DN-3837: Edit Submitted files#188
Conversation
goomens
left a comment
There was a problem hiding this comment.
I'm not entirely sure how the changes here relate to the filename issue you mention? These seem to only be admin permissions changes (and I'm not sure we always want admins to have these permissions?)
Perhaps this issue should be picked up separately from the FE issue as a bug / improvement, so we can refine it a bit more. I did notice some permission issues while working on the FE part, also sometimes there wasn't an edit / replace button, while it should show. In those cases the user didn't have the permission on a specific form, but in general the user did have view permissions. Why this happened: EnsureAuthorizedToEdit previously only checked for Edit / Submit permission on the specific form. Users with ViewAdminTools (e.g. SystemAdmin) have neither, so uploading or deleting a file on an already-submitted form would return 403. The check now also passes if the user has ViewAdminTools, allowing admins to replace or delete uploaded files. But maybe we should use something else than ViewAdminTools? GetAllowedSubmissions previously only included forms the current user had explicit View permission for. Users with only ViewAdminTools (no View) would therefore receive an empty submissions list, causing the frontend to fall back to a historical read-only snapshot instead of live data. The method now additionally includes all submitted forms for users with ViewAdminTools, so the updated file is reflected in the response immediately after upload. I haven't been super invested in the permissions in the back-end, so this might not be the ideal approach. Let me know if you think there is a better way, perhaps a separate ticket and refining is the way to go. |
|
Either way the front-end and back-end permissions should align I guess, so this sounds like a bug, but I'm not sure if this is the ideal fix. Do you have an example where it goes wrong? |
Made a few small changes here because there was an issue:
Hope I fixed this properly, but I'm not completely certain about it. Contains permissions etc. so please review carefully 🙏