Let users delete a folder if it has files in it

What does this MR do?

I simply implemented a for line in case a folder has files that runs deleteFileSync which is also used in the DeleteFilesProcessorService in a for/while loop this does not hit any API endpoint and shouldn't hit the rate limit.

This change does not allow you to delete a folder that has another folder inside it cause I wasn't sure how to efficiently implement it sorting through each child folder if another child folder also had one.

Closes #949

Contribution Guidelines

By submitting this merge request, you agree to follow our Contribution Guidelines

• I agree to follow this project's Contribution Guidelines
• I have made sure to test this merge request

added enhancement label

requested review from @dakkar and @fEmber

uh… this feels wrong on a few different levels (better review tomorrow)

It might feel wrong but in the first place all this does is replace the countBy with instead actually finding all the rows with the folder's ID (obv not including other folders as they are in a different table) and then if it has any the for statement runs through them otherwise it just gets skipped.

calling deleteFileSync possibly thousands of times inside a controller is not a great idea, this should go to a background job (possibly DeleteDriveFiles, with an extra folder-id argument)

and at that point it should be able to delete any folder, regardless of what's inside them (even other folders)

That is probably true but the issue with that is neither DeleteAccount (which I just noticed) nor DeleteDriveFiles know how to handle multiple child folders or even just one folder as they simply go and delete all files from the user while keeping the folders.

(Which is even weirder when considered in combination with DeleteAccount leaving those in the DB)

on that note I just realized that DeleteDriveFiles exists but goes unused as it never gets called anywhere but it essentially just does the same thing as DeleteAccount

1. Take user id as input
2. Delete all files in db that reference user id

sounds like a great opportunity to extend it to take an (optional) folder id as well! then it will delete all files and folders contained in that folder id (or all files if no folder id is provided)

and, as a separate MR, teach DeleteAccount about deleting folders, too

my question would still be how to handle the folders cause i don't know much about typeorm or SQL.

as it stands the drive files only get one folder id assigned to them and that is the one they are in.

That folder id can then be resolved to in the other table that has all the folders and any child folder created in that folder then have it as a parent id now that parent id changes though if you create another child folder inside the child folder that already exists in the main folder.

pseudo-code:

• given a folder id, you can find all folders inside it this.driveFoldersRepository.findBy({userId: theUSer.id, parentId: theFolderId})
• you do a breadth-first visit of the folders tree, so you end up with an array of folder objects to work on
• given a folder id, you can find all files inside it this.driveFilesRepository.findBy({userId: theUser.id, folderId: theFolderId})
• now you have a list of files to delete: delete them this.driveService.deleteFileSync(file)
• then delete the folders this.driveFoldersRepository.delete(folder.id) starting from the deeper ones (if you did a breadth-first visit, appending folders to the resulting list, the deeper ones will be at the end of that list)
  • this may fail if files have been adedd while you were deleting them… in which case you just stop, the user keeps whatever is left and/or calls folders/delete again

(for both findBy, you can also use the "cursor" trick that DeleteDriveFilesProcessorService uses, so you don't get too many rows at once)

added 1 commit

• 7fd6e009 - chore: move import

Compare with previous version

resolved all threads

requested review from @fEmber

approved this merge request