feat(backend, website): rename get-data-to-edit endpoint and surface version comment via it

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

@@ -119,7 +119,9 @@ data class SequenceEntryVersionToEdit(
     override val version: Version,
     val status: Status,
     val groupId: Int,
-    val processedData: ProcessedData<GeneticSequence>,
+    val isRevocation: Boolean = false,
+    @Schema(description = "Null for revocations, which are not preprocessed.")
+    val processedData: ProcessedData<GeneticSequence>? = null,
     val originalData: OriginalData<GeneticSequence>,
     @Schema(description = "The preprocessing will be considered failed if this is not empty")
     val errors: List<PreprocessingAnnotation>? = null,

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

@@ -339,14 +339,17 @@ open class SubmissionController(
         return ResponseEntity.ok().headers(headers).body(streamBody)
     }
 
-    @Operation(description = GET_DATA_TO_EDIT_SEQUENCE_VERSION_DESCRIPTION)
-    @GetMapping("/get-data-to-edit/{accession}/{version}", produces = [MediaType.APPLICATION_JSON_VALUE])
-    fun getSequenceEntryVersionToEdit(
+    @Operation(description = GET_ORIGINAL_DATA_FOR_ENTRY_DESCRIPTION)
+    @GetMapping(
+        "/get-original-data-for-entry/{accession}/{version}",
+        produces = [MediaType.APPLICATION_JSON_VALUE],
+    )
+    fun getOriginalDataForEntry(
         @PathVariable @Valid organism: Organism,
         @PathVariable accession: Accession,
         @PathVariable version: Long,
         @HiddenParam authenticatedUser: AuthenticatedUser,
-    ): SequenceEntryVersionToEdit = submissionDatabaseService.getSequenceEntryVersionToEdit(
+    ): SequenceEntryVersionToEdit = submissionDatabaseService.getOriginalDataForEntry(
         authenticatedUser,
         AccessionVersion(accession, version),
         organism,

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

@@ -86,8 +86,10 @@ The submitted external data cannot be written to the database, e.g. if the acces
 state, if the pipeline submits invalid data or if the name of external metadata updater is not known. Rolls back the whole transaction.
 """
 
-const val GET_DATA_TO_EDIT_SEQUENCE_VERSION_DESCRIPTION = """
-Get original data for a single accession version for subsequent editing and edit/revision.
+const val GET_ORIGINAL_DATA_FOR_ENTRY_DESCRIPTION = """
+Get original data for a single accession version. Used both for displaying read-only details of an entry
+(including for revocations) and as the basis for subsequent editing or revising.
+For revocations, no processed data is returned.
 """
 
 const val GET_SEQUENCES_DESCRIPTION = """

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

@@ -1150,14 +1150,14 @@ class SubmissionDatabaseService(
         )
     }
 
-    fun getSequenceEntryVersionToEdit(
+    fun getOriginalDataForEntry(
         authenticatedUser: AuthenticatedUser,
         accessionVersion: AccessionVersion,
         organism: Organism,
     ): SequenceEntryVersionToEdit {
         log.info {
-            "Getting sequence entry ${accessionVersion.displayAccessionVersion()} " +
-                "by ${authenticatedUser.username} to edit"
+            "Getting original data for sequence entry ${accessionVersion.displayAccessionVersion()} " +
+                "by ${authenticatedUser.username}"
         }
 
         accessionPreconditionValidator.validate {
@@ -1181,21 +1181,16 @@ class SubmissionDatabaseService(
             .where { SequenceEntriesView.accessionVersionEquals(accessionVersion) }
             .first()
 
-        if (selectedSequenceEntry[SequenceEntriesView.isRevocationColumn]) {
-            throw UnprocessableEntityException(
-                "Accession version ${accessionVersion.displayAccessionVersion()} is a revocation.",
-            )
-        }
+        val processedDataValue = selectedSequenceEntry[SequenceEntriesView.processedDataColumn]
+            ?.let { processedDataPostprocessor.retrieveFromStoredValue(it, organism) }
 
         return SequenceEntryVersionToEdit(
             accession = selectedSequenceEntry[SequenceEntriesView.accessionColumn],
             version = selectedSequenceEntry[SequenceEntriesView.versionColumn],
             status = Status.fromString(selectedSequenceEntry[SequenceEntriesView.statusColumn]),
             groupId = selectedSequenceEntry[SequenceEntriesView.groupIdColumn],
-            processedData = processedDataPostprocessor.retrieveFromStoredValue(
-                selectedSequenceEntry[SequenceEntriesView.processedDataColumn]!!,
-                organism,
-            ),
+            isRevocation = selectedSequenceEntry[SequenceEntriesView.isRevocationColumn],
+            processedData = processedDataValue,
             originalData = compressionService.decompressSequencesInOriginalData(
                 selectedSequenceEntry[SequenceEntriesView.unprocessedDataColumn]!!,
             ),

backend/src/test/kotlin/org/loculus/backend/controller/submission/GetDataToEditEndpointTest.kt → backend/src/test/kotlin/org/loculus/backend/controller/submission/GetOriginalDataForEntryEndpointTest.kt

@@ -2,6 +2,7 @@ package org.loculus.backend.controller.submission
 
 import org.hamcrest.CoreMatchers.containsString
 import org.hamcrest.CoreMatchers.`is`
+import org.hamcrest.CoreMatchers.nullValue
 import org.hamcrest.MatcherAssert.assertThat
 import org.junit.jupiter.api.Test
 import org.loculus.backend.api.Status
@@ -21,15 +22,15 @@ import org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPat
 import org.springframework.test.web.servlet.result.MockMvcResultMatchers.status
 
 @EndpointTest
-class GetDataToEditEndpointTest(
+class GetOriginalDataForEntryEndpointTest(
     @Autowired val client: SubmissionControllerClient,
     @Autowired val convenienceClient: SubmissionConvenienceClient,
 ) {
 
     @Test
     fun `GIVEN invalid authorization token THEN returns 401 Unauthorized`() {
         expectUnauthorizedResponse {
-            client.getSequenceEntryToEdit(
+            client.getOriginalDataForEntry(
                 "ShouldNotMatterAtAll",
                 1,
                 jwt = it,
@@ -47,7 +48,7 @@ class GetDataToEditEndpointTest(
             .assertStatusIs(Status.PROCESSED)
             .assertHasError(true)
 
-        val editedData = convenienceClient.getSequenceEntryToEdit(
+        val editedData = convenienceClient.getOriginalDataForEntry(
             accession = firstAccession,
             version = 1,
         )
@@ -61,7 +62,7 @@ class GetDataToEditEndpointTest(
     fun `WHEN I query data for a non-existent accession THEN refuses request with not found`() {
         val nonExistentAccession = "DefinitelyNotExisting"
 
-        client.getSequenceEntryToEdit(nonExistentAccession, 1)
+        client.getOriginalDataForEntry(nonExistentAccession, 1)
             .andExpect(status().isUnprocessableEntity)
             .andExpect(content().contentType(MediaType.APPLICATION_PROBLEM_JSON))
             .andExpect(
@@ -79,9 +80,9 @@ class GetDataToEditEndpointTest(
             organism = DEFAULT_ORGANISM,
         ).first().accession
 
-        client.getSequenceEntryToEdit(firstAccession, 1, organism = DEFAULT_ORGANISM)
+        client.getOriginalDataForEntry(firstAccession, 1, organism = DEFAULT_ORGANISM)
             .andExpect(status().isOk)
-        client.getSequenceEntryToEdit(firstAccession, 1, organism = OTHER_ORGANISM)
+        client.getOriginalDataForEntry(firstAccession, 1, organism = OTHER_ORGANISM)
             .andExpect(status().isUnprocessableEntity)
             .andExpect(content().contentType(MediaType.APPLICATION_PROBLEM_JSON))
             .andExpect(
@@ -99,7 +100,7 @@ class GetDataToEditEndpointTest(
 
         convenienceClient.prepareDataTo(Status.PROCESSED, errors = true)
 
-        client.getSequenceEntryToEdit("1", nonExistentAccessionVersion)
+        client.getOriginalDataForEntry("1", nonExistentAccessionVersion)
             .andExpect(status().isUnprocessableEntity)
             .andExpect(content().contentType(MediaType.APPLICATION_PROBLEM_JSON))
             .andExpect(
@@ -114,7 +115,7 @@ class GetDataToEditEndpointTest(
         val firstAccession = convenienceClient.prepareDataTo(Status.PROCESSED, errors = true).first().accession
 
         val userNameThatDoesNotHavePermissionToQuery = "theOneWhoMustNotBeNamed"
-        client.getSequenceEntryToEdit(
+        client.getOriginalDataForEntry(
             accession = firstAccession,
             version = 1,
             jwt = generateJwtFor(userNameThatDoesNotHavePermissionToQuery),
@@ -135,7 +136,7 @@ class GetDataToEditEndpointTest(
             )
             .first()
 
-        client.getSequenceEntryToEdit(
+        client.getOriginalDataForEntry(
             accession = accessionVersion.accession,
             version = accessionVersion.version,
             jwt = jwtForSuperUser,
@@ -146,12 +147,23 @@ class GetDataToEditEndpointTest(
     }
 
     @Test
-    fun `GIVEN revocation version awaiting approval THEN throws unprocessable entity error`() {
-        val accessionVersion = convenienceClient.prepareDefaultSequenceEntriesToAwaitingApprovalForRevocation()
-            .first()
+    fun `GIVEN revocation awaiting approval THEN returns original data with version comment and null processed data`() {
+        val versionComment = "revocation reason for test"
+        val approvedAccession = convenienceClient.prepareDefaultSequenceEntriesToApprovedForRelease().first().accession
+        val accessionVersion = convenienceClient.revokeSequenceEntries(
+            listOf(approvedAccession),
+            versionComment = versionComment,
+        ).first()
+
+        val originalData = convenienceClient.getOriginalDataForEntry(
+            accession = accessionVersion.accession,
+            version = accessionVersion.version,
+        )
 
-        client.getSequenceEntryToEdit(accessionVersion.accession, accessionVersion.version)
-            .andExpect(status().isUnprocessableEntity)
-            .andExpect(jsonPath("\$.detail", containsString("is a revocation")))
+        assertThat(originalData.accession, `is`(accessionVersion.accession))
+        assertThat(originalData.version, `is`(accessionVersion.version))
+        assertThat(originalData.isRevocation, `is`(true))
+        assertThat(originalData.processedData, `is`(nullValue()))
+        assertThat(originalData.originalData.metadata["versionComment"], `is`(versionComment))
     }
 }

backend/src/test/kotlin/org/loculus/backend/controller/submission/SubmissionControllerClient.kt

@@ -172,14 +172,14 @@ class SubmissionControllerClient(private val mockMvc: MockMvc, private val objec
             .param("size", size?.toString()),
     )
 
-    fun getSequenceEntryToEdit(
+    fun getOriginalDataForEntry(
         accession: Accession,
         version: Long,
         organism: String = DEFAULT_ORGANISM,
         groupName: String = DEFAULT_GROUP_NAME,
         jwt: String? = jwtForDefaultUser,
     ): ResultActions = mockMvc.perform(
-        get(addOrganismToPath("/get-data-to-edit/$accession/$version", organism = organism))
+        get(addOrganismToPath("/get-original-data-for-entry/$accession/$version", organism = organism))
             .withAuth(jwt)
             .param("groupName", groupName),
     )

backend/src/test/kotlin/org/loculus/backend/controller/submission/SubmissionConvenienceClient.kt

@@ -351,12 +351,12 @@ class SubmissionConvenienceClient(
             ?: error("Did not find $accession.$version for $userName")
     }
 
-    fun getSequenceEntryToEdit(
+    fun getOriginalDataForEntry(
         accession: Accession,
         version: Long,
         userName: String = DEFAULT_USER_NAME,
     ): SequenceEntryVersionToEdit = deserializeJsonResponse(
-        client.getSequenceEntryToEdit(
+        client.getOriginalDataForEntry(
             accession = accession,
             version = version,
             jwt = generateJwtFor(userName),

backend/src/test/kotlin/org/loculus/backend/controller/submission/SubmitProcessedDataEndpointTest.kt

@@ -105,12 +105,16 @@ class SubmitProcessedDataEndpointTest(
         convenienceClient.getSequenceEntry(accession = accessions.first(), version = 1)
             .assertStatusIs(Status.PROCESSED)
 
-        val sequenceEntryToEdit = convenienceClient.getSequenceEntryToEdit(accession = accessions.first(), version = 1)
-        assertThat(sequenceEntryToEdit.processedData.metadata, hasEntry("qc", DoubleNode(0.987654321)))
-        assertThat(sequenceEntryToEdit.processedData.metadata, hasEntry("age", IntNode(42)))
-        assertThat(sequenceEntryToEdit.processedData.metadata, hasEntry("region", TextNode("Europe")))
-        assertThat(sequenceEntryToEdit.processedData.metadata, hasEntry("pangoLineage", TextNode("XBB.1.5")))
-        assertThat(sequenceEntryToEdit.processedData.metadata, hasEntry("booleanColumn", BooleanNode.TRUE))
+        val sequenceEntryToEdit = convenienceClient.getOriginalDataForEntry(
+            accession = accessions.first(),
+            version = 1,
+        )
+        val metadata = sequenceEntryToEdit.processedData!!.metadata
+        assertThat(metadata, hasEntry("qc", DoubleNode(0.987654321)))
+        assertThat(metadata, hasEntry("age", IntNode(42)))
+        assertThat(metadata, hasEntry("region", TextNode("Europe")))
+        assertThat(metadata, hasEntry("pangoLineage", TextNode("XBB.1.5")))
+        assertThat(metadata, hasEntry("booleanColumn", BooleanNode.TRUE))
     }
 
     @Test
@@ -135,8 +139,8 @@ class SubmitProcessedDataEndpointTest(
         )
             .andExpect(status().isNoContent)
 
-        val processedData = convenienceClient.getSequenceEntryToEdit(accession = accession, version = version)
-            .processedData
+        val processedData = convenienceClient.getOriginalDataForEntry(accession = accession, version = version)
+            .processedData!!
 
         assertThat(processedData.unalignedNucleotideSequences, hasEntry(MAIN_SEGMENT, "NACTG"))
         assertThat(

cli/src/loculus_cli/utils/review_utils.py

@@ -189,7 +189,7 @@ def get_sequence_details(
         backend_url = self.instance_config.backend_url
 
         response = httpx.get(
-            f"{backend_url}/{organism}/get-data-to-edit/{accession}/{version}",
+            f"{backend_url}/{organism}/get-original-data-for-entry/{accession}/{version}",
             headers=self._get_auth_headers(),
         )
         response.raise_for_status()

integration-tests/tests/specs/backend/authentication.spec.ts

@@ -47,7 +47,7 @@ function getBackendBaseUrl(): URL {
 
 test.describe('Backend authentication', () => {
     test('rejects tokens that were not signed by Keycloak', async ({ backendRequest }) => {
-        const response = await backendRequest.get('/dummy-organism/get-data-to-edit/1/1', {
+        const response = await backendRequest.get('/dummy-organism/get-original-data-for-entry/1/1', {
             headers: {
                 Authorization: `Bearer ${tokenSignedWithDifferentKey}`,
             },

website/src/components/Edit/EditableSequences.ts

@@ -68,6 +68,9 @@ export class EditableSequences {
      */
     static fromInitialData(initialData: SequenceEntryToEdit, maxSequencesPerEntry?: number): EditableSequences {
         const maxNumberRows = maxSequencesPerEntry ?? Infinity;
+        if (initialData.processedData === null) {
+            throw new Error('Cannot edit a sequence entry without processed data (e.g. a revocation)');
+        }
         const fastaHeaderMap = EditableSequences.invertRecordMulti(initialData.processedData.sequenceNameToFastaId);
         const existingDataRows = Object.entries(initialData.originalData.unalignedNucleotideSequences).map(
             ([key, value]) => {

website/src/components/ReviewPage/FilesDialog.tsx

@@ -10,7 +10,7 @@ type FilesDialogProps = {
 };
 
 export const FilesDialog: FC<FilesDialogProps> = ({ isOpen, onClose, dataToView }) => {
-    if (!isOpen || !dataToView) return null;
+    if (!isOpen || !dataToView?.processedData) return null;
 
     return (
         <div className='fixed inset-0 flex items-center justify-center z-50 overflow-auto bg-black bg-opacity-30'>

website/src/components/ReviewPage/ReviewCard.tsx

@@ -63,7 +63,11 @@ export const ReviewCard: FC<ReviewCardProps> = ({
     const [isSequencesDialogOpen, setSequencesDialogOpen] = useState(false);
     const [isFilesDialogOpen, setFilesDialogOpen] = useState(false);
     const { isLoading, data } = useGetMetadataAndAnnotations(organism, clientConfig, accessToken, sequenceEntryStatus);
-    const hasFiles = Object.entries(data?.processedData.files ?? {}).length > 0;
+    const hasFiles = Object.entries(data?.processedData?.files ?? {}).length > 0;
+    const revocationVersionComment =
+        sequenceEntryStatus.isRevocation && typeof data?.originalData.metadata.versionComment === 'string'
+            ? data.originalData.metadata.versionComment
+            : undefined;
 
     const notProcessed = sequenceEntryStatus.status !== processedStatus;
 
@@ -83,7 +87,7 @@ export const ReviewCard: FC<ReviewCardProps> = ({
                         keyName={getAccessionVersionString(sequenceEntryStatus)}
                         value={sequenceEntryStatus.submissionId}
                     />
-                    {data !== undefined && (
+                    {data?.processedData != null && (
                         <MetadataList data={data} metadataDisplayNames={metadataDisplayNames} isLoading={isLoading} />
                     )}
                     {sequenceEntryStatus.isRevocation && (
@@ -95,14 +99,26 @@ export const ReviewCard: FC<ReviewCardProps> = ({
                             disableTruncate
                         />
                     )}
+                    {revocationVersionComment !== undefined && (
+                        <KeyValueComponent
+                            accessionVersion={getAccessionVersionString(sequenceEntryStatus)}
+                            keyName='Version comment'
+                            value={revocationVersionComment}
+                            disableTruncate
+                        />
+                    )}
                 </div>
                 <ButtonBar
                     sequenceEntryStatus={sequenceEntryStatus}
                     approveAccessionVersion={approveAccessionVersion}
                     deleteAccessionVersion={deleteAccessionVersion}
                     editAccessionVersion={editAccessionVersion}
-                    viewSequences={data && !notProcessed ? () => setSequencesDialogOpen(true) : undefined}
-                    viewFiles={data && !notProcessed ? () => setFilesDialogOpen(true) : undefined}
+                    viewSequences={
+                        data?.processedData != null && !notProcessed ? () => setSequencesDialogOpen(true) : undefined
+                    }
+                    viewFiles={
+                        data?.processedData != null && !notProcessed ? () => setFilesDialogOpen(true) : undefined
+                    }
                     filesEnabled={filesEnabled}
                     hasFiles={hasFiles}
                 />
@@ -259,9 +275,11 @@ type MetadataListProps = {
 const isAnnotationPresent = (metadataField: string) => (item: ProcessingAnnotation) =>
     item.processedFields[0].name === metadataField;
 
-const MetadataList: FC<MetadataListProps> = ({ data, isLoading, metadataDisplayNames }) =>
-    !isLoading &&
-    Object.entries(data.processedData.metadata).map(([metadataName, value], index) =>
+const MetadataList: FC<MetadataListProps> = ({ data, isLoading, metadataDisplayNames }) => {
+    if (isLoading || data.processedData === null) {
+        return null;
+    }
+    return Object.entries(data.processedData.metadata).map(([metadataName, value], index) =>
         value === null ? null : (
             <KeyValueComponent
                 accessionVersion={getAccessionVersionString(data)}
@@ -273,6 +291,7 @@ const MetadataList: FC<MetadataListProps> = ({ data, isLoading, metadataDisplayN
             />
         ),
     );
+};
 
 type ErrorsProps = {
     errors: ProcessingAnnotation[];
@@ -519,14 +538,14 @@ function useGetMetadataAndAnnotations(
     accessToken: string,
     sequenceEntryStatus: SequenceEntryStatus,
 ) {
-    const { status, accession, version, isRevocation } = sequenceEntryStatus;
-    return backendClientHooks(clientConfig).useGetDataToEdit(
+    const { status, accession, version } = sequenceEntryStatus;
+    return backendClientHooks(clientConfig).useGetOriginalDataForEntry(
         {
             headers: createAuthorizationHeader(accessToken),
             params: { organism, accession, version },
         },
         {
-            enabled: status !== receivedStatus && status !== inProcessingStatus && !isRevocation,
+            enabled: status !== receivedStatus && status !== inProcessingStatus,
         },
     );
 }