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

Revert Translation Matching Logic to Use FileName #581

Merged
merged 2 commits into from
Nov 8, 2023
Merged

Conversation

mohamede1945
Copy link
Collaborator

This pull request reverts a recent change (#526) in the matching logic of translations from using Translation.Id to fileName. The update was initially implemented to enhance the accuracy of translation matching. However, it was observed that new versions of the same translation are assigned new IDs, leading to inconsistencies in the matching process.

Issue

After updating the matching logic to utilize Translation.Id, we encountered a significant issue where new versions of translations received different IDs, breaking the expected continuity and leading to mismatches in our application. This behavior was not anticipated and has affected the stability of the translation matching feature.

Resolution

To address this, we've decided to revert to using the fileName as the matching reference. The fileName has been confirmed with the backend team to be a stable and unchanging field, ensuring consistent matching even when new versions of translations are released.

Testing

The changes have been tested locally, and existing automated tests have been updated to reflect this new logic. Manual testing was also conducted to ensure that translations are being matched correctly with their respective fileName.

This reverts commit 4091557.

Translation.ID changes every time a db upgrade happens. Instead,
we use fileName as it will not change across upgrades.
As we cannot change the Id of the translation through UPDATE query.
Copy link

codecov bot commented Nov 8, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (423b821) 41.26% compared to head (27329aa) 41.39%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #581      +/-   ##
==========================================
+ Coverage   41.26%   41.39%   +0.13%     
==========================================
  Files         496      496              
  Lines       19052    19058       +6     
==========================================
+ Hits         7862     7890      +28     
+ Misses      11190    11168      -22     
Files Coverage Δ
...lationService/Sources/TranslationsRepository.swift 93.10% <100.00%> (-0.23%) ⬇️
...ionService/Tests/TranslationsRepositoryTests.swift 100.00% <100.00%> (+21.87%) ⬆️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mohamede1945 mohamede1945 merged commit a47189a into main Nov 8, 2023
2 checks passed
@mohamede1945 mohamede1945 deleted the afifi branch November 8, 2023 02:10
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

Successfully merging this pull request may close these issues.

1 participant