Skip to content

Commit

Permalink
Refactor Tos -> TermsOfService
Browse files Browse the repository at this point in the history
  • Loading branch information
Ghost-in-a-Jar committed Nov 16, 2023
1 parent d2bbc35 commit c075f75
Show file tree
Hide file tree
Showing 14 changed files with 109 additions and 109 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ trait OldUserRoutes extends SamUserDirectives with SamRequestContextDirectives {
get {
withUserAllowInactive(samRequestContext) { samUser =>
complete {
tosService.getTosComplianceStatus(samUser, samRequestContext).map { tosAcceptanceStatus =>
tosService.getTermsOfServiceComplianceStatus(samUser, samRequestContext).map { tosAcceptanceStatus =>
StatusCodes.OK -> Option(JsBoolean(tosAcceptanceStatus.permitsSystemUsage))
}
}
Expand Down Expand Up @@ -144,7 +144,7 @@ trait OldUserRoutes extends SamUserDirectives with SamRequestContextDirectives {
} ~
path("termsOfServiceComplianceStatus") {
get {
complete(tosService.getTosComplianceStatus(user, samRequestContext))
complete(tosService.getTermsOfServiceComplianceStatus(user, samRequestContext))
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ trait TermsOfServiceRoutes extends SamUserDirectives {
parameters("doc".as[String].?) { (doc: Option[String]) =>
val docSet = doc.map(_.split(",").toSet).getOrElse(Set.empty)
if (docSet.subsetOf(Set(tosService.termsOfServiceTextKey, tosService.privacyPolicyTextKey)))
complete(tosService.getTosText(docSet))
complete(tosService.getTermsOfServiceText(docSet))
else
complete(StatusCodes.BadRequest)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,9 @@ trait DirectoryDAO {

def acceptTermsOfService(userId: WorkbenchUserId, tosVersion: String, samRequestContext: SamRequestContext): IO[Boolean]
def rejectTermsOfService(userId: WorkbenchUserId, tosVersion: String, samRequestContext: SamRequestContext): IO[Boolean]
def getUserTos(userId: WorkbenchUserId, samRequestContext: SamRequestContext): IO[Option[SamUserTos]]
def getUserTosVersion(userId: WorkbenchUserId, tosVersion: Option[String], samRequestContext: SamRequestContext): IO[Option[SamUserTos]]
def getUserTosHistory(userId: WorkbenchUserId, samRequestContext: SamRequestContext, limit: Integer): IO[List[SamUserTos]]
def getUserTermsOfService(userId: WorkbenchUserId, samRequestContext: SamRequestContext): IO[Option[SamUserTos]]
def getUserTermsOfServiceVersion(userId: WorkbenchUserId, tosVersion: Option[String], samRequestContext: SamRequestContext): IO[Option[SamUserTos]]
def getUserTermsOfServiceHistory(userId: WorkbenchUserId, samRequestContext: SamRequestContext, limit: Integer): IO[List[SamUserTos]]
def createPetManagedIdentity(petManagedIdentity: PetManagedIdentity, samRequestContext: SamRequestContext): IO[PetManagedIdentity]
def loadPetManagedIdentity(petManagedIdentityId: PetManagedIdentityId, samRequestContext: SamRequestContext): IO[Option[PetManagedIdentity]]
def getUserFromPetManagedIdentity(petManagedIdentityObjectId: ManagedIdentityObjectId, samRequestContext: SamRequestContext): IO[Option[SamUser]]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -648,7 +648,7 @@ class PostgresDirectoryDAO(protected val writeDbRef: DbReference, protected val
}
}

override def getUserTos(userId: WorkbenchUserId, samRequestContext: SamRequestContext): IO[Option[SamUserTos]] =
override def getUserTermsOfService(userId: WorkbenchUserId, samRequestContext: SamRequestContext): IO[Option[SamUserTos]] =
readOnlyTransaction("getUserTos", samRequestContext) { implicit session =>
val tosTable = TosTable.syntax
val column = TosTable.column
Expand All @@ -664,7 +664,7 @@ class PostgresDirectoryDAO(protected val writeDbRef: DbReference, protected val
userTosRecordOpt.map(TosTable.unmarshalUserRecord)
}

override def getUserTosVersion(userId: WorkbenchUserId, tosVersion: Option[String], samRequestContext: SamRequestContext): IO[Option[SamUserTos]] = {
override def getUserTermsOfServiceVersion(userId: WorkbenchUserId, tosVersion: Option[String], samRequestContext: SamRequestContext): IO[Option[SamUserTos]] = {
if (tosVersion.isEmpty) return IO(None)
readOnlyTransaction("getUserTos", samRequestContext) { implicit session =>
val tosTable = TosTable.syntax
Expand All @@ -682,7 +682,7 @@ class PostgresDirectoryDAO(protected val writeDbRef: DbReference, protected val
}
}

override def getUserTosHistory(userId: WorkbenchUserId, samRequestContext: SamRequestContext, limit: Integer): IO[List[SamUserTos]] =
override def getUserTermsOfServiceHistory(userId: WorkbenchUserId, samRequestContext: SamRequestContext, limit: Integer): IO[List[SamUserTos]] =
readOnlyTransaction("getUserTosHistory", samRequestContext) { implicit session =>
val tosTable = TosTable.syntax
val column = TosTable.column
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ class TosService(
)
)

def getTosText(docSet: Set[String]): IO[String] =
def getTermsOfServiceText(docSet: Set[String]): IO[String] =
docSet match {
case set if set.isEmpty => IO.pure(termsOfServiceText)
case set if set.equals(Set(privacyPolicyTextKey)) => IO.pure(privacyPolicyText)
Expand All @@ -86,7 +86,7 @@ class TosService(

@Deprecated
def getTosDetails(samUser: SamUser, samRequestContext: SamRequestContext): IO[OldTermsOfServiceDetails] =
directoryDao.getUserTos(samUser.id, samRequestContext).map { tos =>
directoryDao.getUserTermsOfService(samUser.id, samRequestContext).map { tos =>
OldTermsOfServiceDetails(isEnabled = true, tosConfig.isGracePeriodEnabled, tosConfig.version, tos.map(_.version))
}

Expand Down Expand Up @@ -114,14 +114,14 @@ class TosService(

// Note: if version is None, then the query will return the last accepted ToS info for the user
private def loadTosRecordForUser(userId: WorkbenchUserId, version: Option[String], samRequestContext: SamRequestContext): IO[SamUserTos] =
directoryDao.getUserTosVersion(userId, version, samRequestContext).map {
directoryDao.getUserTermsOfServiceVersion(userId, version, samRequestContext).map {
case Some(samUserTos) => samUserTos
case None => throw new WorkbenchExceptionWithErrorReport(ErrorReport(StatusCodes.NotFound, s"Could not find Terms of Service entry for user:${userId}"))
}

def getTermsOfServiceHistoryForUser(userId: WorkbenchUserId, samRequestContext: SamRequestContext, limit: Integer): IO[TermsOfServiceHistory] =
ensureAdminIfNeeded[TermsOfServiceHistory](userId, samRequestContext) {
directoryDao.getUserTosHistory(userId, samRequestContext, limit).map {
directoryDao.getUserTermsOfServiceHistory(userId, samRequestContext, limit).map {
case samUserTosHistory if samUserTosHistory.isEmpty =>
throw new WorkbenchExceptionWithErrorReport(ErrorReport(StatusCodes.NotFound, s"Could not find any Terms of Service entries for user:${userId}"))
case samUserTosHistory =>
Expand All @@ -131,9 +131,9 @@ class TosService(
}
}

def getTosComplianceStatus(samUser: SamUser, samRequestContext: SamRequestContext): IO[TermsOfServiceComplianceStatus] = for {
latestUserTos <- directoryDao.getUserTos(samUser.id, samRequestContext)
previousUserTos <- directoryDao.getUserTosVersion(samUser.id, tosConfig.previousVersion, samRequestContext)
def getTermsOfServiceComplianceStatus(samUser: SamUser, samRequestContext: SamRequestContext): IO[TermsOfServiceComplianceStatus] = for {
latestUserTos <- directoryDao.getUserTermsOfService(samUser.id, samRequestContext)
previousUserTos <- directoryDao.getUserTermsOfServiceVersion(samUser.id, tosConfig.previousVersion, samRequestContext)
userHasAcceptedLatestVersion = userHasAcceptedLatestTosVersion(latestUserTos)
permitsSystemUsage = tosAcceptancePermitsSystemUsage(samUser, latestUserTos, previousUserTos)
} yield TermsOfServiceComplianceStatus(samUser.id, userHasAcceptedLatestVersion, permitsSystemUsage)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ class UserService(
allUsersStatus <- directoryDAO.isGroupMember(allUsersGroup.id, user.id, samRequestContext) recover { case _: NameNotFoundException =>
false
}
tosComplianceStatus <- tosService.getTosComplianceStatus(user, samRequestContext)
tosComplianceStatus <- tosService.getTermsOfServiceComplianceStatus(user, samRequestContext)
adminEnabled <- directoryDAO.isEnabled(user.id, samRequestContext)
} yield {
// We are removing references to LDAP but this will require an API version change here, so we are leaving
Expand Down Expand Up @@ -382,15 +382,15 @@ class UserService(
// Mixing up the endpoint to return user info AND status information is only causing problems and confusion
def getUserStatusInfo(user: SamUser, samRequestContext: SamRequestContext): IO[UserStatusInfo] =
for {
tosAcceptanceDetails <- tosService.getTosComplianceStatus(user, samRequestContext)
tosAcceptanceDetails <- tosService.getTermsOfServiceComplianceStatus(user, samRequestContext)
} yield UserStatusInfo(user.id.value, user.email.value, tosAcceptanceDetails.permitsSystemUsage && user.enabled, user.enabled)

def getUserStatusDiagnostics(userId: WorkbenchUserId, samRequestContext: SamRequestContext): IO[Option[UserStatusDiagnostics]] =
openTelemetry.time("api.v1.user.statusDiagnostics.time", API_TIMING_DURATION_BUCKET) {
directoryDAO.loadUser(userId, samRequestContext).flatMap {
case Some(user) =>
// pulled out of for comprehension to allow concurrent execution
val tosAcceptanceStatus = tosService.getTosComplianceStatus(user, samRequestContext)
val tosAcceptanceStatus = tosService.getTermsOfServiceComplianceStatus(user, samRequestContext)
val adminEnabledStatus = directoryDAO.isEnabled(user.id, samRequestContext)
val allUsersStatus = cloudExtensions.getOrCreateAllUsersGroup(directoryDAO, samRequestContext).flatMap { allUsersGroup =>
directoryDAO.isGroupMember(allUsersGroup.id, user.id, samRequestContext) recover { case e: NameNotFoundException => false }
Expand Down Expand Up @@ -496,7 +496,7 @@ class UserService(

def getUserAllowances(samUser: SamUser, samRequestContext: SamRequestContext): IO[SamUserAllowances] =
for {
tosStatus <- tosService.getTosComplianceStatus(samUser, samRequestContext)
tosStatus <- tosService.getTermsOfServiceComplianceStatus(samUser, samRequestContext)
} yield SamUserAllowances(
allowed = samUser.enabled && tosStatus.permitsSystemUsage,
enabled = samUser.enabled,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ class MockSamRoutesBuilder(allUsersGroup: WorkbenchGroup)(implicit system: Actor
this
}

def withTosHistoryForUser(samUser: SamUser, tosHistory: TermsOfServiceHistory): MockSamRoutesBuilder = {
mockTosServiceBuilder.withTosHistoryForUser(samUser, tosHistory)
def withTermsOfServiceHistoryForUser(samUser: SamUser, tosHistory: TermsOfServiceHistory): MockSamRoutesBuilder = {
mockTosServiceBuilder.withTermsOfServiceHistoryForUser(samUser, tosHistory)
this
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ class TermsOfServiceRouteSpec extends AnyFunSpec with Matchers with ScalatestRou
val record2 = TermsOfServiceHistoryRecord(TosTable.REJECT, "0", Instant.now.minusSeconds(5))
val mockSamRoutesBuilder = new MockSamRoutesBuilder(allUsersGroup)
.withEnabledUser(defaultUser)
.withTosHistoryForUser(defaultUser, TermsOfServiceHistory(List(record1, record2)))
.withTermsOfServiceHistoryForUser(defaultUser, TermsOfServiceHistory(List(record1, record2)))

Get(s"/api/termsOfService/v1/user/${defaultUser.id}/history") ~> mockSamRoutesBuilder.build.route ~> check {
withClue(s"${responseAs[String]} is not parsable as an instance of `TermsOfServiceHistoryRecord`.") {
Expand Down Expand Up @@ -195,7 +195,7 @@ class TermsOfServiceRouteSpec extends AnyFunSpec with Matchers with ScalatestRou
val record2 = TermsOfServiceHistoryRecord(TosTable.REJECT, "0", Instant.now.minusSeconds(5))
val mockSamRoutesBuilder = new MockSamRoutesBuilder(allUsersGroup)
.withEnabledUser(defaultUser)
.withTosHistoryForUser(defaultUser, TermsOfServiceHistory(List(record1, record2)))
.withTermsOfServiceHistoryForUser(defaultUser, TermsOfServiceHistory(List(record1, record2)))

Get(s"/api/termsOfService/v1/user/self/history") ~> mockSamRoutesBuilder.build.route ~> check {
withClue(s"${responseAs[String]} is not parsable as an instance of `TermsOfServiceHistoryRecord`.") {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -330,14 +330,14 @@ class MockDirectoryDAO(val groups: mutable.Map[WorkbenchGroupIdentity, Workbench
true
}

override def getUserTos(userId: WorkbenchUserId, samRequestContext: SamRequestContext): IO[Option[SamUserTos]] =
override def getUserTermsOfService(userId: WorkbenchUserId, samRequestContext: SamRequestContext): IO[Option[SamUserTos]] =
loadUser(userId, samRequestContext).map {
case None => None
case Some(_) =>
userTos.get(userId)
}

override def getUserTosVersion(userId: WorkbenchUserId, tosVersion: Option[String], samRequestContext: SamRequestContext): IO[Option[SamUserTos]] =
override def getUserTermsOfServiceVersion(userId: WorkbenchUserId, tosVersion: Option[String], samRequestContext: SamRequestContext): IO[Option[SamUserTos]] =
loadUser(userId, samRequestContext).map {
case None => None
case Some(_) =>
Expand All @@ -347,7 +347,7 @@ class MockDirectoryDAO(val groups: mutable.Map[WorkbenchGroupIdentity, Workbench
}
}

override def getUserTosHistory(userId: WorkbenchUserId, samRequestContext: SamRequestContext, limit: Integer): IO[List[SamUserTos]] =
override def getUserTermsOfServiceHistory(userId: WorkbenchUserId, samRequestContext: SamRequestContext, limit: Integer): IO[List[SamUserTos]] =
loadUser(userId, samRequestContext).map {
case None => List.empty
case Some(_) => userTosHistory.getOrElse(userId, List.empty)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ case class MockDirectoryDaoBuilder() extends IdiomaticMockito {
mockedDirectoryDAO.setUserRegisteredAt(any[WorkbenchUserId], any[Instant], any[SamRequestContext]) returns IO.unit
mockedDirectoryDAO.setUserAttributes(any[SamUserAttributes], any[SamRequestContext]) returns IO.unit
mockedDirectoryDAO.getUserAttributes(any[WorkbenchUserId], any[SamRequestContext]) returns IO(None)
mockedDirectoryDAO.getUserTosVersion(any[WorkbenchUserId], any[Some[String]], any[SamRequestContext]) returns IO(None)
mockedDirectoryDAO.getUserTermsOfServiceVersion(any[WorkbenchUserId], any[Some[String]], any[SamRequestContext]) returns IO(None)

def withHealthyDatabase: MockDirectoryDaoBuilder = {
mockedDirectoryDAO.checkStatus(any[SamRequestContext]) returns IO(true)
Expand Down Expand Up @@ -116,24 +116,24 @@ case class MockDirectoryDaoBuilder() extends IdiomaticMockito {
def withAcceptedTermsOfServiceForUser(samUser: SamUser, tosVersion: String): MockDirectoryDaoBuilder = {
makeUserExist(samUser)
val samUserTos = SamUserTos(samUser.id, tosVersion, TosTable.ACCEPT, Instant.now)
mockedDirectoryDAO.getUserTos(eqTo(samUser.id), any[SamRequestContext]) returns IO(Option(samUserTos))
mockedDirectoryDAO.getUserTosVersion(eqTo(samUser.id), eqTo(Some(tosVersion)), any[SamRequestContext]) returns IO(Option(samUserTos))
mockedDirectoryDAO.getUserTermsOfService(eqTo(samUser.id), any[SamRequestContext]) returns IO(Option(samUserTos))
mockedDirectoryDAO.getUserTermsOfServiceVersion(eqTo(samUser.id), eqTo(Some(tosVersion)), any[SamRequestContext]) returns IO(Option(samUserTos))
this
}

def withRejectedTermsOfServiceForUser(samUser: SamUser, tosVersion: String): MockDirectoryDaoBuilder = {
makeUserExist(samUser)
val samUserTos = SamUserTos(samUser.id, tosVersion, TosTable.REJECT, Instant.now)
mockedDirectoryDAO.getUserTos(eqTo(samUser.id), any[SamRequestContext]) returns IO(Option(samUserTos))
mockedDirectoryDAO.getUserTosVersion(eqTo(samUser.id), eqTo(Some(tosVersion)), any[SamRequestContext]) returns IO(Option(samUserTos))
mockedDirectoryDAO.getUserTermsOfService(eqTo(samUser.id), any[SamRequestContext]) returns IO(Option(samUserTos))
mockedDirectoryDAO.getUserTermsOfServiceVersion(eqTo(samUser.id), eqTo(Some(tosVersion)), any[SamRequestContext]) returns IO(Option(samUserTos))
this
}

def withTermsOfServiceHistoryForUser(samUser: SamUser, tosHistory: List[SamUserTos]): MockDirectoryDaoBuilder = {
makeUserExist(samUser)
mockedDirectoryDAO.getUserTos(eqTo(samUser.id), any[SamRequestContext]) returns IO(Option(tosHistory.head))
mockedDirectoryDAO.getUserTosVersion(eqTo(samUser.id), any[Option[String]], any[SamRequestContext]) returns IO(Option(tosHistory.head))
mockedDirectoryDAO.getUserTosHistory(eqTo(samUser.id), any[SamRequestContext], any[Integer]) returns IO(tosHistory)
mockedDirectoryDAO.getUserTermsOfService(eqTo(samUser.id), any[SamRequestContext]) returns IO(Option(tosHistory.head))
mockedDirectoryDAO.getUserTermsOfServiceVersion(eqTo(samUser.id), any[Option[String]], any[SamRequestContext]) returns IO(Option(tosHistory.head))
mockedDirectoryDAO.getUserTermsOfServiceHistory(eqTo(samUser.id), any[SamRequestContext], any[Integer]) returns IO(tosHistory)
this
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1601,7 +1601,7 @@ class PostgresDirectoryDAOSpec extends RetryableAnyFreeSpec with Matchers with B
dao.acceptTermsOfService(defaultUser.id, tosConfig.version, samRequestContext).unsafeRunSync() shouldBe true

// Assert
val userTos = dao.getUserTos(defaultUser.id, samRequestContext).unsafeRunSync()
val userTos = dao.getUserTermsOfService(defaultUser.id, samRequestContext).unsafeRunSync()
userTos should not be empty
userTos.get.createdAt should beAround(Instant.now())
userTos.get.action shouldBe TosTable.ACCEPT
Expand All @@ -1615,7 +1615,7 @@ class PostgresDirectoryDAOSpec extends RetryableAnyFreeSpec with Matchers with B
dao.acceptTermsOfService(defaultUser.id, "2", samRequestContext).unsafeRunSync() shouldBe true

// Assert
val userTos = dao.getUserTos(defaultUser.id, samRequestContext).unsafeRunSync()
val userTos = dao.getUserTermsOfService(defaultUser.id, samRequestContext).unsafeRunSync()
userTos should not be empty
userTos.get.createdAt should beAround(Instant.now())
userTos.get.action shouldBe TosTable.ACCEPT
Expand All @@ -1631,7 +1631,7 @@ class PostgresDirectoryDAOSpec extends RetryableAnyFreeSpec with Matchers with B
dao.rejectTermsOfService(user.id, tosConfig.version, samRequestContext).unsafeRunSync() shouldBe true

// Assert
val userTos = dao.getUserTos(user.id, samRequestContext).unsafeRunSync()
val userTos = dao.getUserTermsOfService(user.id, samRequestContext).unsafeRunSync()
userTos should not be empty
userTos.get.createdAt should beAround(Instant.now())
userTos.get.action shouldBe TosTable.REJECT
Expand All @@ -1646,7 +1646,7 @@ class PostgresDirectoryDAOSpec extends RetryableAnyFreeSpec with Matchers with B
dao.rejectTermsOfService(user.id, tosConfig.version, samRequestContext).unsafeRunSync() shouldBe true

// Assert
val userTos = dao.getUserTos(user.id, samRequestContext).unsafeRunSync()
val userTos = dao.getUserTermsOfService(user.id, samRequestContext).unsafeRunSync()
userTos should not be empty
userTos.get.createdAt should beAround(Instant.now())
userTos.get.action shouldBe TosTable.REJECT
Expand All @@ -1661,7 +1661,7 @@ class PostgresDirectoryDAOSpec extends RetryableAnyFreeSpec with Matchers with B
dao.createUser(user, samRequestContext).unsafeRunSync()

// Assert
val userTos = dao.getUserTos(user.id, samRequestContext).unsafeRunSync()
val userTos = dao.getUserTermsOfService(user.id, samRequestContext).unsafeRunSync()
userTos should be(None)
}
}
Expand Down
Loading

0 comments on commit c075f75

Please sign in to comment.