Issue 5777 - Update crowdin interface

localeTools.py

Index: localeTools.py
diff --git a/localeTools.py b/localeTools.py
index 140f22c1f7bb8298ce12bf7c1d4547a47de346a4..598f822a83688675cd730fc7fe96dc3e662b2873 100644
--- a/localeTools.py
+++ b/localeTools.py
@@ -7,6 +7,7 @@ import os
 import sys
 import codecs
 import json
+import urlparse
 import urllib
 import urllib2
 from StringIO import StringIO
@@ -92,6 +93,24 @@ chromeLocales = [
     'zh-TW',
 ]
 
+CROWDIN_AP_URL = 'https://api.crowdin.com/api/project/{}/{}'
+
+
+def crowdin_url(project_name, action, key, get={}):
+    """Create a valid url for a crowdin endpoint."""
+    url = CROWDIN_AP_URL.format(project_name, action)
+
+    get.update({'key': key})

[Vasily Kuznetsov, 2017/09/26 11:05:52]

Why not just `get['key'] = key`? Seems simpler.

[tlucas, 2017/09/26 12:13:13]

Done.

+
+    scheme, netloc, path, params, query, fragment = urlparse.urlparse(url)
+
+    query = dict(urlparse.parse_qsl(query))

[Vasily Kuznetsov, 2017/09/26 11:05:52]

Could there be anything in this query? It seems like it can only come from the
`action` argument, but in all of the calls to `crowdin_url` `action` is just a
simple path fragment without anything like "?foo=bar&baz=qux" that could end up
in the query. Or am I missing something?

[tlucas, 2017/09/26 12:13:13]

No, you are right - currently, there can't be query parameters - but i somehow
don't like the idea that there shouldn't be any (e.g. when someone imports this
function and expects it to keep the original query - as i did during
development).

In case someone absolutely opposes this, i would just drop a possibly present
query - but currently i'd like to keep it.

[Vasily Kuznetsov, 2017/09/26 13:11:36]

Acknowledged.

+    query.update(get)
+
+    return urlparse.urlunparse((
+        scheme, netloc, path, params, urllib.urlencode(query), fragment
+    ))
+
 
 class OrderedDict(dict):
     def __init__(self):
@@ -298,17 +317,21 @@ def setupTranslations(localeConfig, projectName, key):
 
     allowed = set()
     allowedLocales = json.load(urllib2.urlopen(
-        'https://crowdin.com/languages/languages_list?callback='
-    ))
+        crowdin_url(projectName, 'supported-languages', key, {'json': 1})))
+
     for locale in allowedLocales:
-        allowed.add(locale['code'])
+        allowed.add(locale['crowdin_code'])
     if not allowed.issuperset(locales):
         print "Warning, following locales aren't allowed by server: " + ', '.join(locales - allowed)
 
     locales = list(locales & allowed)
     locales.sort()
     params = urllib.urlencode([('languages[]', locale) for locale in locales])
-    result = urllib2.urlopen('http://api.crowdin.net/api/project/%s/edit-project?key=%s' % (projectName, key), params).read()
+
+    result = urllib2.urlopen(
+        crowdin_url(projectName, 'edit-project', key), data=params
+    ).read()
+
     if result.find('<success') < 0:
         raise Exception('Server indicated that the operation was not successful\n' + result)
 
@@ -334,7 +357,8 @@ def postFiles(files, url):
 
 
 def updateTranslationMaster(localeConfig, metadata, dir, projectName, key):
-    result = json.load(urllib2.urlopen('http://api.crowdin.net/api/project/%s/info?key=%s&json=1' % (projectName, key)))
+    result = json.load(urllib2.urlopen(
+        crowdin_url(projectName, 'info', key, {'json': 1})))
 
     existing = set(map(lambda f: f['name'], result['files']))
     add = []
@@ -362,12 +386,16 @@ def updateTranslationMaster(localeConfig, metadata, dir, projectName, key):
                     add.append((newName, data))
 
     if len(add):
-        titles = urllib.urlencode([('titles[%s]' % name, re.sub(r'\.json', '', name)) for name, data in add])
-        postFiles(add, 'http://api.crowdin.net/api/project/%s/add-file?key=%s&type=chrome&%s' % (projectName, key, titles))
+        data = {'titles[{}]'.format(name): re.sub(r'\.json', '', name)
+                for name, data in add}
+        data.update({'type': 'chrome'})

[Vasily Kuznetsov, 2017/09/26 11:05:52]

Also could be just `data['type'] = 'chrome'`.

[tlucas, 2017/09/26 12:13:13]

Done.

+        postFiles(add, crowdin_url(projectName, 'add-file', key, data))
     if len(update):
-        postFiles(update, 'http://api.crowdin.net/api/project/%s/update-file?key=%s' % (projectName, key))
+        postFiles(update, crowdin_url(projectName, 'update-file', key))
     for file in existing:
-        result = urllib2.urlopen('http://api.crowdin.net/api/project/%s/delete-file?key=%s&file=%s' % (projectName, key, file)).read()
+        result = urllib2.urlopen(
+            crowdin_url(projectName, 'delete-file', key, {'file': file})
+        ).read()
         if result.find('<success') < 0:
             raise Exception('Server indicated that the operation was not successful\n' + result)
 
@@ -392,17 +420,19 @@ def uploadTranslations(localeConfig, metadata, dir, locale, projectName, key):
             if data:
                 files.append((newName, data))
     if len(files):
-        postFiles(files, 'http://api.crowdin.net/api/project/%s/upload-translation?key=%s&language=%s' % (
-            projectName, key, mapLocale(localeConfig['name_format'], locale))
-        )
+        language = mapLocale(localeConfig['name_format'], locale)
+        postFiles(files,

[Vasily Kuznetsov, 2017/09/26 11:05:52]

It would probably be more readable if this was split into two statements: 

    url = crowdin_url(...)
    postFiles(files, url)

What do you think?

[tlucas, 2017/09/26 12:13:13]

Done.

+                  crowdin_url(projectName, 'upload-translation', key,
+                              {'language': language}))
 
 
 def getTranslations(localeConfig, projectName, key):
-    result = urllib2.urlopen('http://api.crowdin.net/api/project/%s/export?key=%s' % (projectName, key)).read()
+    result = urllib2.urlopen(crowdin_url(projectName, 'export', key)).read()

[Vasily Kuznetsov, 2017/09/26 11:05:52]

This pattern of `urllib2.urlopen(crowdin_url(....)).read()` seems to be a common
one. Maybe makes sense to make a function for it.

[tlucas, 2017/09/26 12:13:13]

Yes - most of the time, the result was used to detect server side errors -
outsourced this too.

     if result.find('<success') < 0:
         raise Exception('Server indicated that the operation was not successful\n' + result)
 
-    result = urllib2.urlopen('http://api.crowdin.net/api/project/%s/download/all.zip?key=%s' % (projectName, key)).read()
+    result = urllib2.urlopen(
+        crowdin_url(projectName, 'download/all.zip', key)).read()
     zip = ZipFile(StringIO(result))
     dirs = {}