Unobsolete GET instance copy endpoint, copy based on Authenticated
#1228
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| var actual = await Assert.ThrowsAsync<PlatformHttpException>( | ||
| async () => await controller.CopyInstance("ttd", "copy-instance", instanceOwnerPartyId, instanceGuid) | ||
| ); |
Check warning
Code scanning / CodeQL
Useless assignment to local variable Warning test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
To fix the problem, we need to remove the unnecessary assignment to the variable actual. The Assert.ThrowsAsync method call should be retained to ensure that the expected exception is thrown, but the result of this call does not need to be assigned to a variable.
- Remove the assignment to the variable
actualon line 259. - Retain the
Assert.ThrowsAsyncmethod call to ensure the test still verifies that thePlatformHttpExceptionis thrown.
| @@ -258,3 +258,3 @@ | ||
| var controller = fixture.ServiceProvider.GetRequiredService<InstancesController>(); | ||
| var actual = await Assert.ThrowsAsync<PlatformHttpException>( | ||
| await Assert.ThrowsAsync<PlatformHttpException>( | ||
| async () => await controller.CopyInstance("ttd", "copy-instance", instanceOwnerPartyId, instanceGuid) |
|
| @@ -597,7 +600,8 @@ await _processEngine.HandleEventsAndUpdateStorage( | |||
|
|
|||
| /// <summary> | |||
| /// This method handles the copy endpoint for when a user wants to create a copy of an existing instance. | |||
| /// The endpoint will primarily be accessed directly by a user clicking the copy button for an archived instance. | |||
| /// The endpoint will primarily be accessed directly by a user clicking the copy button for an archived instance | |||
| /// from the message box in the Altinn 2 portal/Altinn 3 arbeidsflate. | |||
There was a problem hiding this comment.
| /// from the message box in the Altinn 2 portal/Altinn 3 arbeidsflate. | |
| /// from the message box in the Altinn 2 portal/Altinn 3 arbeidsflate. | |
| /// WARNING: The endpoint will be removed in an updated version of the app packages as soon as we have an | |
| /// alternative where dialogporten/arbeidsflate where it doesn't need to rely on the user cookie propagating | |
| /// through a GET request. |
There was a problem hiding this comment.
There's discussions going on about the future of Altinn authentication (where/how to store session info, not doing token exchange etc etc) but there is nothing concrete yet, so I don't think it's appropriate to make any assertions on what we'll do at that point. Whatever it is, it has to be planned and agreed upon with the Arbeidsflate team
There was a problem hiding this comment.
The important part is that the endpoint might be removed without warning when arbeidsflate has an alternative and stops using it for apps newer than a specific version (or however we decide to do the transition)
There was a problem hiding this comment.
Is your point that this is only meant to be consumed by Arbeidsflate? And that we don't care about potential breakage for any other possible consumer? If so I agree that we should document that someplace, but I don't think it belongs in an XML doc comment that doesn't go anywhere
Description
Related Issue(s)
Verification
Documentation