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

draft-api: Update priority field #334

Merged
merged 9 commits into from
Nov 14, 2023
35 changes: 35 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,35 @@
/*
* Part of NDLA common.
* Copyright (C) 2023 NDLA
*
* See LICENSE
*/

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 All @@ -57,6 +57,7 @@ object Draft {
Json4s.serializer(DraftStatus),
Json4s.serializer(ArticleType),
Json4s.serializer(RevisionStatus),
Json4s.serializer(Priority),
NDLADate.Json4sSerializer
) ++
JavaTimeSerializers.all ++
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 @@ -10,7 +10,7 @@ package no.ndla.draftapi.integration
import com.typesafe.scalalogging.StrictLogging
import enumeratum.Json4s
import no.ndla.common.model.NDLADate
import no.ndla.common.model.domain.{ArticleType, Availability}
import no.ndla.common.model.domain.{ArticleType, Availability, Priority}
import no.ndla.common.model.domain.draft.{Draft, DraftStatus, RevisionStatus}
import no.ndla.draftapi.Props
import no.ndla.draftapi.service.ConverterService
Expand Down Expand Up @@ -41,6 +41,7 @@ trait SearchApiClient {
new EnumNameSerializer(Availability) +
Json4s.serializer(DraftStatus) +
Json4s.serializer(ArticleType) +
Json4s.serializer(Priority) +
Json4s.serializer(RevisionStatus) ++
JavaTimeSerializers.all ++
JavaTypesSerializers.all +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,5 +47,6 @@ case class Article(
@(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, unspecified") 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,6 @@ 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") prioritized: Option[Boolean],
@(ApiModelProperty @field)(description = "If the article should be prioritized. Possible values are prioritized, on-hold, unspecified") priority: Option[String]
)
Original file line number Diff line number Diff line change
Expand Up @@ -41,5 +41,6 @@ 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") prioritized: Option[Boolean],
@(ApiModelProperty @field)(description = "If the article should be prioritized. Possible values are prioritized, on-hold, unspecified") 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 @@ -76,6 +76,15 @@ trait ConverterService {
Responsible(responsibleId = responsibleId, lastUpdated = clock.now())
)

val priority = newArticle.priority
.flatMap(x => common.Priority.valueOfOrError(x).toOption)
.getOrElse(
newArticle.prioritized match {
case Some(true) => Priority.Prioritized
case _ => Priority.Unspecified
}
)

newNotes(newArticle.notes, user, status).map(notes =>
Draft(
id = Some(newArticleId),
Expand Down Expand Up @@ -116,7 +125,7 @@ trait ConverterService {
responsible = responsible,
slug = newArticle.slug,
comments = newCommentToDomain(newArticle.comments),
prioritized = newArticle.prioritized.getOrElse(false),
priority = priority,
started = false
)
)
Expand Down Expand Up @@ -388,7 +397,8 @@ trait ConverterService {
responsible = responsible,
slug = article.slug,
comments = article.comments.map(toApiComment),
prioritized = article.prioritized,
prioritized = article.priority == Priority.Prioritized,
priority = article.priority.entryName,
started = article.started
)
)
Expand Down Expand Up @@ -686,6 +696,16 @@ 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(
article.prioritized match {
case Some(true) => common.Priority.Prioritized
case Some(false) => common.Priority.Unspecified
case None => toMergeInto.priority
}
)

failableFields match {
case Failure(ex) => Failure(ex)
case Success((allNotes, newContent)) =>
Expand All @@ -708,7 +728,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 +815,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 +851,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
Loading