SWC-6610 - Changes from code review

- Table should have null view scope, handle null case in props serializer - Remove extra imports in XML - Rename variables from mock -> stub in test
Sage-Bionetworks · Nov 15, 2023 · 99fd4da · 99fd4da
1 parent 4e7f8fd
commit 99fd4da
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 38 deletions.
diff --git a/src/main/java/org/sagebionetworks/web/client/jsinterop/TableColumnSchemaFormProps.java b/src/main/java/org/sagebionetworks/web/client/jsinterop/TableColumnSchemaFormProps.java
@@ -1,6 +1,7 @@
 package org.sagebionetworks.web.client.jsinterop;
 
 import java.util.List;
+import jsinterop.annotations.JsNullable;
 import jsinterop.annotations.JsOverlay;
 import jsinterop.annotations.JsPackage;
 import jsinterop.annotations.JsType;
@@ -12,7 +13,10 @@
 public class TableColumnSchemaFormProps extends ReactComponentProps {
 
   String entityType;
+
+  @JsNullable
   Object viewScope;
+
   Object[] initialData;
   ReactRef<TableColumnSchemaFormRef> ref;
 
@@ -25,7 +29,9 @@ public static TableColumnSchemaFormProps create(
   ) {
     TableColumnSchemaFormProps props = new TableColumnSchemaFormProps();
     props.entityType = entityType.name();
-    props.viewScope = JSONEntityUtils.toJsInteropCompatibleObject(viewScope);
+    if (viewScope != null) {
+      props.viewScope = JSONEntityUtils.toJsInteropCompatibleObject(viewScope);
+    }
     props.initialData =
       initialData
         .stream()

diff --git a/...rg/sagebionetworks/web/client/widget/table/modal/fileview/CreateTableViewWizardStep2.java b/...rg/sagebionetworks/web/client/widget/table/modal/fileview/CreateTableViewWizardStep2.java
@@ -160,7 +160,7 @@ public ViewScope getViewScope() {
     ViewScope scope = new ViewScope();
     List<String> scopeIds = null;
     if (entity instanceof TableEntity) {
-      return new ViewScope();
+      return null;
     } else if (entity instanceof EntityView) {
       scopeIds = ((EntityView) entity).getScopeIds();
       scope.setViewTypeMask(tableType.getViewTypeMask().longValue());

diff --git a/...gebionetworks/web/client/widget/table/v2/schema/ColumnModelsEditorV2WidgetViewImpl.ui.xml b/...gebionetworks/web/client/widget/table/v2/schema/ColumnModelsEditorV2WidgetViewImpl.ui.xml
@@ -1,9 +1,5 @@
 <ui:UiBinder
   xmlns:ui="urn:ui:com.google.gwt.uibinder"
-  xmlns:g="urn:import:com.google.gwt.user.client.ui"
-  xmlns:b="urn:import:org.gwtbootstrap3.client.ui"
-  xmlns:b.html="urn:import:org.gwtbootstrap3.client.ui.html"
-  xmlns:t="urn:import:org.sagebionetworks.web.client.view.bootstrap.table"
   xmlns:w="urn:import:org.sagebionetworks.web.client.widget"
 >
   <w:ReactComponentDiv ui:field="reactContainer" />

diff --git a/...ionetworks/web/unitclient/widget/table/modal/fileview/CreateTableViewWizardStep2Test.java b/...ionetworks/web/unitclient/widget/table/modal/fileview/CreateTableViewWizardStep2Test.java
@@ -81,13 +81,11 @@ public class CreateTableViewWizardStep2Test {
   String parentId;
   CreateTableViewWizardStep2 widget;
 
-  EntityView mockEntityView;
+  EntityView entityViewStub;
 
-  @Mock
-  TableEntity mockTableEntity;
+  TableEntity tableEntityStub;
 
-  @Mock
-  SubmissionView mockSubmissionView;
+  SubmissionView submissionViewStub;
 
   @Mock
   SynapseClientAsync mockSynapseClient;
@@ -167,12 +165,12 @@ public class CreateTableViewWizardStep2Test {
   @Before
   public void before() {
     MockitoAnnotations.initMocks(this);
-    mockTableEntity = new TableEntity();
-    mockEntityView = new EntityView();
-    mockSubmissionView = new SubmissionView();
-    mockTableEntity.setId(ENTITY_ID);
-    mockEntityView.setId(ENTITY_ID);
-    mockSubmissionView.setId(ENTITY_ID);
+    tableEntityStub = new TableEntity();
+    entityViewStub = new EntityView();
+    submissionViewStub = new SubmissionView();
+    tableEntityStub.setId(ENTITY_ID);
+    entityViewStub.setId(ENTITY_ID);
+    submissionViewStub.setId(ENTITY_ID);
     when(mockGlobalAppState.getPlaceChanger()).thenReturn(mockPlaceChanger);
 
     widget =
@@ -219,8 +217,8 @@ public void before() {
       )
     )
       .thenReturn(mockDefaultSubmissionViewColumnModels);
-    mockEntityView.setScopeIds(mockViewScopeIds);
-    mockSubmissionView.setScopeIds(mockSubmissionViewScopeIds);
+    entityViewStub.setScopeIds(mockViewScopeIds);
+    submissionViewStub.setScopeIds(mockSubmissionViewScopeIds);
     when(mockViewColumnModelResponsePage1.getNextPageToken())
       .thenReturn(NEXT_PAGE_TOKEN);
     when(mockViewColumnModelResponsePage1.getResults())
@@ -252,7 +250,7 @@ void setUpInExperimentalMode() {
 
   @Test
   public void testConfigureTable() {
-    widget.configure(mockTableEntity, TableType.table);
+    widget.configure(tableEntityStub, TableType.table);
     verify(mockEditor).setAddDefaultColumnsButtonVisible(false);
     verify(mockEditor).setAddAnnotationColumnsButtonVisible(false);
     verify(mockEditor).configure(TableType.table, new ArrayList<ColumnModel>());
@@ -262,25 +260,23 @@ public void testConfigureTable() {
   public void testConfigureTableExperimentalMode() {
     setUpInExperimentalMode();
 
-    widget.configure(mockTableEntity, TableType.table);
+    widget.configure(tableEntityStub, TableType.table);
     verify(mockEditorV2)
       .configure(
         eq(EntityType.table),
         viewScopeCaptor.capture(),
         eq(new ArrayList<>())
       );
     ViewScope viewScope = viewScopeCaptor.getValue();
-    assertNull(viewScope.getScope());
-    assertNull(viewScope.getViewType());
-    assertNull(viewScope.getViewEntityType());
+    assertNull(viewScope);
   }
 
   @Test
   public void testConfigureView() {
     verify(mockView).setJobTracker(any(Widget.class));
     verify(mockView).setEditor(any(Widget.class));
 
-    widget.configure(mockEntityView, TableType.file_view);
+    widget.configure(entityViewStub, TableType.file_view);
     verify(mockEditor)
       .configure(TableType.file_view, new ArrayList<ColumnModel>());
     verify(mockEditor).setAddDefaultColumnsButtonVisible(true);
@@ -295,7 +291,7 @@ public void testConfigureViewExperimentalMode() {
     verify(mockView, times(2)).setJobTracker(any(Widget.class));
     verify(mockView, times(2)).setEditor(any(Widget.class));
 
-    widget.configure(mockEntityView, TableType.file_view);
+    widget.configure(entityViewStub, TableType.file_view);
     verify(mockEditorV2)
       .configure(
         eq(EntityType.entityview),
@@ -311,7 +307,7 @@ public void testConfigureViewExperimentalMode() {
 
   @Test
   public void testConfigureSubmissionView() {
-    widget.configure(mockEntityView, TableType.submission_view);
+    widget.configure(entityViewStub, TableType.submission_view);
 
     verify(mockEditor)
       .configure(TableType.submission_view, new ArrayList<ColumnModel>());
@@ -323,7 +319,7 @@ public void testConfigureSubmissionView() {
   @Test
   public void testConfigureSubmissionViewExperimentalMode() {
     setUpInExperimentalMode();
-    widget.configure(mockSubmissionView, TableType.submission_view);
+    widget.configure(submissionViewStub, TableType.submission_view);
 
     verify(mockEditorV2)
       .configure(
@@ -343,7 +339,7 @@ public void testConfigureProjectView() {
     verify(mockView).setJobTracker(any(Widget.class));
     verify(mockView).setEditor(any(Widget.class));
 
-    widget.configure(mockEntityView, TableType.project_view);
+    widget.configure(entityViewStub, TableType.project_view);
     verify(mockEditor)
       .configure(TableType.project_view, new ArrayList<ColumnModel>());
     verify(mockEditor).setAddDefaultColumnsButtonVisible(true);
@@ -358,7 +354,7 @@ public void testConfigureProjectViewExperimentalMode() {
     verify(mockView, times(2)).setJobTracker(any(Widget.class));
     verify(mockView, times(2)).setEditor(any(Widget.class));
 
-    widget.configure(mockEntityView, TableType.project_view);
+    widget.configure(entityViewStub, TableType.project_view);
     verify(mockEditorV2)
       .configure(
         eq(EntityType.entityview),
@@ -375,7 +371,7 @@ public void testConfigureProjectViewExperimentalMode() {
   @Test
   public void testGetPossibleColumnModelsForViewScope() {
     // test is set up so that rpc successfully returns 2 pages, and then stops.
-    widget.configure(mockEntityView, TableType.file_view);
+    widget.configure(entityViewStub, TableType.file_view);
 
     String firstPageToken = null;
     widget.getPossibleColumnModelsForViewScope(firstPageToken);
@@ -401,7 +397,7 @@ public void testGetPossibleColumnModelsForViewScope() {
   @Test
   public void testGetPossibleColumnModelsForProjectViewScope() {
     // test is set up so that rpc successfully returns 2 pages, and then stops.
-    widget.configure(mockEntityView, TableType.project_view);
+    widget.configure(entityViewStub, TableType.project_view);
 
     String firstPageToken = null;
     widget.getPossibleColumnModelsForViewScope(firstPageToken);
@@ -425,7 +421,7 @@ public void testGetPossibleColumnModelsForProjectViewScope() {
 
   @Test
   public void testGetPossibleColumnModelsForSubmissionViewScope() {
-    widget.configure(mockSubmissionView, TableType.submission_view);
+    widget.configure(submissionViewStub, TableType.submission_view);
 
     String firstPageToken = null;
     widget.getPossibleColumnModelsForViewScope(firstPageToken);
@@ -450,7 +446,7 @@ public void testGetPossibleColumnModelsForSubmissionViewScope() {
 
   @Test
   public void testGetPossibleColumnModelsForViewScopeFailure() {
-    widget.configure(mockEntityView, filesFoldersTablesView);
+    widget.configure(entityViewStub, filesFoldersTablesView);
     String error = "error message getting annotation column models";
     Exception ex = new Exception(error);
     String firstPageToken = null;
@@ -489,7 +485,7 @@ public void testOnCancelSuccessfulCleanup() {
     verify(mockWizardPresenter).addCallback(wizardCallbackCaptor.capture());
     ModalWizardWidget.WizardCallback wizardCallback =
       wizardCallbackCaptor.getValue();
-    widget.configure(mockTableEntity, TableType.table);
+    widget.configure(tableEntityStub, TableType.table);
 
     // simulate user cancels
     wizardCallback.onCanceled();
@@ -512,7 +508,7 @@ public void testOnCancelFailedToCleanup() {
     verify(mockWizardPresenter).addCallback(wizardCallbackCaptor.capture());
     ModalWizardWidget.WizardCallback wizardCallback =
       wizardCallbackCaptor.getValue();
-    widget.configure(mockTableEntity, TableType.table);
+    widget.configure(tableEntityStub, TableType.table);
 
     // simulate user cancels
     wizardCallback.onCanceled();
@@ -527,7 +523,7 @@ public void testOnCancelFailedToCleanup() {
   }
 
   private AsynchronousProgressHandler onPrimary() {
-    widget.configure(mockTableEntity, TableType.table);
+    widget.configure(tableEntityStub, TableType.table);
     widget.onPrimary();
     boolean isDeterminate = false;
     ArgumentCaptor<AsynchronousProgressHandler> captor =
@@ -614,7 +610,7 @@ public void testOnPrimaryAsyncFailure() {
 
   @Test
   public void testOnPrimaryFailure() {
-    widget.configure(mockTableEntity, TableType.table);
+    widget.configure(tableEntityStub, TableType.table);
     String error = "error message";
     Exception ex = new Exception(error);
     AsyncMockStubber