Pull-request comment cleanup

NASA-AMMOS · Oct 15, 2024 · a670831 · a670831
1 parent 7c3d33c
commit a670831
Show file tree

Hide file tree

Showing 19 changed files with 202 additions and 186 deletions.
diff --git a/e2e-tests/fixtures/ExternalSources.ts b/e2e-tests/fixtures/ExternalSources.ts
@@ -49,10 +49,13 @@ export class ExternalSources {
   }
 
   async deleteSource(sourceName: string) {
-    await this.selectSource(sourceName);
-    await this.deleteSourceButton.click();
-    await this.deleteSourceButtonConfirmation.click();
-    await expect(this.externalSourcesTable.getByText(sourceName)).not.toBeVisible();
+    // Only delete a source if its visible in the table
+    if (await this.page.getByRole('gridcell', { name: sourceName }).first().isVisible()) {
+      await this.selectSource(sourceName);
+      await this.deleteSourceButton.click();
+      await this.deleteSourceButtonConfirmation.click();
+      await expect(this.externalSourcesTable.getByText(sourceName)).not.toBeVisible();
+    }
   }
 
   async fillInputFile(externalSourceFilePath: string) {
@@ -88,10 +91,10 @@ export class ExternalSources {
     await expect(this.page.getByRole('button', { exact: true, name: sourceTypeName })).toBeVisible();
   }
 
-  async selectEvent() {
+  async selectEvent(eventName: string, sourceName: string = 'example-external-source.json') {
     // Assumes the selected source was the test source, and selects the specific event from it
     // NOTE: This may not be the case, and should be re-visited when we implement deletion for External Sources!
-    await this.selectSource();
+    await this.selectSource(sourceName);
     await this.page.getByRole('gridcell', { name: 'ExampleEvent:1/sc/sc1:1' }).click();
   }
 

diff --git a/e2e-tests/tests/external-sources.test.ts b/e2e-tests/tests/external-sources.test.ts
@@ -37,7 +37,7 @@ test.describe.serial('External Sources', () => {
   });
 
   test('External event form should be shown when an event is selected', async () => {
-    await externalSources.selectEvent();
+    await externalSources.selectEvent('ExampleEvent:1/sc/sc1:1');
     await expect(externalSources.inputFile).not.toBeVisible();
   });
 
@@ -54,7 +54,7 @@ test.describe.serial('External Sources', () => {
   });
 
   test('External event deselection should be shown when a source is selected', async () => {
-    await externalSources.selectEvent();
+    await externalSources.selectEvent('ExampleEvent:1/sc/sc1:1');
     await expect(page.getByLabel('Deselect event')).toBeVisible();
   });
 

diff --git a/e2e-tests/tests/plan-external-source.test.ts b/e2e-tests/tests/plan-external-source.test.ts
@@ -50,25 +50,11 @@ test.afterAll(async () => {
 
   await externalSources.goto();
   // Cleanup all test files that *may* have been uploaded
-  if (await page.getByRole('gridcell', { name: externalSources.externalSourceFileName }).first().isVisible()) {
-    await externalSources.deleteSource(externalSources.externalSourceFileName);
-  }
-
-  if (await page.getByRole('gridcell', { name: externalSources.derivationTestFileKey1 }).first().isVisible()) {
-    await externalSources.deleteSource(externalSources.derivationTestFileKey1);
-  }
-
-  if (await page.getByRole('gridcell', { name: externalSources.derivationTestFileKey2 }).first().isVisible()) {
-    await externalSources.deleteSource(externalSources.derivationTestFileKey2);
-  }
-
-  if (await page.getByRole('gridcell', { name: externalSources.derivationTestFileKey3 }).first().isVisible()) {
-    await externalSources.deleteSource(externalSources.derivationTestFileKey3);
-  }
-
-  if (await page.getByRole('gridcell', { name: externalSources.derivationTestFileKey4 }).first().isVisible()) {
-    await externalSources.deleteSource(externalSources.derivationTestFileKey4);
-  }
+  await externalSources.deleteSource(externalSources.externalSourceFileName);
+  await externalSources.deleteSource(externalSources.derivationTestFileKey1);
+  await externalSources.deleteSource(externalSources.derivationTestFileKey2);
+  await externalSources.deleteSource(externalSources.derivationTestFileKey3);
+  await externalSources.deleteSource(externalSources.derivationTestFileKey4);
 
   await page.close();
   await context.close();

diff --git a/src/components/external-source/ExternalSourceManager.svelte b/src/components/external-source/ExternalSourceManager.svelte
@@ -37,7 +37,7 @@
   import { parseJSONStream } from '../../utilities/generic';
   import { permissionHandler } from '../../utilities/permissionHandler';
   import { featurePermissions } from '../../utilities/permissions';
-  import { convertUTCtoMs, formatDate, getIntervalInMs } from '../../utilities/time';
+  import { convertUTCToMs, formatDate, getIntervalInMs } from '../../utilities/time';
   import { showFailureToast } from '../../utilities/toast';
   import { tooltip } from '../../utilities/tooltip';
   import { required, timestamp } from '../../utilities/validators';
@@ -296,7 +296,7 @@
             ...externalEventsDB,
             duration_ms: getIntervalInMs(externalEventsDB.duration),
             event_type: externalEventsDB.pkey.event_type_name,
-            start_ms: convertUTCtoMs(externalEventsDB.start_time),
+            start_ms: convertUTCToMs(externalEventsDB.start_time),
           };
         })),
     );
