Skip to content

Commit

Permalink
Update priority field
Browse files Browse the repository at this point in the history
- Change naming prioritized -> priority
- Update priority to either be prioritized, on-hold or unspecified
- Add migration to update articles
  • Loading branch information
katrinewi committed Nov 10, 2023
1 parent 28f9e72 commit 657b155
Show file tree
Hide file tree
Showing 28 changed files with 252 additions and 142 deletions.
28 changes: 28 additions & 0 deletions common/src/main/scala/no/ndla/common/model/domain/Priority.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package no.ndla.common.model.domain
import enumeratum._
import no.ndla.common.errors.ValidationException

import scala.util.{Failure, Success, Try}

sealed abstract class Priority(override val entryName: String) extends EnumEntry

object Priority extends Enum[Priority] {
case object Prioritized extends Priority("prioritized")
case object OnHold extends Priority("on-hold")
case object Unspecified extends Priority("unspecified")

val values: IndexedSeq[Priority] = findValues

def all: Seq[String] = Priority.values.map(_.entryName)
def valueOf(s: String): Option[Priority] = Priority.withNameOption(s)

def valueOfOrError(s: String): Try[Priority] =
valueOf(s) match {
case Some(p) => Success(p)
case None =>
val validPriorities = values.map(_.toString).mkString(", ")
Failure(
ValidationException("priority", s"'$s' is not a valid priority. Must be one of $validPriorities")
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ case class Draft(
responsible: Option[Responsible],
slug: Option[String],
comments: Seq[Comment],
prioritized: Boolean,
priority: Priority,
started: Boolean
) extends Content {

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/*
* Part of NDLA draft-api.
* Copyright (C) 2023 NDLA
*
* See LICENSE
*/

package no.ndla.draftapi.db.migration

import io.circe.syntax.EncoderOps
import io.circe.{parser}
import org.flywaydb.core.api.migration.{BaseJavaMigration, Context}
import org.postgresql.util.PGobject
import scalikejdbc.{DB, DBSession, _}

class V52__UpdatePrioritizedField extends BaseJavaMigration {

def countAllRows(implicit session: DBSession): Option[Long] = {
sql"select count(*) from articledata where document is not NULL"
.map(rs => rs.long("count"))
.single()
}

def allRows(offset: Long)(implicit session: DBSession): Seq[(Long, String)] = {
sql"select id, document, article_id from articledata where document is not null order by id limit 1000 offset $offset"
.map(rs => {
(rs.long("id"), rs.string("document"))
})
.list()
}

def updateRow(document: String, id: Long)(implicit session: DBSession): Int = {
val dataObject = new PGobject()
dataObject.setType("jsonb")
dataObject.setValue(document)

sql"update articledata set document = $dataObject where id = $id"
.update()
}

override def migrate(context: Context): Unit = DB(context.getConnection)
.autoClose(false)
.withinTx { session => migrateRows(session) }

def migrateRows(implicit session: DBSession): Unit = {
val count = countAllRows.get
var numPagesLeft = (count / 1000) + 1
var offset = 0L

while (numPagesLeft > 0) {
allRows(offset * 1000).map { case (id, document) =>
updateRow(convertDocument(document), id)
}: Unit
numPagesLeft -= 1
offset += 1
}
}

private[migration] def convertDocument(document: String): String = {
val oldArticle = parser.parse(document).toTry.get
val cursor = oldArticle.hcursor
val oldPrioritized = cursor.downField("prioritized").as[Boolean].toTry.get
val newValue = if (oldPrioritized) {
"prioritized"
} else {
"unspecified"
}
val newJson = cursor.withFocus(_.mapObject(obj => obj.add("priority", newValue.asJson).remove("prioritized")))
newJson.top.get.noSpaces
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,6 @@ case class Article(
@(ApiModelProperty @field)(description = "Object with data representing the editor responsible for this article") responsible: Option[DraftResponsible],
@(ApiModelProperty @field)(description = "The path to the frontpage article") slug: Option[String],
@(ApiModelProperty @field)(description = "Information about comments attached to the article") comments: Seq[Comment],
@(ApiModelProperty @field)(description = "If the article should be prioritized") prioritized: Boolean,
@(ApiModelProperty @field)(description = "If the article should be prioritized. Possible values are prioritized, on-hold, undefined") priority: String,
@(ApiModelProperty @field)(description = "If the article has been edited after last status or responsible change") started: Boolean
)
Original file line number Diff line number Diff line change
Expand Up @@ -38,5 +38,5 @@ case class NewArticle(
@(ApiModelProperty @field)(description = "NDLA ID representing the editor responsible for this article") responsibleId: Option[String],
@(ApiModelProperty @field)(description = "The path to the frontpage article") slug: Option[String],
@(ApiModelProperty @field)(description = "Information about a comment attached to an article") comments: List[NewComment],
@(ApiModelProperty @field)(description = "If the article should be prioritized") prioritized: Option[Boolean]
@(ApiModelProperty @field)(description = "If the article should be prioritized. Possible values are prioritized, on-hold, undefined") priority: Option[String]
)
Original file line number Diff line number Diff line change
Expand Up @@ -41,5 +41,5 @@ case class UpdatedArticle(
@(ApiModelProperty @field)(description = "NDLA ID representing the editor responsible for this article") responsibleId: Deletable[String],
@(ApiModelProperty @field)(description = "The path to the frontpage article") slug: Option[String],
@(ApiModelProperty @field)(description = "Information about a comment attached to an article") comments: Option[List[UpdatedComment]],
@(ApiModelProperty @field)(description = "If the article should be prioritized") prioritized: Option[Boolean]
@(ApiModelProperty @field)(description = "If the article should be prioritized. Possible values are prioritized, on-hold, undefined") priority: Option[String]
)
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ package no.ndla.draftapi.repository
import com.typesafe.scalalogging.StrictLogging
import no.ndla.common.Clock
import no.ndla.common.errors.RollbackException
import no.ndla.common.model.domain.EditorNote
import no.ndla.common.model.domain.{EditorNote, Priority}
import no.ndla.common.model.domain.draft.{Draft, DraftStatus}
import no.ndla.draftapi.integration.DataSource
import no.ndla.draftapi.model.api.{ArticleVersioningException, ErrorHelpers, GenerateIDException, NotFoundException}
Expand Down Expand Up @@ -118,7 +118,7 @@ trait DraftRepository {
.toList,
previousVersionsNotes = article.previousVersionsNotes ++ article.notes,
responsible = if (keepDraftData) article.responsible else None,
prioritized = if (keepDraftData) article.prioritized else false,
priority = if (keepDraftData) article.priority else Priority.Unspecified,
comments = if (keepDraftData) article.comments else Seq.empty
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import com.typesafe.scalalogging.StrictLogging
import no.ndla.common.configuration.Constants.EmbedTagName
import no.ndla.common.errors.{ValidationException, ValidationMessage}
import no.ndla.common.model.api.{DraftCopyright, draft}
import no.ndla.common.model.domain.Responsible
import no.ndla.common.model.domain.{Priority, Responsible}
import no.ndla.common.model.domain.draft.DraftStatus.{IMPORTED, PLANNED}
import no.ndla.common.model.domain.draft.{Comment, Draft, DraftStatus}
import no.ndla.common.model.{NDLADate, RelatedContentLink, api => commonApi, domain => common}
Expand Down Expand Up @@ -116,7 +116,9 @@ trait ConverterService {
responsible = responsible,
slug = newArticle.slug,
comments = newCommentToDomain(newArticle.comments),
prioritized = newArticle.prioritized.getOrElse(false),
priority = common.Priority
.valueOfOrError(newArticle.priority.getOrElse(Priority.Unspecified.entryName))
.getOrElse(common.Priority.Unspecified),
started = false
)
)
Expand Down Expand Up @@ -388,7 +390,7 @@ trait ConverterService {
responsible = responsible,
slug = article.slug,
comments = article.comments.map(toApiComment),
prioritized = article.prioritized,
priority = article.priority.entryName,
started = article.started
)
)
Expand Down Expand Up @@ -686,6 +688,10 @@ trait ConverterService {

val copyright = article.copyright.map(toDomainCopyright).orElse(toMergeInto.copyright)

val priority = article.priority
.map(v => common.Priority.valueOfOrError(v).getOrElse(toMergeInto.priority))
.getOrElse(toMergeInto.priority)

failableFields match {
case Failure(ex) => Failure(ex)
case Success((allNotes, newContent)) =>
Expand All @@ -708,7 +714,7 @@ trait ConverterService {
responsible = responsible,
slug = article.slug.orElse(toMergeInto.slug),
comments = updatedComments,
prioritized = article.prioritized.getOrElse(toMergeInto.prioritized)
priority = priority
)

val articleWithNewContent = article.copy(content = newContent)
Expand Down Expand Up @@ -795,6 +801,10 @@ trait ConverterService {
.map(common.ArticleType.valueOfOrError)
.getOrElse(common.ArticleType.Standard)

val priority = common.Priority
.valueOfOrError(article.priority.getOrElse(Priority.Unspecified.entryName))
.getOrElse(common.Priority.Unspecified)

for {
comments <- updatedCommentToDomainNullDocument(article.comments.getOrElse(List.empty))
notes <- mergedNotes
Expand Down Expand Up @@ -827,7 +837,7 @@ trait ConverterService {
responsible = responsible,
slug = article.slug,
comments = comments,
prioritized = article.prioritized.getOrElse(false),
priority = priority,
started = false
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import no.ndla.common.ContentURIUtil.parseArticleIdAndRevision
import no.ndla.common.configuration.Constants.EmbedTagName
import no.ndla.common.errors.ValidationException
import no.ndla.common.implicits.TryQuestionMark
import no.ndla.common.model.domain.Responsible
import no.ndla.common.model.domain.{Priority, Responsible}
import no.ndla.common.model.domain.draft.DraftStatus.{IN_PROGRESS, PLANNED, PUBLISHED}
import no.ndla.common.model.domain.draft.{Draft, DraftStatus}
import no.ndla.common.model.{NDLADate, domain => common}
Expand Down Expand Up @@ -333,6 +333,19 @@ trait WriteService {
updatedArticle.copy(notes = updatedArticle.notes ++ notes ++ deleted)
}

private def hasResponsibleBeenUpdated(
draft: Draft,
oldDraft: Option[Draft]
): Boolean = {
draft.responsible match {
case None => false
case Some(responsible) =>
val oldResponsibleId = oldDraft.flatMap(_.responsible).map(_.responsibleId)
val hasNewResponsible = !oldResponsibleId.contains(responsible.responsibleId)
hasNewResponsible
}
}

private def updateStartedField(
draft: Draft,
oldDraft: Option[Draft],
Expand All @@ -349,19 +362,27 @@ trait WriteService {
} else if (isAutomaticOnEditTransition && statusWasUpdated) {
draft.copy(started = true)
} else {
val responsibleIdWasUpdated = draft.responsible match {
case None => false
case Some(responsible) =>
val oldResponsibleId = oldDraft.flatMap(_.responsible).map(_.responsibleId)
val hasNewResponsible = !oldResponsibleId.contains(responsible.responsibleId)
hasNewResponsible
}
val responsibleIdWasUpdated = hasResponsibleBeenUpdated(draft, oldDraft)

val shouldReset = statusWasUpdated && !isAutomaticStatusChange || responsibleIdWasUpdated
draft.copy(started = !shouldReset)
}
}

private def updatePriorityField(
draft: Draft,
oldDraft: Option[Draft],
statusWasUpdated: Boolean
): Draft = {
if (draft.priority == Priority.OnHold) {
val responsibleIdWasUpdated = hasResponsibleBeenUpdated(draft, oldDraft)
if (responsibleIdWasUpdated || statusWasUpdated) {
draft.copy(priority = Priority.Unspecified)
} else draft
} else draft

}

private def addPartialPublishNote(
draft: Draft,
user: TokenUser,
Expand Down Expand Up @@ -395,11 +416,13 @@ trait WriteService {
updatedApiArticle,
shouldNotAutoUpdateStatus
)
val withPriority =
updatePriorityField(withStarted, oldArticle, statusWasUpdated)

for {
_ <- contentValidator.validateArticleOnLanguage(toUpdate, language)
domainArticle <- performArticleUpdate(
withStarted,
withPriority,
externalIds,
externalSubjectIds,
isImported,
Expand Down Expand Up @@ -448,7 +471,7 @@ trait WriteService {
tags = Seq.empty,
revisionMeta = Seq.empty,
comments = List.empty,
prioritized = false,
priority = Priority.Unspecified,
started = false,
// LanguageField ordering shouldn't matter:
visualElement = article.visualElement.sorted,
Expand Down
19 changes: 10 additions & 9 deletions draft-api/src/test/scala/no/ndla/draftapi/TestData.scala
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ package no.ndla.draftapi
import no.ndla.common.configuration.Constants.EmbedTagName
import no.ndla.common.model
import no.ndla.common.model.api.DraftCopyright
import no.ndla.common.model.domain.Priority
import no.ndla.common.model.domain.draft.Draft
import no.ndla.common.model.domain.draft.DraftStatus._
import no.ndla.common.model.{NDLADate, api => commonApi, domain => common}
Expand Down Expand Up @@ -107,8 +108,8 @@ object TestData {
responsible = None,
None,
Seq.empty,
false,
false
priority = Priority.Unspecified.entryName,
started = false
)

val blankUpdatedArticle: UpdatedArticle = api.UpdatedArticle(
Expand Down Expand Up @@ -222,7 +223,7 @@ object TestData {
None,
None,
Seq.empty,
false,
Priority.Unspecified.entryName,
false
)

Expand Down Expand Up @@ -273,7 +274,7 @@ object TestData {
None,
None,
Seq.empty,
false,
Priority.Unspecified.entryName,
false
)

Expand Down Expand Up @@ -306,7 +307,7 @@ object TestData {
None,
None,
Seq.empty,
false,
Priority.Unspecified,
false
)

Expand Down Expand Up @@ -339,7 +340,7 @@ object TestData {
None,
None,
Seq.empty,
false,
Priority.Unspecified,
false
)

Expand Down Expand Up @@ -374,7 +375,7 @@ object TestData {
None,
None,
Seq.empty,
false,
Priority.Unspecified,
false
)

Expand Down Expand Up @@ -460,7 +461,7 @@ object TestData {
None,
None,
Seq.empty,
false,
Priority.Unspecified,
false
)

Expand Down Expand Up @@ -514,7 +515,7 @@ object TestData {
None,
None,
Seq.empty,
false,
Priority.Unspecified.entryName,
false
)

Expand Down
Loading

0 comments on commit 657b155

Please sign in to comment.