pkp/pkp-lib#12493 Reviewer suggestion locale update with multilingual data and validation update#12529
pkp/pkp-lib#12493 Reviewer suggestion locale update with multilingual data and validation update#12529touhidurabir wants to merge 3 commits intopkp:stable-3_5_0from
Conversation
…e form as thats whats required for users
…d validation update
| } | ||
|
|
||
| return $locales; | ||
| return Application::get()->getRequest()->getContext()->getSupportedFormLocales(); |
There was a problem hiding this comment.
I see now, that the primary locales are merged with allowedLocales in the MultilingualInput, thats why I don't have to do it here. Its bit of implementation detail which is not intuitive, but it means reducing code here I take it..
| foreach ($multilingualSettingsProps as $prop) { | ||
| if (!isset($row->{$prop})) { | ||
| $row->{$prop} = []; | ||
| } |
There was a problem hiding this comment.
@touhidurabir The idea here is just not to save empty inputs coming from the client and rather make sure we return empty object instead of null in the api for multilingual fields, right?
@asmecher @ewhanson I will tag you here in case you have some preference, because I don't really have strong opinion, but seems like a quite significant shift how we treat the multilingual inputs/outputs in api/database.
To provide context.
If I am for example saving contributor for submission, using multilingual fields - it sends all the fields including empty values. As result current behaviour was to save all these empty string to the database.
But now using the latest patterns - it turned out it saves only the fields which has actual value. Which I think is better behaviour as its not polluting database with empty values. And as result I was hitting the issue, where I was retrieving null value for multilingual fields, which did not get any value at all (see screenshots of request and response specifically for familyName).
I think its good idea to always return empty object from API - so there is one less variance for API consumers to worry about. So no objection for this solution - really just in case you have some suggestions or concerns here.
There was a problem hiding this comment.
The idea here is just not to save empty inputs coming from the client and rather make sure we return empty object instead of null in the api for multilingual fields, right?
Yes . And we can do a bit the opposite also that is, if that multilingual setting props has no value, than instead of null, we can make it not available in the model instance . But I went with this approach as we are defining those as setting props so feel like they should be always available in the default model instance .
There was a problem hiding this comment.
Just to add, I agree that we should not be populating the database with empty rows (and historically we have not done that). So I support this change.
3 participants
for #12493 .