-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
[50254] Filtering on share modal #14075
Conversation
b9aa7b5
to
6cfad47
Compare
8defbc1
to
a3dcb67
Compare
…ut any additional js required
a5b393d
to
8a1b4af
Compare
8a1b4af
to
96c118f
Compare
I added the still open points to the description. Futher, the code needs some cleanup, especially this part. Other than that this ready for a first review. |
app/components/work_packages/share/bulk_permission_button_component.html.erb
Show resolved
Hide resolved
frontend/src/stimulus/controllers/dynamic/work-packages/share/bulk-selection.controller.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice so far, besides the open points and known issues you mentioned, it looks good to me. I'd like to see what the implementation turns out to be once we can chain both filter types at a time. Great job so far!
…other way around). Further, the test has been adapted to it.
I excluded the inheritance problem to #50939 as I am unsure about the expected results and would like to discuss that with product first. |
The `MemberQuery` has a default_scope that checks a non-admin has a certain set of permissions which are not required for the behavior of the share modal (or visibility of work package shares for that matter). Given that visibility is unscoped for the share modal, extracting a separate query class that inherits from the MemberQuery sans the default_scope made sense as a way to keep these two in sync minus the varying visibility levels based on the current_user.
There were a couple of specs failing due to the new query layer and injectable shares into the ModalBodyComponent. This commit standardizes the interface to prevent this in an easy to follow manner.
Thanks for extracting Attila!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Went ahead and fixed the pesky issues you were facing. You can merge whenever you're back :)
Thanks a lot, @aaron-contreras for jumping in! 🤗 |
Open Questions
Handled separatly
When the filter results are empty, the Box switches to the blankslate, where no filter is available. Thus I cannot leave this stage any more.See ticketInherited permissions are in filter result set but might show a different permission if the user was shared separatlySee tickethttps://community.openproject.org/projects/openproject/work_packages/50254/activity