Crowdin API v2, allow editing of any files as XLIFF #633

berezins · 2020-05-24T07:56:00Z

Added support (from customer's prospective):

Crowdin for enterprise
- Works with Crowdin Enterprise accounts of any organization (on any domains)
Open/save/sync as Xliff any file which can be parsed for translations by Crowdin
While continue to work with PO natively as is

Improved

Path to locally stored Crowdin files contains <project name>/<lang code>/<branch><path to file in Crowdin project>
Files opened via Open from Crowdin can be saved/synced back to Crowdin on further open and edit as local file from file system (simplest from File menu's Recent list/history)

Migrated to new:

OAuth improved/updated that works with API v2
API v2

This PR is clean version of #629 with

Work with PO natively (as is without converting to/from Xliff) is back
Clean/arranged/grouped commits
Back to implicit flows with few improvements
- state param passed to OAuth URL and checked when callback received
- OAuth URL redirect via poedit.net replaced with direct to Crowdin OAuth URL but source tracking params added (to identify authentication via Poedit for statistics)

- OAuth URL redirect via poedit.net replaced with direct to Crowdin OAuth URL but source tracking params added (to identify authentication via Poedit for statistics) - `state` param passed to OAuth URL and checked when callback URL with token received - Works already one new API `/user` so it shows that authenticated and full name of the user at the end of authentication process (when Authorize button pressed on authorization page)

JWT parsing without third-party deps

then in deps/wx

As it seems create some problem on MacOS plust looks more simple/elegant

i.e. which are not authorized to get file list etc.

from temporary arbitrary URL returned by API

In dropdown list of "Open from Crowdin" dialog

i.e. `{` from new line unless omitted due of single-line statement

- To keep Poedit workflows/behaviour consistent with how it worked with old Crowdin API v1 - As planned for deletion from API

Without type casts in this commit. - Tried on GCC 7 and 8 with -std=gnu++14 and 17 - Even with latest as of now nlohmann/json@c05bd90 - While no errors on Visual Studio and Xcode without those casts

to make more clear

src/edframe.cpp

In cpprest (casablanka) RESTful client

Now all files are supported

for setting remote (Crowdin side) file extension

- Fixed naming projName=>projIdentifier - Improved error messages to be more clear, excplanatory and precise

In `Catalog` Also improved detection of files opened from Crowdin via `Open from Crowdin` dialog (added "CrowdinSync_" at the beginning of each such filename)

And stuff related to it in `XLIFF1Catalog` and `XLIFF2Catalog`

There needs to be / between the prefix and the relative URLs; to keep the changes minimal, put it in the prefix.

gettext translations are normally uploaded as a POT file, not PO, while translations themselves are PO. Handle this case correctly.

Caught by Visual C++'s warning about comparing bool to int. Rephrased the condition to be more verbose, but more readable and less error-prone.

berezins · 2020-06-22T12:12:45Z

src/catalog.cpp

@@ -602,8 +602,10 @@ void Catalog::SetFileName(const wxString& fn)

 bool Catalog::IsFromCrowdin() const
 {
-    if((m_crowdinFileId > 0 && m_crowdinProjectId) > 0


Yep, that was definitely my "typo" during refactoring and I would never write above intentently :). And many apologies for this. That is why Visual Studio complained about it and it's pure luck it worked as expected (due Crowdin seems never assigns 0 to IDs) . It intended to be (m_crowdinFileId > 0 && m_crowdinProjectId > 0) definitely. Anyway thanks a lot for catching this up and fixing

and I would never write above intentently :)

Of course. I'm simultaneously impressed that MSVC caught it and surprised than neither relatively modern GCC nor Clang did.

vslavik · 2020-06-24T10:18:36Z

Thank you! I have now pushed most of the PR to the crowdin-v2 branch; please use that as the basis for any changes. Some missing bits are because I think I can come up with better solution:

no bSkipUpload
no AttachCloudSync changes

I also improved formatting, fixed broken uses of gettext's _(), restored link attribution code to minimize changes, made some more trivial tweaks. For reference, this is a diff from what I committed to this PR, except for trivial formatting changes. In other words, applying this patch to crowdin-v2 branch restores your PR as-is:
from-committed-to-pr-not-applied-bits.patch.gz

vslavik · 2020-06-24T10:56:39Z

I forgot to say: I also rebased on the new (macOS) networking implementation.

There's one known problem: syncing PO files fails.

Click on Collaborate on a Translation with Others
Choose "Poedit Testing Sandbox" project (I invited you), language e.g. Finnish, file default.po or any of the POT files, click OK.
Immediately click the Sync toolbar button.
Oops:

I thought my changes w.r.t PO vs POT handling caused it, but it's happening with PO too.

vslavik · 2020-06-27T14:50:53Z

There's one known problem: syncing PO files fails.

This is fixed in 54b86a9.

I am now seeing X-Crowdin-Project-ID and X-Crowdin-File-ID headers in PO files downloaded from Crowdin (both via API and manually). It seems that big part of 9a58ee3, namely CrowdinUpdateCatalogIDsFromHeaders, can be safely removed after all?

berezins · 2020-07-01T17:02:07Z

Hi Václav, Fixed in this new PR #641 (using `crowdin-v2` as base) as well as few other issues/bugs found during testing on all platforms (see commits there for more details). One of issues overlaps with that you already fixed as well on your own so far I see but still improved variable scopes/captures to limit them to only where they are really used. Please feel free to review/comment directly in PR or here in email. Or to fix/correct/improve on your own. And sorry for some delay due I was in the trip. Best regards, Serhiy

…

On Wed, Jun 24, 2020 at 1:56 PM Václav Slavík ***@***.***> wrote: I forgot to say: I also rebased on the new (macOS) networking implementation. There's one known problem: syncing PO files fails. 1. Click on *Collaborate on a Translation with Others* 2. Choose "Poedit Testing Sandbox" project (I invited you), language e.g. Finnish, file default.po or any of the POT files, click OK. 3. Immediately click the *Sync* toolbar button. 4. Oops: [image: Screen Shot 2020-06-24 at 12 55 56] <https://user-images.githubusercontent.com/145881/85543111-0d7e3000-b61a-11ea-9ed0-f60e957ed83b.png> I thought my changes w.r.t PO vs POT handling caused it, but it's happening with PO too. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#633 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEWRVUUWIZJMJZOI6X3BNNDRYHLXLANCNFSM4NIZAKJQ> .

berezins · 2020-07-01T17:49:06Z

I am now seeing X-Crowdin-Project-ID and X-Crowdin-File-ID headers in PO files downloaded from Crowdin (both via API and manually). It seems that big part of 9a58ee3 <9a58ee3>, namely CrowdinUpdateCatalogIDsFromHeaders, can be safely removed after all?

Yep, I am starting changes to that same PR #641 to engage above to eliminate that crappy CrowdinUpdateCatalogIDsFromHeaders so you'll see once I will commit and also will let you know by email Also now negotiating adding of `projId` attribute somewhere to top level tag of XLIFF files as well so the same would work for XLIFF as well (i.e. sync/upload of XLIFF files downloaded in browser as well) so if will be added quickly as well from Crowdin side - it would make sense to include it in your upcoming release as well.

…

On Sat, Jun 27, 2020 at 5:51 PM Václav Slavík ***@***.***> wrote: There's one known problem: syncing PO files fails. This is fixed in 54b86a9 <54b86a9> . I am now seeing X-Crowdin-Project-ID and X-Crowdin-File-ID headers in PO files downloaded from Crowdin (both via API and manually). It seems that big part of 9a58ee3 <9a58ee3>, namely CrowdinUpdateCatalogIDsFromHeaders, can be safely removed after all? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#633 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEWRVUUMDANZJAB4X3VNKL3RYYBNRANCNFSM4NIZAKJQ> .

berezins · 2020-07-02T12:04:24Z

- Done using of X-Crowdin-Project-ID and X-Crowdin-File-ID in 43870fc - Crowdin server/API team answered that probable will implement providing project ID for XLIFF files soon as well so probable it would be good to include same approach for XLIFF as well

…

On Wed, Jul 1, 2020 at 8:48 PM Serhiy Berezin ***@***.***> wrote: I am now seeing X-Crowdin-Project-ID and X-Crowdin-File-ID headers in PO > files downloaded from Crowdin (both via API and manually). It seems that > big part of 9a58ee3 > <9a58ee3>, > namely CrowdinUpdateCatalogIDsFromHeaders, can be safely removed after > all? Yep, I am starting changes to that same PR #641 to engage above to eliminate that crappy CrowdinUpdateCatalogIDsFromHeaders so you'll see once I will commit and also will let you know by email Also now negotiating adding of `projId` attribute somewhere to top level tag of XLIFF files as well so the same would work for XLIFF as well (i.e. sync/upload of XLIFF files downloaded in browser as well) so if will be added quickly as well from Crowdin side - it would make sense to include it in your upcoming release as well. On Sat, Jun 27, 2020 at 5:51 PM Václav Slavík ***@***.***> wrote: > There's one known problem: syncing PO files fails. > > This is fixed in 54b86a9 > <54b86a9> > . > > I am now seeing X-Crowdin-Project-ID and X-Crowdin-File-ID headers in PO > files downloaded from Crowdin (both via API and manually). It seems that > big part of 9a58ee3 > <9a58ee3>, > namely CrowdinUpdateCatalogIDsFromHeaders, can be safely removed after > all? > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > <#633 (comment)>, or > unsubscribe > <https://github.com/notifications/unsubscribe-auth/AEWRVUUMDANZJAB4X3VNKL3RYYBNRANCNFSM4NIZAKJQ> > . >

berezins added 20 commits May 24, 2020 02:10

Download and open file in Xliff

fa3751c

Local path matches path in Crowdin

e400607

Upload file as XLIFF

50d4381

OAuth to any organization or personal account

b4a90b7

JWT parsing without third-party deps

Fixed empty first and last name in organization user

750e76a

Hide assets from list

753a5c8

Fix compilation errors on newer wxWidgets versions

8cdd4fe

then in deps/wx

Crashed when to Save file deleted on Crowdin side

2139b6e

Crashed on open cached Crowdin file if logged out

68d5e5e

Leading slash in API requests removed

b202e3c

As it seems create some problem on MacOS plust looks more simple/elegant

Move AttachCloudSync() to GUI code

e7161c0

Hide 'Forbidden' projects

a3c5c45

i.e. which are not authorized to get file list etc.

Domain agnostic file download

caf7330

from temporary arbitrary URL returned by API

Full path in file list including branch

ca4d693

In dropdown list of "Open from Crowdin" dialog

Implicit conversion from json to json_data

1f910e6

Logging using wxLogTrace()

e824933

std explicitly

99261b0

Predominant if/for/while formatting

de53022

i.e. `{` from new line unless omitted due of single-line statement

GetUserInfo() fixed

8f6f90b

berezins mentioned this pull request May 24, 2020

Crowdin API v2, OAuth v2, open/save/sync as Xliff all files with translations #629

Closed

berezins added 7 commits May 25, 2020 20:40

Native PO file support back

e06429f

Buildable on Windows (wxStandardPaths())

98ab380

Buildable on macOS (wxGetHomeDir() undefined)

b5771a5

OAuth: TM (translation memory) scope is not needed

f190355

Removed auto approval of uploaded translations

9b7a084

XLF extension is XLIFF as well

efb1a7d

Removed importDuplicates

89ce540

- To keep Poedit workflows/behaviour consistent with how it worked with old Crowdin API v1 - As planned for deletion from API

berezins force-pushed the crowdin-new-oauth-api-xliff-clean branch 2 times, most recently from 5cdd02c to 5d9204e Compare May 31, 2020 12:59

GCC: "error: operands to ?: have different types"

f7e6799

Without type casts in this commit. - Tried on GCC 7 and 8 with -std=gnu++14 and 17 - Even with latest as of now nlohmann/json@c05bd90 - While no errors on Visual Studio and Xcode without those casts

berezins force-pushed the crowdin-new-oauth-api-xliff-clean branch from 3f21d4e to f7e6799 Compare June 13, 2020 13:51

GetUserInfo() refactored and commented

d81778a

to make more clear

vslavik reviewed Jun 13, 2020

View reviewed changes

src/edframe.cpp Show resolved Hide resolved

berezins added 6 commits June 14, 2020 01:10

Comment for removing "Accept: application/json"

a37bb61

In cpprest (casablanka) RESTful client

mb_str() to str::to_utf8() changed

9bc52ba

Token domain error handling and logging

cc58929

Removed moving unsupported files to the end

0fa48dd

Now all files are supported

Explicit CmpNoCase() != 0 and comment

1d244ec

for setting remote (Crowdin side) file extension

Save/sync PO downloaded via Crowdin UI improved

6df545e

- Fixed naming projName=>projIdentifier - Improved error messages to be more clear, excplanatory and precise

berezins force-pushed the crowdin-new-oauth-api-xliff-clean branch from 3792a79 to 6df545e Compare June 18, 2020 13:18

berezins and others added 6 commits June 18, 2020 16:31

Put together most of Crowdin stuff to IsFromCrowdin()

a711235

In `Catalog` Also improved detection of files opened from Crowdin via `Open from Crowdin` dialog (added "CrowdinSync_" at the beginning of each such filename)

m_language is back in XLIFFCatalog

0dcedfa

And stuff related to it in `XLIFF1Catalog` and `XLIFF2Catalog`

Compilation fix for bSkipUpload

0bb4cb4

Fix bad API URL prefix

e9db2d0

There needs to be / between the prefix and the relative URLs; to keep the changes minimal, put it in the prefix.

Really open PO files natively, even if uploaded as POT

579d3d4

gettext translations are normally uploaded as a POT file, not PO, while translations themselves are PO. Handle this case correctly.

Fix broken IsFromCrowdin() logic

dced359

Caught by Visual C++'s warning about comparing bool to int. Rephrased the condition to be more verbose, but more readable and less error-prone.

berezins commented Jun 22, 2020

View reviewed changes

berezins force-pushed the crowdin-new-oauth-api-xliff-clean branch from e0aa0b7 to dced359 Compare June 22, 2020 13:31

New OAuth app since old Client ID compromised

0f76785

vslavik mentioned this pull request Jun 23, 2020

Race condition on Crowdin upload, messes future syncing #638

Closed

vslavik closed this Jul 2, 2020

vslavik mentioned this pull request Jul 2, 2020

Crowdin v2 fixes #641

Closed

berezins mentioned this pull request Jul 13, 2020

Sync XLIFF downloaded from Crowdin UI in browser (clean) #647

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Crowdin API v2, allow editing of any files as XLIFF #633

Crowdin API v2, allow editing of any files as XLIFF #633

berezins commented May 24, 2020 •

edited

Loading

berezins Jun 22, 2020 •

edited

Loading

vslavik Jun 22, 2020

vslavik commented Jun 24, 2020

vslavik commented Jun 24, 2020

vslavik commented Jun 27, 2020

berezins commented Jul 1, 2020 via email

berezins commented Jul 1, 2020 via email

berezins commented Jul 2, 2020 via email

Crowdin API v2, allow editing of any files as XLIFF #633

Crowdin API v2, allow editing of any files as XLIFF #633

Conversation

berezins commented May 24, 2020 • edited Loading

This PR is clean version of #629 with

berezins Jun 22, 2020 • edited Loading

Choose a reason for hiding this comment

vslavik Jun 22, 2020

Choose a reason for hiding this comment

vslavik commented Jun 24, 2020

vslavik commented Jun 24, 2020

vslavik commented Jun 27, 2020

berezins commented Jul 1, 2020 via email

berezins commented Jul 1, 2020 via email

berezins commented Jul 2, 2020 via email

berezins commented May 24, 2020 •

edited

Loading

berezins Jun 22, 2020 •

edited

Loading