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

Make Emails translatable via transifex #6025 #6038

Merged
merged 5 commits into from
Apr 18, 2023

Conversation

2403905
Copy link
Contributor

@2403905 2403905 commented Apr 12, 2023

Description

Make Emails translatable via transifex

Related Issue

How Has This Been Tested?

  • test environment: local
  • test case 1: tested by existing test suite

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

@update-docs
Copy link

update-docs bot commented Apr 12, 2023

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@2403905 2403905 requested a review from kobergj April 12, 2023 12:37
@2403905 2403905 force-pushed the issue-6025 branch 2 times, most recently from 21fbcaa to 7dd4462 Compare April 12, 2023 15:44
@mmattel
Copy link
Contributor

mmattel commented Apr 13, 2023

  • I am curious why we have in templates.go source strings in german language.
    The default language for TX is english. From there stuff is translated.
  • The other thing is, that I cant identify the "string" to be translated. It looks to me like a big junk of text not separated into blocks. If we stay at this, this is gonny be very hard for translators and error prone.

@dragonchaser
Copy link
Member

I am curious why we have in templates.go source strings in german language.
The default language for TX is english. From there stuff is translated.

This is ported from the old approach where we did not have i18n (yet), the templates then contained both strings, and were sent out this way, this is supposed to change in the future.

@mmattel
Copy link
Contributor

mmattel commented Apr 13, 2023

Just for the record, here is the current output of make l10n-read:
cat ./pkg/service/l10n/notifications.pot

# SOME DESCRIPTIVE TITLE.
# Copyright (C) YEAR THE PACKAGE'S COPYRIGHT HOLDER
# This file is distributed under the same license as the PACKAGE package.
# FIRST AUTHOR <EMAIL@ADDRESS>, YEAR.
#
#, fuzzy
msgid   ""
msgstr  "Project-Id-Version: \n"
        "Report-Msgid-Bugs-To: EMAIL\n"
        "POT-Creation-Date: 2023-04-13 09:31+0200\n"
        "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
        "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
        "Language-Team: LANGUAGE <[email protected]>\n"
        "Language: \n"
        "MIME-Version: 1.0\n"
        "Content-Type: text/plain; charset=CHARSET\n"
        "Content-Transfer-Encoding: 8bit\n"

#: pkg/service/l10n/templates.go:32
msgid   "Hello {ShareGrantee},\n"
        "\n"
        "Your share to {ShareFolder} has expired at {ExpiredAt}\n"
        "\n"
        "Even though this share has been revoked you still might have access through other shares and/or space memberships\n"
        "\n"
        "----------------------------------------------------------\n"
        "\n"
        "Hallo {ShareGrantee},\n"
        "\n"
        "Deine Freigabe zu {ShareFolder} ist am {ExpiredAt} abgelaufen.\n"
        "\n"
        "Obwohl diese Freigabe nicht mehr zur Verfügung steht, könntest du immer noch Zugriff über andere Freigaben und/oder Space Mitgliedschaften haben.\n"
        "\n"
        "\n"
        "---\n"
        "ownCloud - Store. Share. Work.\n"
        "https://owncloud.com\n"
msgstr  ""

#: pkg/service/l10n/templates.go:10
msgid   "Hello {ShareGrantee},\n"
        "\n"
        "{ShareSharer} has shared \"{ShareFolder}\" with you.\n"
        "\n"
        "Click here to view it: {ShareLink}\n"
        "\n"
        "----------------------------------------------------------\n"
        "\n"
        "Hallo {ShareGrantee},\n"
        "\n"
        "{ShareSharer} hat dich zu \"{ShareFolder}\" eingeladen.\n"
        "\n"
        "Klicke hier zum Anzeigen: {ShareLink}\n"
        "\n"
        "\n"
        "---\n"
        "ownCloud - Store. Share. Work.\n"
        "https://owncloud.com"
msgstr  ""

#: pkg/service/l10n/templates.go:99
msgid   "Hello {SpaceGrantee},\n"
        "\n"
        "Your membership of space {SpaceName} has expired at {ExpiredAt}\n"
        "\n"
        "Even though this membership has expired you still might have access through other shares and/or space memberships\n"
        "\n"
        "----------------------------------------------------------\n"
        "\n"
        "Hallo {SpaceGrantee},\n"
        "\n"
        "Deine Mitgliedschaft zu dem Space {SpaceName} ist am {ExpiredAt} abgelaufen.\n"
        "\n"
        "Obwohl diese Mitgliedschaft nicht mehr zur Verfügung steht, könntest du immer noch Zugriff über andere Freigaben und/oder Space Mitgliedschaften haben.\n"
        "\n"
        "\n"
        "---\n"
        "ownCloud - Store. Share. Work.\n"
        "https://owncloud.com"
msgstr  ""

#: pkg/service/l10n/templates.go:55
msgid   "Hello {SpaceGrantee},\n"
        "\n"
        "{SpaceSharer} has invited you to join \"{SpaceName}\".\n"
        "\n"
        "Click here to view it: {ShareLink}\n"
        "\n"
        "----------------------------------------------------------\n"
        "\n"
        "Hallo {SpaceGrantee},\n"
        "\n"
        "{SpaceSharer} hat dich in den Space \"{SpaceName}\" eingeladen.\n"
        "\n"
        "Klicke hier zum Anzeigen: {ShareLink}\n"
        "\n"
        "\n"
        "---\n"
        "ownCloud - Store. Share. Work.\n"
        "https://owncloud.com"
msgstr  ""

#: pkg/service/l10n/templates.go:77
msgid   "Hello {SpaceGrantee},\n"
        "\n"
        "{SpaceSharer} has removed you from \"{SpaceName}\".\n"
        "\n"
        "You might still have access through your other groups or direct membership. Click here to check it: {ShareLink}\n"
        "\n"
        "----------------------------------------------------------\n"
        "\n"
        "Hallo {SpaceGrantee},\n"
        "\n"
        "{SpaceSharer} hat dich aus dem Space \"{SpaceName}\" entfernt.\n"
        "\n"
        "Du könntest über deine anderen Gruppen oder deiner direkten Mitgliedschaft noch Zugriff haben. Klicke hier zum Überprüfen: {ShareLink}\n"
        "\n"
        "\n"
        "---\n"
        "ownCloud - Store. Share. Work.\n"
        "https://owncloud.com"
msgstr  ""

#: pkg/service/l10n/templates.go:98
msgid   "Membership of '{SpaceName}' expired at {ExpiredAt}"
msgstr  ""

#: pkg/service/l10n/templates.go:31
msgid   "Share to '{ShareFolder}' expired at {ExpiredAt}"
msgstr  ""

#: pkg/service/l10n/templates.go:9
msgid   "{ShareSharer} shared '{ShareFolder}' with you"
msgstr  ""

#: pkg/service/l10n/templates.go:54
msgid   "{SpaceSharer} invited you to join {SpaceName}"
msgstr  ""

#: pkg/service/l10n/templates.go:76
msgid   "{SpaceSharer} removed you from {SpaceName}"
msgstr  ""

@mmattel
Copy link
Contributor

mmattel commented Apr 13, 2023

