Recursive tag management in folder contents.#329
Conversation
|
@thet thanks for creating this Pull Request and helping to improve Plone! TL;DR: Finish pushing changes, pass all other checks, then paste a comment: To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass, but it takes 30-60 min. Other CI checks are usually much faster and the Plone Jenkins resources are limited, so when done pushing changes and all other checks pass either start all Jenkins PR jobs yourself, or simply add the comment above in this PR to start all the jobs automatically. Happy hacking! |
thet
force-pushed
the
thet/fc-tags-recurse
branch
2 times, most recently
from
7885684 to
1982096
Compare
|
@jenkins-plone-org please run jobs |
petschki
left a comment
petschki
left a comment
There was a problem hiding this comment.
LGTM.
Minor translation comment.
Allow to recursively add and remove tags in the folder contents tags management popup. @thet
|
Tnx for the reviews! I applied all suggestions, rebased on master and force-pushed to this branch for a clean git history. |
mauritsvanrees
left a comment
mauritsvanrees
left a comment
There was a problem hiding this comment.
We are already in the RC stage, so we shouldn't really be adding features...
But this is small, and contained to one package, so there is not much risk.
Seems to work fine in local testing.
But I'm not sure about the default page handling. You are in the contents of a container and you add/remove tags from its children. Then I don't expect that the container itself gets updated as well if you use this recurse option.
When you edit a default page with the standard Edit form and set a tag, its folder also doesn't get updated.
|
|
||
| def action(self, obj, bypass_recurse=False): | ||
| if check_default_page_via_view(obj, self.request): | ||
| self.action(obj.aq_parent, bypass_recurse=True) |
There was a problem hiding this comment.
I would not expect that the parent folder tags of a default page would get changed too when I click that in the folder contents... though its the same behavior as for the workflow state, but there it makes sense from a permission perspective.
There was a problem hiding this comment.
Right - that's copied code so same behavior here as with the workflow state.
Not sure what I would expect here.
@mauritsvanrees has an argument with the edit-view behavior which also doesn't change the workflow state.
|
@mauritsvanrees I'm fine to not merge this yet resp. creating a branch from master for 6.2 - especially while it's not clear how to handle the default page problem. |
7 participants
Allow to recursively add and remove tags in the folder contents tags management popup. @thet
If your pull request closes an open issue, include the exact text below, immediately followed by the issue number. When your pull request gets merged, then that issue will close automatically.
Closes #