BundleExporter.CheckBundleExportStatus cannot deserialize status "in_progress" properly. about crowdin-api-client-dotnet HOT 19 CLOSED

After some investigation, I found that both these status representations are being returned by the v2 API;

• "status": "inProgress" (Translations -> Build Status)
• "status": "in_progress" (Bundles -> Build Status)

A proper fix should probably be done on the API server side instead of in this package.

I can still offer a fix, but I don't really like the options I have of fixing it, what I considered:

• Adding a new BuildStatus enum specifically for Crowdin.Api.Bundles (probably what I would go for).
• Adding a new attribute which is also considered by the DescriptionEnumConverter (feels hacky)
• Remove any underscores before doing a case insensitive equals check and using the current comparison (also feels hacky).
• More hacky stuff 😓.

from crowdin-api-client-dotnet.

@PatrickMNL thanks a lot for the investigation!

This should definitely be fixed on the API side. Passing it on to the development team

from crowdin-api-client-dotnet.

@andrii-bodnar I have added 'temporary workaround' suggestion in this PR: #169

from crowdin-api-client-dotnet.

@PatrickMNL sure, I am going to release a new version in a few minutes

from crowdin-api-client-dotnet.

Ignore the nuget feed question, I see it just got updated! Thanks again 👍!

from crowdin-api-client-dotnet.

from crowdin-api-client-dotnet.

@andrii-bodnar I don't mind picking this up myself, if I can get permissions to open pull requests.

from crowdin-api-client-dotnet.

@PatrickMNL great, thank you! 🚀

Feel free to fork the repo and create a PR

from crowdin-api-client-dotnet.

Alright, glad we're on the same page, thanks @andrii-bodnar! Would u have a ballpark estimate about when we can expect the fix? Just checking if it's worth waiting or that we should build in a small work around for now.

from crowdin-api-client-dotnet.

@PatrickMNL I will provide the exact estimate as soon as possible. It would be great if we could make a temporary workaround just to not block further integration.

from crowdin-api-client-dotnet.

Thanks for considering and merging the PR @andrii-bodnar, when can we expect a new release of the NuGet package? I would like to start using it with the fixes included 😄.

from crowdin-api-client-dotnet.

@PatrickMNL just released a new version - 2.14.3

from crowdin-api-client-dotnet.

@andrii-bodnar Thanks for the release! I see that the NuGet feed did not yet receive the new version, does that take a while or was it perhaps missed during the update?

from crowdin-api-client-dotnet.

@PatrickMNL thanks for your contribution to this project!

@andrii-bodnar can we consider migration to permanent solution for this case? For example add extra [Description] attribute with "in_progress" value or create separate enum for Bundle API. As OP mentioned before.

Probably new value from API is already used by end users. We can avoid breaking backward compatibility just by fixing this on client side

from crowdin-api-client-dotnet.

Hi @PatrickMNL,

let's create a new class for bundle build status.

Having the status response in the snake_case is more correct.

Due to some historical reasons, it was introduced in camelCase for the Build Project Translation and Build Project Directory Translation API methods and can't be changed now.

Currently, the following API endpoints have the snake_case status response:

• Export Bundle
• Export Glossary
• Import Glossary
• Apply Pre-Translation
• Export TM
• Import TM

as well as the correcponding Check ... status

from crowdin-api-client-dotnet.

@andrii-bodnar thanks for the reply.

Can do, definitely, are you aware that introducing the new enum (class?), would cause 'breaking changes' when updating the package, as in, people will need to move their own Bundle implementations to this newly created enum.

If we're okay with that, than I can introduce a PR for that.

Something small to consider; If we're introducing breaking changes, we are also able to fix the existing BuildStatus and move the older incorrectly named variants to something like a LegacyBuildStatus.

from crowdin-api-client-dotnet.

@PatrickMNL Yes, we'll add a warning to the release notes about these breaking changes.

Something small to consider; If we're introducing breaking changes, we are also able to fix the existing BuildStatus and move the older incorrectly named variants to something like a LegacyBuildStatus.

I think this is a good idea!

from crowdin-api-client-dotnet.

@andrii-bodnar I have created the above PR as potential fix, I am able to test the Bundle API against our live implementation, we don't have a Translation/Directory API implementation internally, so I am unable to test those, is that something you can verify?

from crowdin-api-client-dotnet.

@PatrickMNL thanks a lot!

We have some examples in this repo, maybe it would be possible to test the new code using these examples?

from crowdin-api-client-dotnet.