Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle node deletion event #1228

Merged
merged 3 commits into from
Sep 13, 2022
Merged

Handle node deletion event #1228

merged 3 commits into from
Sep 13, 2022

Conversation

skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Sep 9, 2022

To remove files from album when they are deleted.

Signed-off-by: Louis Chemineau [email protected]

From #1161
Fix #1226

To remove files from album when they are deleted.

Signed-off-by: Louis Chemineau <[email protected]>
@skjnldsv skjnldsv added bug Something isn't working 3. to review Waiting for reviews high High priority labels Sep 9, 2022
@skjnldsv skjnldsv self-assigned this Sep 9, 2022
Copy link
Collaborator

@artonge artonge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

Unrelated, but damn you gotta love how powerful git is!

@artonge
Copy link
Collaborator

artonge commented Sep 9, 2022

Note that you'll still have to remove broken files in the database

@skjnldsv
Copy link
Member Author

skjnldsv commented Sep 10, 2022

Note that you'll still have to remove broken files in the database

Broken files?
Broken albums you mean?
We could also add a check when we get the children and remove the missing references.
While this could be expensive, this would only be a failsafe as the event would already be there

@skjnldsv
Copy link
Member Author

Also, if the node gets put to trash, does it fire the NodeDeletedEvent ?
Because the fileId would still exists, no?

Signed-off-by: John Molakvoæ <[email protected]>
@artonge
Copy link
Collaborator

artonge commented Sep 10, 2022

Also, if the node gets put to trash, does it fire the NodeDeletedEvent ?
Because the fileId would still exists, no?

I thought that NodeDeletedEvent was for when the file is put in the trashbin, confused

@skjnldsv
Copy link
Member Author

I thought that NodeDeletedEvent was for when the file is put in the trashbin, confused

Well, what if you disabled the trashbin? :p

@skjnldsv
Copy link
Member Author

skjnldsv commented Sep 13, 2022

Added some check in the propfind where it was throwing before

@skjnldsv skjnldsv merged commit de90112 into master Sep 13, 2022
@skjnldsv skjnldsv deleted the fix/node-deletion branch September 13, 2022 10:35
@artonge
Copy link
Collaborator

artonge commented Sep 14, 2022

@skjnldsv for folders, the NodeDeletedEvent is only triggered for the folders, and not media inside the folder. Are you aware of another event that will be triggered for every files, or should we walk the folder manually ?

@skjnldsv
Copy link
Member Author

skjnldsv commented Sep 14, 2022

@skjnldsv for folders, the NodeDeletedEvent is only triggered for the folders, and not media inside the folder. Are you aware of another event that will be triggered for every files, or should we walk the folder manually ?

Ah!
There is the old hooks iirc. e.g
https://github.com/nextcloud/server/blob/ac2bc2384efe3c15ff987b87a7432bc60d545c67/lib/private/Files/Node/HookConnector.php#L149-L157

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug Something isn't working high High priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

500 error if you remove a file belonging to an album
2 participants