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

Submission and metadata languages separated from UI and form languages #9309

Merged
merged 1 commit into from
Mar 21, 2024

Conversation

jyhein
Copy link
Contributor

@jyhein jyhein commented Sep 18, 2023

  • In Website Settings > Setup > Languages and in Administration > Hosted Journals > Settings Wizard > Journal Settings > Languages, separate Website Languages and Submission Languages settings. In the website languages are UI and Forms, and in the submission languages are submission and metadata.
  • Submission metadata can be edited even if metadata isn't checked in that language in the settings.

Copy link
Collaborator

@bozana bozana left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jyhein, great work!
There are so many changes, so I am not sure I deeply understood/covered them all.
I would like to test it all, and then maybe take another look once you go over these comments.
Some questions might be also only for me to understand better, thus feel free to comment...

api/v1/vocabs/PKPVocabHandler.php Outdated Show resolved Hide resolved
controllers/api/file/PKPManageFileApiHandler.php Outdated Show resolved Hide resolved
classes/author/maps/Schema.php Outdated Show resolved Hide resolved
classes/context/Context.php Outdated Show resolved Hide resolved
classes/author/Repository.php Outdated Show resolved Hide resolved
pages/authorDashboard/PKPAuthorDashboardHandler.php Outdated Show resolved Hide resolved
pages/workflow/PKPWorkflowHandler.php Outdated Show resolved Hide resolved
templates/admin/contextSettings.tpl Outdated Show resolved Hide resolved
templates/controllers/grid/languages/addLanguageForm.tpl Outdated Show resolved Hide resolved
templates/management/website.tpl Outdated Show resolved Hide resolved
Copy link
Collaborator

@bozana bozana left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few more comments

@bozana
Copy link
Collaborator

bozana commented Oct 18, 2023

@jyhein, you would also need to implement a migration script for the upgrade, that will consider the current/old site and journal submission language settings and save them using the new settings names.
Also, could you please use git rebase instead of merge?

@jyhein
Copy link
Contributor Author

jyhein commented Oct 27, 2023

Hey @bozana,
I made the requested changes but lot of them are different now from the original because I also added languages from the multilingual props of the publication object instead of just its locale (=submission locale)

@bozana
Copy link
Collaborator

bozana commented Oct 27, 2023

@jyhein, I will then take a new look at it all... Thanks a lot!

@jyhein jyhein force-pushed the f5000_7431 branch 5 times, most recently from b27a82c to ed48bba Compare November 3, 2023 10:55
@bozana
Copy link
Collaborator

bozana commented Nov 8, 2023

Hi @jyhein,

I have tested a few cases and find out some cases we have not considered yet. I think that in the same way as you consider author languages in Publication::getLanguages() also other objects needs to be considered:
Vocabs: can be in supportedSubmissionMetadataLocales, and they are saved in a different DB table. Thus, if a metadata language is then later removed, looking for the existing publication and author locales only does not consider existing vocabs.
Galleys' locale: can be one of the supportedSubmissionLocales. If the submission locale is later removed, the galley can not be edited correctly. The galley locale however is only relevant for galleys, galleys forms, and does not need to be considered in Publication::getLanguages().
Submission File: metadata of a submission file can be in supportedSubmissionMetadataLocales. These locales are not considered anywhere, in case the metadata locale is removed later. Also here, I think, the submission files locales need to be considered only for submission files, submission file forms. Also, not only galley files needs to be considered, but all submission files, also from other workflow stages.

Currently citations i.e. the DB table citation_settings seems not to be important.
I see you are using supportedSubmissionLocales in classes/submission/Repository.php. Currently there are no multilingual submission settings, so I suppose it is correct 🤔 but maybe needs to be keep in mind if a multilingual setting comes... (just a note for us all).

I think the same locales concept needs to be considered also for/in the submission wizard.

I wanted to double check a few other things -- for example the new param publicationId for vocabs API -- but maybe I will leave it for later, once we solved these few comments.

