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

allow Folder deletion #13

Closed
grahamu opened this issue Feb 18, 2016 · 8 comments
Closed

allow Folder deletion #13

grahamu opened this issue Feb 18, 2016 · 8 comments
Milestone

Comments

@grahamu
Copy link
Contributor

grahamu commented Feb 18, 2016

Since Documents can be deleted, surely Folders should be deletable as well?

@grahamu
Copy link
Contributor Author

grahamu commented Feb 18, 2016

If we allow folder deletion, what happens when deleting a non-empty Folder?

@paltman
Copy link
Member

paltman commented Feb 20, 2016

Since it's destructive, perhaps we should provide a confirmation screen listing all the files that that will be deleted as well.

@paltman paltman modified the milestone: 16.04 Mar 18, 2016
@paltman paltman modified the milestones: 16.04, Documents Starter Project: Milestone 0 Mar 31, 2016
@ethankent
Copy link
Contributor

I'm not sure how many documents are allowed in a folder. If there is no limit, we might want to consider doing the mass-delete as some sort of job. Just a thought for a future feature.

@paltman
Copy link
Member

paltman commented Apr 1, 2016

@ethankent good thinking. can you implement the actual deletion as a hookset method so the site builder could override the default functionality and offline it to whatever queuing system (e.g. celery) they happen to be using or desire to use?

@ethankent
Copy link
Contributor

I'd be happy to take a look at that. However, I'd also like to raise one more question...

Since we're now beginning to raise issues related to recursion, we should at least consider whether (and when) we might want to offload some of these concerns to a library focused on tree structures.

  1. Should we consider incorporating a library like django-mptt or django-treebeard?
  2. If yes, should we consider implementing non-recursive deletion now and implementing recursive deletion with the help of such a library?

@paltman
Copy link
Member

paltman commented Apr 1, 2016

@ethankent good points. I'd like @brosner to weigh on this. For now, let's just implement it with the recursion, as horrible as that could get in a large tree, but it have all the logic implemented in the hookset method. It is a bit of kicking the can down the road a bit, but very interested in getting to feature complete with the simplest thing possible for now, while provide some hooks for people to override things while we determine more performant solutions for future releases.

@ethankent
Copy link
Contributor

Hi Everyone,

Just wanted to let you know that I have a PR on the way for this. But I'm trying to get the required PR for pinax-theme-bootstrap first. This order should prevent the build from breaking.

@ethankent
Copy link
Contributor

PR submitted! Let me know if you see any improvements I can make.

@paltman paltman closed this as completed Apr 10, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants