From 279ebe6ec46e2526bba9f6bd90c5e032a4e8a9de Mon Sep 17 00:00:00 2001 From: Tony <43834836+lofoyet@users.noreply.github.com> Date: Wed, 3 Apr 2024 15:09:00 -0700 Subject: [PATCH] IHRDS 3363 note section for test (#933) * added note section and display the first line of note in the preview * add edit -> save logic and display * unify placeholder text --- .../com/iheart/thomas/abtest/AbtestAlg.scala | 12 ++++++ .../iheart/thomas/abtest/model/package.scala | 8 +++- .../http4s/abtest/AbtestManagementUI.scala | 39 ++++++++++++++++-- .../thomas/abtest/admin/index.scala.html | 5 ++- .../thomas/abtest/admin/showTest.scala.html | 40 ++++++++++++++++--- .../thomas/abtest/admin/testForm.scala.html | 17 +++++++- .../twirl/com/iheart/thomas/main.scala.html | 5 ++- 7 files changed, 112 insertions(+), 14 deletions(-) diff --git a/core/src/main/scala/com/iheart/thomas/abtest/AbtestAlg.scala b/core/src/main/scala/com/iheart/thomas/abtest/AbtestAlg.scala index d43906d3..94beab77 100644 --- a/core/src/main/scala/com/iheart/thomas/abtest/AbtestAlg.scala +++ b/core/src/main/scala/com/iheart/thomas/abtest/AbtestAlg.scala @@ -53,6 +53,11 @@ trait AbtestAlg[F[_]] extends TestsDataProvider[F] with FeatureRetriever[F] { spec: AbtestSpec ): F[Entity[Abtest]] + def updateTestNote( + testId: TestId, + note: String + ): F[Entity[Abtest]] + /** @param feature * @return * tests for this feature in chronological descending order @@ -294,6 +299,13 @@ final class DefaultAbtestAlg[F[_]]( } yield r + override def updateTestNote(testId: TestId, note: String): F[Entity[Abtest]] = { + for { + test <- getTest(testId) + r <- abTestDao.update(test.copy(data = test.data.copy(note = Some(note)))) + } yield r + } + def continue(testSpec: AbtestSpec): F[Entity[Abtest]] = addTestWithLock(testSpec) { for { diff --git a/core/src/main/scala/com/iheart/thomas/abtest/model/package.scala b/core/src/main/scala/com/iheart/thomas/abtest/model/package.scala index 0745a3ac..5da014e3 100644 --- a/core/src/main/scala/com/iheart/thomas/abtest/model/package.scala +++ b/core/src/main/scala/com/iheart/thomas/abtest/model/package.scala @@ -55,7 +55,8 @@ package model { userMetaCriteria: UserMetaCriteria = None, salt: Option[String] = None, segmentRanges: List[GroupRange] = Nil, - specialization: Option[Abtest.Specialization] = None) { + specialization: Option[Abtest.Specialization] = None, + note: Option[String] = None){ val hasEligibilityControl: Boolean = requiredTags.nonEmpty || userMetaCriteria.nonEmpty @@ -101,6 +102,8 @@ package model { end = end.map(_.toOffsetDateTimeSystemDefault), groups = groups.map(g => g.copy(meta = g.meta)) ) + + def shortNote: String = note.map(_.split("\n").head).getOrElse("") } /** Data used to create an A/B tests @@ -160,7 +163,8 @@ package model { userMetaCriteria: UserMetaCriteria = None, reshuffle: Boolean = false, segmentRanges: List[GroupRange] = Nil, - specialization: Option[Abtest.Specialization] = None) { + specialization: Option[Abtest.Specialization] = None, + note: Option[String] = None) { val startI = start.toInstant val endI = end.map(_.toInstant) diff --git a/http4s/src/main/scala/com/iheart/thomas/http4s/abtest/AbtestManagementUI.scala b/http4s/src/main/scala/com/iheart/thomas/http4s/abtest/AbtestManagementUI.scala index e633abb6..dd18a369 100644 --- a/http4s/src/main/scala/com/iheart/thomas/http4s/abtest/AbtestManagementUI.scala +++ b/http4s/src/main/scala/com/iheart/thomas/http4s/abtest/AbtestManagementUI.scala @@ -359,6 +359,29 @@ class AbtestManagementUI[F[_]]( Ok(redirect(reverseRoutes.tests, s"Successfully $message.")) } + case se @ POST -> Root / "tests" / testId / "note" asAuthed u => + get(testId).flatMap { test => + se.request + .as[NoteForm] + .redeemWith( + e => BadRequest(errorMsg(displayError(e))), + note => + alg + .updateTestNote( + testId.coerce[EntityId], + note.note.get + ) + .flatMap(redirectToTest) + .handleErrorWith(_ => + u.canManage(test) + .flatMap(_ => + BadRequest( + ) + ) + ) + ) + } + // Add new test to a feature case se @ POST -> Root / "features" / feature / "tests" asAuthed u => se.request @@ -551,7 +574,8 @@ object AbtestManagementUI { alternativeIdName: Option[MetaFieldName] = None, userMetaCriteria: UserMetaCriteria = None, reshuffle: Boolean = false, - segmentRanges: List[GroupRange] = Nil) { + segmentRanges: List[GroupRange] = Nil, + note: Option[String] = None) { def toAbtestSpec[F[_]: Functor: Clock]( u: User, feature: FeatureName @@ -568,13 +592,16 @@ object AbtestManagementUI { alternativeIdName = alternativeIdName, userMetaCriteria = userMetaCriteria, reshuffle = reshuffle, - segmentRanges = segmentRanges + segmentRanges = segmentRanges, + note = note ) } } } + case class NoteForm(note: Option[String] = Some("")) + implicit val userGroupQueryFormDecoder: FormDataDecoder[UserGroupQuery] = { implicit val mapQPD = mapQueryParamDecoder ( @@ -595,9 +622,15 @@ object AbtestManagementUI { fieldOptional[MetaFieldName]("alternativeIdName"), fieldOptional[UserMetaCriterion.And]("userMetaCriteria"), fieldEither[Boolean]("reshuffle").default(false), - list[GroupRange]("segmentRanges") + list[GroupRange]("segmentRanges"), + fieldOptional[String]("note") ).mapN(SpecForm.apply).sanitized + implicit val noteFormDecoder: FormDataDecoder[NoteForm] = + ( + fieldOptional[String]("note") + ).map(NoteForm.apply).sanitized + implicit val FeatureFormDecoder: FormDataDecoder[Feature] = { implicit val mapQPD = mapQueryParamDecoder diff --git a/http4s/src/main/twirl/com/iheart/thomas/abtest/admin/index.scala.html b/http4s/src/main/twirl/com/iheart/thomas/abtest/admin/index.scala.html index e360c985..b0bf9da5 100644 --- a/http4s/src/main/twirl/com/iheart/thomas/abtest/admin/index.scala.html +++ b/http4s/src/main/twirl/com/iheart/thomas/abtest/admin/index.scala.html @@ -42,7 +42,7 @@
-
+
@test.data.name @@ -69,6 +69,9 @@
+
+
@test.data.shortNote
+
diff --git a/http4s/src/main/twirl/com/iheart/thomas/abtest/admin/showTest.scala.html b/http4s/src/main/twirl/com/iheart/thomas/abtest/admin/showTest.scala.html index 4dfa162c..696bdd65 100644 --- a/http4s/src/main/twirl/com/iheart/thomas/abtest/admin/showTest.scala.html +++ b/http4s/src/main/twirl/com/iheart/thomas/abtest/admin/showTest.scala.html @@ -20,7 +20,6 @@ @topNav("A/B Test for feature " + test.data.feature, "A/B Tests") { -
A/B Test @test.data.name for @@ -93,8 +92,27 @@ }
+
+
+
+
+ Description +
+
+ + + +
+ + +
+ +
+
+
+
-
+
@@ -132,8 +150,20 @@
- - } - + diff --git a/http4s/src/main/twirl/com/iheart/thomas/abtest/admin/testForm.scala.html b/http4s/src/main/twirl/com/iheart/thomas/abtest/admin/testForm.scala.html index 974302d2..ad1fa7a0 100644 --- a/http4s/src/main/twirl/com/iheart/thomas/abtest/admin/testForm.scala.html +++ b/http4s/src/main/twirl/com/iheart/thomas/abtest/admin/testForm.scala.html @@ -20,7 +20,8 @@ @msg
} -
+ +
@@ -100,6 +101,20 @@
+@if(!readonly) { +
+
+ Description +
+
+
+ + +
+
+
+} +