[no ticket][risk=no] Supplemented creator information (#8969)

all-of-us · Dec 10, 2024 · f57e48a · f57e48a
1 parent 9b58577
commit f57e48a
Show file tree

Hide file tree

Showing 18 changed files with 86 additions and 33 deletions.
diff --git a/.../main/java/org/pmiops/workbench/actionaudit/targetproperties/WorkspaceTargetProperty.java b/.../main/java/org/pmiops/workbench/actionaudit/targetproperties/WorkspaceTargetProperty.java
@@ -14,7 +14,7 @@ public enum WorkspaceTargetProperty implements ModelBackedTargetProperty<Workspa
   TERRA_NAME("terra_name", Workspace::getTerraName),
   NAMESPACE("namespace", Workspace::getNamespace),
   CDR_VERSION_ID("cdr_version_id", Workspace::getCdrVersionId),
-  CREATOR("creator", Workspace::getCreator),
+  CREATOR("creator", workspace -> workspace.getCreatorUser().getUserName()),
   ACCESS_TIER_SHORT_NAME("access_tier_short_name", Workspace::getAccessTierShortName),
 
   // all fields below here relate to research purpose

diff --git a/api/src/main/java/org/pmiops/workbench/utils/mappers/WorkspaceMapper.java b/api/src/main/java/org/pmiops/workbench/utils/mappers/WorkspaceMapper.java
@@ -48,7 +48,10 @@ public interface WorkspaceMapper {
   @Mapping(target = "displayName", source = "dbWorkspace.name")
   @Mapping(target = "terraName", source = "fcWorkspace.name")
   @Mapping(target = "googleBucketName", source = "fcWorkspace.bucketName")
-  @Mapping(target = "creator", source = "dbWorkspace.creator.username")
+  @Mapping(target = "creatorUser.userName", source = "dbWorkspace.creator.username")
+  // Need to work with security before exposing
+  // Should change to contactEmail or institutionalEmail
+  @Mapping(target = "creatorUser.email", ignore = true)
   @Mapping(
       target = "initialCredits.expirationEpochMillis",
       source = "dbWorkspace.creator",
@@ -124,7 +127,10 @@ default List<WorkspaceResponse> toApiWorkspaceResponseList(
   ResearchPurpose workspaceToResearchPurpose(DbWorkspace dbWorkspace);
 
   @Mapping(target = "cdrVersionId", source = "cdrVersion")
-  @Mapping(target = "creator", source = "creator.username")
+  @Mapping(target = "creatorUser.userName", source = "creator.username")
+  @Mapping(
+      target = "creatorUser.email",
+      ignore = true) // need to work with security before exposing
   @Mapping(target = "initialCredits.expired", source = "dbWorkspace.initialCreditsExpired")
   @Mapping(
       target = "initialCredits.expirationEpochMillis",

diff --git a/api/src/main/resources/workbench-api.yaml b/api/src/main/resources/workbench-api.yaml
@@ -7279,6 +7279,10 @@ components:
           type: string
         creator:
           type: string
+          description: |
+            # TODO: Remove this field. Deprecated. Use creatorUser instead.
+        creatorUser:
+          $ref: '#/components/schemas/User'
         billingAccountName:
           type: string
         googleBucketName:

diff --git a/api/src/test/java/org/pmiops/workbench/actionaudit/auditors/WorkspaceAuditorTest.java b/api/src/test/java/org/pmiops/workbench/actionaudit/auditors/WorkspaceAuditorTest.java
@@ -37,6 +37,7 @@
 import org.pmiops.workbench.db.model.DbWorkspace;
 import org.pmiops.workbench.model.BillingStatus;
 import org.pmiops.workbench.model.ResearchPurpose;
+import org.pmiops.workbench.model.User;
 import org.pmiops.workbench.model.Workspace;
 import org.pmiops.workbench.model.WorkspaceAccessLevel;
 import org.pmiops.workbench.utils.mappers.CommonMappers;
@@ -113,14 +114,17 @@ public void setUp() {
             .researchOutcomeList(Collections.emptyList());
     final long now = System.currentTimeMillis();
 
+    User creator = new User();
+    creator.setUserName("[email protected]");
+
     workspace1 =
         new Workspace()
             .etag("etag_1")
             .name("DbWorkspace 1")
             .terraName("dbworkspace1")
             .namespace("aou-rw-local1-c4be869a")
             .cdrVersionId("1")
-            .creator("[email protected]")
+            .creatorUser(creator)
             .billingAccountName("big-bux")
             .googleBucketName("bucket o' science")
             .accessTierShortName(AccessTierService.REGISTERED_TIER_SHORT_NAME)
@@ -279,6 +283,8 @@ public void testFireEditAction_sendsChangedProperties() {
             .timeReviewed(workspace1.getResearchPurpose().getTimeReviewed() + 1000L)
             .controlSet(!workspace1.getResearchPurpose().isControlSet());
     final int rpChanges = 5;
+    User creator = new User();
+    creator.setUserName("[email protected]");
 
     Workspace editedWorkspace =
         clone(workspace1)
@@ -287,7 +293,7 @@ public void testFireEditAction_sendsChangedProperties() {
             // changes
             .name("a new name")
             .namespace("a new namespace")
-            .creator("[email protected]");
+            .creatorUser(creator);
     final int wsChanges = 3;
 
     workspaceAuditor.fireEditAction(workspace1, editedWorkspace, dbWorkspace1.getWorkspaceId());
@@ -304,7 +310,7 @@ private Workspace clone(Workspace in) {
         .name(in.getName())
         .namespace(in.getNamespace())
         .cdrVersionId(in.getCdrVersionId())
-        .creator(in.getCreator())
+        .creatorUser(in.getCreatorUser())
         .billingAccountName(in.getBillingAccountName())
         .googleBucketName(in.getGoogleBucketName())
         .accessTierShortName(in.getAccessTierShortName())

diff --git a/...t/java/org/pmiops/workbench/actionaudit/targetproperties/TargetPropertyExtractorTest.java b/...t/java/org/pmiops/workbench/actionaudit/targetproperties/TargetPropertyExtractorTest.java
@@ -8,6 +8,7 @@
 import org.pmiops.workbench.access.AccessTierService;
 import org.pmiops.workbench.model.ResearchPurpose;
 import org.pmiops.workbench.model.SpecificPopulationEnum;
+import org.pmiops.workbench.model.User;
 import org.pmiops.workbench.model.Workspace;
 
 class TargetPropertyExtractorTest {
@@ -16,6 +17,8 @@ class TargetPropertyExtractorTest {
   @BeforeEach
   void setUp() {
     long now = System.currentTimeMillis();
+    User creator = new User();
+    creator.setUserName("[email protected]");
 
     var researchPurpose1 =
         new ResearchPurpose()
@@ -52,7 +55,7 @@ void setUp() {
             .terraName("dbworkspace1")
             .namespace("aou-rw-local1-c4be869a")
             .cdrVersionId("1")
-            .creator("[email protected]")
+            .creatorUser(creator)
             .accessTierShortName(AccessTierService.REGISTERED_TIER_SHORT_NAME)
             .researchPurpose(researchPurpose1);
   }

diff --git a/...t/java/org/pmiops/workbench/actionaudit/targetproperties/WorkspaceTargetPropertyTest.java b/...t/java/org/pmiops/workbench/actionaudit/targetproperties/WorkspaceTargetPropertyTest.java
@@ -14,6 +14,7 @@
 import org.pmiops.workbench.access.AccessTierService;
 import org.pmiops.workbench.model.ResearchPurpose;
 import org.pmiops.workbench.model.SpecificPopulationEnum;
+import org.pmiops.workbench.model.User;
 import org.pmiops.workbench.model.Workspace;
 
 public class WorkspaceTargetPropertyTest {
@@ -42,13 +43,15 @@ public void setUp() {
             .anticipatedFindings("a 4-leaf clover");
 
     long now = System.currentTimeMillis();
+    User creator = new User();
+    creator.setUserName("[email protected]");
 
     workspace1 =
         new Workspace()
             .name("Workspace 1")
             .terraName("workspace1")
             .namespace("aou-rw-local1-c4be869a")
-            .creator("[email protected]")
+            .creatorUser(creator)
             .cdrVersionId("1")
             .researchPurpose(researchPurposeAllFieldsPopulated)
             .creationTime(now)
@@ -61,7 +64,7 @@ public void setUp() {
             .name("Workspace 2")
             .terraName("workspace2")
             .namespace("aou-rw-local1-c4be869a")
-            .creator("[email protected]")
+            .creatorUser(creator)
             .cdrVersionId("33")
             .researchPurpose(researchPurpose2)
             .creationTime(now)

diff --git a/api/src/test/java/org/pmiops/workbench/api/WorkspacesControllerTest.java b/api/src/test/java/org/pmiops/workbench/api/WorkspacesControllerTest.java
@@ -618,7 +618,8 @@ private List<RawlsWorkspaceACLUpdate> convertUserRolesToUpdateAclRequestList(
   private Workspace createWorkspaceAndGrantAccess(WorkspaceAccessLevel accessLevel) {
     Workspace ws = createWorkspace();
     ws = workspacesController.createWorkspace(ws).getBody();
-    stubGetWorkspace(ws.getNamespace(), ws.getTerraName(), ws.getCreator(), accessLevel);
+    stubGetWorkspace(
+        ws.getNamespace(), ws.getTerraName(), ws.getCreatorUser().getUserName(), accessLevel);
     return ws;
   }
 
@@ -669,7 +670,7 @@ public void testCreateWorkspace() {
     assertThat(retrievedWorkspace.getCdrVersionId()).isEqualTo(cdrVersionId);
     assertThat(retrievedWorkspace.getAccessTierShortName())
         .isEqualTo(registeredTier.getShortName());
-    assertThat(retrievedWorkspace.getCreator()).isEqualTo(LOGGED_IN_USER_EMAIL);
+    assertThat(retrievedWorkspace.getCreatorUser().getUserName()).isEqualTo(LOGGED_IN_USER_EMAIL);
     assertThat(retrievedWorkspace.getName()).isEqualTo(testWorkspaceDisplayName);
     assertThat(retrievedWorkspace.getDisplayName()).isEqualTo(testWorkspaceDisplayName);
     assertThat(retrievedWorkspace.getTerraName()).isEqualTo(testWorkspaceTerraName);
@@ -905,7 +906,7 @@ public void testGetWorkspaceOperation_withWorkspace() {
     stubGetWorkspace(
         workspace.getNamespace(),
         workspace.getTerraName(),
-        workspace.getCreator(),
+        workspace.getCreatorUser().getUserName(),
         WorkspaceAccessLevel.READER);
 
     WorkspaceOperation operation =
@@ -1248,7 +1249,10 @@ public void testReaderUpdateWorkspaceThrows() {
           UpdateWorkspaceRequest request = new UpdateWorkspaceRequest();
           request.setWorkspace(ws);
           stubGetWorkspace(
-              ws.getNamespace(), ws.getTerraName(), ws.getCreator(), WorkspaceAccessLevel.READER);
+              ws.getNamespace(),
+              ws.getTerraName(),
+              ws.getCreatorUser().getUserName(),
+              WorkspaceAccessLevel.READER);
           workspacesController.updateWorkspace(ws.getNamespace(), ws.getTerraName(), request);
         });
   }
@@ -1264,7 +1268,10 @@ public void testWriterUpdateWorkspaceThrows() {
           UpdateWorkspaceRequest request = new UpdateWorkspaceRequest();
           request.setWorkspace(ws);
           stubGetWorkspace(
-              ws.getNamespace(), ws.getTerraName(), ws.getCreator(), WorkspaceAccessLevel.WRITER);
+              ws.getNamespace(),
+              ws.getTerraName(),
+              ws.getCreatorUser().getUserName(),
+              WorkspaceAccessLevel.WRITER);
           workspacesController.updateWorkspace(ws.getNamespace(), ws.getTerraName(), request);
         });
   }
@@ -2102,7 +2109,8 @@ private void assertConceptSetClone(
     assertThat(clonedConceptSet.getName()).isEqualTo(originalConceptSet.getName());
     assertThat(clonedConceptSet.getDomain()).isEqualTo(originalConceptSet.getDomain());
     assertThat(clonedConceptSet.getCriteriums()).isEqualTo(originalConceptSet.getCriteriums());
-    assertThat(clonedConceptSet.getCreator()).isEqualTo(clonedWorkspace.getCreator());
+    assertThat(clonedConceptSet.getCreator())
+        .isEqualTo(clonedWorkspace.getCreatorUser().getUserName());
     assertThat(clonedConceptSet.getCreationTime()).isEqualTo(clonedWorkspace.getCreationTime());
     assertThat(clonedConceptSet.getLastModifiedTime())
         .isEqualTo(clonedWorkspace.getLastModifiedTime());
@@ -2180,7 +2188,7 @@ public void testCloneWorkspaceDifferentOwner() {
             .getBody()
             .getWorkspace();
 
-    assertThat(workspace2.getCreator()).isEqualTo(cloner.getUsername());
+    assertThat(workspace2.getCreatorUser().getUserName()).isEqualTo(cloner.getUsername());
   }
 
   @Test
@@ -2397,7 +2405,7 @@ public void testCloneWorkspaceIncludeUserRoles() {
             .getBody()
             .getWorkspace();
 
-    assertThat(clonedWorkspace.getCreator()).isEqualTo(cloner.getUsername());
+    assertThat(clonedWorkspace.getCreatorUser().getUserName()).isEqualTo(cloner.getUsername());
 
     verify(fireCloudService)
         .updateWorkspaceACL(
@@ -2839,7 +2847,10 @@ public void testGetBillingUsage() {
     Workspace ws = createWorkspace();
     ws = workspacesController.createWorkspace(ws).getBody();
     stubGetWorkspace(
-        ws.getNamespace(), ws.getTerraName(), ws.getCreator(), WorkspaceAccessLevel.OWNER);
+        ws.getNamespace(),
+        ws.getTerraName(),
+        ws.getCreatorUser().getUserName(),
+        WorkspaceAccessLevel.OWNER);
     when(mockInitialCreditsService.getWorkspaceFreeTierBillingUsage(any())).thenReturn(cost);
 
     WorkspaceBillingUsageResponse workspaceBillingUsageResponse =
@@ -2855,7 +2866,10 @@ public void testGetBillingUsageWithoutAccess() {
           Workspace ws = createWorkspace();
           ws = workspacesController.createWorkspace(ws).getBody();
           stubGetWorkspace(
-              ws.getNamespace(), ws.getTerraName(), ws.getCreator(), WorkspaceAccessLevel.READER);
+              ws.getNamespace(),
+              ws.getTerraName(),
+              ws.getCreatorUser().getUserName(),
+              WorkspaceAccessLevel.READER);
           workspacesController.getBillingUsage(ws.getNamespace(), ws.getTerraName());
         });
   }
@@ -2865,7 +2879,10 @@ public void testGetBillingUsageWithNoSpend() {
     Workspace ws = createWorkspace();
     ws = workspacesController.createWorkspace(ws).getBody();
     stubGetWorkspace(
-        ws.getNamespace(), ws.getTerraName(), ws.getCreator(), WorkspaceAccessLevel.OWNER);
+        ws.getNamespace(),
+        ws.getTerraName(),
+        ws.getCreatorUser().getUserName(),
+        WorkspaceAccessLevel.OWNER);
     WorkspaceBillingUsageResponse workspaceBillingUsageResponse =
         workspacesController.getBillingUsage(ws.getNamespace(), ws.getTerraName()).getBody();
     assertThat(workspaceBillingUsageResponse.getCost()).isEqualTo(0.0d);

diff --git a/api/src/test/java/org/pmiops/workbench/utils/TestMockFactory.java b/api/src/test/java/org/pmiops/workbench/utils/TestMockFactory.java
@@ -61,6 +61,7 @@
 import org.pmiops.workbench.model.ResearchPurpose;
 import org.pmiops.workbench.model.SexAtBirthV2;
 import org.pmiops.workbench.model.SexualOrientationV2;
+import org.pmiops.workbench.model.User;
 import org.pmiops.workbench.model.Workspace;
 import org.pmiops.workbench.model.YesNoPreferNot;
 import org.pmiops.workbench.rawls.model.RawlsWorkspaceAccessLevel;
@@ -119,6 +120,11 @@ public static Workspace createWorkspace(
     List<ResearchOutcomeEnum> ResearchOutcomeEnumsList = new ArrayList<>();
     ResearchOutcomeEnumsList.add(ResearchOutcomeEnum.IMPROVED_RISK_ASSESMENT);
 
+    User creator = new User();
+    creator.setUserName("[email protected]");
+    creator.setGivenName("Jay");
+    creator.setFamilyName("Tester");
+
     return new Workspace()
         .etag("\"1\"")
         .name(workspaceDisplayName)
@@ -131,7 +137,7 @@ public static Workspace createWorkspace(
         .billingAccountName(WORKSPACE_BILLING_ACCOUNT_NAME)
         .googleProject(DEFAULT_GOOGLE_PROJECT)
         .creationTime(1588097211621L)
-        .creator("[email protected]")
+        .creatorUser(creator)
         .creationTime(Instant.parse("2000-01-01T00:00:00.00Z").toEpochMilli())
         .lastModifiedTime(1588097211621L)
         .googleProject(DEFAULT_GOOGLE_PROJECT)

diff --git a/api/src/test/java/org/pmiops/workbench/utils/mappers/WorkspaceMapperTest.java b/api/src/test/java/org/pmiops/workbench/utils/mappers/WorkspaceMapperTest.java
@@ -67,6 +67,8 @@
 public class WorkspaceMapperTest {
   private static final String FIRECLOUD_NAMESPACE = "aou-xxxxxxx";
   private static final String CREATOR_EMAIL = "[email protected]";
+  private static final String CREATOR_GIVEN_NAME = "Oscar";
+  private static final String CREATOR_FAMILY_NAME = "Calhoun";
   private static final long CREATOR_USER_ID = 101L;
   private static final long WORKSPACE_DB_ID = 222L;
   private static final int WORKSPACE_VERSION = 2;
@@ -145,6 +147,8 @@ public void setUp() {
     final DbUser creatorUser =
         new DbUser()
             .setUsername(CREATOR_EMAIL)
+            .setGivenName(CREATOR_GIVEN_NAME)
+            .setFamilyName(CREATOR_FAMILY_NAME)
             .setUserId(CREATOR_USER_ID)
             .setUserInitialCreditsExpiration(
                 new DbUserInitialCreditsExpiration()
@@ -213,7 +217,9 @@ public void testConvertsDbToApiWorkspace() {
     assertThat(ws.getName()).isEqualTo(WORKSPACE_AOU_NAME);
     assertThat(ws.getNamespace()).isEqualTo(FIRECLOUD_NAMESPACE);
     assertThat(ws.getCdrVersionId()).isEqualTo(Long.toString(CDR_VERSION_ID));
-    assertThat(ws.getCreator()).isEqualTo(CREATOR_EMAIL);
+    assertThat(ws.getCreatorUser().getUserName()).isEqualTo(CREATOR_EMAIL);
+    assertThat(ws.getCreatorUser().getGivenName()).isEqualTo(CREATOR_GIVEN_NAME);
+    assertThat(ws.getCreatorUser().getFamilyName()).isEqualTo(CREATOR_FAMILY_NAME);
     assertThat(ws.getInitialCredits().getExpirationEpochMillis())
         .isEqualTo(INITIAL_CREDITS_EXPIRATION_TIMESTAMP.getTime());
     assertThat(ws.getGoogleBucketName()).isEqualTo(FIRECLOUD_BUCKET_NAME);

diff --git a/ui/src/app/components/common-env-conf-panels/environment-informed-action-panel.tsx b/ui/src/app/components/common-env-conf-panels/environment-informed-action-panel.tsx
@@ -130,7 +130,7 @@ export const EnvironmentInformedActionPanel = ({
           <CostsDrawnFrom
             {...{ creatorFreeCreditsRemaining }}
             usingInitialCredits={isUsingFreeTierBillingAccount(workspace)}
-            userIsCreator={profile.username === workspace.creator}
+            userIsCreator={profile.username === workspace.creatorUser.userName}
             billingAccountName={workspace.billingAccountName}
             style={{
               borderLeft: `1px solid ${colorWithWhiteness(colors.dark, 0.5)}`,
@@ -143,7 +143,7 @@ export const EnvironmentInformedActionPanel = ({
         <CostsDrawnFrom
           {...{ creatorFreeCreditsRemaining }}
           usingInitialCredits={isUsingFreeTierBillingAccount(workspace)}
-          userIsCreator={profile.username === workspace.creator}
+          userIsCreator={profile.username === workspace.creatorUser.userName}
           billingAccountName={workspace.billingAccountName}
         />
       )}

diff --git a/ui/src/app/pages/admin/workspace/admin-workspace.tsx b/ui/src/app/pages/admin/workspace/admin-workspace.tsx
@@ -155,7 +155,7 @@ export class AdminWorkspaceImpl extends React.Component<Props, State> {
               <>
                 <Collaborators
                   {...{ collaborators }}
-                  creator={workspace.creator}
+                  creator={workspace.creatorUser.userName}
                 />
                 <CohortBuilder {...{ workspaceObjects }} />
                 <CloudStorageObjects

diff --git a/ui/src/app/pages/admin/workspace/basic-information.tsx b/ui/src/app/pages/admin/workspace/basic-information.tsx
@@ -27,7 +27,7 @@ export const BasicInformation = ({
         </WorkspaceInfoField>
         <WorkspaceInfoField labelText='Billing Account Type'>
           {isUsingFreeTierBillingAccount(workspace)
-            ? `Initial credits (${workspace.creator})`
+            ? `Initial credits (${workspace.creatorUser.userName})`
             : 'User provided'}
         </WorkspaceInfoField>
         {isUsingFreeTierBillingAccount(workspace) && (

diff --git a/ui/src/app/pages/workspace/workspace-card.tsx b/ui/src/app/pages/workspace/workspace-card.tsx
@@ -296,7 +296,8 @@ export const WorkspaceCard = fp.flow(withNavigation)(
                     </div>
                     {useFeaturedWorkspacePageUi && (
                       <div style={{ fontSize: 12 }}>
-                        Created By: {workspace.creator.split('@')[0]}
+                        Created By:{' '}
+                        {workspace.creatorUser.userName.split('@')[0]}
                       </div>
                     )}
                   </FlexColumn>

diff --git a/ui/src/app/pages/workspace/workspace-edit.spec.tsx b/ui/src/app/pages/workspace/workspace-edit.spec.tsx
@@ -134,7 +134,7 @@ describe(WorkspaceEdit.name, () => {
         ],
         researchOutcomeList: [ResearchOutcomeEnum.DECREASE_ILLNESS_BURDEN],
       },
-      creator: ProfileStubVariables.PROFILE_STUB.username,
+      creatorUser: { userName: ProfileStubVariables.PROFILE_STUB.username },
     };
 
     workspaceEditMode = WorkspaceEditMode.Create;