Changement du type pour le Mandat #815
Conversation
There was a problem hiding this comment.
Je pense que si tu veux avoir un modèle plus propre, il ne faut pas rajouter de la serialization dedans. J'avais créé serializers.DataModel dans cette optique, je pense que c'est la solution la plus simple à maintenir à long terme. Peut-être en renommant JsonDataModel et en créant PgDataModel avec les serializers anorm et des case class qui correspondent aux lignes (genre ApplicationRow) lorsque le modèle ne match plus la BDD.
| implicit val Reads: Reads[Answer] = Json | ||
| .reads[Answer] | ||
| .map(answer => | ||
| answer.copy(creationDate = answer.creationDate.withZoneSameInstant(Time.timeZoneParis)) | ||
| ) | ||
|
|
||
| implicit val Writes: Writes[Answer] = Json.writes[Answer] | ||
|
|
||
| implicit val answerListParser: anorm.Column[List[Answer]] = | ||
| nonNull { (value, meta) => | ||
| val MetaDataItem(qualified, _, _) = meta | ||
| value match { | ||
| case json: org.postgresql.util.PGobject => | ||
| Json.parse(json.getValue).as[List[Answer]].asRight[Nothing] | ||
| case json: String => Json.parse(json).as[List[Answer]].asRight[Nothing] | ||
| case _ => | ||
| TypeDoesNotMatch( | ||
| s"Cannot convert $value: ${className(value)} to List[Answer] for column $qualified" | ||
| ).asLeft[List[Answer]] | ||
| } | ||
| } |
There was a problem hiding this comment.
En fait avoir la logique de serialization dans le modèle ça n'a pas de sens. Ce n'est pas clair, ça mixe de la logique métier avec de la logique de serialization. Aussi juste la dépendance "modèle dépends des serializers anorm/json" ça sens pas bon.
app/models/Application.scala
Outdated
| ) | ||
| } | ||
|
|
||
| final case class Mandat(_type: MandatType, date: String) |
There was a problem hiding this comment.
J'aime pas trop le _ avant le nom, car ça signifie souvent "privé" ou "non utilisé, mais quand même utile" dans certains autres langages. Que penses-tu de type_ ? ou simplement `type` ?
There was a problem hiding this comment.
type_ c'est ok pour moi. A la base j'avais mis type, mais le parsing Twirl n'a pas l'air d'aimer :(
app/models/Application.scala
Outdated
| implicit val Parser: RowParser[Application] = | ||
| (get[UUID]("id") ~ | ||
| get[ZonedDateTime]("creation_date").map(_.withZoneSameInstant(Time.timeZoneParis)) ~ | ||
| str(columnName = "creator_user_name") ~ | ||
| get[UUID]("creator_user_id") ~ | ||
| str(columnName = "subject") ~ | ||
| str(columnName = "description") ~ | ||
| get[Map[String, String]]("user_infos") ~ | ||
| get[Map[UUID, String]]("invited_users") ~ | ||
| get[UUID]("area") ~ | ||
| bool(columnName = "irrelevant") ~ | ||
| get[List[Answer]]("answers") ~ | ||
| int("internal_id") ~ | ||
| bool("closed") ~ | ||
| get[List[UUID]]("seen_by_user_ids") ~ | ||
| str("usefulness").? ~ | ||
| get[ZonedDateTime]("closed_date").? ~ | ||
| bool("expert_invited") ~ | ||
| bool("has_selected_subject") ~ | ||
| str("category").? ~ | ||
| get[Map[String, Long]]("files") ~ | ||
| Application.Mandat.Parser) | ||
| .map { | ||
| case id ~ creation ~ creatorName ~ creatorId ~ subject ~ description ~ userInfos ~ invitedUsers | ||
| ~ area ~ irrelevant ~ answers ~ internalId ~ closed ~ seen ~ usefulness ~ closedDate ~ experts ~ hasSelectedSubject ~ category ~ files ~ mandat => | ||
| Application( | ||
| id, | ||
| creation, | ||
| creatorName, | ||
| creatorId, | ||
| subject, | ||
| description, | ||
| userInfos, | ||
| invitedUsers, | ||
| area, | ||
| irrelevant, | ||
| answers, | ||
| internalId, | ||
| closed, | ||
| seen, | ||
| usefulness, | ||
| closedDate, | ||
| experts, | ||
| hasSelectedSubject, | ||
| category, | ||
| files, | ||
| mandat | ||
| ) | ||
| } |
There was a problem hiding this comment.
Non mais simplement définis une ApplicationRow qui matche la ligne PG, et tu fais des méthodes de conversion Application <=> ApplicationRow, c'est tellement plus simple. Et les erreurs de consistence peuvent être loggués dans la transformation avec un log.warn puisqu'en théorie si ApplicationRow => Application fail, c'est un bug de notre part, ce n'est pas de la faute de l'utilisateur.
| @@ -5,6 +5,7 @@ import java.time.{LocalDate, ZonedDateTime} | |||
| import java.util.UUID | |||
|
|
|||
| import actions._ | |||
| import cats.implicits.catsSyntaxOptionId | |||
There was a problem hiding this comment.
| import cats.implicits.catsSyntaxOptionId |
mdulac
changed the title
| @@ -5,7 +5,7 @@ import java.time.{LocalDate, ZonedDateTime} | |||
| import java.util.UUID | |||
|
|
|||
| import actions._ | |||
| import cats.implicits.catsSyntaxTuple2Semigroupal | |||
| import cats.implicits.{catsSyntaxOptionId, catsSyntaxTuple2Semigroupal} | |||
There was a problem hiding this comment.
| import cats.implicits.{catsSyntaxOptionId, catsSyntaxTuple2Semigroupal} |
tu es sûr que c'est nécessaire ?
|
|
||
| object Answer { | ||
|
|
||
| implicit val Reads: Reads[Answer] = Json | ||
| .reads[Answer] | ||
| .map(answer => | ||
| answer.copy(creationDate = answer.creationDate.withZoneSameInstant(Time.timeZoneParis)) | ||
| ) | ||
|
|
||
| implicit val Writes: Writes[Answer] = Json.writes[Answer] | ||
|
|
||
| implicit val answerListParser: anorm.Column[List[Answer]] = | ||
| nonNull { (value, meta) => | ||
| val MetaDataItem(qualified, _, _) = meta | ||
| value match { | ||
| case json: org.postgresql.util.PGobject => | ||
| Json.parse(json.getValue).as[List[Answer]].asRight[Nothing] | ||
| case json: String => Json.parse(json).as[List[Answer]].asRight[Nothing] | ||
| case _ => | ||
| TypeDoesNotMatch( | ||
| s"Cannot convert $value: ${className(value)} to List[Answer] for column $qualified" | ||
| ).asLeft[List[Answer]] | ||
| } | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
Du coup ces modifs ne sont plus nécessaires ?
| implicit val MandatTypeParser: Column[Option[MandatType]] = | ||
| implicitly[Column[Option[String]]] | ||
| .map(_.flatMap(dataModelDeserialization)) |
| hasSelectedSubject: Boolean = false, | ||
| category: Option[String] = Option.empty[String], | ||
| files: Map[String, Long] = Map.empty[String, Long], | ||
| mandatType: Option[Application.Mandat.MandatType], |
There was a problem hiding this comment.
Pour postgres c'est un varchar, que penses-tu de mettre la conversion dans toApplication plutôt ?
| mandatType: Option[Application.Mandat.MandatType], | |
| mandatType: Option[String], |
No description provided.