Review page tweaks (#1162)

* review page tweaks * try to fix test * click confirm buttons in test * try waiting * sleep * remove click of discard * format * simplify * order preprocessing by accession column * order by submitted at then accession * try this * format * give keycloak longer * fix isLoggedIn errors * update * revert submission database changes * e2e * format * try this * update * Update keycloak-deployment.yaml * include Hidden * format * update * upload test results * update * more attempts
loculus-project · Feb 28, 2024 · 5372e33 · 5372e33
1 parent 15bd9b0
commit 5372e33
Show file tree

Hide file tree

Showing 6 changed files with 104 additions and 28 deletions.
diff --git a/.github/workflows/e2e-k3d.yml b/.github/workflows/e2e-k3d.yml
@@ -111,6 +111,13 @@ jobs:
           name: playwright-report
           path: website/playwright-report/
           retention-days: 30
+      - name: Upload Test Results
+        if: always()
+        uses: actions/upload-artifact@v4
+        with:
+          name: test-results
+          path: website/test-results/
+          retention-days: 30
 
       - name: List running pods
         if: ${{ !cancelled() }}

diff --git a/website/src/components/ConfirmationDialog.tsx b/website/src/components/ConfirmationDialog.tsx
@@ -9,7 +9,7 @@ type ConfirmationDialogProps = {
 
 type DisplayConfirmationProps = {
     dialogText: string;
-    onConfirmation: () => Promise<void>;
+    onConfirmation: () => Promise<void> | void;
 };
 
 export const ConfirmationDialog: FC<ConfirmationDialogProps> = ({ dialogText, onConfirmation, onClose }) => {
@@ -25,12 +25,12 @@ export const ConfirmationDialog: FC<ConfirmationDialogProps> = ({ dialogText, on
 
             <div className='flex justify-end gap-4 mt-4'>
                 <form method='dialog'>
-                    <button className='btn btn-error' onClick={onClose}>
+                    <button className='btn loculusColor text-white hover:bg-primary-700' onClick={onClose}>
                         Cancel
                     </button>
                 </form>
                 <form method='dialog'>
-                    <button className='btn loculusColor' onClick={onConfirmation}>
+                    <button className='btn loculusColor text-white hover:bg-primary-700' onClick={onConfirmation}>
                         Confirm
                     </button>
                 </form>

diff --git a/website/src/components/ReviewPage/ReviewCard.tsx b/website/src/components/ReviewPage/ReviewCard.tsx
@@ -18,15 +18,15 @@ import type { ClientConfig } from '../../types/runtimeConfig.ts';
 import { createAuthorizationHeader } from '../../utils/createAuthorizationHeader.ts';
 import { displayMetadataField } from '../../utils/displayMetadataField.ts';
 import { getAccessionVersionString } from '../../utils/extractAccessionVersion.ts';
-import Edit from '~icons/bxs/edit';
-import Trash from '~icons/bxs/trash';
-import Send from '~icons/fa/send';
+import BiTrash from '~icons/bi/trash';
+import ClarityNoteEditLine from '~icons/clarity/note-edit-line';
 import Note from '~icons/fluent/note-24-filled';
 import QuestionMark from '~icons/fluent/tag-question-mark-24-filled';
 import Locked from '~icons/fluent-emoji-high-contrast/locked';
 import Unlocked from '~icons/fluent-emoji-high-contrast/unlocked';
 import EmptyCircle from '~icons/grommet-icons/empty-circle';
 import TickOutline from '~icons/mdi/tick-outline';
+import WpfPaperPlane from '~icons/wpf/paper-plane';
 
 type ReviewCardProps = {
     sequenceEntryStatus: SequenceEntryStatus;
@@ -103,14 +103,14 @@ const ButtonBar: FC<ButtonBarProps> = ({
         } pl-3 inline-block mr-2 mb-2 text-xl`;
 
     return (
-        <div className='flex space-x-2 mb-auto pt-3.5'>
+        <div className='flex space-x-1 mb-auto pt-3.5'>
             <button
                 className={buttonBarClass(sequenceEntryStatus.status !== awaitingApprovalStatus)}
                 onClick={approveAccessionVersion}
                 data-tooltip-id={'approve-tooltip' + sequenceEntryStatus.accession}
                 disabled={sequenceEntryStatus.status !== awaitingApprovalStatus}
             >
-                <Send />
+                <WpfPaperPlane />
             </button>
             <Tooltip
                 id={'approve-tooltip' + sequenceEntryStatus.accession}
@@ -136,7 +136,7 @@ const ButtonBar: FC<ButtonBarProps> = ({
                     sequenceEntryStatus.status !== awaitingApprovalStatus
                 }
             >
-                <Edit />
+                <ClarityNoteEditLine />
             </button>
             <Tooltip
                 id={'edit-tooltip' + sequenceEntryStatus.accession}
@@ -160,7 +160,7 @@ const ButtonBar: FC<ButtonBarProps> = ({
                     sequenceEntryStatus.status !== awaitingApprovalStatus
                 }
             >
-                <Trash />
+                <BiTrash />
             </button>
             <Tooltip
                 id={'delete-tooltip' + sequenceEntryStatus.accession}

diff --git a/website/src/components/ReviewPage/ReviewPage.spec.tsx b/website/src/components/ReviewPage/ReviewPage.spec.tsx
@@ -114,15 +114,20 @@ describe('ReviewPage', () => {
             expect(getByText(awaitingApprovalTestData.accession)).toBeDefined();
         });
 
+        getByText('Discard sequences').click();
+
         await waitFor(() => {
             expect(getByText((text) => text.includes('Discard 1 sequences with errors'))).toBeDefined();
             expect(getByText((text) => text.includes('Release 1 sequences without errors'))).toBeDefined();
         });
 
         mockRequest.backend.getSequences(200, generateGetSequencesResponse([]));
 
-        getByText((text) => text.includes('Discard 1 sequences with errors')).click();
         getByText((text) => text.includes('Release 1 sequences without errors')).click();
+        await waitFor(() => {
+            expect(getByText('Confirm')).toBeDefined();
+        });
+        getByText((text) => text.includes('Confirm')).click();
 
         await waitFor(() => {
             expect(getByText('No sequences to review')).toBeDefined();

diff --git a/website/src/components/ReviewPage/ReviewPage.tsx b/website/src/components/ReviewPage/ReviewPage.tsx
@@ -1,3 +1,4 @@
+import { Menu } from '@headlessui/react';
 import { Pagination } from '@mui/material';
 import { type ChangeEvent, type FC, useState } from 'react';
 
@@ -15,8 +16,14 @@ import {
     type SequenceEntryStatus,
 } from '../../types/backend.ts';
 import { type ClientConfig } from '../../types/runtimeConfig.ts';
+import { displayConfirmationDialog } from '../ConfirmationDialog.tsx';
 import { ManagedErrorFeedback, useErrorFeedbackState } from '../common/ManagedErrorFeedback.tsx';
 import { withQueryProvider } from '../common/withQueryProvider.tsx';
+import BiTrash from '~icons/bi/trash';
+import IwwaArrowDown from '~icons/iwwa/arrow-down';
+import WpfPaperPlane from '~icons/wpf/paper-plane';
+const menuItemClassName = `group flex rounded-md items-center w-full px-2 py-2 text-sm
+hover:bg-gray-400 bg-gray-500 text-white text-left mb-1`;
 
 type ReviewPageProps = {
     clientConfig: ClientConfig;
@@ -104,28 +111,73 @@ const InnerReviewPage: FC<ReviewPageProps> = ({ clientConfig, organism, accessTo
     );
 
     const bulkActionButtons = (
-        <div className='flex justify-end'>
-            {errorCount > 0 && showErrors && (
-                <button
-                    className='border rounded-md p-1 bg-gray-500 text-white px-2'
-                    onClick={() => {
-                        hooks.deleteSequenceEntries({
-                            scope: deleteProcessedDataWithErrorsScope.value,
-                        });
-                    }}
-                >
-                    Discard {errorCount} sequences with errors
-                </button>
+        <div className='flex justify-end items-center gap-3'>
+            {processedCount + errorCount > 0 && (
+                <Menu as='div' className='relative inline-block text-left'>
+                    <Menu.Button className='border rounded-md p-1 bg-gray-500 text-white px-2'>
+                        <BiTrash className='inline-block w-4 h-4 -mt-0.5 mr-1.5' />
+                        Discard sequences
+                        <IwwaArrowDown className='inline-block ml-1 w-3 h-3 -mt-0.5' />
+                    </Menu.Button>
+                    <Menu.Items className='origin-top-right absolute z-50 bg-white'>
+                        <div className='py-1'>
+                            {errorCount > 0 && showErrors && (
+                                <Menu.Item>
+                                    <button
+                                        className={menuItemClassName}
+                                        onClick={() =>
+                                            displayConfirmationDialog({
+                                                dialogText:
+                                                    'Are you sure you want to discard all sequences with errors?',
+                                                onConfirmation: () => {
+                                                    hooks.deleteSequenceEntries({
+                                                        scope: deleteProcessedDataWithErrorsScope.value,
+                                                    });
+                                                },
+                                            })
+                                        }
+                                    >
+                                        <BiTrash className='inline-block w-4 h-4 -mt-0.5 mr-1.5' />
+                                        Discard {errorCount} sequences with errors
+                                    </button>
+                                </Menu.Item>
+                            )}
+                            <Menu.Item>
+                                <button
+                                    className={menuItemClassName}
+                                    onClick={() =>
+                                        displayConfirmationDialog({
+                                            dialogText: `Are you sure you want to discard all ${processedCount + errorCount} processed sequences?`,
+                                            onConfirmation: () => {
+                                                hooks.deleteSequenceEntries({
+                                                    scope: deleteAllDataScope.value,
+                                                });
+                                            },
+                                        })
+                                    }
+                                >
+                                    <BiTrash className='inline-block w-4 h-4 -mt-0.5 mr-1.5' />
+                                    Discard all {processedCount + errorCount} processed sequences
+                                </button>
+                            </Menu.Item>
+                        </div>
+                    </Menu.Items>
+                </Menu>
             )}
             {processedCount > 0 && (
                 <button
-                    className='border rounded-md p-1 bg-gray-500 text-white px-2 ml-2'
+                    className='border rounded-md p-1 bg-gray-500 text-white px-2'
                     onClick={() =>
-                        hooks.approveProcessedData({
-                            scope: approveAllDataScope.value,
+                        displayConfirmationDialog({
+                            dialogText: 'Are you sure you want to release all sequences without errors?',
+                            onConfirmation: () =>
+                                hooks.approveProcessedData({
+                                    scope: approveAllDataScope.value,
+                                }),
                         })
                     }
                 >
+                    <WpfPaperPlane className='inline-block w-4 h-4 -mt-0.5 mr-1.5' />
                     Release {processedCount} sequences without errors
                 </button>
             )}

diff --git a/website/tests/pages/review/review.page.ts b/website/tests/pages/review/review.page.ts
@@ -10,11 +10,18 @@ type ReviewPageOverview = {
 
 export class ReviewPage {
     public readonly approveAllButton: Locator;
+    public readonly deleteFirstButton: Locator;
     public readonly deleteAllButton: Locator;
+    public readonly confirmButton: Locator;
 
     constructor(public readonly page: Page) {
         this.approveAllButton = page.getByRole('button', { name: 'Release', exact: false });
-        this.deleteAllButton = page.getByRole('button', { name: 'Discard', exact: false });
+        this.deleteFirstButton = page.getByRole('button', { name: 'Discard sequences', exact: false });
+        this.deleteAllButton = page.getByText('Discard all', {
+            exact: false,
+        });
+
+        this.confirmButton = page.getByRole('button', { name: 'Confirm', exact: false });
     }
 
     public async goto() {
@@ -64,10 +71,15 @@ export class ReviewPage {
     public async approveAll() {
         await expect(this.approveAllButton).toBeVisible();
         await this.approveAllButton.click();
+        await expect(this.confirmButton).toBeVisible();
+        await this.confirmButton.click();
     }
 
     public async deleteAll() {
-        await expect(this.deleteAllButton).toBeVisible();
+        await expect(this.deleteFirstButton).toBeVisible();
+        await this.deleteFirstButton.click();
         await this.deleteAllButton.click();
+        await expect(this.confirmButton).toBeVisible();
+        await this.confirmButton.click();
     }
 }