Closed Bug 1405404 Opened 6 years ago Closed 6 years ago

Pocket recommendations do not show in Deutsch multi-locale builds

Categories

(Firefox for Android Graveyard :: Activity Stream, defect, P1)

Product:
Firefox for Android Graveyard
Component:
Activity Stream
Platform:
All
Android
Type:
defect

Tracking

(firefox57 verified, firefox58 verified)

Status:
VERIFIED FIXED
Milestone:
Firefox 58
Tracking Flags:
Tracking Status
firefox57 --- verified
firefox58 --- verified

People

(Reporter: mcomella, Assigned: mcomella)

References

Details

Attachments

(1 file)

Bug 1405404: Add 'de' to list of pocket whitelisted locales.
59 bytes, text/x-review-board-request
liuche
: review+
ritu
: approval-mozilla-beta+
Details 
I didn't implement this initially in bug 1404460: "de" is used for multi-locale builds but cannot be selected by Android so I thought it was unnecessary. We should add it to the whitelist.
Comment on attachment 8914839 [details]
Bug 1405404: Add 'de' to list of pocket whitelisted locales.

https://reviewboard.mozilla.org/r/186110/#review191134

::: mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/ActivityStreamConfiguration.java:26
(Diff revision 2)
>   */
>  public class ActivityStreamConfiguration {
>  
>      private static final Set<Locale> pocketEnabledLocales;
>      @VisibleForTesting static final String[] pocketEnabledLocaleTags = new String[] {
>              // Sorted alphabetically to preserve blame for additions/removals.

I wonder if we should document here that new locales being added should be cross-referenced with both Fennec locale and Android locale.
Attachment #8914839 - Flags: review?(liuche) → review+
Comment on attachment 8914839 [details]
Bug 1405404: Add 'de' to list of pocket whitelisted locales.

https://reviewboard.mozilla.org/r/186110/#review191134

> I wonder if we should document here that new locales being added should be cross-referenced with both Fennec locale and Android locale.

I added a note.
Pushed by michael.l.comella@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/0bf07287d614
Add 'de' to list of pocket whitelisted locales. r=liuche
Comment on attachment 8914839 [details]
Bug 1405404: Add 'de' to list of pocket whitelisted locales.

This must be uplifted in unison with bug 1404460 (and should land after).

Approval Request Comment
[Feature/Bug causing the regression]: bug 1404460.
[User impact if declined]: Users using the Deutsch language override in Fennec settings will not get Pocket stories.
[Is this code covered by automated tests?]: Lightly (we ensure the locale says that Pocket should be enabled, as expected).
[Has the fix been verified in Nightly?]: No
[Needs manual test from QE? If yes, steps to reproduce]: Ideally, see bug 1404460 comment 11.
[List of other uplifts needed for the feature/fix]: bug 1404460 (before this).
[Is the change risky?]: Trivial.
[Why is the change risky/not risky?]: We add a one locale, "de", to an existing list of whitelisted locales so assuming the current code works, there should be no problems.
[String changes made/needed]: None
Attachment #8914839 - Flags: approval-mozilla-beta?
Pushed by michael.l.comella@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/0db210721aa4
Add 'de' to list of pocket whitelisted locales. r=liuche
Note: I didn't have an opportunity to test this test change because multi-locale builds are breaking gradle builds but I'm confident in the change.
Backout by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/autoland/rev/fc879542e9ae
Backed out changeset 0bf07287d614 for failing android-test. r=backout
^ is the backout I described in comment 8, which occurred prior to my relanding in comment 10.

(In reply to Michael Comella (:mcomella) from comment #11)
> Note: I didn't have an opportunity to test this test change because
> multi-locale builds are breaking gradle builds but I'm confident in the
> change.

I tested locally: it passed. (I blew out my multi-locale build for the gradle build)
Comment on attachment 8914839 [details]
Bug 1405404: Add 'de' to list of pocket whitelisted locales.

This is needed for Bug 1404460, Beta57+
Attachment #8914839 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
https://hg.mozilla.org/mozilla-central/rev/0db210721aa4
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox58: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
Devices:
 - Nexus 5 (Android 6.0.1);
 - Samsung Galaxy Tab 3	(Android 7);
 - Google Pixel (Android 8).

Verified this on multiple devices and pocket stories are being displayed when changing to Deutsch.
Status: RESOLVED → VERIFIED
status-firefox57: fixed → verified
status-firefox58: fixed → verified
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.