feat(mobile app): add admin mobile settings for apple watch#38911
feat(mobile app): add admin mobile settings for apple watch#38911divyanshu-patil wants to merge 2 commits intoRocketChat:developfrom
Conversation
|
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 |
|
WalkthroughA new Apple Watch section is introduced in the mobile settings configuration, adding support for customizable quick action replies. The setting defines a public string property with default quick response options and associated internationalization labels. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/meteor/server/settings/mobile.tspackages/i18n/src/locales/en.i18n.json
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/server/settings/mobile.ts
🧠 Learnings (1)
📚 Learning: 2025-11-05T20:53:57.761Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37357
File: apps/meteor/ee/server/startup/federation.ts:39-74
Timestamp: 2025-11-05T20:53:57.761Z
Learning: In Rocket.Chat (apps/meteor/app/settings/server/CachedSettings.ts), the settings.watchMultiple() method immediately invokes its callback with current values if all requested settings exist in the store, then continues watching for subsequent changes. It does not wait for a setting to change before the first invocation.
Applied to files:
apps/meteor/server/settings/mobile.ts
🔇 Additional comments (1)
apps/meteor/server/settings/mobile.ts (1)
23-23: This review comment is incorrect. Setting keys in Rocket.Chat do not require registration incore-typingsfor type-safe access viasettings.get().The
get()method inCachedSettings.tsaccepts any string for the setting ID parameter (_id: ISetting['_id']), whereISetting['_id']is simply astringtype. There is no literal type union of known setting keys in core-typings, and existing settings likeForce_Screen_LockandAllow_Save_Media_to_Galleryare not registered as literal types. Type safety for the returned value comes from the generic type parameter (<T extends SettingValue>), not from key registration.Likely an incorrect or invalid review comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/meteor/server/settings/mobile.ts`:
- Around line 23-28: The setting ID and label are mismatched: you're adding a
setting via this.add('Apple_Watch_Quick_Actions', ...) while the i18n label is
Apple_Watch_Quick_Replies_Default; change the setting ID to
'Apple_Watch_Quick_Replies' in the this.add call to match the i18n key, and
update any other code/tests/migrations that reference Apple_Watch_Quick_Actions
to use Apple_Watch_Quick_Replies so terminology is consistent across the
codebase.
| await this.add('Apple_Watch_Quick_Actions', 'OK,Yes,No,On my way,Will follow up shortly', { | ||
| type: 'string', | ||
| i18nLabel: 'Apple_Watch_Quick_Replies_Default', | ||
| i18nDescription: 'Apple_Watch_Quick_Actions_Description', | ||
| public: true, | ||
| }); |
There was a problem hiding this comment.
Terminology mismatch between setting ID (Quick_Actions) and i18nLabel (Quick_Replies).
The setting is called Apple_Watch_Quick_Actions but the i18nLabel references Apple_Watch_Quick_Replies_Default. Apple Watch Quick Actions and Quick Replies are distinct features. Given the PR description and the linked React Native PR both refer to "quick replies", the setting ID should be aligned:
🔧 Proposed fix
- await this.add('Apple_Watch_Quick_Actions', 'OK,Yes,No,On my way,Will follow up shortly', {
+ await this.add('Apple_Watch_Quick_Replies', 'OK,Yes,No,On my way,Will follow up shortly', {
type: 'string',
- i18nLabel: 'Apple_Watch_Quick_Replies_Default',
- i18nDescription: 'Apple_Watch_Quick_Actions_Description',
+ i18nLabel: 'Apple_Watch_Quick_Replies',
+ i18nDescription: 'Apple_Watch_Quick_Replies_Description',
public: true,
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await this.add('Apple_Watch_Quick_Actions', 'OK,Yes,No,On my way,Will follow up shortly', { | |
| type: 'string', | |
| i18nLabel: 'Apple_Watch_Quick_Replies_Default', | |
| i18nDescription: 'Apple_Watch_Quick_Actions_Description', | |
| public: true, | |
| }); | |
| await this.add('Apple_Watch_Quick_Replies', 'OK,Yes,No,On my way,Will follow up shortly', { | |
| type: 'string', | |
| i18nLabel: 'Apple_Watch_Quick_Replies', | |
| i18nDescription: 'Apple_Watch_Quick_Replies_Description', | |
| public: true, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/meteor/server/settings/mobile.ts` around lines 23 - 28, The setting ID
and label are mismatched: you're adding a setting via
this.add('Apple_Watch_Quick_Actions', ...) while the i18n label is
Apple_Watch_Quick_Replies_Default; change the setting ID to
'Apple_Watch_Quick_Replies' in the this.add call to match the i18n key, and
update any other code/tests/migrations that reference Apple_Watch_Quick_Actions
to use Apple_Watch_Quick_Replies so terminology is consistent across the
codebase.
Proposed changes (including videos or screenshots)
added admin setting for watchos quick replies
Issue(s)
RocketChat/Rocket.Chat.ReactNative#6957
Steps to test or reproduce
Further comments
Summary by CodeRabbit