Contributor Roles and Type #2164
Conversation
jyhein
force-pushed
the
f857
branch
3 times, most recently
from
4e19869 to
13fa21b
Compare
bozana
left a comment
bozana
left a comment
There was a problem hiding this comment.
Hi Jyrki, it all looks good. Just a few comments, regarding migration and ONIX export.
Thanks a lot, great work!
| contributorTypePerson: 'PERSON', | ||
| contributorRoleAuthor: 1, | ||
| contributorRoleChapterAuthor: 3, | ||
| contributorRoleVolumeEditor: 4, |
There was a problem hiding this comment.
Maybe to add translator even if it is not currently used
| <migration class="PKP\migration\upgrade\v3_6_0\I11917_TaskTemplateDueDate"/> | ||
| <migration class="PKP\migration\upgrade\v3_6_0\I11913_AssociatePublicationsWithReviewRounds"/> | ||
| <migration class="PKP\migration\upgrade\v3_6_0\I857_ContributorRolesTypes"/> | ||
| <migration class="APP\migration\upgrade\v3_6_0\I857_ContributorRolesTypes"/> |
There was a problem hiding this comment.
Only APP can stay, that should then call the parent class
| use PKP\install\DowngradeNotSupportedException; | ||
| use PKP\migration\Migration; | ||
|
|
||
| class I857_ContributorRolesTypes extends Migration |
There was a problem hiding this comment.
Maybe to extend the PKP class and then first run parent::up(), and then only use this one in upgrade.xml
| collect($author->getContributorRoles()) | ||
| ->map(function (ContributorRole $contributorRole) use ($isEditedVolume): string { | ||
| $contributorRoleIdentifier = $contributorRole->getAttribute('contributor_role_identifier'); | ||
| $editor = $contributorRoleIdentifier === ContributorRoleIdentifier::EDITOR->getName(); |
There was a problem hiding this comment.
If I see it correctly userGroup can never be Press Editor (default.groups.name.editor): when I submit an article as Press Editor, I always have userGroup author per default, or the contributor role that I have chosen.
So I am not sure, if we should only consider volumeEditor => B01.
Else, we would need to consider userGroup = Press Editor in the migration too...
🤔
Jyrki, would you mind testing/double checking it? If there is a situation when an author can have userGroup=PressEditor? If it is not possible, then maybe to only consider volume editor for this export...
There was a problem hiding this comment.
I tried with dbarnes (Press editor) and that $editor => 'B21' happens
| ->select(['setting_name']) | ||
| ->where('setting_name', '=', 'isVolumeEditor') | ||
| ->delete(); | ||
| } |
There was a problem hiding this comment.
I think we need to consider this setting in migration, to set the contributor role to EDITOR if this setting is true.
It seems the user group of an contributor could be author, but also this setting could exist.
So, if user group of an existing contributor is not volume editor (i.e. there is no EDITOR contributor role already set in parent::up()) and this setting is true, then add EDITOR as contributor role as well.
jyhein
force-pushed
the
f857
branch
3 times, most recently
from
c839091 to
bf0ddce
Compare
2 participants
Contributor Roles and Type:
Add contributor Roles to Author
Add contributor Type to Author
Remove user group functions from Author
Add possibility to add more roles than author and translator
Selected contributor type changes what contributor form fields are required
More than one contributor role can be selected in the contributor form
Remove isVolumeEditor