Note that NOTIFICATIONS_EMAIL_TEMPLATE_PATH (see referenced issue: #6025) is still present and I doubt that it is needed (but I can be wrong). @dragonchaser

Imho we should deprecate it with no replacement.

@micbar
Copy link
Contributor

micbar commented Apr 13, 2023

Hmm, i am late to the party, but i am not ok by removing the template files. IMO this was a miscommunication.

@micbar
Copy link
Contributor

micbar commented Apr 13, 2023

@mmattel So no deprecation is planned, we still need the email templates. But they should be translatable.

services/notifications/pkg/service/l10n/templates.go Outdated Show resolved Hide resolved
services/notifications/pkg/service/l10n/templates.go Outdated Show resolved Hide resolved
services/notifications/pkg/service/l10n/templates.go Outdated Show resolved Hide resolved
services/notifications/pkg/service/l10n/templates.go Outdated Show resolved Hide resolved
services/notifications/pkg/service/l10n/templates.go Outdated Show resolved Hide resolved
@2403905
Copy link
Contributor Author

2403905 commented Apr 13, 2023

@2403905 2403905 closed this Apr 13, 2023
@2403905 2403905 reopened this Apr 13, 2023
@2403905
Copy link
Contributor Author

2403905 commented Apr 13, 2023

Hmm, i am late to the party, but i am not ok by removing the template files. IMO this was a miscommunication.

Do you mean, I have to keep using the templates? https://github.com/owncloud/ocis/tree/master/services/notifications/pkg/email/templates

@micbar
Copy link
Contributor

micbar commented Apr 13, 2023

Hmm, i am late to the party, but i am not ok by removing the template files. IMO this was a miscommunication.

Do you mean, I have to keep using the templates? https://github.com/owncloud/ocis/tree/master/services/notifications/pkg/email/templates

yes.

@mmattel
Copy link
Contributor

mmattel commented Apr 13, 2023

We must add some documentation in the readme.md of the notification service.

The transifex translation add in to the email templates.
The optional environment variable NOTIFICATIONS_TRANSLATION_PATH added to config.
@mmattel
Copy link
Contributor

mmattel commented Apr 17, 2023

Regarding the text, which is a copy of the userlog service and technically fine, BUT...

Compared to userlog where the source strings are embedded in the code, we have here sources that are files (as far I understood...). This makes things a bit different.

  • Where does one find the source files provided by ocis
    (pkg/email/templates/shares/shareCreated.email.body.tmpl)
  • How can one change the sources with own content and
  • What happens to the source files when ocis gets an upgrade
  • What are the naming and content rules for email files

We need to describe this in the readme.

@mmattel
Copy link
Contributor

mmattel commented Apr 17, 2023

I roughly remember that we (ownCloud) focus on the language code but not on the territory, though we do not prevent that for custom translations.
Means that the source file should be de and not de_DE.
(pkg/email/l10n/locale/de_DE/LC_MESSAGES/notifications.po)

And maybe I have missed the discussion about that, but shouldnt the source be en...?

@2403905
Copy link
Contributor Author

2403905 commented Apr 17, 2023

I roughly remember that we (ownCloud) focus on the language code but not on the territory, though we do not prevent that for custom translations. Means that the source file should be de and not de_DE. (pkg/email/l10n/locale/de_DE/LC_MESSAGES/notifications.po)

And maybe I have missed the discussion about that, but shouldnt the source be en...?

This file is not contains any payload. It was added because we use an embed folder in a code and the golang compiler failed if this folder is empty. I followed the same way that it was in a userlog service

services/notifications/pkg/config/config.go Outdated Show resolved Hide resolved
services/notifications/pkg/email/email.go Outdated Show resolved Hide resolved
services/notifications/pkg/service/service.go Outdated Show resolved Hide resolved
services/userlog/pkg/service/conversion.go Outdated Show resolved Hide resolved
@ownclouders
Copy link
Contributor

returned value swapped
@sonarcloud
Copy link

sonarcloud bot commented Apr 17, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

15.9% 15.9% Coverage
0.0% 0.0% Duplication

Copy link
Member

@dragonchaser dragonchaser left a comment

Choose a reason for hiding this comment

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

lgtm

@kobergj kobergj merged commit db4ff51 into owncloud:master Apr 18, 2023
ownclouders pushed a commit that referenced this pull request Apr 18, 2023
Make Emails translatable via transifex #6025
@micbar micbar mentioned this pull request May 3, 2023
89 tasks
@ScharfViktor ScharfViktor mentioned this pull request May 4, 2023
86 tasks
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.

Make Emails translatable via transifex
7 participants