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

Convert disclaimer to new OptLanguageFields type #578

Merged
merged 4 commits into from
Jan 16, 2025

Conversation

jnatten
Copy link
Contributor

@jnatten jnatten commented Jan 16, 2025

Denne PR'en er større enn den trenger å være for å fikse problemet, men jeg tror språktypene kan bli enklere å jobbe med i fremtiden!
Min drøm er at alle språktypene våre kan konverteres til denne (Evt LanguageFields typen som er veldig lik).

Sånn det funker:
LanguageFields (Opt og ikke) serialiseres til et objekt istedenfor en liste.
Feks:

{ "nb": "Dette er en bokmål disclaimer", "nn": "Dette er ein nynorsk disclaimer" }

Den som heter OptLanguageFields lar deg også definere at et språk ikke skal ha en variant.
Feks lar det oss gjøre at disclaimer kan ha en bokmål versjon, og nynorsk versjonen fallbacker til bokmål, mens engelsk har ingenting og returnerer heller ingen fallback.

Hopefully these types will be used with all stored language fields in
the future.
This patch adds a migration of disclaimer to `OptLanguageFields` as well
as retypes the `diclaimer` of the domain type to `OptLanguageFields`.
@jnatten jnatten requested a review from a team January 16, 2025 12:10
Copy link
Member

@gunnarvelle gunnarvelle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Korleis skal ed bruke dette? Vil en tom streng i disclaimer på en ny språkversjon bli
language:nn, value:__notwanted__? Og om du sender inn feks " " så blir det lagra slik?

Comment on lines 30 to 37
val oldArticle = parser.parse(document).toTry.get
oldArticle.hcursor.downField(fieldName).focus match {
case None => addEmptyLanguageField(oldArticle)
case Some(f) if f.isNull => addEmptyLanguageField(oldArticle)
case Some(disclaimers) =>
val disclaimerVector = disclaimers.asArray.get
val converted = convertOldLanguageField(disclaimerVector)
val newArticle = oldArticle.withObject(_.remove(fieldName).add(fieldName, converted).toJson)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Denne er vel generisk så kanskje navna også skal være det? oldDocument og Some(values)

@jnatten
Copy link
Contributor Author

jnatten commented Jan 16, 2025

Korleis skal ed bruke dette? Vil en tom streng i disclaimer på en ny språkversjon bli language:nn, value:__notwanted__? Og om du sender inn feks " " så blir det lagra slik?

Ed forholder ikke seg noe annerledes til dette egentlig. Så ja, om man sender inn tom-streng (i alle fall i disclaimer sitt tilfelle) så blir det lagret som NotWanted klassen som gjør at uthenting short-circuit'er før den prøver å lete etter fallback språkene.

@jnatten jnatten force-pushed the optional-language-rework branch from 30af8ca to ab6e40a Compare January 16, 2025 12:37
Copy link
Member

@gunnarvelle gunnarvelle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fint å få dette inn

@jnatten jnatten merged commit d88a95f into master Jan 16, 2025
40 checks passed
@jnatten jnatten deleted the optional-language-rework branch January 16, 2025 13:32
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.

2 participants