chore(subscription): localize slack notification messages - #2778
Merged
Conversation
The feature stale, experiment running and MAU count notifications were sent in Japanese regardless of the recipient's language setting. Only the domain event notification honored it. Move the hardcoded Japanese strings into the locale bundle and pass the existing localizer (built from Recipient.Language in notify()) down to the three remaining attachment builders. This also covers the days-left line of the experiment list, which was inlined in slack.go rather than in message.go. message.go is removed since localizedMessage() has no callers left. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ants The Template suffix marks message IDs that must be resolved with MustLocalizeWithTemplate. Every other templated constant in this file follows it, so align the three subscription notification keys that take a template field. NotificationExperimentRunning keeps no suffix since it has no placeholder and is resolved with MustLocalize. Only the Go identifiers change; the message IDs and the localized data are untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
kimurakazuhiro-c
marked this pull request as ready for review
August 20, 2026 04:25
kimurakazuhiro-c
requested review from
cre8ivejp,
hvn2k1 and
t-kikuc
as code owners
August 20, 2026 04:25
There was a problem hiding this comment.
Pull request overview
Localizes Slack subscription notifications using the shared locale system.
Changes:
- Adds English and Japanese notification translations.
- Passes recipient localizers into attachment builders.
- Adds bilingual attachment tests and removes legacy localization code.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
pkg/subscription/sender/notifier/slack.go |
Localizes notification attachments. |
pkg/subscription/sender/notifier/slack_test.go |
Tests English and Japanese rendering. |
pkg/subscription/sender/notifier/message.go |
Removes legacy Japanese-only messages. |
pkg/locale/localizer.go |
Defines notification locale keys. |
pkg/locale/localizedata/ja.yaml |
Adds Japanese translations. |
pkg/locale/localizedata/en.yaml |
Adds English translations. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…essage Address review comments on PR #2778: - "{{ .Field_1 }} days left" rendered "`1` days left" for an experiment with exactly one day remaining. Reword it as "Days left: {{ .Field_1 }}" so the English notification is grammatical for singular, plural, zero and negative values, and update the test expectation accordingly. The Japanese message is already count-neutral and stays unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
|
Closes #699 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Replaces the hardcoded Japanese-only Slack notification messages with the shared
locale.Localizer, so feature-stale, running-experiment, and MAU-count notifications are rendered in the recipient's language.Background / Why this PR is needed
pkg/subscription/sender/notifier/message.goheld the notification text as Japanese string literals and had alocalizedMessage()helper that ignored thelocargument entirely — every call site passedlocale.Jawith a// handle loc if multi-lang is necessarynote. Meanwhile the rest of the codebase already resolves messages throughlocale.Localizerbacked bypkg/locale/localizedata/*.yaml, andcreateAttachmentalready receives alocalizer. This PR removes the parallel mechanism and moves these messages onto the standard path so English recipients get English text.Points
message.gois deleted outright, along withmsgType,errUnknownMsgType, and the*errdetails.LocalizedMessagevalues. Nothing else referenced them.create*Attachmentmethods keep theirerrorreturn even though the localizer path itself no longer fails —createFeatureStaleAttachmentandcreateExperimentRunningAttachmentstill build URLs that can error, and keeping the signature uniform avoids churn increateAttachment.MustLocalize/MustLocalizeWithTemplateare used rather than the error-returning variants, matching how the rest of the codebase consumes the localizer.{{ .Field_1 }} days left/残り {{ .Field_1 }} 日) rather than interpolating a translated "days" word, because the number's position differs between the two languages. The backtick-wrapped count is passed in as the template field so the Slack code formatting is preserved.enandja.