From ebc35a93b182cf39e77460f4c8d2c19b804789b1 Mon Sep 17 00:00:00 2001 From: Nick Grosenbacher Date: Fri, 4 Oct 2024 15:01:47 -0400 Subject: [PATCH] Merge changes from release-516 into develop --- .github/workflows/main.yaml | 4 + .../web/client/FeatureFlagKey.java | 3 + .../web/client/SynapseJavascriptClient.java | 28 +++++ .../entity/download/UploadDialogWidget.java | 34 +++++- .../widget/entity/download/Uploader.java | 52 ++++++++- .../entity/download/UploadDialogTest.java | 100 +++++++++++++++++- .../widget/entity/download/UploaderTest.java | 21 +++- 7 files changed, 228 insertions(+), 14 deletions(-) diff --git a/.github/workflows/main.yaml b/.github/workflows/main.yaml index 1bfb81912b..e667b89662 100644 --- a/.github/workflows/main.yaml +++ b/.github/workflows/main.yaml @@ -31,6 +31,10 @@ jobs: MAVEN_USERNAME: ${{ secrets.PLATFORM_ARTIFACTORY_USER }} MAVEN_USERPWD: ${{ secrets.PLATFORM_ARTIFACTORY_PWD }} + - name: setup-job-summary + run: | + echo "| version | ${{ env.pomversion }} |" > $GITHUB_STEP_SUMMARY + call-test: runs-on: ubuntu-latest if: ${{ github.event_name == 'pull_request' }} diff --git a/src/main/java/org/sagebionetworks/web/client/FeatureFlagKey.java b/src/main/java/org/sagebionetworks/web/client/FeatureFlagKey.java index 489779ba58..0960d6cddd 100644 --- a/src/main/java/org/sagebionetworks/web/client/FeatureFlagKey.java +++ b/src/main/java/org/sagebionetworks/web/client/FeatureFlagKey.java @@ -37,6 +37,9 @@ public enum FeatureFlagKey { // If enabled, use the re-implemented ACL Editor for entities REACT_ENTITY_ACL_EDITOR("REACT_ENTITY_ACL_EDITOR"), + // If enabled, sharing settings will appear in a dialog immediately after uploading one or more files. + SHOW_SHARING_SETTINGS_AFTER_UPLOAD("SHOW_SHARING_SETTINGS_AFTER_UPLOAD"), + // Last flag is used only for tests TEST_FLAG_ONLY("TEST_FLAG_ONLY"); diff --git a/src/main/java/org/sagebionetworks/web/client/SynapseJavascriptClient.java b/src/main/java/org/sagebionetworks/web/client/SynapseJavascriptClient.java index 3f017b56c8..0a9a2c15ac 100644 --- a/src/main/java/org/sagebionetworks/web/client/SynapseJavascriptClient.java +++ b/src/main/java/org/sagebionetworks/web/client/SynapseJavascriptClient.java @@ -3295,4 +3295,32 @@ public FluentFuture deleteSessionAccessToken() { String url = getAuthServiceUrl() + SESSION_ACCESS_TOKEN; return getFuture(cb -> doDelete(url, cb)); } + + public void getEntityBenefactorAcl( + String entityId, + AsyncCallback cb + ) { + // Retrieving the benefactor ACL is always permitted regardless of permissions, so only retrieve that part of the bundle. + EntityBundleRequest request = new EntityBundleRequest(); + request.setIncludeBenefactorACL(true); + String url = getRepoServiceUrl() + ENTITY + "/" + entityId + BUNDLE2; + + doPost( + url, + request, + OBJECT_TYPE.EntityBundle, + true, + new AsyncCallback() { + @Override + public void onSuccess(EntityBundle result) { + cb.onSuccess(result.getBenefactorAcl()); + } + + @Override + public void onFailure(Throwable caught) { + cb.onFailure(caught); + } + } + ); + } } diff --git a/src/main/java/org/sagebionetworks/web/client/widget/entity/download/UploadDialogWidget.java b/src/main/java/org/sagebionetworks/web/client/widget/entity/download/UploadDialogWidget.java index fce5ca3c1b..527f41c389 100644 --- a/src/main/java/org/sagebionetworks/web/client/widget/entity/download/UploadDialogWidget.java +++ b/src/main/java/org/sagebionetworks/web/client/widget/entity/download/UploadDialogWidget.java @@ -1,21 +1,38 @@ package org.sagebionetworks.web.client.widget.entity.download; +import com.google.gwt.event.shared.EventBus; import com.google.gwt.user.client.ui.Widget; import com.google.inject.Inject; import org.sagebionetworks.repo.model.Entity; +import org.sagebionetworks.web.client.FeatureFlagConfig; +import org.sagebionetworks.web.client.FeatureFlagKey; +import org.sagebionetworks.web.client.events.EntityUpdatedEvent; import org.sagebionetworks.web.client.utils.CallbackP; import org.sagebionetworks.web.client.widget.SynapseWidgetPresenter; +import org.sagebionetworks.web.client.widget.sharing.EntityAccessControlListModalWidget; public class UploadDialogWidget implements UploadDialogWidgetView.Presenter, SynapseWidgetPresenter { private UploadDialogWidgetView view; private Uploader uploader; + private final EventBus eventBus; + private final EntityAccessControlListModalWidget entityAclEditor; + private final FeatureFlagConfig featureFlagConfig; @Inject - public UploadDialogWidget(UploadDialogWidgetView view, Uploader uploader) { + public UploadDialogWidget( + UploadDialogWidgetView view, + Uploader uploader, + EventBus eventBus, + EntityAccessControlListModalWidget entityAccessControlListModalWidget, + FeatureFlagConfig featureFlagConfig + ) { this.view = view; this.uploader = uploader; + this.eventBus = eventBus; + this.entityAclEditor = entityAccessControlListModalWidget; + this.featureFlagConfig = featureFlagConfig; view.setPresenter(this); } @@ -40,8 +57,21 @@ public void configure( view.configureDialog(title, body); // add handlers for closing the window - uploader.setSuccessHandler(() -> { + uploader.setSuccessHandler(benefactorId -> { view.hideDialog(); + if ( + benefactorId != null && + featureFlagConfig.isFeatureEnabled( + FeatureFlagKey.SHOW_SHARING_SETTINGS_AFTER_UPLOAD + ) + ) { + entityAclEditor.configure( + benefactorId, + () -> eventBus.fireEvent(new EntityUpdatedEvent(benefactorId)), + true + ); + entityAclEditor.setOpen(true); + } }); uploader.setCancelHandler(() -> { diff --git a/src/main/java/org/sagebionetworks/web/client/widget/entity/download/Uploader.java b/src/main/java/org/sagebionetworks/web/client/widget/entity/download/Uploader.java index 7b46fcb42f..37a489e1e1 100644 --- a/src/main/java/org/sagebionetworks/web/client/widget/entity/download/Uploader.java +++ b/src/main/java/org/sagebionetworks/web/client/widget/entity/download/Uploader.java @@ -12,6 +12,7 @@ import com.google.gwt.user.client.ui.Widget; import com.google.inject.Inject; import java.util.List; +import org.sagebionetworks.repo.model.AccessControlList; import org.sagebionetworks.repo.model.Entity; import org.sagebionetworks.repo.model.Folder; import org.sagebionetworks.repo.model.attachment.UploadResult; @@ -790,10 +791,30 @@ public void onSuccess(Entity result) { entity = result; view.showInfo(DisplayConstants.TEXT_LINK_SUCCESS); if (successHandler != null) { - successHandler.onSuccessfulUpload(); - } + jsClient.getEntityBenefactorAcl( + result.getId(), + new AsyncCallback() { + @Override + public void onSuccess(AccessControlList benefactorAcl) { + if (benefactorAcl.getId().equals(entity.getId())) { + // Don't show the ACL modal if the entity is its own benefactor + successHandler.onSuccessfulUpload(null); + } else { + successHandler.onSuccessfulUpload(benefactorAcl.getId()); + } + entityUpdated(); + } - entityUpdated(); + @Override + public void onFailure(Throwable caught) { + view.showErrorMessage(caught.getMessage()); + // Upload was still a success, benefactor ID is not required to continue + successHandler.onSuccessfulUpload(null); + entityUpdated(); + } + } + ); + } } @Override @@ -1009,9 +1030,30 @@ private void uploadSuccess() { view.resetToInitialState(); resetUploadProgress(); if (successHandler != null) { - successHandler.onSuccessfulUpload(); + jsClient.getEntityBenefactorAcl( + entityId, + new AsyncCallback() { + @Override + public void onSuccess(AccessControlList benefactorAcl) { + if (benefactorAcl.getId().equals(entityId)) { + // Don't show the ACL modal if the entity is its own benefactor + successHandler.onSuccessfulUpload(null); + } else { + successHandler.onSuccessfulUpload(benefactorAcl.getId()); + } + entityUpdated(); + } + + @Override + public void onFailure(Throwable caught) { + view.showErrorMessage(caught.getMessage()); + // Upload was still a success, benefactor ID is not required to continue. + successHandler.onSuccessfulUpload(null); + entityUpdated(); + } + } + ); } - entityUpdated(); } private void resetUploadProgress() { diff --git a/src/test/java/org/sagebionetworks/web/unitclient/widget/entity/download/UploadDialogTest.java b/src/test/java/org/sagebionetworks/web/unitclient/widget/entity/download/UploadDialogTest.java index a61643f87b..5c2fe4316e 100644 --- a/src/test/java/org/sagebionetworks/web/unitclient/widget/entity/download/UploadDialogTest.java +++ b/src/test/java/org/sagebionetworks/web/unitclient/widget/entity/download/UploadDialogTest.java @@ -1,23 +1,32 @@ package org.sagebionetworks.web.unitclient.widget.entity.download; +import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.Matchers.any; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; -import com.google.gwt.user.client.ui.Widget; +import com.google.gwt.event.shared.EventBus; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Captor; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; import org.sagebionetworks.repo.model.Entity; +import org.sagebionetworks.web.client.FeatureFlagConfig; +import org.sagebionetworks.web.client.FeatureFlagKey; import org.sagebionetworks.web.client.events.CancelHandler; import org.sagebionetworks.web.client.events.UploadSuccessHandler; +import org.sagebionetworks.web.client.jsinterop.EntityAclEditorModalProps; import org.sagebionetworks.web.client.utils.CallbackP; import org.sagebionetworks.web.client.widget.entity.download.UploadDialogWidget; import org.sagebionetworks.web.client.widget.entity.download.UploadDialogWidgetView; import org.sagebionetworks.web.client.widget.entity.download.Uploader; +import org.sagebionetworks.web.client.widget.sharing.EntityAccessControlListModalWidget; @RunWith(MockitoJUnitRunner.Silent.class) public class UploadDialogTest { @@ -28,15 +37,84 @@ public class UploadDialogTest { @Mock Uploader mockUploader; + @Mock + EventBus mockEventBus; + + @Mock + EntityAccessControlListModalWidget mockEntityAccessControlListModalWidget; + + @Mock + FeatureFlagConfig mockFeatureFlagConfig; + + @Captor + ArgumentCaptor uploadSuccessCaptor; + + @Captor + ArgumentCaptor< + EntityAclEditorModalProps.Callback + > updateAclSuccessCallbackCaptor; + UploadDialogWidget widget; @Before public void before() throws Exception { - widget = new UploadDialogWidget(view, mockUploader); + widget = + new UploadDialogWidget( + view, + mockUploader, + mockEventBus, + mockEntityAccessControlListModalWidget, + mockFeatureFlagConfig + ); } @Test public void testConfigure() { + when( + mockFeatureFlagConfig.isFeatureEnabled( + FeatureFlagKey.SHOW_SHARING_SETTINGS_AFTER_UPLOAD + ) + ) + .thenReturn(false); + + String title = "dialog title"; + Entity entity = mock(Entity.class); + String parentEntityId = "parent"; + CallbackP fileHandleIdCallback = mock(CallbackP.class); + boolean isEntity = true; + widget.configure( + title, + entity, + parentEntityId, + fileHandleIdCallback, + isEntity + ); + + verify(mockUploader) + .configure(entity, parentEntityId, fileHandleIdCallback, isEntity); + verify(view).configureDialog(eq(title), any()); + + verify(mockUploader).setSuccessHandler(uploadSuccessCaptor.capture()); + verify(mockUploader).setCancelHandler(any(CancelHandler.class)); + + // simulate a successful upload + String benefactorId = "syn123"; + uploadSuccessCaptor.getValue().onSuccessfulUpload(benefactorId); + verify(view).hideDialog(); + verify(mockEntityAccessControlListModalWidget, never()) + .configure(eq(benefactorId), any(), anyBoolean()); + verify(mockEntityAccessControlListModalWidget, never()).setOpen(true); + } + + @Test + public void testSharingSettingsFeatureEnabled() { + when( + mockFeatureFlagConfig.isFeatureEnabled( + FeatureFlagKey.SHOW_SHARING_SETTINGS_AFTER_UPLOAD + ) + ) + .thenReturn(true); + String title = "dialog title"; Entity entity = mock(Entity.class); String parentEntityId = "parent"; @@ -54,8 +132,24 @@ public void testConfigure() { .configure(entity, parentEntityId, fileHandleIdCallback, isEntity); verify(view).configureDialog(eq(title), any()); - verify(mockUploader).setSuccessHandler(any(UploadSuccessHandler.class)); + verify(mockUploader).setSuccessHandler(uploadSuccessCaptor.capture()); verify(mockUploader).setCancelHandler(any(CancelHandler.class)); + + // simulate a successful upload + String benefactorId = "syn123"; + uploadSuccessCaptor.getValue().onSuccessfulUpload(benefactorId); + verify(view).hideDialog(); + verify(mockEntityAccessControlListModalWidget) + .configure( + eq(benefactorId), + updateAclSuccessCallbackCaptor.capture(), + eq(true) + ); + verify(mockEntityAccessControlListModalWidget).setOpen(true); + + // Simulate a successful ACL save + updateAclSuccessCallbackCaptor.getValue().run(); + verify(mockEventBus).fireEvent(any()); } @Test diff --git a/src/test/java/org/sagebionetworks/web/unitclient/widget/entity/download/UploaderTest.java b/src/test/java/org/sagebionetworks/web/unitclient/widget/entity/download/UploaderTest.java index 4fb03875e8..c8064eae3e 100644 --- a/src/test/java/org/sagebionetworks/web/unitclient/widget/entity/download/UploaderTest.java +++ b/src/test/java/org/sagebionetworks/web/unitclient/widget/entity/download/UploaderTest.java @@ -6,7 +6,6 @@ import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyInt; -import static org.mockito.Matchers.anyLong; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.doAnswer; @@ -30,13 +29,13 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; -import org.mockito.ArgumentMatchers; import org.mockito.Captor; import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.invocation.InvocationOnMock; import org.mockito.junit.MockitoJUnitRunner; import org.mockito.stubbing.Answer; +import org.sagebionetworks.repo.model.AccessControlList; import org.sagebionetworks.repo.model.Entity; import org.sagebionetworks.repo.model.FileEntity; import org.sagebionetworks.repo.model.Folder; @@ -166,6 +165,7 @@ public class UploaderTest { private final Long defaultSynapseStorageId = 1L; public static final String SUCCESS_FILE_HANDLE = "99999"; + public static final String UPLOAD_BENEFACTOR_ID = "syn12345"; @Before public void before() throws Exception { @@ -291,6 +291,11 @@ public void testSetNewExternalPath() throws Exception { // this is the full success test // if entity is null, it should call synapseClient.createExternalFile() to create the FileEntity and // associate the path. + AsyncMockStubber + .callSuccessWith(new AccessControlList().setId(UPLOAD_BENEFACTOR_ID)) + .when(mockSynapseJavascriptClient) + .getEntityBenefactorAcl(anyString(), any(AsyncCallback.class)); + uploader.setExternalFilePath( "http://fakepath.url/blah.xml", "", @@ -307,8 +312,10 @@ public void testSetNewExternalPath() throws Exception { eq(storageLocationId), any() ); + verify(mockSynapseJavascriptClient) + .getEntityBenefactorAcl(anyString(), any(AsyncCallback.class)); verify(mockView).showInfo(anyString()); - verify(mockUploadSuccessHandler).onSuccessfulUpload(); + verify(mockUploadSuccessHandler).onSuccessfulUpload(UPLOAD_BENEFACTOR_ID); } @Test @@ -411,11 +418,17 @@ public void testDirectUploadHappyCase() throws Exception { .callSuccessWith(testEntity) .when(mockSynapseJavascriptClient) .getEntity(anyString(), any(OBJECT_TYPE.class), any(AsyncCallback.class)); + AsyncMockStubber + .callSuccessWith(new AccessControlList().setId(UPLOAD_BENEFACTOR_ID)) + .when(mockSynapseJavascriptClient) + .getEntityBenefactorAcl(anyString(), any(AsyncCallback.class)); uploader.handleUploads(); verify(mockGlobalApplicationState).clearDropZoneHandler(); // SWC-5161 (cleared on handleUploads) verify(mockView).disableSelectionDuringUpload(); verify(mockSynapseClient) .setFileEntityFileHandle(any(), any(), any(), any()); + verify(mockSynapseJavascriptClient) + .getEntityBenefactorAcl(anyString(), any(AsyncCallback.class)); verify(mockView).hideLoading(); assertEquals(UploadType.S3, uploader.getCurrentUploadType()); // verify upload success @@ -423,7 +436,7 @@ public void testDirectUploadHappyCase() throws Exception { verify(mockView).showSingleFileUploaded("entityID"); verify(mockView).clear(); verify(mockView, times(2)).resetToInitialState(); - verify(mockUploadSuccessHandler).onSuccessfulUpload(); + verify(mockUploadSuccessHandler).onSuccessfulUpload(UPLOAD_BENEFACTOR_ID); verify(mockEventBus).fireEvent(any(EntityUpdatedEvent.class)); }