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.

Sign up for GitHub

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

Jump to bottom

Multilingual Emails #8044

Open
wants to merge 24 commits into
base: main
Choose a base branch
from
Open

Multilingual Emails #8044

wants to merge 24 commits into from

Conversation

tylerjmchugh
Copy link
Contributor

Emails are currently sent in the UI language of the sender regardless of the preferences of the receiver.

This PR implements a framework for generating multilingual emails including:

  • A configuration field in settings>feedback to specify which languages to send emails in.
  • Another configuration field to specify the "translation follows" text at the start of the email. (Can include multiple languages).

Email messages for all specified languages will be concatenated with a separator.
If the settings fields are left empty, default back to the UI language.

Related feature request: #7966

image

Checklist

  • I have read the contribution guidelines
  • Pull request provided for main branch, backports managed with label
  • Good housekeeping of code, cleaning up comments, tests, and documentation
  • Clean commit history broken into understandable chucks, avoiding big commits with hundreds of files, cautious of reformatting and whitespace changes
  • Clean commit messages, longer verbose messages are encouraged
  • API Changes are identified in commit messages
  • Testing provided for features or enhancements using automatic tests
  • User documentation provided for new features or enhancements in manual
  • Build documentation provided for development instructions in README.md files
  • Library management using pom.xml dependency management. Update build documentation with intended library use and library tutorials or documentation

@tylerjmchugh
Add settings fields for localized emails
146f184
@tylerjmchugh
Add spring bean to initialize feedback email locales from settings field
c7c4991
@tylerjmchugh
Modify settings manager to update feedback locales when the settings …
80a7d26 
…fields are saved
@tylerjmchugh
Create classes for localized emails, their components, and the compon…
acf4147 
…ents' parameters
@tylerjmchugh
Implement lang in getIndexField
88f2068
@tylerjmchugh
Localize workflow status emails
3fab7c3
@tylerjmchugh
Localize metadata publication emails
da0f545
@tylerjmchugh
Localize user feedback emails
cc83101
@tylerjmchugh
Localize RegisterApi emails
3d6b6ce
@tylerjmchugh
Localize PasswordApi emails
9988d7d
@tylerjmchugh
Localize WatchListNotifier emails
c94dd7b
@tylerjmchugh
Localize MailApi emails
461e4c5
@tylerjmchugh
Update migration script to only insert settings fields if not present
a8f4d11
@tylerjmchugh
Add static enum imports for readability
bd1dc23
@tylerjmchugh
Log a warning when a locale is invalid or missing
3f5a3af
@tylerjmchugh
Update log modules and messages
2a56de3
@ianwallen ianwallen added this to the 4.4.5 milestone May 16, 2024
ianwallen
ianwallen approved these changes May 16, 2024
View reviewed changes
Copy link
Contributor

@ianwallen ianwallen left a comment

Choose a reason for hiding this comment

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

Tested locally and works as expected.

I'm not sure if some of the new files are in the preferred location - I will let others comment on if some of the new files should be placed in different location.

@tylerjmchugh tylerjmchugh mentioned this pull request May 16, 2024
Merged
10 tasks
@tylerjmchugh tylerjmchugh marked this pull request as ready for review May 16, 2024 18:45
@fxprunayre
Copy link
Member

I'm not sure if some of the new files are in the preferred location - I will let others comment on if some of the new files should be placed in different location.

LGTM.

@josegar74
Copy link
Member

josegar74 commented May 17, 2024 •
edited

Tested with the default configuration (no languages configured), I get a NullPointerException when submitting a metadata to review:

https://github.com/geonetwork/core-geonetwork/pull/8044/files#diff-8ffe27a26ea5dc5d13d71bdc2dfbd7afd4aaea80037f18bf304d9754aaaf7818R182

It fails internally in

https://github.com/geonetwork/core-geonetwork/pull/8044/files#diff-1c4dbe5c43914f0b170604f63ec220e17bdf72b3dd6b6a4951c8c13583c50336R309

It happens also when configuring the languages in the settings.

@ianwallen ianwallen mentioned this pull request May 17, 2024
Merged
@tylerjmchugh
Merge remote-tracking branch 'refs/remotes/fork/main' into main.multi…
ae96904 
…lingual-emails

# Conflicts:
#	core/src/main/java/org/fao/geonet/kernel/search/EsSearchManager.java
#	core/src/main/java/org/fao/geonet/util/XslUtil.java
#	index/src/main/java/org/fao/geonet/index/es/EsRestClient.java
@tylerjmchugh
Trim language codes defined in settings to handle spaces after commas
6c4f653
@tylerjmchugh
Copy link
Contributor Author

Tested with the default configuration (no languages configured), I get a NullPointerException when submitting a metadata to review:

https://github.com/geonetwork/core-geonetwork/pull/8044/files#diff-8ffe27a26ea5dc5d13d71bdc2dfbd7afd4aaea80037f18bf304d9754aaaf7818R182

It fails internally in

https://github.com/geonetwork/core-geonetwork/pull/8044/files#diff-1c4dbe5c43914f0b170604f63ec220e17bdf72b3dd6b6a4951c8c13583c50336R309

Reproduced the first issue and it seems to be fixed after merging the changes from #8042.

It happens also when configuring the languages in the settings.

I am however unable to reproduce the issue while configuring the languages.

@josegar74 josegar74 modified the milestones: 4.4.5, 4.4.6 Jun 3, 2024
josegar74
josegar74 approved these changes Jun 4, 2024
View reviewed changes
Copy link
Member

@josegar74 josegar74 left a comment

Choose a reason for hiding this comment

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

Changes look good, the error is solved.

Please wait to merge it until 4.4.5 is released.

core/src/main/java/org/fao/geonet/kernel/metadata/DefaultStatusActions.java
} catch (MissingResourceException e) {
failedToFindASpecificTextTemplate = true;
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
}
}
if ((failedToFindASpecificSubjectTemplate) && (failedToFindASpecificTextTemplate)) break;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added loop breaking logic in latest push

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@josegar74 josegar74 josegar74 approved these changes

@ianwallen ianwallen ianwallen approved these changes

@fxprunayre fxprunayre Awaiting requested review from fxprunayre

Assignees
No one assigned
Labels
backport 4.2.x enhancement
Projects
None yet
Milestone
4.4.6
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@tylerjmchugh @fxprunayre @josegar74 @ianwallen