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

Only set hideByline if not already set #571

Merged
merged 2 commits into from
Jan 8, 2025

Conversation

gunnarvelle
Copy link
Member

Dette fikser en logisk feil i forrige versjon av migreringa. Det som skjedde då var at om hidebyline var satt så blei ikkje bildeid sendt til imageid, noko som førte til at hidebyline blei satt til false for bilder som var satt inn med hidebyline=true fordi images.find ikkje har bildet som sjekkes.
Dette henter litt fleire bilder fra image-api, men oppdaterer kun dei som ikkje har verdien satt allerede.

@gunnarvelle gunnarvelle requested a review from a team January 7, 2025 12:32
val image = images.find(i => i.id == imageId)
embed
.attr("data-hide-byline", s"${image.exists(i => !i.copyright.license.license.equals("COPYRIGHTED"))}"): Unit
val noHideByline = !embed.hasAttr("data-hide-byline")
Copy link
Contributor

Choose a reason for hiding this comment

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

Det ble aldri satt data-hide-byline=false?

Copy link
Member Author

Choose a reason for hiding this comment

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

Mulig, men tanken min var å droppe å sette dersom verdien allerede er satt.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ble disse aldri kjørt siden vi oppdaterer migreringer fremfor å lage nye?

Copy link
Member Author

Choose a reason for hiding this comment

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

Den er ikkje kjørt i prod endå. Skal kopiere data fra prod til staging for å verifisere.

@gunnarvelle gunnarvelle merged commit 3e9f1f4 into master Jan 8, 2025
8 checks passed
@gunnarvelle gunnarvelle deleted the only-update-hide-byline-if-not-set branch January 8, 2025 10:36
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