Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error when trying to send an asset that already exists on the remote #113

Closed
jkobus opened this issue Jul 19, 2017 · 7 comments
Closed

Error when trying to send an asset that already exists on the remote #113

jkobus opened this issue Jul 19, 2017 · 7 comments

Comments

@jkobus
Copy link
Contributor

jkobus commented Jul 19, 2017

  1. I have new translation keys on my website
  2. I use profiler to send them to remote: "Send selected translations to SaaS"
  3. I added a few more translation keys and reloaded the website page
  4. On profiler page under "missing" i see all keys - even keys that were sent in step 2.
  5. I click: Send selected translations to SaaS
  6. I see error: Asset already exists called "xxx"

The reason is that bundle does not know that these translations were already sent.
The only way of knowing that is to see if translation exists.

If asset on the remote exists, but was not translated, it will not be downloaded.

I think about accepting this type of response from SaaS so it will not break the whole process, or - to check if asset already exists and ignore it if it does.

@jkobus jkobus changed the title Error when trying to sync an asset that already exists on the remote Error when trying to send an asset that already exists on the remote Jul 19, 2017
@Nyholm
Copy link
Member

Nyholm commented Jul 19, 2017

After we send translations to sass we also need to update the local storage (XLIF). But you are sending untranslated stuff?

It does not seams like there is an error... I agree with you. We should not display it as a "Oh no, this is a catastrophe". If we send things to sass and they already exists the everything should be good, right?

@jkobus
Copy link
Contributor Author

jkobus commented Jul 19, 2017

Yes, untranslated.
If we'll add untranslated asset to local xliff file it will be translated to blank string I guess.

Anyway, some kind of "insert if does not exist" would be enough. Then, the meaning of the button "Send selected translations to SaaS" would be more like "Sync selected translations with SaaS", as we won't know if any of the selected assets was added to the remote either by our profiler-tool or by someone else.

@jkobus
Copy link
Contributor Author

jkobus commented Jul 19, 2017

Theres also one thing: when you create a new key and view it in the profiler, pressing "sync" will only add it to local xliff file (with "target text" same as the key) without sending it to the SaaS. The only way to send that translation is by using translation:sync command.

Expected behaviour would be to send it to SaaS and not adding it to the local xliff file.
This should happen when you edit the translation, and such change should be sent to SaaS and local translation file.

@Nyholm
Copy link
Member

Nyholm commented Jul 19, 2017

That "sync" button is more "download". Not sure what we want.

Also, when we press "edit", we do a download before showing anything.

@jkobus
Copy link
Contributor Author

jkobus commented Jul 19, 2017

Sync on non-existing-on-remote asset would add it to the translation file without notes about params, while the "Send ...." button below does that.

@jkobus
Copy link
Contributor Author

jkobus commented Jul 19, 2017

it looks like you already thought about it:

        try {
            // Create asset first
            $this->client->asset()->create($projectKey, $message->getKey());
            $this->client->translations()->create($projectKey, $message->getKey(), $message->getLocale(), $message->getTranslation());
        } catch (AssetConflictException $e) {
            // This is okey
            $isNewAsset = false;
        }

but theres a small typo in client:

        if ($response->getStatusCode() !== 201) {
            $this->handleErrors($response);
        }

        if ($response->getStatusCode() === 409) {
            throw Exception\Domain\AssetConflictException::create($id);
        }

it should be:

        if ($response->getStatusCode() === 409) {
            throw Exception\Domain\AssetConflictException::create($id);
        }

        if ($response->getStatusCode() !== 201) {
            $this->handleErrors($response);
        }

I'll send a PR tomorrow as this worked for me and all translations were sent successfully.

@Nyholm
Copy link
Member

Nyholm commented Jul 24, 2017

Thank you for fixing this issue.

Ref: FriendsOfApi/localise.biz#22

@Nyholm Nyholm closed this as completed Jul 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants