search

quran / quran-ios

QuranEngine is the engine powering the Quran.com iOS app.
Apache License 2.0
446 stars 153 forks source link

Revert Translation Matching Logic to Use FileName #581

Closed mohamede1945 closed 10 months ago

mohamede1945 commented 10 months ago

This pull request reverts a recent change (https://github.com/quran/quran-ios/pull/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.

codecov[bot] commented 10 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

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

Additional details and impacted files ```diff @@ 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](https://app.codecov.io/gh/quran/quran-ios/pull/581?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quran) | Coverage Δ | | |---|---|---| | [...lationService/Sources/TranslationsRepository.swift](https://app.codecov.io/gh/quran/quran-ios/pull/581?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quran#diff-RG9tYWluL1RyYW5zbGF0aW9uU2VydmljZS9Tb3VyY2VzL1RyYW5zbGF0aW9uc1JlcG9zaXRvcnkuc3dpZnQ=) | `93.10% <100.00%> (-0.23%)` | :arrow_down: | | [...ionService/Tests/TranslationsRepositoryTests.swift](https://app.codecov.io/gh/quran/quran-ios/pull/581?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quran#diff-RG9tYWluL1RyYW5zbGF0aW9uU2VydmljZS9UZXN0cy9UcmFuc2xhdGlvbnNSZXBvc2l0b3J5VGVzdHMuc3dpZnQ=) | `100.00% <100.00%> (+21.87%)` | :arrow_up: | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/quran/quran-ios/pull/581/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quran)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.