pkp/pkp-lib#9658 send and accept invitation views and php integrations #10558
Conversation
ipula
changed the title
There was a problem hiding this comment.
Hey @ipula, thanks for putting this together! It's looking really good. Just a few general comments and questions along with a few more minor style/typo requests.
In general:
- Invitation search for users should strip start/end whitespace out:
dbarnes@mailinator.comanddbarnes@mailinator.comare different and a search for the former won't return the latter. - "Next" link in user & roles page for invitations is a clickable link even if there are no invitations.
- Invitation workflow doesn't work for inviting a user to a role again that they had previously been removed from. This is a larger issue that I don't know is within the scope here, but I found it while going through this. Pinging @defstat for this. For example, if a user has a role that has been "removed" i.e. ended, and an editor wants to add them to that same role again with a new start date, it's currently impossible.
- The phrase "OJS" and "journal" are used throughout the locale keys. I've indicated these ones with comments, but I think for this PR it would probably be best to get this merged, and address that separately afterwards. So you can keep the comments there, but I wouldn't worry about them right now.
Thanks again and let me know if you have any questions.
templates/management/access.tpl
Outdated
| @@ -18,6 +18,7 @@ | |||
|
|
|||
| <tabs :track-history="true"> | |||
| <tab id="users" label="{translate key="manager.users"}"> | |||
| <user-invitation-manager></user-invitation-manager> | |||
There was a problem hiding this comment.
Should use tabs instead of spcaes.
There was a problem hiding this comment.
This line should use tabs like the rest of the file.
The OJS specific locale were moved to OJS locale, so that other PKP applications may customize them easily. |
|
@ewhanson I made some changes to white-spaces and hide next button in ui-library in this pr pkp/ui-library#439 |
ipula
changed the title
Thanks @withanage. Have PRs with equivalents been created for OPS and OMP been created? We'll need those too. Thanks! |
There was a problem hiding this comment.
Thanks @ipula for the quick changes! Just a few other smaller style things along with a question about journal specific language for @withanage. Once that's taken care of it should be ready for merge!
locale/en/invitation.po
Outdated
|
|
||
| msgid "acceptInvitation.privacyStatement.label" | ||
| msgstr "Yes, I agree to have my data collected and stored according to the" |
There was a problem hiding this comment.
I see, thanks @ipula. I think it would be better to include this as a variable in the locale string directly rather than appending it to the end so other languages can change both the text and placement within the sentence.
locale/en/invitation.po
Outdated
|
|
||
| msgid "invitation.role.masthead" | ||
| msgstr "Journal Masthead" |
There was a problem hiding this comment.
Anything journal-related should probably move to the applications as well. If you've already done this for the OJS language, could you do this for the word "journal" as well, @withanage?
locale/en/invitation.po
Outdated
|
|
||
| msgid "userInvitation.roleTable.journalMasthead" | ||
| msgstr "Journal Masthead" |
There was a problem hiding this comment.
Usage of journal. Should be moved to app. cc: @withanage.
locale/en/invitation.po
Outdated
|
|
||
| msgid "userInvitation.search.userNotFound" | ||
| msgstr "The user does not have a role in this journal" |
There was a problem hiding this comment.
Usage of journal. Should be moved to app. cc: @withanage.
locale/en/invitation.po
Outdated
| msgstr "The user does not have a role in this journal" | ||
|
|
||
| msgid "userInvitation.search.userFound" |
There was a problem hiding this comment.
Usage of journal. Should be moved to app. cc: @withanage.
templates/management/access.tpl
Outdated
| @@ -18,6 +18,7 @@ | |||
|
|
|||
| <tabs :track-history="true"> | |||
| <tab id="users" label="{translate key="manager.users"}"> | |||
| <user-invitation-manager></user-invitation-manager> | |||
There was a problem hiding this comment.
This line should use tabs like the rest of the file.
|
@ewhanson all the strings with "Jourals" were also moved to journal locale |
No description provided.