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

add rfc for nested collections #48

Closed
wants to merge 1 commit into from
Closed

add rfc for nested collections #48

wants to merge 1 commit into from

Conversation

SebJansen
Copy link

This RFC proposes the ability to nest collections to improve media management.

@kaedroho kaedroho added the 1:New label Feb 13, 2020
@chosak
Copy link
Member

chosak commented Feb 13, 2020

Related: issue wagtail/wagtail#3380, PRs wagtail/wagtail#3407 and wagtail/wagtail#4106.

Highlighting one design decision needed from wagtail/wagtail#4106 (comment): how should the collection hierarchy be shown to users with incomplete permissions over the whole tree?

@SebJansen
Copy link
Author

Highlighting one design decision needed from wagtail/wagtail#4106 (comment): how should the collection hierarchy be shown to users with incomplete permissions over the whole tree?

By default the collection-chooser isn't shown when user has no permission to see a collection.

Assuming the following tree of collections:
level1
...level2
......level3

If a user has access to level 2, but not 1, my implementation shows level 2 to be the root and doesn't 'know' about level 1

I'm very keen on hearing whether my implementation is adequate to be merged upstream.
May I do a PR or should I improve the implementation/design?

@gasman
Copy link
Contributor

gasman commented May 13, 2020

Thanks @SebJansen! Have given your implementation a try, and it's all working well and behaving as I'd expect it to. I think it's fine for permissions to pass down to child collections - that's consistent with how it works for pages.

In the interface for adding/editing collections, intuitively I feel like I want to click through to the parent to add a child collection, but that's only really a minor point, and getting the initial functionality in place is more important than UI tweaks.

I noticed a couple of places that don't have the new hierarchical dropdown yet - the add/edit image views, and the group permission settings - but besides that, I think this is ready for a PR 👍

@coredumperror
Copy link

I'm glad I found this, because I'm also currently working on implementing the same thing. My code shop has been using a modified version of wagtail/wagtail#4106, patched onto a custom fork of Wagtail 2.3 for months, and it's worked great. We're now (finally) in the process of upgrading to Wagtail 2.9, and I've been tasked with building a fork of 2.9 with our Hierarchical Collections functionality.

But with gasman saying that SebJansen's code looks good, I figure it'll be better to work with his implementation. So I'm going to check it out and see if it suits my shop's needs. If so, I'll be happy to contribute any improvements I can make, because we would very much like this functionality to officially make it into Wagtail ASAP.

@coredumperror
Copy link

coredumperror commented Jun 5, 2020

Alright! Finally got this up and running on my dev machine, and can check out how it works.

So far, I like it! That "return" icon is a good idea for indicating child relationships. And I like the way the Parent setting has a special that disables the option for setting a Collection as it's own child, or it's own descendant's child. However, I noticed a major bug. The select presented to you when you want to set an Image or a Document as a member of a Collection doesn't do the nesting. Everything's at the top level. Another minor bug is that the Root collection is shown as being at the same level as the rest of the top-level Collections, even though it's actually their parent. Was that intentional, perhaps? I'll dive into the code tomorrow and fix the nesting issue on the Image and Document forms. And also check the whole thing out in more detail, and perhaps find some other enhancements to make.

@ababic
Copy link

ababic commented Sep 5, 2020

Closing this, as Nested Collections are now a thing :D Big thanks to @SebJansen, @coredumperror, @gasman and all the other devs that reviewed things along the way.

@ababic ababic closed this Sep 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants