Skip to content
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

Include metadataLocales in /submissions/{submissionId} API #10329

Closed
jardakotesovec opened this issue Aug 20, 2024 · 18 comments
Closed

Include metadataLocales in /submissions/{submissionId} API #10329

jardakotesovec opened this issue Aug 20, 2024 · 18 comments
Assignees
Labels
Enhancement:1:Minor A new feature or improvement that can be implemented in less than 3 days.
Milestone

Comments

@jardakotesovec
Copy link
Contributor

jardakotesovec commented Aug 20, 2024

Describe the issue

While working on migrating Publications to the side modal. I need to be able to configure the publication metadata forms correctly client side. For that I need supported metadata locales for specific submission.

Could we add following to the api/submissions/{submissionId} response?

{
  ...
  metadataLocales: [
	{key: 'en', label: 'English'},
	{key: 'fr_CA', label: 'French (Canada)'},
  ]
}

This would come from.

        $locales = collect($submissionContext->getSupportedSubmissionMetadataLocaleNames() + $submission->getPublicationLanguageNames())
            ->map(fn (string $name, string $locale) => ['key' => $locale, 'label' => $name])
            ->sortBy('key')
            ->values()
            ->toArray();

What application are you using?
OJS, OMP or OPS version 3.5

PRs:

@jardakotesovec jardakotesovec added the Enhancement:1:Minor A new feature or improvement that can be implemented in less than 3 days. label Aug 20, 2024
@jardakotesovec jardakotesovec added this to the 3.5.0 LTS milestone Aug 20, 2024
@bozana bozana self-assigned this Aug 21, 2024
bozana added a commit to bozana/ojs that referenced this issue Aug 21, 2024
@bozana
Copy link
Collaborator

bozana commented Aug 21, 2024

@jardakotesovec, could you test if this is what you need?

@jardakotesovec
Copy link
Contributor Author

@bozana Thank you! Looking good. I am not sure if this might generate individual sql queries per each submission on endpoints like /submissions?

As I run into couple more issues with the publication forms - I did create discussion to explore alternative solution than client side manipulation. Lets see how that goes and than will come back to this one.

#10331

@Vitaliy-1
Copy link
Collaborator

I am not sure if this might generate individual sql queries per each submission on endpoints like /submissions?

Yes, this will produce 2 additional sql queries per submission in the list.

@bozana
Copy link
Collaborator

bozana commented Aug 22, 2024

Doesn't /submissions use summary props, where this metadataLocales is not included?

@Vitaliy-1
Copy link
Collaborator

I was referring to the backend submission controller, parts where we mapping submissions to the submissions list.

@jardakotesovec
Copy link
Contributor Author

I will close this one for now in favour of #10336.

We might need to do this once we would shift form building towards JS, but let's not get ahead of ourselves too much.

@jardakotesovec
Copy link
Contributor Author

So it turned out this still could be useful for some forms.

@Vitaliy-1 Could you please advise how to make this either performant or other option that would work fine for me to expose it only on /submissions/{submissionId}, so it does not cause problems on the listings we have.

@bozana
Copy link
Collaborator

bozana commented Aug 30, 2024

As far as I can see this property is not included into (backend) submission listing, s. https://github.com/pkp/pkp-lib/blob/main/classes/submission/maps/Schema.php#L62.

@Vitaliy-1
Copy link
Collaborator

Thanks, not included then! My bad.

@jardakotesovec
Copy link
Contributor Author

@Vitaliy-1 @bozana Oki, in that case are we comfortable to merge it?

@jardakotesovec
Copy link
Contributor Author

@bozana I see that your implementation would return only array of the locale names.

Would be possible to include the labels as mentioned in original issue?

I could get the labels some other way if we could assume that whatever locales are selected for given journal stay same - but since it can gets changes - I can't think of reliable way of getting the labels for all locales that might occur for given submission.

@bozana
Copy link
Collaborator

bozana commented Sep 5, 2024

@jardakotesovec, let me see -- it should be possible for me to change it...

@bozana
Copy link
Collaborator

bozana commented Sep 5, 2024

Hi @jardakotesovec, what do you mean with

Would be possible to include the labels as mentioned in original issue?

That JSON part looks for me locally like this:
"metadataLocales":[{"key":"en","label":"English"},{"key":"fr_CA","label":"French (Canada)"}]
Isn't it correct?

@jardakotesovec
Copy link
Contributor Author

@bozana I see, ok. From the schema it seems that it would be only array of strings? Thats what confused me.

"metadataLocales": {
			"type": "array",
			"items": {
				"type": "string",
				"validation": [
					"regex:/^([A-Za-z]{2,4})(?<sc>[_-]([A-Za-z]{4,5}|[0-9]{4}))?([_-]([A-Za-z]{2}|[0-9]{3}))?(@[a-z]{2,30}(?&sc)?)?$/"
				]
			}
		},

@bozana
Copy link
Collaborator

bozana commented Sep 5, 2024

Ah, yes, you are right -- the schema part looks wrong... Let me think about it...

@bozana
Copy link
Collaborator

bozana commented Sep 5, 2024

How about having this:
"metadataLocales":{"en":"English","fr_CA":"French (Canada)"}
Or would you rather have it with key and value?

@jardakotesovec
Copy link
Contributor Author

@bozana Thank you, yes this shape is perfectly fine. Can we merge it? :-)

bozana added a commit to bozana/pkp-lib that referenced this issue Sep 6, 2024
@bozana
Copy link
Collaborator

bozana commented Sep 6, 2024

OK, I have slightly changed it, now we have "metadataLocales":{"en":"English","fr_CA":"French (Canada)"}.
@Vitaliy-1, could you maybe take a look at/review the PR -- if you would have any other suggestion?
(If not, I can merge)

bozana added a commit to bozana/ojs that referenced this issue Sep 6, 2024
bozana added a commit to bozana/pkp-lib that referenced this issue Sep 9, 2024
bozana added a commit to bozana/ojs that referenced this issue Sep 9, 2024
bozana added a commit to bozana/omp that referenced this issue Sep 9, 2024
bozana added a commit to bozana/ops that referenced this issue Sep 9, 2024
bozana added a commit that referenced this issue Sep 10, 2024
#10329 Include metadataLocales in /submissions/{submission…
@bozana bozana closed this as completed Sep 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement:1:Minor A new feature or improvement that can be implemented in less than 3 days.
Projects
None yet
Development

No branches or pull requests

3 participants