-
Notifications
You must be signed in to change notification settings - Fork 461
clean up invtation handler #11137
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
clean up invtation handler #11137
Conversation
7991caa
to
99fd632
Compare
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.
@ipula Thanks. I have added some review comments for this.
...ation/invitations/userRoleAssignment/handlers/UserRoleAssignmentInviteRedirectController.php
Outdated
Show resolved
Hide resolved
...ses/invitation/invitations/userRoleAssignment/resources/UserRoleAssignmentInviteResource.php
Outdated
Show resolved
Hide resolved
...invitation/invitations/userRoleAssignment/resources/BaseUserRoleAssignmentInviteResource.php
Outdated
Show resolved
Hide resolved
d583d6b
to
0f738a5
Compare
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.
@ipula thanks! There are still unresolved and new comments on that change.
.../invitation/invitations/userRoleAssignment/handlers/UserRoleAssignmentInviteUIController.php
Outdated
Show resolved
Hide resolved
.../invitation/invitations/userRoleAssignment/handlers/UserRoleAssignmentInviteUIController.php
Show resolved
Hide resolved
.../invitation/invitations/userRoleAssignment/handlers/UserRoleAssignmentInviteUIController.php
Outdated
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.
@ipula thanks. some comments from the previous review remained unresolved. Just adding them again here with 1 addition.
...ses/invitation/invitations/userRoleAssignment/resources/UserRoleAssignmentInviteResource.php
Outdated
Show resolved
Hide resolved
...ses/invitation/invitations/userRoleAssignment/resources/UserRoleAssignmentInviteResource.php
Outdated
Show resolved
Hide resolved
28451f8
to
6c25c8b
Compare
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.
@ipula Thanks for your work on this! I’ve left a few comments that are important for ensuring everything works as expected.
I also proposed some changes here: ipula#4 — could you please review them and test if they work for the specific invitation case?
You'll also need to make sure the UI is updated to call the new handler introduced in that PR.
Once we confirm this works, we can move on to test the other invitation types and address any potential security concerns.
Thanks again — let me know if anything’s unclear or if you'd like to discuss further. Ping me when ready.
pkp#11118 Proposed fixes for cleanup
close this PR. this related to |
#11118