@@ -530,7 +530,6 @@
             </Input>
 
             <Collapse
-              className="anchor-collapse"
               defaultExpanded={false}
               title="Event Types"
               tooltipContent="View Contained Event Types"
@@ -544,7 +543,6 @@
               {/await}
             </Collapse>
             <Collapse
-              className="anchor-collapse"
               defaultExpanded={false}
               title="Metadata"
               tooltipContent="View Event Source Metadata"
@@ -568,44 +566,43 @@
                 {catchError(error)}
               {/await}
             </Collapse>
-            <div style="padding-bottom:20px">
-              <Collapse
-                className="used-in-plans-collapse"
-                defaultExpanded={false}
-                title="Used in Plans"
-                tooltipContent="View plans this source is used in"
+            <Collapse
+              className="used-in-plans-collapse"
+              defaultExpanded={false}
+              title="Used in Plans"
+              tooltipContent="View plans this source is used in"
+            >
+              {#if selectedSourceLinkedDerivationGroupsPlans.length > 0}
+                {#each selectedSourceLinkedDerivationGroupsPlans as linkedPlanDerivationGroup}
+                  <i>
+                    <a href="{base}/plans/{linkedPlanDerivationGroup.plan_id}">
+                      {$plans.find(plan => {
+                        return linkedPlanDerivationGroup.plan_id === plan.id;
+                      })?.name}
+                    </a>
+                  </i>
+                {/each}
+              {:else}
+                <i class="st-typography-body">Not used in any plans</i>
+              {/if}
+            </Collapse>
+
+            <div class="selected-source-delete">
+              <button
+                class="st-button danger w-100"
+                use:permissionHandler={{
+                  hasPermission: hasDeleteExternalSourcePermissionOnSelectedSource,
+                  permissionError: deletePermissionError,
+                }}
+                on:click|stopPropagation={async () => {
+                  if (selectedSource !== null) {
+                    onDeleteExternalSource([selectedSource]);
+                  }
+                }}
               >
-                {#if selectedSourceLinkedDerivationGroupsPlans.length > 0}
-                  {#each selectedSourceLinkedDerivationGroupsPlans as linkedPlanDerivationGroup}
-                    <i>
-                      <a href="{base}/plans/{linkedPlanDerivationGroup.plan_id}">
-                        {$plans.find(plan => {
-                          return linkedPlanDerivationGroup.plan_id === plan.id;
-                        })?.name}
-                      </a></i
-                    >
-                  {/each}
-                {:else}
-                  <i class="st-typography-body">Not used in any plans</i>
-                {/if}
-              </Collapse>
+                Delete external source
+              </button>
             </div>
-
-            <button
-              class="st-button danger w-100"
-              style="margin-bottom:auto;"
-              use:permissionHandler={{
-                hasPermission: hasDeleteExternalSourcePermissionOnSelectedSource,
-                permissionError: deletePermissionError,
-              }}
-              on:click|stopPropagation={async () => {
-                if (selectedSource !== null) {
-                  onDeleteExternalSource([selectedSource]);
-                }
-              }}
-            >
-              Delete external source
-            </button>
           </fieldset>
         </div>
       {:else}
@@ -762,7 +759,7 @@
               {columnDefs}
               hasDeletePermission={hasDeleteExternalSourcePermissionOnRow}
               singleItemDisplayText="External Source"
-              pluralItemDisplayText="External Source"
+              pluralItemDisplayText="External Sources"
               {filterExpression}
               items={$externalSources.map(externalSource => {
                 return { ...externalSource, id: getExternalSourceSlimRowId(externalSource) };
@@ -815,9 +812,9 @@
               />
             </div>
           {:else if $externalSources.length}
-            <p style="padding-left: 5px">Select a source to view contents.</p>
+            <p class="selected-source-prompt">Select a source to view contents.</p>
           {:else}
-            <p style="padding-left: 5px">No External Sources present.</p>
+            <p class="selected-source-prompt">No External Sources present.</p>
           {/if}
         </svelte:fragment>
       </Panel>
@@ -901,4 +898,12 @@
   .selected-source-forms {
     height: 100%;
   }
+
+  .selected-source-prompt {
+    padding-left: 4px;
+  }
+
+  .selected-source-delete {
+    padding-top: 12px;
+  }
 </style>
diff --git a/src/components/external-source/ExternalSourcePanelEntry.svelte b/src/components/external-source/ExternalSourcePanelEntry.svelte
@@ -116,7 +116,6 @@
         </Collapse>
       {/each}
       <Collapse
-        className="anchor-collapse"
         defaultExpanded={false}
         title="Event Types"
         tooltipContent="View Contained Event Types"

diff --git a/src/components/external-source/ExternalSourceUpdateCard.svelte b/src/components/external-source/ExternalSourceUpdateCard.svelte
@@ -21,10 +21,10 @@
   }>();
 
   let mappedSources: { [sourceType: string]: { [derivationGroup: string]: ExternalSourceSlim[] } } = {};
-  let hasAcknowledgePermission: boolean = false;
+  let hasUpdatePermission: boolean = false;
 
   $: if ($plan !== null) {
-    hasAcknowledgePermission = featurePermissions.derivationGroupAcknowledgement.canUpdate(user, $plan);
+    hasUpdatePermission = featurePermissions.derivationGroupAcknowledgement.canUpdate(user, $plan);
   }
 
   $: sources.forEach(source => {
@@ -95,7 +95,8 @@
         class="st-button secondary hover-fix"
         on:click={() => dispatch('dismiss')}
         use:permissionHandler={{
-          hasPermission: hasAcknowledgePermission,
+          hasPermission: hasUpdatePermission,
+          permissionError: "You do not have permission to acknowledge this external source."
         }}
       >
         Dismiss

diff --git a/src/components/external-source/ExternalSourcesPanel.svelte b/src/components/external-source/ExternalSourcesPanel.svelte
@@ -82,7 +82,6 @@
       {},
     );
   }
-  planDerivationGroupLinks.subscribe(_ => (mappedDerivationGroups = {})); // clear the map...
   $: filteredDerivationGroups.forEach(group => {
     // ...and repopulate it every time the links change. this handles deletion correctly
     if (group.source_type_name) {

diff --git a/src/components/modals/CreateGroupsOrTypesModal.svelte b/src/components/modals/CreateGroupsOrTypesModal.svelte
@@ -115,6 +115,7 @@
                 on:click|preventDefault={onCreateDerivationGroup}
                 use:permissionHandler={{
                   hasPermission: hasCreateDerivationGroupPermission,
+                  permissionError: "You do not have permission to create a derivation group."
                 }}
               >
                 Create
@@ -142,6 +143,7 @@
                 on:click|preventDefault={onCreateExternalSourceType}
                 use:permissionHandler={{
                   hasPermission: hasCreateExternalSourceTypePermission,
+                  permissionError: "You do not have permission to create an external source type."
                 }}
               >
                 Create
@@ -169,6 +171,7 @@
                 on:click|preventDefault={onCreateExternalEventType}
                 use:permissionHandler={{
                   hasPermission: hasCreateExternalEventTypePermission,
+                  permissionError: "You do not have permission to create an external event type."
                 }}
               >
                 Create