From 33011e37f09edaa980c13f5d09f1784b971e0c0d Mon Sep 17 00:00:00 2001 From: Nick Grosenbacher Date: Thu, 3 Oct 2024 13:19:28 -0400 Subject: [PATCH 1/5] SWC-6776, SWC-7096 - fix issue where parentId was null (getting the benefactor of the entityId should be fine) --- .../web/client/SynapseJavascriptClient.java | 29 ++++++++ .../widget/entity/download/Uploader.java | 71 ++++++++++--------- 2 files changed, 67 insertions(+), 33 deletions(-) diff --git a/src/main/java/org/sagebionetworks/web/client/SynapseJavascriptClient.java b/src/main/java/org/sagebionetworks/web/client/SynapseJavascriptClient.java index 3f017b56c8..6068c7f5c3 100644 --- a/src/main/java/org/sagebionetworks/web/client/SynapseJavascriptClient.java +++ b/src/main/java/org/sagebionetworks/web/client/SynapseJavascriptClient.java @@ -3295,4 +3295,33 @@ public FluentFuture deleteSessionAccessToken() { String url = getAuthServiceUrl() + SESSION_ACCESS_TOKEN; return getFuture(cb -> doDelete(url, cb)); } + + public FluentFuture getEntityBenefactorAcl( + String entityId + ) { + // 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; + + return getFuture(cb -> + doPost( + url, + request, + OBJECT_TYPE.EntityBundle, + true, + new AsyncCallback() { + @Override + public void onFailure(Throwable caught) { + cb.onFailure(caught); + } + + @Override + public void onSuccess(EntityBundle result) { + cb.onSuccess(result.getBenefactorAcl()); + } + } + ) + ); + } } 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 5132e89bb4..b707fe7c2b 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 @@ -791,25 +791,27 @@ public void onSuccess(Entity result) { entity = result; view.showInfo(DisplayConstants.TEXT_LINK_SUCCESS); if (successHandler != null) { - synapseClient.getEntityBenefactorAcl( - result.getId(), - new AsyncCallback() { - @Override - public void onSuccess(AccessControlList benefactorAcl) { - successHandler.onSuccessfulUpload(benefactorAcl.getId()); - } + jsClient + .getEntityBenefactorAcl(result.getId()) + .addCallback( + new FutureCallback() { + @Override + public void onSuccess(AccessControlList benefactorAcl) { + 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); - } - } - ); + @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(); + } + }, + directExecutor() + ); } - - entityUpdated(); } @Override @@ -1025,24 +1027,27 @@ private void uploadSuccess() { view.resetToInitialState(); resetUploadProgress(); if (successHandler != null) { - synapseClient.getEntityBenefactorAcl( - parentEntityId, - new AsyncCallback() { - @Override - public void onSuccess(AccessControlList benefactorAcl) { - successHandler.onSuccessfulUpload(benefactorAcl.getId()); - } + jsClient + .getEntityBenefactorAcl(entityId) + .addCallback( + new FutureCallback() { + @Override + public void onSuccess(AccessControlList benefactorAcl) { + 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); - } - } - ); + @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(); + } + }, + directExecutor() + ); } - entityUpdated(); } private void resetUploadProgress() { From 87a4fd0bb7800a36a5910f5c1c3ecc784ac17b94 Mon Sep 17 00:00:00 2001 From: Nick Grosenbacher Date: Thu, 3 Oct 2024 13:45:07 -0400 Subject: [PATCH 2/5] SWC-6776, SWC-7096 - Fix tests, use AsyncCallback for easier mocking --- .../web/client/SynapseJavascriptClient.java | 35 +++++---- .../widget/entity/download/Uploader.java | 74 ++++++++++--------- .../widget/entity/download/UploaderTest.java | 8 +- 3 files changed, 61 insertions(+), 56 deletions(-) diff --git a/src/main/java/org/sagebionetworks/web/client/SynapseJavascriptClient.java b/src/main/java/org/sagebionetworks/web/client/SynapseJavascriptClient.java index 6068c7f5c3..0a9a2c15ac 100644 --- a/src/main/java/org/sagebionetworks/web/client/SynapseJavascriptClient.java +++ b/src/main/java/org/sagebionetworks/web/client/SynapseJavascriptClient.java @@ -3296,32 +3296,31 @@ public FluentFuture deleteSessionAccessToken() { return getFuture(cb -> doDelete(url, cb)); } - public FluentFuture getEntityBenefactorAcl( - String entityId + 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; - return getFuture(cb -> - doPost( - url, - request, - OBJECT_TYPE.EntityBundle, - true, - new AsyncCallback() { - @Override - public void onFailure(Throwable caught) { - cb.onFailure(caught); - } + doPost( + url, + request, + OBJECT_TYPE.EntityBundle, + true, + new AsyncCallback() { + @Override + public void onSuccess(EntityBundle result) { + cb.onSuccess(result.getBenefactorAcl()); + } - @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/Uploader.java b/src/main/java/org/sagebionetworks/web/client/widget/entity/download/Uploader.java index b707fe7c2b..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 @@ -791,26 +791,29 @@ public void onSuccess(Entity result) { entity = result; view.showInfo(DisplayConstants.TEXT_LINK_SUCCESS); if (successHandler != null) { - jsClient - .getEntityBenefactorAcl(result.getId()) - .addCallback( - new FutureCallback() { - @Override - public void onSuccess(AccessControlList benefactorAcl) { + 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(); - } - }, - directExecutor() - ); + @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(); + } + } + ); } } @@ -1027,26 +1030,29 @@ private void uploadSuccess() { view.resetToInitialState(); resetUploadProgress(); if (successHandler != null) { - jsClient - .getEntityBenefactorAcl(entityId) - .addCallback( - new FutureCallback() { - @Override - public void onSuccess(AccessControlList benefactorAcl) { + 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(); } + 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(); - } - }, - directExecutor() - ); + @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(); + } + } + ); } } 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 edf8ee069b..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 @@ -293,7 +293,7 @@ public void testSetNewExternalPath() throws Exception { // associate the path. AsyncMockStubber .callSuccessWith(new AccessControlList().setId(UPLOAD_BENEFACTOR_ID)) - .when(mockSynapseClient) + .when(mockSynapseJavascriptClient) .getEntityBenefactorAcl(anyString(), any(AsyncCallback.class)); uploader.setExternalFilePath( @@ -312,7 +312,7 @@ public void testSetNewExternalPath() throws Exception { eq(storageLocationId), any() ); - verify(mockSynapseClient) + verify(mockSynapseJavascriptClient) .getEntityBenefactorAcl(anyString(), any(AsyncCallback.class)); verify(mockView).showInfo(anyString()); verify(mockUploadSuccessHandler).onSuccessfulUpload(UPLOAD_BENEFACTOR_ID); @@ -420,14 +420,14 @@ public void testDirectUploadHappyCase() throws Exception { .getEntity(anyString(), any(OBJECT_TYPE.class), any(AsyncCallback.class)); AsyncMockStubber .callSuccessWith(new AccessControlList().setId(UPLOAD_BENEFACTOR_ID)) - .when(mockSynapseClient) + .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(mockSynapseClient) + verify(mockSynapseJavascriptClient) .getEntityBenefactorAcl(anyString(), any(AsyncCallback.class)); verify(mockView).hideLoading(); assertEquals(UploadType.S3, uploader.getCurrentUploadType()); From dfdc9e8bebe083f6e18d6bed0e2fc3fa6b106d40 Mon Sep 17 00:00:00 2001 From: Xavier Schildwachter Date: Thu, 3 Oct 2024 12:09:52 -0700 Subject: [PATCH 3/5] Add job summary (#5538) --- .github/workflows/main.yaml | 4 ++++ 1 file changed, 4 insertions(+) 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' }} From c411df67a52599d6530fa6d1d4d789ab4307de6b Mon Sep 17 00:00:00 2001 From: Nick Grosenbacher Date: Fri, 4 Oct 2024 13:19:10 -0400 Subject: [PATCH 4/5] SWC-6776 - Put feature in experimental mode under feature flag --- .../web/client/FeatureFlagKey.java | 3 + .../entity/download/UploadDialogWidget.java | 14 ++++- .../entity/download/UploadDialogTest.java | 56 ++++++++++++++++++- 3 files changed, 70 insertions(+), 3 deletions(-) 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/widget/entity/download/UploadDialogWidget.java b/src/main/java/org/sagebionetworks/web/client/widget/entity/download/UploadDialogWidget.java index af2608df27..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 @@ -4,6 +4,8 @@ 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; @@ -16,18 +18,21 @@ public class UploadDialogWidget private Uploader uploader; private final EventBus eventBus; private final EntityAccessControlListModalWidget entityAclEditor; + private final FeatureFlagConfig featureFlagConfig; @Inject public UploadDialogWidget( UploadDialogWidgetView view, Uploader uploader, EventBus eventBus, - EntityAccessControlListModalWidget entityAccessControlListModalWidget + EntityAccessControlListModalWidget entityAccessControlListModalWidget, + FeatureFlagConfig featureFlagConfig ) { this.view = view; this.uploader = uploader; this.eventBus = eventBus; this.entityAclEditor = entityAccessControlListModalWidget; + this.featureFlagConfig = featureFlagConfig; view.setPresenter(this); } @@ -54,7 +59,12 @@ public void configure( // add handlers for closing the window uploader.setSuccessHandler(benefactorId -> { view.hideDialog(); - if (benefactorId != null) { + if ( + benefactorId != null && + featureFlagConfig.isFeatureEnabled( + FeatureFlagKey.SHOW_SHARING_SETTINGS_AFTER_UPLOAD + ) + ) { entityAclEditor.configure( benefactorId, () -> eventBus.fireEvent(new EntityUpdatedEvent(benefactorId)), 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 8aed2efd02..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,9 +1,12 @@ 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.event.shared.EventBus; import org.junit.Before; @@ -14,6 +17,8 @@ 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; @@ -38,6 +43,9 @@ public class UploadDialogTest { @Mock EntityAccessControlListModalWidget mockEntityAccessControlListModalWidget; + @Mock + FeatureFlagConfig mockFeatureFlagConfig; + @Captor ArgumentCaptor uploadSuccessCaptor; @@ -55,12 +63,58 @@ public void before() throws Exception { view, mockUploader, mockEventBus, - mockEntityAccessControlListModalWidget + 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"; From e56d463f115f60fd12e77a6b3354fda6c523bef9 Mon Sep 17 00:00:00 2001 From: Nick Grosenbacher Date: Fri, 4 Oct 2024 16:32:46 -0400 Subject: [PATCH 5/5] Reapply "SWC-6776 - Show ACL editor after successful upload" (#5535) This reverts commit f25c2d430ed990e72b44e3594ab385192d7af1fe. --- .../client/events/UploadSuccessHandler.java | 6 ++- .../jsinterop/EntityAclEditorModalProps.java | 5 +- .../entity/download/UploadDialogWidget.java | 24 +++++++++- .../widget/entity/download/Uploader.java | 35 +++++++++++++- .../EntityAccessControlListModalWidget.java | 6 +++ ...ntityAccessControlListModalWidgetImpl.java | 12 ++++- .../entity/download/UploadDialogTest.java | 46 +++++++++++++++++-- .../widget/entity/download/UploaderTest.java | 21 +++++++-- 8 files changed, 141 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/sagebionetworks/web/client/events/UploadSuccessHandler.java b/src/main/java/org/sagebionetworks/web/client/events/UploadSuccessHandler.java index 3108212f7b..cf3c6af051 100644 --- a/src/main/java/org/sagebionetworks/web/client/events/UploadSuccessHandler.java +++ b/src/main/java/org/sagebionetworks/web/client/events/UploadSuccessHandler.java @@ -1,5 +1,9 @@ package org.sagebionetworks.web.client.events; public interface UploadSuccessHandler { - void onSuccessfulUpload(); + /** + * Called when one or more files have been successfully uploaded. + * @param benefactorId the benefactor ID of all uploaded files. May be null if request to get benefactor fails + */ + void onSuccessfulUpload(String benefactorId); } diff --git a/src/main/java/org/sagebionetworks/web/client/jsinterop/EntityAclEditorModalProps.java b/src/main/java/org/sagebionetworks/web/client/jsinterop/EntityAclEditorModalProps.java index f51b48428d..8d067a33de 100644 --- a/src/main/java/org/sagebionetworks/web/client/jsinterop/EntityAclEditorModalProps.java +++ b/src/main/java/org/sagebionetworks/web/client/jsinterop/EntityAclEditorModalProps.java @@ -17,13 +17,15 @@ public interface Callback { public boolean open; public Callback onUpdateSuccess; public Callback onClose; + public boolean isAfterUpload; @JsOverlay public static EntityAclEditorModalProps create( String entityId, boolean open, Callback onUpdateSuccess, - Callback onClose + Callback onClose, + boolean isAfterUpload ) { EntityAclEditorModalProps props = new EntityAclEditorModalProps(); @@ -31,6 +33,7 @@ public static EntityAclEditorModalProps create( props.open = open; props.onUpdateSuccess = onUpdateSuccess; props.onClose = onClose; + props.isAfterUpload = isAfterUpload; return props; } } 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..af2608df27 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,33 @@ 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.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; @Inject - public UploadDialogWidget(UploadDialogWidgetView view, Uploader uploader) { + public UploadDialogWidget( + UploadDialogWidgetView view, + Uploader uploader, + EventBus eventBus, + EntityAccessControlListModalWidget entityAccessControlListModalWidget + ) { this.view = view; this.uploader = uploader; + this.eventBus = eventBus; + this.entityAclEditor = entityAccessControlListModalWidget; view.setPresenter(this); } @@ -40,8 +52,16 @@ public void configure( view.configureDialog(title, body); // add handlers for closing the window - uploader.setSuccessHandler(() -> { + uploader.setSuccessHandler(benefactorId -> { view.hideDialog(); + if (benefactorId != null) { + 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..5132e89bb4 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,7 +791,22 @@ public void onSuccess(Entity result) { entity = result; view.showInfo(DisplayConstants.TEXT_LINK_SUCCESS); if (successHandler != null) { - successHandler.onSuccessfulUpload(); + synapseClient.getEntityBenefactorAcl( + result.getId(), + new AsyncCallback() { + @Override + public void onSuccess(AccessControlList benefactorAcl) { + successHandler.onSuccessfulUpload(benefactorAcl.getId()); + } + + @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(); @@ -1009,7 +1025,22 @@ private void uploadSuccess() { view.resetToInitialState(); resetUploadProgress(); if (successHandler != null) { - successHandler.onSuccessfulUpload(); + synapseClient.getEntityBenefactorAcl( + parentEntityId, + new AsyncCallback() { + @Override + public void onSuccess(AccessControlList benefactorAcl) { + successHandler.onSuccessfulUpload(benefactorAcl.getId()); + } + + @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(); } diff --git a/src/main/java/org/sagebionetworks/web/client/widget/sharing/EntityAccessControlListModalWidget.java b/src/main/java/org/sagebionetworks/web/client/widget/sharing/EntityAccessControlListModalWidget.java index 8ad84f0636..9208466432 100644 --- a/src/main/java/org/sagebionetworks/web/client/widget/sharing/EntityAccessControlListModalWidget.java +++ b/src/main/java/org/sagebionetworks/web/client/widget/sharing/EntityAccessControlListModalWidget.java @@ -12,5 +12,11 @@ void configure( EntityAclEditorModalProps.Callback onUpdateSuccess ); + void configure( + String entityId, + EntityAclEditorModalProps.Callback onUpdateSuccess, + boolean isAfterUpload + ); + void setOpen(boolean open); } diff --git a/src/main/java/org/sagebionetworks/web/client/widget/sharing/EntityAccessControlListModalWidgetImpl.java b/src/main/java/org/sagebionetworks/web/client/widget/sharing/EntityAccessControlListModalWidgetImpl.java index 02a241c5e5..384f37da82 100644 --- a/src/main/java/org/sagebionetworks/web/client/widget/sharing/EntityAccessControlListModalWidgetImpl.java +++ b/src/main/java/org/sagebionetworks/web/client/widget/sharing/EntityAccessControlListModalWidgetImpl.java @@ -31,13 +31,23 @@ public class EntityAccessControlListModalWidgetImpl public void configure( String entityId, EntityAclEditorModalProps.Callback onUpdateSuccess + ) { + configure(entityId, onUpdateSuccess, false); + } + + @Override + public void configure( + String entityId, + EntityAclEditorModalProps.Callback onUpdateSuccess, + boolean isAfterUpload ) { componentProps = EntityAclEditorModalProps.create( entityId, false, onUpdateSuccess, - () -> setOpen(false) + () -> setOpen(false), + isAfterUpload ); renderComponent(); } 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..8aed2efd02 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 @@ -5,19 +5,23 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; -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.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,11 +32,31 @@ public class UploadDialogTest { @Mock Uploader mockUploader; + @Mock + EventBus mockEventBus; + + @Mock + EntityAccessControlListModalWidget mockEntityAccessControlListModalWidget; + + @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 + ); } @Test @@ -54,8 +78,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..edf8ee069b 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(mockSynapseClient) + .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(mockSynapseClient) + .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(mockSynapseClient) + .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(mockSynapseClient) + .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)); }