Contributor Roles and Type #11765

jyhein · 2025-09-01T06:21:44Z

Contributor Roles and Type

Add contributor Roles to Author
Add contributor Type to Author
Remove user group functions from Author
Remove translator from user groups
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

bozana

Hi @jyhein,
Thanks a lot for the huge work!
I have not finished my review yet, but wanted to let you know about a few things I am wondering/thinking about:
I am wondering if we could have author object with the array of ContributorRole objects. Currently, it is not easy to understand where what is set or get, when, why...
I think it is then more similar to how else we have it in the code, and maybe also cleaner, easier to read and understand.
E.g. sometimes we set contributor roles with localized names, sometimes with all names... This should actually be handled (if localized or not) when we want to do something with author's contributor roles.
Also, it would be good to have add, edit, delete functions in contributorRole Repo, that would get an object.
I believe also in the ContributorRoleForm you could work with ContributorRole objects.
I believe it would be cleaner, easier to read it all...
What do you think?

Also, there are a few other comments...
And I will continue to review...
Could you maybe rebase and let me know, then I would also try to test it locally....

Thanks a lot!

bozana · 2025-09-16T09:04:56Z

api/v1/submissions/PKPSubmissionController.php


        $params = $this->convertStringsToSchema(PKPSchemaService::SCHEMA_AUTHOR, $illuminateRequest->input());
+        // Remove extra data that is irrelevant to the selected contributor type
+        $this->removeIrrelevantContributorTypeData($params);


I am wondering if we should rather put this into the Repository validate() function...
Here and above...
Let me think... 🤔

bozana · 2025-09-16T09:09:32Z

classes/author/Author.php

+        return match ($this->getData('contributorType')) {
+            ContributorType::PERSON->getName() => parent::getFullName($preferred, $familyFirst, $preferredLocale),
+            ContributorType::ORGANIZATION->getName() => $this->getLocalizedOrganizationName($preferredLocale),
+            ContributorType::ANONYMOUS->getName() => __('submission.submit.contributorType.anonymous'),


Provide $preferredLocale as parameter for __():
__('submission.submit.contributorType.anonymous', locale: $preferredLocale)
or
__('submission.submit.contributorType.anonymous', [], $preferredLocale)

bozana · 2025-09-16T09:22:38Z

classes/author/Author.php

+        /**
+     * Get organization name.
+     */
+    public function getOrganizationName(string $locale): string|array|null


also null can be given, to get an array of all localized names, no? It also seems to be the default parameter, because above, in the function getFullNames() we call it without parameters.

bozana · 2025-09-16T09:25:07Z

classes/author/Author.php

+     *
+     * @return string
+     */
+    public function getLocalizedOrganizationName(string $preferredLocale = null)


to add return type like this:
public function getLocalizedOrganizationName(string $preferredLocale = null): string
?

bozana · 2025-09-16T09:28:23Z

classes/author/DAO.php

        }

-        Repo::creditContributorRole()->addCreditRoles($author->getCreditRoles(), $newAuthorId);
+        Repo::creditContributorRole()->addCreditRoles($author->getCreditRoles(false), $newAuthorId);


author->getCreditRoles() does not expect any parameter

bozana · 2025-09-17T11:22:32Z

schemas/author.json

+			"readOnly": true,
+			"items": {
+				"type": "object",
+				"multilingual": "true"


shell here also the properties be defined/listed?

bozana · 2025-09-17T11:23:26Z

schemas/author.json

 		},
+		"contributorRoles": {
+			"type": "object",
+			"description": "The names of this contributor's roles in the publication, such as 'Author' or 'Translator', and the translations",


Wouldn't it be better to have the array of objects here: That way we know also the role identifier and can match it when exporting?

bozana · 2025-09-17T11:26:35Z

pages/authorDashboard/PKPAuthorDashboardHandler.php


        // Get full details of the working publication and the current publication
-        $mapper = Repo::publication()->getSchemaMap($submission, $contextUserGroups, $contextGenres);
+        $mapper = Repo::publication()->getSchemaMap($submission, $contextGenres);


should then the arguments in the function getSchemaMap() in lib/pkp/classes/publication/Repository be changed as well?

bozana · 2025-09-17T11:46:53Z

classes/author/Author.php

+    {
+        $this->setData('contributorRoles', !$contextId
+            ? $contributorRoles
+            : Repo::contributorRole()->getByContextIdAndIdentifiers($contextId, $contributorRoles)


Somehow I think it would be cleaner to have array of objects/ContributorRoles in this author's property.

bozana · 2025-09-17T12:53:09Z

classes/author/Repository.php


        foreach ($authors as $author) {
-            if (empty($author->getGivenName($newLocale))) {
+            if (ContributorType::PERSON->getName() && empty($author->getGivenName($newLocale))) {


Shouldn't we check if author->getData('contributorType') == ContributorType::PERSON->getName() instead?

jardakotesovec · 2025-10-06T12:40:15Z

api/v1/contributorRoles/ContributorRoleController.php

+    /**
+     * Get form data
+     */
+    public function getContributorRoleFormData(Request $illuminateRequest): JsonResponse


I would suggest not making it form specific. Just expose it as any other data. I think its still valuable even if its not data from the database.

Endpoint could be contributorRoles/contributorRoleIdentifier
just with response
["EDITOR", "CHAIR"]

Tagging @ewhanson If he has some preference on this.

Just to note that the identifiers are not all but what are currently not in use

@jyhein Not sure if I understand. Can you elaborate more? Thanks.

The identifiers are enum and some of them are saved to the db with translations. That function returns the identifiers that are not in the db for the form. Identifier per context is unique in db

bozana mentioned this pull request Jul 1, 2025

Contributor metadata enhancements: Credit Roles, Contributor Roles and Contributor Types #11378

Open

bozana reviewed Sep 17, 2025

View reviewed changes

jyhein force-pushed the f857 branch from 114974d to 175420c Compare September 18, 2025 08:13

jyhein force-pushed the f857 branch 6 times, most recently from 3d6c902 to 6e7408a Compare October 1, 2025 18:09

Contributor Roles and Type

f0d12c9

jyhein force-pushed the f857 branch from 6e7408a to e722861 Compare October 2, 2025 17:09

Contributor Roles and Type

0a19f74

jyhein force-pushed the f857 branch from e722861 to 0a19f74 Compare October 2, 2025 18:43

jardakotesovec reviewed Oct 6, 2025

View reviewed changes

Contributor Roles and Type #11765

Are you sure you want to change the base?

Contributor Roles and Type #11765

Uh oh!

Conversation

jyhein commented Sep 1, 2025

Uh oh!

bozana left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants