-
Notifications
You must be signed in to change notification settings - Fork 55
pkp/pkp-lib#11826 Update task template form to add "Mark as Unrestricted" & "Limit access to user groups" fields #750
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
base: main
Are you sure you want to change the base?
Conversation
cc141db to
ecd394f
Compare
jardakotesovec
left a comment
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.
Two minor suggestions. I think it make sense to list all roles..
| let label = `${participant.fullName} ${username}`; | ||
|
|
||
| if (participant.userName === currentUser.getCurrentUserName()) { | ||
| if (participant.username === currentUser.getCurrentUserName()) { |
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.
Minor thing. Even though username should be unique - checking by id feels safer.
| label, | ||
| subLabel: withSubLabel ? participant.roleName : null, | ||
| value: participant.id, | ||
| subLabel: withSubLabel ? participant.roles?.[0]?.name : null, |
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.
Current discussions are listing all roles. Its not all roles of that user - but all roles that given user is assigned to the submission. I see that @Vitaliy-1 handled that nicely already on the backend.
Anyway - I think just comma separated list here would be good.
…roups field to task template form
…e payload when saving task template
ecd394f to
02d6fe1
Compare
No description provided.