1091469 - Use the Google Play API to update Listing Copy and Feature Descriptions

Status: RESOLVED FIXED

(Marketing :: Copy, task)

Description

[Sylvestre Ledru [:Sylvestre]]

Just like we are doing in bug 1045531 (use of the Google play API to push the APK), we should use this API to update the description from the l10n platform.

Comment 1

[Matej Novak [:matej]]

(In reply to Sylvestre Ledru [:sylvestre] from comment #0)
> Just like we are doing in bug 1045531 (use of the Google play API to push
> the APK), we should use this API to update the description from the l10n
> platform.

I don't think I'm the right person to be assigned to this as I don't know what any of that means.

Comment 2

[Sylvestre Ledru [:Sylvestre]]

My bad, I did a lazy clone and forgot to update the field. I am probably going to do it.

Comment 3

[Sylvestre Ledru [:Sylvestre]]

So, I have a working patch feeding from Pascal's interface:
https://l10n.mozilla-community.org/~pascalc/google_play_description/

However, we (Pascal and I) noticed that Google play:
* does not manage a bunch of locales "an", "as", "bn-IN", "cy", "es-MX", "eu", "ff", "fy-NL", "ga-IE", "gd", "gl", "gu-IN", "hy-AM", "is", "kk", "kn", "ml", "mr", "or", "pa-IN", "sq", "ta", "te"
* are not using the same language codes as us.
Here is a (bad) mapping:
        if locale == "cs":
            locale = "cs-CZ"
        if locale == "da":
            locale = "da-DK"
        if locale == "de":
            locale = "de-DE"
        if locale == "fr":
            locale = "fr-FR"
        if locale == "es-AR":
            locale = "es-419"
        if locale == "fi":
            locale = "fi-FI"
        if locale == "hu":
            locale = "hu-HU"
        if locale == "it":
            locale = "it-IT"
        if locale == "ja":
            locale = "ja-JP"
        if locale == "ko":
            locale = "ko-KR"
        if locale == "nl":
            locale = "nl-NL"
        if locale == "pl":
            locale = "pl-PL"
        if locale == "ru":
            locale = "ru-RU"
        if locale == "tr":
            locale = "tr-TR"

This is working (I have been playing with the Firefox aurora application which is disabled).

Comment 4

[Sylvestre Ledru [:Sylvestre]]

I will clean up my code and propose a patch.

Comment 5

[Sylvestre Ledru [:Sylvestre]]

Attachment: update-description-apk.diff

Comment 6

[Sylvestre Ledru [:Sylvestre]]

Depends on bug 1045531 because it provides the authentication.

Works with:
python update_apk_description.py --package-name org.mozilla.fennec_aurora --credentials googleplay.p12 --service-account [e-mail]  --update-apk-description

Comment 7

[Francesco Lodolo [:flod]]

Comment on attachment 8516643 [details] [diff] [review]
update-description-apk.diff

Review of attachment 8516643 [details] [diff] [review]:
-----------------------------------------------------------------

::: scripts/update_apk_description.py
@@ +99,5 @@
> +        response = urllib2.urlopen(ALL_LOCALES_URL)
> +        return json.load(response)
> +
> +    def locale_mapping(self, locale):
> +        if locale == "cs":

A dictionary would make things a bit more readable

    mappings = {
        'cs': "cs-CZ",
        "da": "da-DK",
        etc.
    }
    if locale in mappings:
        return mappings[locale]
    else:
        return locale

Comment 8

[Sylvestre Ledru [:Sylvestre]]

Attachment: update-description-apk.diff

Right, thanks

Comment 9

[Sylvestre Ledru [:Sylvestre]]

Attachment: update-description-apk.diff

Using the virtual env

Comment 10

[Sylvestre Ledru [:Sylvestre]]

Attachment: update-description-apk.diff

So, no more hardcoded locales, pascal is providing that from his web interface.
http://demos.mozfr.org/google_play_copy/web/api/

Comment 11

[[e-mail] (:bhearsum)]

Comment on attachment 8518187 [details] [diff] [review]
update-description-apk.diff

Review of attachment 8518187 [details] [diff] [review]:
-----------------------------------------------------------------

This looks generally okay, just a few questions below. We're probably going to need to talk about how to integrate this and bug 1045531 with the release automation at some point, too, since just landing this script won't make anything happen automatically...

::: scripts/update_apk_description.py
@@ +17,5 @@
> +from mozharness.base.script import BaseScript
> +from mozharness.mozilla.googleplay import GooglePlayMixin
> +from mozharness.base.python import VirtualenvMixin
> +
> +BASE_URL = "http://demos.mozfr.org/google_play_copy/web/"

This shouldn't be hardcoded, please make it configurable. It's fine to use this value as the default, though.

@@ +55,5 @@
> +
> +    # We have 3 apps. Make sure that their names are correct
> +    package_name_values = ("org.mozilla.fennec_aurora",
> +                           "org.mozilla.firefox_beta",
> +                           "org.mozilla.firefox")

What happens if we try to submit something that's not one of these three values?

@@ +158,5 @@
> +                          'title': 'Firefox Aurora'}).execute()
> +
> +            except client.AccessTokenRefreshError:
> +                self.log('The credentials have been revoked or expired,'
> +                         'please re-run the application to re-authorize')

Is it worthwhile to automatically retry this, or does it require human intervention?

Comment 12

[Sylvestre Ledru [:Sylvestre]]

Attachment: attachment.cgi?id=8518187

Thanks for the "URL change idea", much better now!

> since just landing this script won't make anything happen automatically...
Indeed, I think we have to discuss with Pascal and Arcadio. I don't know how often we want to run that...

> What happens if we try to submit something that's not one of these three values?
fatal:
        if self.config['package_name'] not in self.package_name_values:
            self.fatal("Unknown package name value " +
                       self.config['package_name'])

> Is it worthwhile to automatically retry this, or does it require human intervention?
I see three potential failures:
* Google is down
* the arguments are incorrect (no p12 file, wrong credentials)
* we used our API quota (unlikely thing it is something like 100 000 operations per day).
So, I don't think we should do it.

Comment 13

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8522337 [details] [diff] [review]
attachment.cgi?id=8518187

merged:
https://hg.mozilla.org/build/mozharness/rev/ca641817e1a4

I did a minor change (Pascal updated the json file).

Comment 14

[Sylvestre Ledru [:Sylvestre]]

This patch might have to be updated if we decide to use different strings for the different channels.

Comment 15

[Pascal Chevrel:pascalc (PTO until April 26)]

Currently, only the release channel is implemented in the l10n API. If we do localize Aurora and Beta descriptions in the future, I will implement the needed API but I don't have yet that information and I prefer not to add complexity to the API for requests not defined yet by marketing.  

I documented how to use these future APIs there:
https://l10n.mozilla-community.org/~pascalc/google_play_description/api/

By default, if the channel is not specified, it is the release channel.

Comment 16

[Release Engineering SlaveAPI Service]

In production: https://hg.mozilla.org/build/mozharness/rev/ca641817e1a4

Comment 17

[Sylvestre Ledru [:Sylvestre]]

I created a new bug (bug 1110115) to implement comment #15.

Comment 18

[Sylvestre Ledru [:Sylvestre]]

So, we are running the update of the Google play description of Firefox release every day a 14:00 Paris time.
We will do the same for beta later.