Skip to content

Commit

Permalink
fix(backend): Fix submitter of revoked sequences to be the revoker (#…
Browse files Browse the repository at this point in the history
…3246)

* (CoPilot generated attempt) Fix submitter of revoked sequences to be the revoker

Fixes #3244

Update the submitter of a revoked sequence to the revoker

* Modify `backend/src/main/kotlin/org/loculus/backend/service/submission/SubmissionDatabaseService.kt` to set the `submitterColumn` to the revoker's account during the revocation process
* Change the `revoke` function to use the revoker's username for the `submitterColumn` instead of cloning the old submitter information

---

For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/loculus-project/loculus/issues/3244?shareId=XXXX-XXXX-XXXX-XXXX).

* Add test for #3244 (#3248)

* Assert some more things that we don't seem to so far to avoid similar errors from happening in future

* fix tests

---------

Co-authored-by: Cornelius Roemer <[email protected]>
  • Loading branch information
theosanderson and corneliusroemer authored Nov 20, 2024
1 parent 04e661b commit a2907ce
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -772,7 +772,7 @@ class SubmissionDatabaseService(
else -> stringParam(versionComment)
},
SequenceEntriesTable.submissionIdColumn,
SequenceEntriesTable.submitterColumn,
stringParam(authenticatedUser.username),
SequenceEntriesTable.groupIdColumn,
dateTimeParam(dateProvider.getCurrentDateTime()),
booleanParam(true), SequenceEntriesTable.organismColumn,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,25 @@ fun SequenceEntryStatus.assertHasError(error: Boolean): SequenceEntryStatus {
return this
}

fun SequenceEntryStatus.assertSubmitterIs(submitter: String): SequenceEntryStatus {
assertThat(this.submitter, `is`(submitter))
return this
}

fun SequenceEntryStatus.assertGroupIdIs(groupId: Int): SequenceEntryStatus {
assertThat(this.groupId, `is`(groupId))
return this
}

fun SequenceEntryStatus.assertIsRevocationIs(revoked: Boolean): SequenceEntryStatus {
if (revoked) {
assertThat(this.isRevocation, `is`(true))
} else {
assertThat(this.isRevocation, `is`(false))
}
return this
}

fun expectUnauthorizedResponse(isModifyingRequest: Boolean = false, apiCall: (jwt: String?) -> ResultActions) {
val response = apiCall(null)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,12 @@ import org.loculus.backend.controller.DEFAULT_ORGANISM
import org.loculus.backend.controller.DEFAULT_USER_NAME
import org.loculus.backend.controller.EndpointTest
import org.loculus.backend.controller.OTHER_ORGANISM
import org.loculus.backend.controller.SUPER_USER_NAME
import org.loculus.backend.controller.assertGroupIdIs
import org.loculus.backend.controller.assertHasError
import org.loculus.backend.controller.assertIsRevocationIs
import org.loculus.backend.controller.assertStatusIs
import org.loculus.backend.controller.assertSubmitterIs
import org.loculus.backend.controller.expectUnauthorizedResponse
import org.loculus.backend.controller.generateJwtFor
import org.loculus.backend.controller.jwtForSuperUser
Expand Down Expand Up @@ -36,7 +41,7 @@ class RevokeEndpointTest(
}

@Test
fun `GIVEN entries with 'APPROVED_FOR_RELEASE' THEN returns revocation version in status AWAITING_APPROVAL`() {
fun `GIVEN entries with 'APPROVED_FOR_RELEASE' THEN returns revocation version in status PROCESSED`() {
val accessions = convenienceClient.prepareDefaultSequenceEntriesToApprovedForRelease().map { it.accession }

client.revokeSequenceEntries(accessions)
Expand All @@ -46,8 +51,14 @@ class RevokeEndpointTest(
.andExpect(jsonPath("\$[0].accession").value(accessions.first()))
.andExpect(jsonPath("\$[0].version").value(2))

val originalEntry = convenienceClient.getSequenceEntry(accession = accessions.first(), version = 1)

convenienceClient.getSequenceEntry(accession = accessions.first(), version = 2)
.assertStatusIs(PROCESSED)
.assertHasError(false)
.assertGroupIdIs(originalEntry.groupId)
.assertIsRevocationIs(true)
.assertSubmitterIs(DEFAULT_USER_NAME)
}

@Test
Expand Down Expand Up @@ -94,7 +105,7 @@ class RevokeEndpointTest(
}

@Test
fun `WHEN superuser revokes entries of other group THEN revocation version is created`() {
fun `WHEN superuser revokes entries of other group THEN superuser is submitter of revocation entry`() {
val accessions = convenienceClient
.prepareDefaultSequenceEntriesToApprovedForRelease(username = DEFAULT_USER_NAME)
.map { it.accession }
Expand All @@ -106,8 +117,14 @@ class RevokeEndpointTest(
.andExpect(jsonPath("\$[0].accession").value(accessions.first()))
.andExpect(jsonPath("\$[0].version").value(2))

val originalEntry = convenienceClient.getSequenceEntry(accession = accessions.first(), version = 1)

convenienceClient.getSequenceEntry(accession = accessions.first(), version = 2)
.assertStatusIs(PROCESSED)
.assertHasError(false)
.assertGroupIdIs(originalEntry.groupId)
.assertIsRevocationIs(true)
.assertSubmitterIs(SUPER_USER_NAME)
}

@Test
Expand Down

0 comments on commit a2907ce

Please sign in to comment.