Repository navigation
fix: add English labels for app permissions that had none - #43118
mrityunjay-pandey wants to merge 3 commits into
Conversation
The app permission review showed raw Apps_Permissions_ keys for sixteen permissions defined in AppPermissions.
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
🦋 Changeset detectedLatest commit: f0ed1e5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Add labels and verify translation coverage packages/i18n/src/locales/en.i18n.json, apps/meteor/client/views/marketplace/helpers/appPermissionTranslations.spec.ts, .changeset/app-permission-labels.md |
The English locale adds labels for app permissions. The test checks for a corresponding English key for every AppPermissions value. The changeset records patch releases for @rocket.chat/i18n and @rocket.chat/meteor. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix · Severity of issue fixed: Low
Possibly related PRs
- RocketChat/Rocket.Chat#43065: Fixes translation-key normalization for nested permission names; this PR adds missing labels and checks all
AppPermissionsvalues. - RocketChat/Rocket.Chat#42890: Includes the same test for English translation keys across
AppPermissionsvalues.
Suggested labels: type: bug
Suggested reviewers: dougfabris, ggazzo, gabriellsh
Merge Risk: 
Minimal · up to f0ed1
-
The added labels and coverage test introduce no established merge-blocking risk. The separate rendering mismatch remains outside this change.
Pre-merge checks: 
5
-
Full details: Docstring Coverage
-
Explanation
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files. (2 skipped: 2 unsupported.)
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Warning
Errors were encountered while retrieving linked issues.
Errors: 1
-
- JIRA integration encountered authorization issues. Please disconnect and reconnect the integration in the CodeRabbit UI.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
Proposed changes
AppPermissionsListlabels each permission an app asks for withApps_Permissions_<name>(dots turned into underscores). Sixteen permissions defined inpackages/apps-engine/src/definition/metadata/AppPermissions.tshave no such key inen.i18n.json, so the permission review shown when installing or updating an app lists them as raw keys, for exampleApps_Permissions_threads_read.This adds English labels for them, worded after what each one allows in
packages/apps/src/server(bridges and managers):contact.read/contact.writeemail.sendlivechat-message.multiplemoderation.read/moderation.writeoauth-app.read/oauth-app.writeoutbound-communication.providerole.read/role.writethreads.readui.registerButtonsvideo-conference-providervideo-conference.read/video-conference.writeOnly
en.i18n.jsonchanges; other locales fall back to English until translated (docs/i18n.md).Issue(s)
No existing issue. Found by comparing every name in
AppPermissionswith theApps_Permissions_keys.Steps to test or reproduce
Install or update an app whose manifest requests, for example,
threads.readorui.registerButtons: the permission list shows the raw key before this change and the label after it.appPermissionTranslations.spec.tschecks that every permission inAppPermissionshas an English label; the sixteen above fail without this change.yarn workspace @rocket.chat/i18n checkpasses and the marketplace suites pass (22 suites).Further comments
The spec builds the key with
replaceAll('.', '_');AppPermissionsListitself still usesreplace, which only matters forroom.system.view-alland is fixed separately in #43065.Summary by CodeRabbit