fix: consolidating deletion logic in ExtensionService#1935
Conversation
gnugomez
force-pushed
the
gnugomez/main/delete-extensions-consolidation
branch
from
f2be33b to
26a8425
Compare
autumnfound
left a comment
autumnfound
left a comment
There was a problem hiding this comment.
Overall LGTM, but I haven't run the server locally to do a full test.
Assisted-By: anthropic:claude-opus-4-8[1m]
netomi
force-pushed
the
gnugomez/main/delete-extensions-consolidation
branch
from
26a8425 to
e919387
Compare
|
|
||
| search.removeSearchEntry(extension); | ||
| logs.logAction(admin, ResultJson.success("Deleted " + NamingUtil.toExtensionId(extension))); | ||
| extensions.deleteExtension(admin, extension); |
There was a problem hiding this comment.
we cant just call the version from the ExtensionService as this one does not take the admin role into account afaict.
There was a problem hiding this comment.
So reviewing this a bit more extensions.deleteExtension(admin, extension) should not really be problematic since it only takes the user to log the action, the overload that's taking the user into account is deleteExtension(UserData user, String namespaceName, String extensionName, List<TargetPlatformVersionJson> targetVersions) since it's the only user-facing one.
This is quite confusing tho, we should rename it or even move it out to it's own service.
|
will refactor that PR a bit due to the comments I made |
3 participants
This PR is a followup of #1919, while consolidating the deletion logic, the race condition has also been fixed in the admin dashboard extension deletion flow.
I'm also adding the corresponding front-end changes to re-fetch the data if the error status equals to 409.