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.

Sign up for GitHub

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

Jump to bottom

ID-787 Get user/self terms of service history. #1254

Merged
merged 15 commits into from
Nov 28, 2023
Merged

ID-787 Get user/self terms of service history. #1254

merged 15 commits into from
Nov 28, 2023

Conversation

Ghost-in-a-Jar
Copy link
Contributor

@Ghost-in-a-Jar Ghost-in-a-Jar commented Nov 14, 2023 •
edited
Loading

Ticket: https://broadworkbench.atlassian.net/browse/ID-811

What:

Adds endpoints for getting user_id/self terms of service history. Only admins can grab history for other users besides themselves.

I also renamed Tos -> TermsOfService in a few places for consistency

Also, added the ability to only return the latest terms of service accept/reject vs just returning the latest of either action. When users call the terms of service details endpoint now they will receive info on their latest acceptance even if they rejected most recently.

Why:

Improving tos ergonomics, auditing

How:

Added two endpoints: {user_id}/history and self/history.

PR checklist

  • I've followed the instructions if I've made any changes to the API, especially if they're breaking changes
  • I've filled out the Security Risk Assessment (requires Broad Internal network access) and attached the result to the JIRA ticket

Ghost-in-a-Jar
Ghost-in-a-Jar commented Nov 14, 2023
View reviewed changes
src/main/scala/org/broadinstitute/dsde/workbench/sam/api/TermsOfServiceRoutes.scala Outdated
@@ -9,6 +9,7 @@ import org.broadinstitute.dsde.workbench.sam.model.api.SamJsonSupport._
import org.broadinstitute.dsde.workbench.sam.model.api.SamUser
import org.broadinstitute.dsde.workbench.sam.service.TosService
import org.broadinstitute.dsde.workbench.sam.util.SamRequestContext
import spray.json.DefaultJsonProtocol._
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

took me forever to figure out i had to import this

@Ghost-in-a-Jar Ghost-in-a-Jar force-pushed the ID-787-get-user-tos-history branch from 29b9c57 to 39f1d2c Compare November 16, 2023 18:39
@Ghost-in-a-Jar Ghost-in-a-Jar force-pushed the ID-787-get-user-tos-history branch from aa511f1 to 2e131c3 Compare November 16, 2023 18:50
@Ghost-in-a-Jar Ghost-in-a-Jar force-pushed the ID-787-get-user-tos-history branch 2 times, most recently from 2f6bee3 to c075f75 Compare November 16, 2023 19:05
@Ghost-in-a-Jar Ghost-in-a-Jar force-pushed the ID-787-get-user-tos-history branch from c075f75 to db670cb Compare November 16, 2023 19:06
@Ghost-in-a-Jar
address kai comment
a4bdc6a
@Ghost-in-a-Jar
Merge branch 'develop' into ID-787-get-user-tos-history
cdbdfa9 
# Conflicts:
#	src/main/scala/org/broadinstitute/dsde/workbench/sam/api/TermsOfServiceRoutes.scala
#	src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresDirectoryDAO.scala
#	src/main/scala/org/broadinstitute/dsde/workbench/sam/model/SamModel.scala
#	src/main/scala/org/broadinstitute/dsde/workbench/sam/service/TosService.scala
#	src/test/scala/org/broadinstitute/dsde/workbench/sam/service/TosServiceSpec.scala
@Ghost-in-a-Jar Ghost-in-a-Jar force-pushed the ID-787-get-user-tos-history branch 3 times, most recently from 3dc54dd to af304ac Compare November 27, 2023 17:57
@Ghost-in-a-Jar Ghost-in-a-Jar force-pushed the ID-787-get-user-tos-history branch from af304ac to 7bb8b6c Compare November 27, 2023 18:43
@Ghost-in-a-Jar Ghost-in-a-Jar force-pushed the ID-787-get-user-tos-history branch 2 times, most recently from a7c93fe to dba05a7 Compare November 27, 2023 20:19
Ghost-in-a-Jar
Ghost-in-a-Jar commented Nov 27, 2023
View reviewed changes
src/main/scala/org/broadinstitute/dsde/workbench/sam/service/TosService.scala
@@ -100,7 +107,7 @@ class TosService(
}

private def ensureLatestTermsOfService(userId: WorkbenchUserId, samRequestContext: SamRequestContext): IO[SamUserTos] = for {
maybeTermsOfServiceRecord <- directoryDao.getUserTos(userId, samRequestContext)
maybeTermsOfServiceRecord <- directoryDao.getUserTermsOfService(userId, samRequestContext, Option(TosTable.ACCEPT))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

more context on this change https://broadinstitute.slack.com/archives/C0DSD41QT/p1701111037789719?thread_ts=1701103991.553039&cid=C0DSD41QT

this endpoint will always return data on the latest acceptance even if the user's latest action was to reject.

@Ghost-in-a-Jar
Merge branch 'develop' into ID-787-get-user-tos-history
9133c34 
# Conflicts:
#	src/main/scala/org/broadinstitute/dsde/workbench/sam/api/TermsOfServiceRoutes.scala
#	src/main/scala/org/broadinstitute/dsde/workbench/sam/service/UserService.scala
#	src/test/scala/org/broadinstitute/dsde/workbench/sam/api/MockSamRoutesBuilder.scala
@Ghost-in-a-Jar
post merge last touches
522d37e
@sonarqubecloud SonarQubeCloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
10.5% 10.5% Duplication

@Ghost-in-a-Jar Ghost-in-a-Jar merged commit 9a5ecd9 into develop Nov 28, 2023
17 checks passed
@Ghost-in-a-Jar Ghost-in-a-Jar deleted the ID-787-get-user-tos-history branch November 28, 2023 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sjkobori sjkobori sjkobori approved these changes

@Shakespeared Shakespeared Shakespeared approved these changes

@tlangs tlangs tlangs approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Ghost-in-a-Jar @tlangs @Shakespeared @sjkobori