[basket] should not error for unsupported language

VERIFIED FIXED

Status

defect
VERIFIED FIXED
6 years ago
5 years ago

People

(Reporter: pmac, Assigned: pmac)

Tracking

(Blocks 1 bug)

Production
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [kb=1257477] )

Attachments

(1 attachment)

41 bytes, text/x-github-pull-request
Details | Review
Assignee

Description

6 years ago
It should store all language preferences in Exact Target for future use and analytics.
Assignee

Comment 1

6 years ago
One question I have on this is whether the language column in Exact Target can store only 2 characters, or 5 for things like "pt-BR"?
Flags: needinfo?(jdavis)

Comment 2

6 years ago
The "LANGUAGE_ISO2" field in "Master_Subscribers" can hold up to 5 characters; however - if we pass through more than just the 2, I have to search for "contains en" instead of "is en" (which I currently do for the languages). 

This works well for now with a limited number of possible locales that are passed through to ExactTarget. If we start collecting any/all locales, I'm curious how many other languages have country codes that might mess with the "contains" query. I can't think of any off the top of my head other than "es-ES" - but that would be Spanish no matter the query.

Can you think of other locale combos that could/would mess up segmenting overall language?
Flags: needinfo?(jdavis)
Assignee

Comment 3

6 years ago
(In reply to Jessilyn Davis from comment #2)
> This works well for now with a limited number of possible locales that are
> passed through to ExactTarget. If we start collecting any/all locales, I'm
> curious how many other languages have country codes that might mess with the
> "contains" query. I can't think of any off the top of my head other than
> "es-ES" - but that would be Spanish no matter the query.
> 
> Can you think of other locale combos that could/would mess up segmenting
> overall language?

I can't think of any, but that doesn't mean they don't exist. How hard would it be to do a "starts with" query instead of a "contains"? That's the real answer. In SQL proper it would just be "WHERE LANGUAGE_ISO2 LIKE 'es%';" or something like that. Hopefully ET makes that kind of thing easy. It would be nice to collect the full thing since things like zh-TW and zh-CN are quite different I believe.

Comment 4

6 years ago
"starts with" (aka "begins with" in ExcactTarget's Filters) works!  Good call.

Let's pass through the full 5 characters!

For my notes:
* For EN we'll make it "starts with EN" or "is blank"
* All the other current newsletter languages should be good to filter with "starts with")
* As we launch other languages, we'll adapt how to best filter and segment.
Assignee

Comment 5

6 years ago
Posted file pull request
Assignee

Updated

6 years ago
Assignee: nobody → pmac
Status: NEW → ASSIGNED
Assignee

Updated

6 years ago
Whiteboard: [kb=1257477]
Assignee

Updated

5 years ago
Blocks: 968423

Comment 6

5 years ago
Commits pushed to master at https://github.com/mozilla/basket

https://github.com/mozilla/basket/commit/0a8661bee542138140fbd319b9304681aef1895b
Fix bug 964848: Accept all properly formatted language codes.

https://github.com/mozilla/basket/commit/bc14fa0e15729b3ec85f6d49c274d3e053b68d7c
Merge pull request #94 from pmclanahan/store-all-lang-prefs-964848

Fix bug 964848: Accept all properly formatted language codes.

Updated

5 years ago
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED

Comment 7

5 years ago
Thanks pmac! \o/

Mike - do you know who can make an API call to basket with language = it  and can let me know what unique email address they used? I can then check ExactTarget to make sure their info was passed through correctly with a non-newsletter supported language and verify this bug.

Thanks!
Flags: needinfo?(malexis)

Comment 8

5 years ago
Pmac - this needs to be fixed for the OS bug: https://bugzilla.mozilla.org/show_bug.cgi?id=964342#c39

When using an es-PE (Spanish + Peru) combo, the OS signup form did not work. When using DE (German + Germany) it worked beautifully.

What's the next steps to make this work with the OS FTU form?
Flags: needinfo?(pmac)
Assignee

Comment 9

5 years ago
Just need to do a prod push. I can get on that whenever we're ready.
Flags: needinfo?(pmac)

Comment 10

5 years ago
Prod push happened in Bug 977201

Pmac tested and verified via IRC that these scenarios worked:

es-PE (peru)
en-US
de
Status: RESOLVED → VERIFIED
Depends on: 977201
Flags: needinfo?(malexis)
You need to log in before you can comment on or make changes to this bug.