Skip to content

Add published count#856

Open
gunnarvelle wants to merge 7 commits intomasterfrom
add-published-count
Open

Add published count#856
gunnarvelle wants to merge 7 commits intomasterfrom
add-published-count

Conversation

@gunnarvelle
Copy link
Member

https://github.com/NDLANO/Issues/issues/4493

Legger til publishedCount på draft, article og i search-api.

@gunnarvelle gunnarvelle marked this pull request as ready for review February 3, 2026 13:24
@gunnarvelle gunnarvelle requested a review from a team February 4, 2026 07:16
@jnatten
Copy link
Contributor

jnatten commented Feb 4, 2026

Som jeg skrev på issuet, vil egentlig en published-count løse problemet de beskriver på trello?
Vil de ikke egentlig sortere artikler basert på første gangen den ble publisert?

Copy link
Contributor

@amatho amatho left a comment

Choose a reason for hiding this comment

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

Ser ut som migreringene funker fint, men det mangler logikk for å oppdatere publishedCount ved vanlig publiseringsflyt (både i draft-api og article-api)

override lazy val whereClause: SQLSyntax = sqls"$columnNameSQL is not null"

private def countOtherVersions(revision: Int, articleId: Long)(implicit session: DBSession): Long = {
sql"select count(*) from $tableNameSQL where revision < $revision and article_id = $articleId"
Copy link
Contributor

Choose a reason for hiding this comment

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

Ser at det tar ganske lang tid å kjøre denne lokalt med dump fra test, men ikke sikkert det er verdt bryet å skulle skrive den raskere 😄

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 jo avhengig av å telle opp gamle varianter av artikler, så tar jo tid. Kom gjerne med forslag til måter å gjøre den kjappere.

Copy link
Contributor

Choose a reason for hiding this comment

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

Virker som det går an å gjøre det med en SQL-only migrering, men det ble ganske syre, så tenker det er fint som det er 😄

Copy link
Contributor

Choose a reason for hiding this comment

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

Kanskje lurt med noen tester på disse migreringene?

Copy link
Member Author

Choose a reason for hiding this comment

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

Migreringene er avhengig av nye spørringer, så blir fort litt komplekst. Men kan sikkert mocke noe her. Ser på det.

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.

3 participants