From 05c01b358ec6a7cffbf12cf0fd40c9ec2be7aa03 Mon Sep 17 00:00:00 2001 From: Trevyn Langsford Date: Tue, 12 Sep 2023 16:18:48 -0400 Subject: [PATCH] ID-779 Add some new fields to user profiles (#1209) * ID-779 Add some new fields to user profiles * more test coverage * update swagger docs --- src/main/resources/swagger/api-docs.yaml | 6 ++ .../firecloud/model/ModelJsonProtocol.scala | 4 +- .../dsde/firecloud/model/Profile.scala | 16 ++++-- .../dsde/firecloud/model/ProfileSpec.scala | 56 ++++++++++++++----- .../webservice/RegisterApiServiceSpec.scala | 4 +- .../webservice/UserApiServiceSpec.scala | 4 +- 6 files changed, 68 insertions(+), 22 deletions(-) diff --git a/src/main/resources/swagger/api-docs.yaml b/src/main/resources/swagger/api-docs.yaml index 651040aaa..7a172feea 100755 --- a/src/main/resources/swagger/api-docs.yaml +++ b/src/main/resources/swagger/api-docs.yaml @@ -7770,6 +7770,12 @@ components: description: The Terms of Service url, "app.terra.bio/#terms-of-service", which a user must include to show they accept the Terms of Service + department: + type: string + description: User's department within an institution + interestInTerra: + type: string + description: A string of comma-separated values of why the User is interested in Terra PublishConfigurationIngest: required: - configurationName diff --git a/src/main/scala/org/broadinstitute/dsde/firecloud/model/ModelJsonProtocol.scala b/src/main/scala/org/broadinstitute/dsde/firecloud/model/ModelJsonProtocol.scala index 11a6d887f..6cb81f926 100644 --- a/src/main/scala/org/broadinstitute/dsde/firecloud/model/ModelJsonProtocol.scala +++ b/src/main/scala/org/broadinstitute/dsde/firecloud/model/ModelJsonProtocol.scala @@ -213,8 +213,8 @@ object ModelJsonProtocol extends WorkspaceJsonSupport with SprayJsonSupport { implicit val impFireCloudKeyValue = jsonFormat2(FireCloudKeyValue) implicit val impThurloeKeyValue = jsonFormat2(ThurloeKeyValue) implicit val impThurloeKeyValues = jsonFormat2(ThurloeKeyValues) - implicit val impBasicProfile = jsonFormat10(BasicProfile) - implicit val impProfile = jsonFormat11(Profile.apply) + implicit val impBasicProfile = jsonFormat12(BasicProfile) + implicit val impProfile = jsonFormat13(Profile.apply) implicit val impProfileWrapper = jsonFormat2(ProfileWrapper) implicit val impProfileKVP = jsonFormat2(ProfileKVP) implicit val impTerraPreference = jsonFormat2(TerraPreference) diff --git a/src/main/scala/org/broadinstitute/dsde/firecloud/model/Profile.scala b/src/main/scala/org/broadinstitute/dsde/firecloud/model/Profile.scala index 91d8a32ab..1f595a55c 100644 --- a/src/main/scala/org/broadinstitute/dsde/firecloud/model/Profile.scala +++ b/src/main/scala/org/broadinstitute/dsde/firecloud/model/Profile.scala @@ -29,7 +29,9 @@ case class BasicProfile ( programLocationState: String, programLocationCountry: String, termsOfService: Option[String], - researchArea: Option[String] + researchArea: Option[String], + department: Option[String], + interestInTerra: Option[String], ) extends mappedPropVals { require(ProfileValidator.nonEmpty(firstName), "first name must be non-empty") require(ProfileValidator.nonEmpty(lastName), "last name must be non-empty") @@ -52,7 +54,9 @@ case class Profile ( programLocationCountry: String, researchArea: Option[String], linkedNihUsername: Option[String] = None, - linkExpireTime: Option[Long] = None + linkExpireTime: Option[Long] = None, + department: Option[String], + interestInTerra: Option[String], ) extends mappedPropVals { require(ProfileValidator.nonEmpty(firstName), "first name must be non-empty") require(ProfileValidator.nonEmpty(lastName), "last name must be non-empty") @@ -67,9 +71,9 @@ case class Profile ( object Profile { // increment this number every time you make a change to the user-provided profile fields - val currentVersion:Int = 4 + val currentVersion:Int = 5 - val requiredKeys = List("firstName", "lastName", "title", "institute", "programLocationCity", + val requiredKeys = List("firstName", "lastName", "title", "institute", "department", "programLocationCity", "programLocationState", "programLocationCountry") def apply(wrapper: ProfileWrapper): Profile = { @@ -95,7 +99,9 @@ object Profile { linkExpireTime = mappedKVPs.get("linkExpireTime") match { case Some(time) => Some(time.toLong) case _ => None - } + }, + department = mappedKVPs.get("department"), + interestInTerra = mappedKVPs.get("interestInTerra") ) } diff --git a/src/test/scala/org/broadinstitute/dsde/firecloud/model/ProfileSpec.scala b/src/test/scala/org/broadinstitute/dsde/firecloud/model/ProfileSpec.scala index 335b87614..44ab8f14e 100644 --- a/src/test/scala/org/broadinstitute/dsde/firecloud/model/ProfileSpec.scala +++ b/src/test/scala/org/broadinstitute/dsde/firecloud/model/ProfileSpec.scala @@ -23,7 +23,9 @@ class ProfileSpec extends AnyFreeSpec with Matchers { programLocationCity = randomString, programLocationState = randomString, programLocationCountry = randomString, - termsOfService = Some(termsOfServiceUrl) + termsOfService = Some(termsOfServiceUrl), + department = Some(randomString), + interestInTerra = Some(randomString) ) basicProfile shouldNot be(null) } @@ -38,7 +40,9 @@ class ProfileSpec extends AnyFreeSpec with Matchers { programLocationCity = randomString, programLocationState = randomString, programLocationCountry = randomString, - termsOfService = Some(termsOfServiceUrl) + termsOfService = Some(termsOfServiceUrl), + department = Some(randomString), + interestInTerra = Some(randomString) ) basicProfile shouldNot be(null) } @@ -52,7 +56,9 @@ class ProfileSpec extends AnyFreeSpec with Matchers { researchArea = Some(randomString), programLocationCity = randomString, programLocationState = randomString, - programLocationCountry = randomString + programLocationCountry = randomString, + department = Some(randomString), + interestInTerra = Some(randomString) ) profile shouldNot be(null) } @@ -66,7 +72,9 @@ class ProfileSpec extends AnyFreeSpec with Matchers { researchArea = Some(randomString), programLocationCity = randomString, programLocationState = randomString, - programLocationCountry = randomString + programLocationCountry = randomString, + department = Some(randomString), + interestInTerra = Some(randomString) ) profile shouldNot be(null) } @@ -80,10 +88,28 @@ class ProfileSpec extends AnyFreeSpec with Matchers { researchArea = Some(randomString), programLocationCity = randomString, programLocationState = randomString, - programLocationCountry = randomString + programLocationCountry = randomString, + department = Some(randomString), + interestInTerra = Some(randomString) ) profile shouldNot be(null) } + + "Profile instantiated with ProfileWrapper is valid" in { + val pw = ProfileWrapper("123", List( + FireCloudKeyValue(Some("firstName"), Some("test-firstName")), + FireCloudKeyValue(Some("lastName"), Some("test-lastName")), + FireCloudKeyValue(Some("title"), Some("test-title")), + FireCloudKeyValue(Some("institute"), Some("test-institute")), + FireCloudKeyValue(Some("department"), Some("test-department")), + FireCloudKeyValue(Some("programLocationCity"), Some("test-programLocationCity")), + FireCloudKeyValue(Some("programLocationState"), Some("test-programLocationState")), + FireCloudKeyValue(Some("programLocationCountry"), Some("test-programLocationCountry")), + FireCloudKeyValue(Some("contactEmail"), Some("test-contactEmail@noreply.com")) + )) + val profile = Profile(pw) + profile shouldNot be(null) + } } "Incorrectly formed profiles" - { @@ -99,6 +125,8 @@ class ProfileSpec extends AnyFreeSpec with Matchers { programLocationState = "", programLocationCountry = "", None, + None, + None, None ) } @@ -115,7 +143,9 @@ class ProfileSpec extends AnyFreeSpec with Matchers { researchArea = Some(randomString), programLocationCity = randomString, programLocationState = randomString, - programLocationCountry = randomString + programLocationCountry = randomString, + department = Some(randomString), + interestInTerra = Some(randomString) ) } ex shouldNot be(null) @@ -134,7 +164,7 @@ class ProfileSpec extends AnyFreeSpec with Matchers { )) "getString" - { - "returns None if key doesn't exist" - { + "returns None if key doesn't exist" in { val targetKey = "nonexistent" // assert key does not exist in sample data pw.keyValuePairs.find(_.key.contains(targetKey)) shouldBe None @@ -142,7 +172,7 @@ class ProfileSpec extends AnyFreeSpec with Matchers { val actual = ProfileUtils.getString(targetKey, pw) actual shouldBe None } - "returns None if key exists but value doesn't" - { + "returns None if key exists but value doesn't" in { val targetKey = "imnothing" // assert key exists in sample data with no value val targetKV = pw.keyValuePairs.find(_.key.contains(targetKey)) @@ -152,14 +182,14 @@ class ProfileSpec extends AnyFreeSpec with Matchers { val actual = ProfileUtils.getString(targetKey, pw) actual shouldBe None } - "returns Some(String) if key and value exist" - { + "returns Some(String) if key and value exist" in { val targetKey = "imastring" val actual = ProfileUtils.getString(targetKey, pw) actual shouldBe Some("hello") } } "getLong" - { - "returns None if key doesn't exist" - { + "returns None if key doesn't exist" in { val targetKey = "nonexistent" // assert key does not exist in sample data pw.keyValuePairs.find(_.key.contains(targetKey)) shouldBe None @@ -168,7 +198,7 @@ class ProfileSpec extends AnyFreeSpec with Matchers { actual shouldBe None } - "returns None if key exists but value doesn't" - { + "returns None if key exists but value doesn't" in { val targetKey = "imnothing" // assert key exists in sample data with no value val targetKV = pw.keyValuePairs.find(_.key.contains(targetKey)) @@ -178,7 +208,7 @@ class ProfileSpec extends AnyFreeSpec with Matchers { val actual = ProfileUtils.getLong(targetKey, pw) actual shouldBe None } - "returns None if key and value exist but value is not a Long" - { + "returns None if key and value exist but value is not a Long" in { val targetKey = "imnotalong" // assert the key exists ProfileUtils.getString(targetKey, pw) shouldBe Some("not-a-long") @@ -186,7 +216,7 @@ class ProfileSpec extends AnyFreeSpec with Matchers { val actual = ProfileUtils.getLong(targetKey, pw) actual shouldBe None } - "returns Some(Long) if key and value exist and value is Long-able" - { + "returns Some(Long) if key and value exist and value is Long-able" in { val targetKey = "imalong" val actual = ProfileUtils.getLong(targetKey, pw) actual shouldBe Some(1556724034L) diff --git a/src/test/scala/org/broadinstitute/dsde/firecloud/webservice/RegisterApiServiceSpec.scala b/src/test/scala/org/broadinstitute/dsde/firecloud/webservice/RegisterApiServiceSpec.scala index 6cc4cd0c7..d51f3c151 100644 --- a/src/test/scala/org/broadinstitute/dsde/firecloud/webservice/RegisterApiServiceSpec.scala +++ b/src/test/scala/org/broadinstitute/dsde/firecloud/webservice/RegisterApiServiceSpec.scala @@ -164,7 +164,9 @@ final class RegisterApiServiceSpec extends BaseServiceSpec with RegisterApiServi programLocationCity = randomString, programLocationState = randomString, programLocationCountry = randomString, - termsOfService = if (hasTermsOfService) Some(termsOfServiceUrl) else None + termsOfService = if (hasTermsOfService) Some(termsOfServiceUrl) else None, + department = Some(randomString), + interestInTerra = Some(randomString) ) } } diff --git a/src/test/scala/org/broadinstitute/dsde/firecloud/webservice/UserApiServiceSpec.scala b/src/test/scala/org/broadinstitute/dsde/firecloud/webservice/UserApiServiceSpec.scala index 524ff45ea..6561a93d9 100644 --- a/src/test/scala/org/broadinstitute/dsde/firecloud/webservice/UserApiServiceSpec.scala +++ b/src/test/scala/org/broadinstitute/dsde/firecloud/webservice/UserApiServiceSpec.scala @@ -48,7 +48,9 @@ class UserApiServiceSpec extends BaseServiceSpec with SamMockserverUtils programLocationCity = randomAlpha(), programLocationState = randomAlpha(), programLocationCountry = randomAlpha(), - termsOfService = None + termsOfService = None, + department = Some(randomAlpha()), + interestInTerra = Some(randomAlpha()) ) val allProperties: Map[String, String] = fullProfile.propertyValueMap