feat(website): review pages are now per-group (#1586)

backend/src/main/kotlin/org/loculus/backend/api/SubmissionTypes.kt

@@ -62,6 +62,7 @@ data class AccessionVersionsFilterWithDeletionScope(
         description = ACCESSION_VERSIONS_FILTER_DESCRIPTION,
     )
     val accessionVersionsFilter: List<AccessionVersion>? = null,
+    val groupIdsFilter: List<Int>? = null,
     @Schema(
         description = "Scope for deletion. If scope is set to 'ALL', all sequences are deleted. " +
             "If scope is set to 'PROCESSED_WITH_ERRORS', only processed sequences with errors are deleted. " +
@@ -81,6 +82,7 @@ data class AccessionVersionsFilterWithApprovalScope(
         description = ACCESSION_VERSIONS_FILTER_DESCRIPTION,
     )
     val accessionVersionsFilter: List<AccessionVersion>? = null,
+    val groupIdsFilter: List<Int>? = null,
     @Schema(
         description = "Scope for approval. If scope is set to 'ALL', all sequences are approved. " +
             "If scope is set to 'WITHOUT_WARNINGS', only sequences without warnings are approved.",

backend/src/main/kotlin/org/loculus/backend/controller/SubmissionController.kt

@@ -294,6 +294,7 @@ class SubmissionController(
     ): List<AccessionVersion> = submissionDatabaseService.approveProcessedData(
         authenticatedUser = authenticatedUser,
         accessionVersionsFilter = body.accessionVersionsFilter,
+        groupIdsFilter = body.groupIdsFilter,
         organism = organism,
         scope = body.scope,
     )
@@ -321,6 +322,7 @@ class SubmissionController(
     ): List<AccessionVersion> = submissionDatabaseService.deleteSequenceEntryVersions(
         body.accessionVersionsFilter,
         authenticatedUser,
+        body.groupIdsFilter,
         organism,
         body.scope,
     )

backend/src/main/kotlin/org/loculus/backend/service/submission/SubmissionDatabaseService.kt

@@ -276,9 +276,24 @@ class SubmissionDatabaseService(
         )
     }
 
+    private fun getGroupCondition(groupIdsFilter: List<Int>?, authenticatedUser: AuthenticatedUser): Op<Boolean> {
+        return if (groupIdsFilter != null) {
+            groupManagementPreconditionValidator.validateUserIsAllowedToModifyGroups(
+                groupIdsFilter,
+                authenticatedUser,
+            )
+            SequenceEntriesView.groupIsOneOf(groupIdsFilter)
+        } else if (authenticatedUser.isSuperUser) {
+            Op.TRUE
+        } else {
+            SequenceEntriesView.groupIsOneOf(groupManagementDatabaseService.getGroupIdsOfUser(authenticatedUser))
+        }
+    }
+
     fun approveProcessedData(
         authenticatedUser: AuthenticatedUser,
         accessionVersionsFilter: List<AccessionVersion>?,
+        groupIdsFilter: List<Int>?,
         organism: Organism,
         scope: ApproveDataScope,
     ): List<AccessionVersion> {
@@ -315,8 +330,10 @@ class SubmissionDatabaseService(
             Op.TRUE
         }
 
+        val groupCondition = getGroupCondition(groupIdsFilter, authenticatedUser)
+
         val accessionVersionsToUpdate = SequenceEntriesView
-            .select { statusCondition and accessionCondition and scopeCondition }
+            .select { statusCondition and accessionCondition and scopeCondition and groupCondition }
             .map { AccessionVersion(it[SequenceEntriesView.accessionColumn], it[SequenceEntriesView.versionColumn]) }
 
         if (accessionVersionsToUpdate.isEmpty()) {
@@ -448,17 +465,7 @@ class SubmissionDatabaseService(
 
         val listOfStatuses = statusesFilter ?: Status.entries
 
-        val groupCondition = if (groupIdsFilter != null) {
-            groupManagementPreconditionValidator.validateUserIsAllowedToModifyGroups(
-                groupIdsFilter,
-                authenticatedUser,
-            )
-            SequenceEntriesView.groupIsOneOf(groupIdsFilter)
-        } else if (authenticatedUser.isSuperUser) {
-            Op.TRUE
-        } else {
-            SequenceEntriesView.groupIsOneOf(groupManagementDatabaseService.getGroupIdsOfUser(authenticatedUser))
-        }
+        val groupCondition = getGroupCondition(groupIdsFilter, authenticatedUser)
 
         val baseQuery = SequenceEntriesView
             .join(
@@ -611,6 +618,7 @@ class SubmissionDatabaseService(
     fun deleteSequenceEntryVersions(
         accessionVersionsFilter: List<AccessionVersion>?,
         authenticatedUser: AuthenticatedUser,
+        groupIdsFilter: List<Int>?,
         organism: Organism,
         scope: DeleteSequenceScope,
     ): List<AccessionVersion> {
@@ -655,9 +663,11 @@ class SubmissionDatabaseService(
             DeleteSequenceScope.ALL -> SequenceEntriesView.statusIsOneOf(listOfDeletableStatuses)
         }
 
+        val groupCondition = getGroupCondition(groupIdsFilter, authenticatedUser)
+
         val sequenceEntriesToDelete = SequenceEntriesView
             .slice(SequenceEntriesView.accessionColumn, SequenceEntriesView.versionColumn)
-            .select { accessionCondition and scopeCondition }
+            .select { accessionCondition and scopeCondition and groupCondition }
             .map {
                 AccessionVersion(
                     it[SequenceEntriesView.accessionColumn],

website/src/components/ReviewPage/ReviewPage.spec.tsx

@@ -3,7 +3,7 @@ import { describe, expect, test } from 'vitest';
 
 import { ReviewPage } from './ReviewPage.tsx';
 import { openDataUseTerms } from '../../../tests/e2e.fixture.ts';
-import { mockRequest, testAccessToken, testConfig, testOrganism } from '../../../vitest.setup.ts';
+import { mockRequest, testAccessToken, testConfig, testGroups, testOrganism } from '../../../vitest.setup.ts';
 import {
     approvedForReleaseStatus,
     awaitingApprovalStatus,
@@ -13,9 +13,16 @@ import {
     type SequenceEntryStatus,
 } from '../../types/backend.ts';
 
+const testGroup = testGroups[0];
+
 function renderReviewPage() {
     return render(
-        <ReviewPage organism={testOrganism} accessToken={testAccessToken} clientConfig={testConfig.public} />,
+        <ReviewPage
+            group={testGroup}
+            organism={testOrganism}
+            accessToken={testAccessToken}
+            clientConfig={testConfig.public}
+        />,
     );
 }
 
@@ -96,6 +103,22 @@ describe('ReviewPage', () => {
         });
     });
 
+    test('should request data from the right group', async () => {
+        let requestedGroupFilter: string | null = null;
+        mockRequest.backend.getSequences(200, generateGetSequencesResponse([]), (request) => {
+            const params = new URL(request.url).searchParams;
+            requestedGroupFilter = params.get('groupIdsFilter');
+        });
+
+        const { getByText } = renderReviewPage();
+
+        await waitFor(() => {
+            expect(getByText('You do not currently have any unreleased sequences awaiting review.')).toBeDefined();
+        });
+
+        expect(requestedGroupFilter).toBe(testGroup.groupId.toString());
+    });
+
     test('should render the review page and show button to bulk delete/approve all erroneous sequences', async () => {
         mockRequest.backend.getSequences(
             200,

website/src/components/ReviewPage/ReviewPage.tsx

@@ -11,6 +11,7 @@ import {
     deleteAllDataScope,
     deleteProcessedDataWithErrorsScope,
     type GetSequencesResponse,
+    type Group,
     hasErrorsStatus,
     inProcessingStatus,
     type PageQuery,
@@ -34,6 +35,7 @@ let oldSequenceData: GetSequencesResponse | null = null;
 type ReviewPageProps = {
     clientConfig: ClientConfig;
     organism: string;
+    group: Group;
     accessToken: string;
 };
 
@@ -66,12 +68,12 @@ const NumberAndVisibility = ({
     );
 };
 
-const InnerReviewPage: FC<ReviewPageProps> = ({ clientConfig, organism, accessToken }) => {
+const InnerReviewPage: FC<ReviewPageProps> = ({ clientConfig, organism, group, accessToken }) => {
     const { errorMessage, isErrorOpen, openErrorFeedback, closeErrorFeedback } = useErrorFeedbackState();
 
     const [pageQuery, setPageQuery] = useState<PageQuery>({ page: 1, size: pageSizeOptions[2] });
 
-    const hooks = useSubmissionOperations(organism, clientConfig, accessToken, openErrorFeedback, pageQuery);
+    const hooks = useSubmissionOperations(organism, group, clientConfig, accessToken, openErrorFeedback, pageQuery);
 
     const showErrors = hooks.includedStatuses.includes(hasErrorsStatus);
     const showUnprocessed =
@@ -229,6 +231,7 @@ const InnerReviewPage: FC<ReviewPageProps> = ({ clientConfig, organism, accessTo
                                                     'Are you sure you want to discard all sequences with errors?',
                                                 onConfirmation: () => {
                                                     hooks.deleteSequenceEntries({
+                                                        groupIdsFilter: [group.groupId],
                                                         scope: deleteProcessedDataWithErrorsScope.value,
                                                     });
                                                 },
@@ -248,6 +251,7 @@ const InnerReviewPage: FC<ReviewPageProps> = ({ clientConfig, organism, accessTo
                                             dialogText: `Are you sure you want to discard all ${finishedCount} processed sequences?`,
                                             onConfirmation: () => {
                                                 hooks.deleteSequenceEntries({
+                                                    groupIdsFilter: [group.groupId],
                                                     scope: deleteAllDataScope.value,
                                                 });
                                             },
@@ -270,6 +274,7 @@ const InnerReviewPage: FC<ReviewPageProps> = ({ clientConfig, organism, accessTo
                             dialogText: 'Are you sure you want to release all valid sequences?',
                             onConfirmation: () =>
                                 hooks.approveProcessedData({
+                                    groupIdsFilter: [group.groupId],
                                     scope: approveAllDataScope.value,
                                 }),
                         })
@@ -293,12 +298,14 @@ const InnerReviewPage: FC<ReviewPageProps> = ({ clientConfig, organism, accessTo
                             approveAccessionVersion={() =>
                                 hooks.approveProcessedData({
                                     accessionVersionsFilter: [sequence],
+                                    groupIdsFilter: [group.groupId],
                                     scope: approveAllDataScope.value,
                                 })
                             }
                             deleteAccessionVersion={() =>
                                 hooks.deleteSequenceEntries({
                                     accessionVersionsFilter: [sequence],
+                                    groupIdsFilter: [group.groupId],
                                     scope: deleteAllDataScope.value,
                                 })
                             }

website/src/hooks/useSubmissionOperations.ts

@@ -6,6 +6,7 @@ import { backendApi } from '../services/backendApi.ts';
 import { backendClientHooks } from '../services/serviceHooks.ts';
 import {
     awaitingApprovalStatus,
+    type Group,
     hasErrorsStatus,
     inProcessingStatus,
     type PageQuery,
@@ -17,6 +18,7 @@ import { stringifyMaybeAxiosError } from '../utils/stringifyMaybeAxiosError.ts';
 
 export function useSubmissionOperations(
     organism: string,
+    group: Group,
     clientConfig: ClientConfig,
     accessToken: string,
     openErrorFeedback: (message: string) => void,
@@ -32,6 +34,7 @@ export function useSubmissionOperations(
                 organism,
             },
             queries: {
+                groupIdsFilter: group.groupId.toString(),
                 initialStatusesFilter: allRelevantStatuses.join(','),
                 statusesFilter: includedStatuses.join(','),
                 page: pageQuery.page - 1,

website/src/pages/[organism]/submission/[groupId]/review.astro

@@ -1,24 +1,30 @@
 ---
 import { ReviewPage } from '../../../../components/ReviewPage/ReviewPage';
-import NeedToLogin from '../../../../components/common/NeedToLogin.astro';
+import SubmissionPageWrapper from '../../../../components/Submission/SubmissionPageWrapper.astro';
 import { getRuntimeConfig } from '../../../../config';
-import BaseLayout from '../../../../layouts/BaseLayout.astro';
 import { type ClientConfig } from '../../../../types/runtimeConfig';
 import { getAccessToken } from '../../../../utils/getAccessToken';
+import { getGroupsAndCurrentGroup } from '../../../../utils/submissionPages';
+
 const organism = Astro.params.organism!;
-const accessToken = getAccessToken(Astro.locals.session)!;
+const groupsResult = await getGroupsAndCurrentGroup(Astro.params, Astro.locals.session);
+
 const clientConfig: ClientConfig = getRuntimeConfig().public;
 ---
 
-{
-    accessToken ? (
-        <BaseLayout title='Current submissions'>
-            <h1 class='title'>Current submissions</h1>
-            <ReviewPage clientConfig={clientConfig} organism={organism} accessToken={accessToken} client:load />
-        </BaseLayout>
-    ) : (
-        <BaseLayout title='Current submissions'>
-            <NeedToLogin />
-        </BaseLayout>
-    )
-}
+<SubmissionPageWrapper title='Review current submissions' organism={organism} groupsResult={groupsResult}>
+    {
+        groupsResult.match(
+            ({ currentGroup: group }) => (
+                <ReviewPage
+                    clientConfig={clientConfig}
+                    organism={organism}
+                    group={group}
+                    accessToken={getAccessToken(Astro.locals.session)!}
+                    client:load
+                />
+            ),
+            () => undefined,
+        )
+    }
+</SubmissionPageWrapper>

website/src/types/backend.ts

@@ -61,6 +61,7 @@ export const approveProcessedDataWithoutWarningsScope = z.literal('WITHOUT_WARNI
 
 export const accessionVersionsFilterWithApprovalScope = accessionVersionsFilter.merge(
     z.object({
+        groupIdsFilter: z.array(z.number()),
         scope: z.union([approveAllDataScope, approveProcessedDataWithoutWarningsScope]),
     }),
 );
@@ -72,6 +73,7 @@ export const deleteProcessedDataWithWarningsScope = z.literal('PROCESSED_WITH_WA
 
 export const accessionVersionsFilterWithDeletionScope = accessionVersionsFilter.merge(
     z.object({
+        groupIdsFilter: z.array(z.number()),
         scope: z.union([
             deleteAllDataScope,
             deleteProcessedAndRevocationConfirmationDataScope,

website/tests/util/backendCalls.ts

@@ -48,9 +48,14 @@ export const submitRevisedDataViaApi = async (accessions: Accession[], token: st
     return response.value;
 };
 
-export const approveProcessedData = async (accessionVersions: AccessionVersion[], token: string): Promise<void> => {
+export const approveProcessedData = async (
+    accessionVersions: AccessionVersion[],
+    token: string,
+    groupId: number,
+): Promise<void> => {
     const body = {
         accessionVersionsFilter: accessionVersions,
+        groupIdsFilter: [groupId],
         scope: 'ALL' as const,
     };
 
@@ -64,7 +69,11 @@ export const approveProcessedData = async (accessionVersions: AccessionVersion[]
     }
 };
 
-export const revokeReleasedData = async (accessions: Accession[], token: string): Promise<AccessionVersion[]> => {
+export const revokeReleasedData = async (
+    accessions: Accession[],
+    token: string,
+    groupId: number,
+): Promise<AccessionVersion[]> => {
     const body = {
         accessions,
     };
@@ -83,7 +92,11 @@ export const revokeReleasedData = async (accessions: Accession[], token: string)
 
     const confirmationResponse = await backendClient.call(
         'approveProcessedData',
-        { scope: 'ALL', accessionVersionsFilter: accessionVersions },
+        {
+            scope: 'ALL',
+            accessionVersionsFilter: accessionVersions,
+            groupIdsFilter: [groupId],
+        },
         {
             params: { organism: dummyOrganism.key },
             headers: createAuthorizationHeader(token),

website/tests/util/prepareDataToBe.ts

@@ -56,7 +56,7 @@ const prepareDataToBeAwaitingApproval = async (token: string, groupId: number) =
 const prepareDataToBeApprovedForRelease = async (token: string, groupId: number) => {
     const sequenceEntries = await prepareDataToBeAwaitingApproval(token, groupId);
 
-    await approveProcessedData(sequenceEntries, token);
+    await approveProcessedData(sequenceEntries, token, groupId);
 
     return sequenceEntries;
 };
@@ -67,6 +67,7 @@ const prepareDataToBeRevoked = async (token: string, groupId: number) => {
     return revokeReleasedData(
         sequenceEntries.map((entry) => entry.accession),
         token,
+        groupId,
     );
 };
 
@@ -87,7 +88,7 @@ const prepareDataToBeRevisedForRelease = async (token: string, groupId: number)
     }));
     await fakeProcessingPipeline.submit(options);
 
-    await approveProcessedData(submittedRevisionAccessionVersion, token);
+    await approveProcessedData(submittedRevisionAccessionVersion, token, groupId);
 
     return submittedRevisionAccessionVersion;
 };