[elfkuzco]

Rationale

This PR adds the logic to automatically update title languages from book in the logic to add book to title which is called when book is being processed or re-processed. Logic for automatic update of title language is described in #527 (comment)

Changes

• add env variable MIN_LANGUAGES_FOR_TITLE_SYNC to set how many languages both book and title need to have for automatic update to run
• add function to update title languages from book
• move update_title_metadata_from_book to title module due to cyclic imports

This closes #527

[codecov]

Codecov Report

❌ Patch coverage is 72.82609% with 25 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.78%. Comparing base (3b393e9) to head (8fd5af0).
⚠️ Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
backend/src/cms_backend/utils/database.py	26.66%	11 Missing ⚠️
backend/src/cms_backend/db/title.py	75.00%	10 Missing ⚠️
backend/src/cms_backend/db/book_actions.py	85.71%	1 Missing and 1 partial ⚠️
backend/src/cms_backend/mill/main.py	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #610      +/-   ##
==========================================
- Coverage   81.92%   81.78%   -0.15%     
==========================================
  Files          78       78              
  Lines        5323     5375      +52     
  Branches      657      668      +11     
==========================================
+ Hits         4361     4396      +35     
- Misses        762      778      +16     
- Partials      200      201       +1     

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[benoit74]

I realize now what itched me since quite some time: updating the title must be done only when book is moved to production. A book blocked in staging (and which might be deleted without ever going to production) should not update the title yet.

This is true for the language but also for all other metadata. This is also what will allow editor to validate book metadata on first addition of a book to a blank title.

In other word, we alter the title metadata only when book is automatically moved to production or manually promoted.

I propose to move this to a dedicated issue unless you feel like it could be solved in this same PR. I let you decide and act accordingly (update PR or open issue linked to this comment).

[elfkuzco]

I realize now what itched me since quite some time: updating the title must be done only when book is moved to production. A book blocked in staging (and which might be deleted without ever going to production) should not update the title yet.

What about when it's the first book and title is already missing all mandatory metadata? Book won't ever be moved to production if it doesn't update the mandatory metadata

[benoit74]

What about when it's the first book and title is already missing all mandatory metadata?

First books should force a manual validation of all metadata. Not moving them to production directly is what we want in fact, this has been an oversight so far IMHO.

[elfkuzco]

This is true for the language but also for all other metadata. This is also what will allow editor to validate book metadata on first addition of a book to a blank title.

In other word, we alter the title metadata only when book is automatically moved to production or manually promoted.

Sorry but I'm a bit conflicted on the purpose of this PR. I thought it was supposed to reduce the number of books blocked in staging by ensuring language issue isn't the blocker.

Looking at the second sentence, I don't see how book will be automatically moved to prod if it's blocked because of a language issue because the issues get computed before deciding on it's destination. So, essentially, this PR will not prevent books from being blocked in staging and will still require editor to manually let them through. Only benefit can come from if we want to show editor that we will update languages in the book actions .

[benoit74]

Sorry but I'm a bit conflicted on the purpose of this PR. I thought it was supposed to reduce the number of books blocked in staging by ensuring language issue isn't the blocker.

Looking at the second sentence, I don't see how book will be automatically moved to prod if it's blocked because of a language issue because the issues get computed before deciding on it's destination. So, essentially, this PR will not prevent books from being blocked in staging and will still require editor to manually let them through. Only benefit can come from if we want to show editor that we will update languages in the book actions .

This is indeed the hard part of this PR. I promised I did not silenced it on purpose ^^

We do not want to immediately update the title until we know the book will move to production, but we need to ignore the language changes under some conditions to take the decision about book quality (staging or production) and to raise issues (we do not want to warn the user) and still update title with these changes once decision has been made to move the book to production (automated or manual promotion).

For instance I feel like this means we should flag these not as "informational" issues.

Is it clearer? Note that I might still miss some details.

I also don't have a very strong idea about how we can implement this in a efficient way (avoiding code duplicates) so I'm open to suggestions, including the fact this is just too complex or that for simplicity we should have some code duplication (with proper tests, it is not that much of a concern, especially if some of the logic or at least constants are shared like you proposed so far).

[elfkuzco]

We do not want to immediately update the title until we know the book will move to production, but we need to ignore the language changes under some conditions to take the decision about book quality (staging or production) and to raise issues (we do not want to warn the user) and still update title with these changes once decision has been made to move the book to production (automated or manual promotion).

I have updated the code to include a predicate function that determines whether to include Language as a contributing factor to metadata mismatch issue. This way, we can update the title only when we know the book will move to production (as there are no metadata issues)

For instance I feel like this means we should flag these not as "informational" issues.

i think you are talking about unknown_language issue which is a separate issue arising from language iso-code being unknown. This can be overriden by env var to support unknown languages if pycountry becomes outdated.

[benoit74]

Thank you, one minor change

[benoit74]

i think you are talking about unknown_language issue which is a separate issue arising from language iso-code being unknown. This can be overriden by env var to support unknown languages if pycountry becomes outdated.

Nope, I was referring to the fact that with this PR (actual) code, when the book is blocked in staging for other purposes and the user fetches promotion actions, then you do not show at all in promotion action that Language will get updated when it has changed is ignored. This is a bit misleading and I was anticipating that we might want to show that information. I still don't know is this is pure complexity on my side and no-one will care to not show that information, or if we should show this as "informational" (it will happen in all cases).

[elfkuzco]

i think you are talking about unknown_language issue which is a separate issue arising from language iso-code being unknown. This can be overriden by env var to support unknown languages if pycountry becomes outdated.

Nope, I was referring to the fact that with this PR (actual) code, when the book is blocked in staging for other purposes and the user fetches promotion actions, then you do not show at all in promotion action that Language will get updated when it has changed is ignored. This is a bit misleading and I was anticipating that we might want to show that information. I still don't know is this is pure complexity on my side and no-one will care to not show that information, or if we should show this as "informational" (it will happen in all cases).

I have added a new action type update_title_language which is purely informational so user knows/sees the change

[benoit74]

LGTM, thank you