BC-6352 - Deleting the creator field causes the files not to be displayed anymore #3396
Conversation
…ters to see files created by them) in homeworks
controllers/homework.js
Outdated
| submissionFilesStorageData.files = filesStorage.files.filter((file) => submitters.has(file.creatorId)); | ||
| submissionFilesStorageData.readonly = readonly || (!isCreator && isTeacher); | ||
|
|
||
| const gradeFilesStorageData = _.clone(filesStorage); | ||
| gradeFilesStorageData.files = filesStorage.files.filter((file) => teachers.has(file.creatorId)); |
There was a problem hiding this comment.
Why do you remove it completly? What is with other user? For example:
you has removed a Student, but a Teacher or other Student look this submission wath than?
There was a problem hiding this comment.
@SevenWaysDP It just removes a requirement for the creator to be present among the person that can access the file which can be sometimes not true as we have to remove the creator reference from the files. The files should remain accessible to everyone that have access to it.
There was a problem hiding this comment.
Sorry @bn-pass but I would like to test this again briefly. There are so many scenarios
There was a problem hiding this comment.
There was a problem hiding this comment.
Yes, there is another way, or, yes, this is a real problem? ;)
This is a real problem!
There was a problem hiding this comment.
@SevenWaysDP where did you spot that behaviour? Was it on the testing environment for this branch or on your local development machine and you only checked out this branch in the
schulcloud-client? I'm afraid it could be related to the changes made to the "authorization" logic in the backend (on the same branch)... I don't think changes made here (in theschulcloud-client) could cause that behaviour as I've only removed the check for the creator presence among thesubmittersorteachers(as we still want to be able to access some files uploaded by no longer "present" user.
This is a problem if you remove because submission and grading files have parentType = submission and we sort them by creatorId in Frondend
There was a problem hiding this comment.
@bn-pass Please do not merge, I will clarify this with Cedric and Ingwert.
There was a problem hiding this comment.
@dyedwiper @SevenWaysDP As the ticket has received a higher priority I've modified the solution to a bit safer approach which came from a fact that the issue has not been created by this particular ticket's changes (as it hasn't added the files' creatorId removal - it has been added in another ticket, already merged to main so it will already happen on all the executed data deletion requests). I think it's a sweet spot between what is available right now to us and what we want to achieve in the follow-up ticket (BC-6609).
So thanks to the changes made within the 4e555a6 commit, the filter will now be applied to all the files that still have the creatorId reference defined, but the files that do not have this reference will simply be added to every tab (will be selected by any filter) - thanks to this any files should not be left unavailable in any case. And after the BC-6609 will be developed and merged this inconvenience will be fixed (but unfortunately not for the old files as they will already have the creatorId reference removed) as we'll then be able to decide whether it's a submission-related file or grading-related file based on some flag or something.
Please re-review all the changes and let me know what's the conclusion - in my opinion it should be enough to not block this PR anymore, but you have the final voice of course.
…at has the creatorId defined
|
…ters to see files created by them) in homeworks
Description
Links to Tickets or other pull requests
https://ticketsystem.dbildungscloud.de/browse/BC-6352
Changes
Data Security
Deployment
New Repos, NPM packages or vendor scripts
Screenshots of UI changes
Approval for review