Thanks a lot, great work! 🙏 ❤️

@bozana
Copy link
Collaborator

bozana commented Nov 8, 2023

One more case, I have just find out:
If the last publication does not have anything in German, but the previous does, the German will not be visible.

@jyhein jyhein force-pushed the f5000_7431 branch 2 times, most recently from 2ad90cd to 12f38c2 Compare November 15, 2023 08:28
$primaryLocale = $request->getContext()->getPrimaryLocale();
$allowedLocales = $request->getContext()->getData('supportedSubmissionLocales');
$submissionLocale = $submission->getData('locale');
$allowedLocales = $request->getContext()->getSupportedSubmissionMetadataLocales();
Copy link
Collaborator

@bozana bozana Nov 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jyhein, do you know where is this function used? -- I wanted to test, but I do not know...

EDIT: I am wondering why do we not need getPublicationLanguages() here...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left this comment for me, to investigate further... :-)

Copy link
Collaborator

@bozana bozana left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @jyhein,
I went through the code. Really good work!
I think here a I have only one question :-)
Thanks a lot!

classes/dev/ComposerScript.php Outdated Show resolved Hide resolved
@bozana
Copy link
Collaborator

bozana commented Feb 12, 2024

Hi @jyhein,
I wanted to write it here as well:
One more thing is left: now that we use Weblate locales, I think we would need to consider the old UI locales in the upgrade that do not match with Weblate locales and are used in submission and metadata. I think you already created a list and how those UI locales needs to be changed. This is not changing the whole UI locales, just those used in submissions and metadata, so that the data are correct after the upgrade.
Maybe to use a new upgrade script for that?

And: The fully new files created by you should now have 2024 instead of 2023 in copyright in header :-)

classes/i18n/Locale.php Outdated Show resolved Hide resolved
@jyhein jyhein force-pushed the f5000_7431 branch 4 times, most recently from 6162eee to fa0dbc6 Compare February 19, 2024 20:49
@bozana
Copy link
Collaborator

bozana commented Mar 8, 2024

Hi @jyhein, I went through the changes. All great! Just a few comments are left (FormValidatorCustom in OJS and OPS, and TABLE_SCHEMA in the upgrade script).
One more thing that you would need to do is to consider locales different from Weblate locales for submission and metadata locales in the upgrade script:
I think you said those locales differ:
"be":"be@cyrillic",
"bs_Latn":"bs",
"fr":"fr_FR",
"nb_NO":"nb",
"sr_Cyrl":"sr@cyrillic",
"sr_Latn":"sr@latin",
"uz":"uz@cyrillic",
"uz_Latn":"uz@latin",
"zh_Hans":"zh_CN"
So, I think, you would need to see if there is any of these locales in the following DB tables:
OJS, OMP, OPS:
author_settings
controlled_vocab_entry_settings
publication_galley_settings
publication_galleys
publication_settings
submission_file_settings
submission_settings
submissions

  • only OMP:
    publication_format_settings
    submission_chapter_settings
    And to replace them with the appropriate Weblate locale.

I have spoken with @ajnyga:
Because the issue #9707 is still to be implemented, you could eventually already consider those different locales listed above in the DataObject::getLocalePrecedence(). For example: if Locale::getLocale() is 'be@cyrillic' also consider 'be' i.e. if the data does not exist in 'be@cyrillic' see if it exists in 'be' next. I think this is the only place where this is relevant for now -- when we want to display submission metadata when such a UI locale is used -- right? Else, maybe we can also leave it as it is -- the submission data will then not be able to be displayed correctly for those UI locales till the issue is solved.
Or have I forgotten something else that needs to be considered?

Thanks a lot!

@jyhein jyhein force-pushed the f5000_7431 branch 2 times, most recently from d6c1acd to 2dc8523 Compare March 13, 2024 10:07
@bozana bozana merged commit c6b863d into pkp:main Mar 21, 2024
1 check passed
@jyhein jyhein deleted the f5000_7431 branch April 25, 2024 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants