chore: API Endpoint support for setting action type - #39983
Merged
Conversation
Contributor
|
Looks like this PR is ready to merge! 🎉 |
|
Contributor
WalkthroughThe PR introduces support for action-type settings that can be executed via either Rocket.Chat server methods or HTTP endpoints. The backend now excludes endpoint-based actions from direct method execution, while new client components provide separate handling paths for method vs. endpoint-based actions through conditional routing. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #39983 +/- ##
===========================================
- Coverage 70.57% 70.52% -0.06%
===========================================
Files 3263 3270 +7
Lines 116660 116773 +113
Branches 21067 21095 +28
===========================================
+ Hits 82336 82353 +17
- Misses 32271 32360 +89
- Partials 2053 2060 +7
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
aleksandernsilva
force-pushed
the
chore/setting-action-endpoint
branch
from
March 31, 2026 14:47
66ae30d to
ec206f6
Compare
|
✅ Layne — scan passed No security issues found on latest push. |
KevLehman
commented
Mar 31, 2026
KevLehman
commented
Mar 31, 2026
KevLehman
commented
Mar 31, 2026
KevLehman
marked this pull request as ready for review
March 31, 2026 21:11
coderabbitai
Bot
added
the
type: feature
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/meteor/client/views/admin/settings/Setting/inputs/ActionInputBase.tsx (1)
24-26: Error handling may not properly display all error types.The
errorcaught in the catch block is typed asunknown, but it's passed directly todispatchToastMessage. If the error is not a string or anErrorobject with amessageproperty, the toast may not display a meaningful message to the user.🔧 Consider normalizing the error message
} catch (error) { - dispatchToastMessage({ type: 'error', message: error }); + dispatchToastMessage({ + type: 'error', + message: error instanceof Error ? error.message : String(error), + }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/client/views/admin/settings/Setting/inputs/ActionInputBase.tsx` around lines 24 - 26, The catch block in ActionInputBase.tsx currently passes the caught unknown error directly to dispatchToastMessage, which may not render meaningful text for non-string errors; update the catch to normalize the error before calling dispatchToastMessage (in the catch in the function containing dispatchToastMessage): if error is an instance of Error use error.message, otherwise coerce to a readable string (e.g., String(error) or JSON.stringify when appropriate), and pass that normalized string as the message to dispatchToastMessage so the toast always shows a clear message.packages/core-typings/src/ISetting.ts (1)
11-11: Consider unifyingSettingValueActionandSettingActionEndpointtypes.Two similar types exist:
SettingValueAction(line 11):{ method: string; path: string }- loose typingSettingActionEndpoint(line 127):{ method: 'GET' | 'POST' | 'PUT' | 'DELETE'; path: string }- strict HTTP methods
SettingValueActionis in theSettingValueunion (used broadly), whileSettingActionEndpointis used specifically inISettingAction.value. This could lead to inconsistent validation—an action setting could technically have a method value that passes theSettingValuecheck but isn't a valid HTTP method.🔧 Consider using the stricter type consistently
-export type SettingValueAction = { method: string; path: string }; +export type SettingValueAction = { method: 'GET' | 'POST' | 'PUT' | 'DELETE'; path: string };Or remove
SettingValueActionfrom theSettingValueunion if action settings should only useISettingAction.valuewith its own type constraints.Also applies to: 127-127
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core-typings/src/ISetting.ts` at line 11, The two action types are inconsistent: SettingValueAction is loose while SettingActionEndpoint is strict; update the types so action values use one consistent definition—either replace SettingValueAction in the SettingValue union with the stricter SettingActionEndpoint type, or unify both by redefining SettingValueAction to match SettingActionEndpoint (method: 'GET'|'POST'|'PUT'|'DELETE'; path: string); ensure ISettingAction.value references the unified type so action settings cannot carry invalid HTTP methods (adjust any references to SettingValue, SettingValueAction, SettingActionEndpoint, and ISettingAction.value accordingly).
🤖 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/app/api/server/v1/settings.ts`:
- Around line 351-359: The current branch that checks isSettingAction(setting)
&& isSettingsUpdatePropsActions(bodyParams) && bodyParams.execute &&
!isActionSettingWithEndpoint(setting.value) calls
Meteor.callAsync(setting.value) and returns API.v1.success(), but
endpoint-shaped actions (isActionSettingWithEndpoint(setting.value) === true)
fall through and return API.v1.failure(); update the logic so that when
bodyParams.execute is true and the setting is an endpoint-shaped action you
explicitly return API.v1.success() (either by adding an else branch that returns
API.v1.success() when isActionSettingWithEndpoint(setting.value) is true, or by
changing the condition to handle endpoint-shaped actions separately), leaving
the Meteor.callAsync call only for non-endpoint actions; ensure references:
isSettingAction, isSettingsUpdatePropsActions, bodyParams.execute,
isActionSettingWithEndpoint, Meteor.callAsync, and API.v1.success are used to
locate and implement the fix.
In
`@apps/meteor/client/views/admin/settings/Setting/inputs/EndpointActionInput.tsx`:
- Around line 14-17: EndpointActionInput currently bypasses typing by calling
useEndpoint(endpoint.method, endpoint.path) and invoking it with an unsafe cast
({} as never), which both sends an incorrect empty body and hides the mismatch
between useEndpoint's return type and ActionInputBase's expected Promise<{
message: TranslationKey; params?: string[] }>. Fix by changing
EndpointActionInput to accept or build the correct request body based on
endpoint parameter metadata instead of using {} as never, wrap the raw
useEndpoint call in a typed adapter that maps the actual
Serialized<OperationResult<...>> response into the { message, params? } shape
expected by ActionInputBase (e.g., create a callEndpointWrapper function used in
onAction that transforms types and payload), and/or add a dedicated
action-endpoint response type in `@rocket.chat/rest-typings` so useEndpoint’s
return aligns with ActionInputBase.
---
Nitpick comments:
In `@apps/meteor/client/views/admin/settings/Setting/inputs/ActionInputBase.tsx`:
- Around line 24-26: The catch block in ActionInputBase.tsx currently passes the
caught unknown error directly to dispatchToastMessage, which may not render
meaningful text for non-string errors; update the catch to normalize the error
before calling dispatchToastMessage (in the catch in the function containing
dispatchToastMessage): if error is an instance of Error use error.message,
otherwise coerce to a readable string (e.g., String(error) or JSON.stringify
when appropriate), and pass that normalized string as the message to
dispatchToastMessage so the toast always shows a clear message.
In `@packages/core-typings/src/ISetting.ts`:
- Line 11: The two action types are inconsistent: SettingValueAction is loose
while SettingActionEndpoint is strict; update the types so action values use one
consistent definition—either replace SettingValueAction in the SettingValue
union with the stricter SettingActionEndpoint type, or unify both by redefining
SettingValueAction to match SettingActionEndpoint (method:
'GET'|'POST'|'PUT'|'DELETE'; path: string); ensure ISettingAction.value
references the unified type so action settings cannot carry invalid HTTP methods
(adjust any references to SettingValue, SettingValueAction,
SettingActionEndpoint, and ISettingAction.value accordingly).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 49fb75b6-f0e5-4a9e-9c62-a8d8bbdf2412
📒 Files selected for processing (6)
apps/meteor/app/api/server/v1/settings.tsapps/meteor/client/views/admin/settings/Setting/inputs/ActionInputBase.tsxapps/meteor/client/views/admin/settings/Setting/inputs/ActionSettingInput.tsxapps/meteor/client/views/admin/settings/Setting/inputs/EndpointActionInput.tsxapps/meteor/client/views/admin/settings/Setting/inputs/MethodActionInput.tsxpackages/core-typings/src/ISetting.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: 🔎 Code Check / TypeScript
- GitHub Check: 🔎 Code Check / Code Lint
- GitHub Check: 🔨 Test Unit / Unit Tests
- GitHub Check: 🔨 Test Storybook / Test Storybook
- GitHub Check: 📦 Meteor Build (coverage)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: CodeQL-Build
🧰 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/client/views/admin/settings/Setting/inputs/EndpointActionInput.tsxapps/meteor/app/api/server/v1/settings.tsapps/meteor/client/views/admin/settings/Setting/inputs/MethodActionInput.tsxapps/meteor/client/views/admin/settings/Setting/inputs/ActionSettingInput.tsxapps/meteor/client/views/admin/settings/Setting/inputs/ActionInputBase.tsxpackages/core-typings/src/ISetting.ts
🧠 Learnings (16)
📓 Common learnings
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 35995
File: apps/meteor/app/api/server/v1/rooms.ts:1107-1112
Timestamp: 2026-02-23T17:53:18.785Z
Learning: In Rocket.Chat PR reviews, maintain strict scope boundaries—when a PR is focused on a specific endpoint (e.g., rooms.favorite), avoid reviewing or suggesting changes to other endpoints that were incidentally refactored (e.g., rooms.invite) unless explicitly requested by maintainers.
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38974
File: apps/meteor/app/api/server/v1/im.ts:220-221
Timestamp: 2026-02-24T19:09:09.561Z
Learning: In RocketChat/Rocket.Chat OpenAPI migration PRs for apps/meteor/app/api/server/v1 endpoints, maintainers prefer to avoid any logic changes; style-only cleanups (like removing inline comments) may be deferred to follow-ups to keep scope tight.
Learnt from: smirk-dev
Repo: RocketChat/Rocket.Chat PR: 39625
File: apps/meteor/app/api/server/v1/push.ts:85-97
Timestamp: 2026-03-14T14:58:58.834Z
Learning: In RocketChat/Rocket.Chat, the `push.token` POST/DELETE endpoints in `apps/meteor/app/api/server/v1/push.ts` were already migrated to the chained router API pattern on `develop` prior to PR `#39625`. `cleanTokenResult` (which strips `authToken` and returns `PushTokenResult`) and `isPushTokenPOSTProps`/`isPushTokenDELETEProps` validators already exist on `develop`. PR `#39625` only migrates `push.get` and `push.info` to the chained pattern. Do not flag `cleanTokenResult` or `PushTokenResult` as newly introduced behavior-breaking changes when reviewing this PR.
📚 Learning: 2026-03-27T14:52:56.865Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39892
File: apps/meteor/client/views/room/contextualBar/Threads/Thread.tsx:150-155
Timestamp: 2026-03-27T14:52:56.865Z
Learning: In Rocket.Chat, there are two different `ModalBackdrop` components with different prop APIs. During review, confirm the import source: (1) `rocket.chat/fuselage` `ModalBackdrop` uses `ModalBackdropProps` based on `BoxProps` (so it supports `onClick` and other Box/DOM props) and does not have an `onDismiss` prop; (2) `rocket.chat/ui-client` `ModalBackdrop` uses a narrower props interface like `{ children?: ReactNode; onDismiss?: () => void }` and handles Escape keypress and outside mouse-up, and it does not forward arbitrary DOM props such as `onClick`. Flag mismatched props (e.g., `onDismiss` passed to the fuselage component or `onClick` passed to the ui-client component) and ensure the usage matches the correct component being imported.
Applied to files:
apps/meteor/client/views/admin/settings/Setting/inputs/EndpointActionInput.tsxapps/meteor/client/views/admin/settings/Setting/inputs/MethodActionInput.tsxapps/meteor/client/views/admin/settings/Setting/inputs/ActionSettingInput.tsxapps/meteor/client/views/admin/settings/Setting/inputs/ActionInputBase.tsx
📚 Learning: 2026-03-16T21:50:42.118Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39676
File: .changeset/migrate-users-register-openapi.md:3-3
Timestamp: 2026-03-16T21:50:42.118Z
Learning: In RocketChat/Rocket.Chat OpenAPI migration PRs, removing endpoint types and validators from `rocket.chat/rest-typings` (e.g., `UserRegisterParamsPOST`, `/v1/users.register` entry) is the *required* migration pattern per RocketChat/Rocket.Chat-Open-API#150 Rule 7 ("No More rest-typings or Manual Typings"). The endpoint type is re-exposed via a module augmentation `.d.ts` file in the consuming package (e.g., `packages/web-ui-registration/src/users-register.d.ts`). This is NOT a breaking change — the correct changeset bump for `rocket.chat/rest-typings` in this scenario is `minor`, not `major`. Do not flag this as a breaking change during OpenAPI migration reviews.
Applied to files:
apps/meteor/app/api/server/v1/settings.tspackages/core-typings/src/ISetting.ts
📚 Learning: 2026-03-15T14:31:28.969Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39647
File: apps/meteor/app/api/server/v1/users.ts:710-757
Timestamp: 2026-03-15T14:31:28.969Z
Learning: In RocketChat/Rocket.Chat, the `UserCreateParamsPOST` type in `apps/meteor/app/api/server/v1/users.ts` (migrated from `packages/rest-typings/src/v1/users/UserCreateParamsPOST.ts`) intentionally has `fields: string` (non-optional) and `settings?: IUserSettings` without a corresponding AJV schema entry. This is a pre-existing divergence carried over verbatim from the original rest-typings source (PR `#39647`). Do not flag this type/schema misalignment during the OpenAPI migration review — it is tracked as a separate follow-up fix.
Applied to files:
apps/meteor/app/api/server/v1/settings.ts
📚 Learning: 2026-02-24T19:09:01.522Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38974
File: apps/meteor/app/api/server/v1/im.ts:220-221
Timestamp: 2026-02-24T19:09:01.522Z
Learning: In Rocket.Chat OpenAPI migration PRs for endpoints under apps/meteor/app/api/server/v1, avoid introducing logic changes. Only perform scope-tight changes that preserve behavior; style-only cleanups (e.g., removing inline comments) may be deferred to follow-ups to keep the migration PR focused.
Applied to files:
apps/meteor/app/api/server/v1/settings.ts
📚 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/app/api/server/v1/settings.ts
📚 Learning: 2026-03-14T14:58:58.834Z
Learnt from: smirk-dev
Repo: RocketChat/Rocket.Chat PR: 39625
File: apps/meteor/app/api/server/v1/push.ts:85-97
Timestamp: 2026-03-14T14:58:58.834Z
Learning: In RocketChat/Rocket.Chat, the `push.token` POST/DELETE endpoints in `apps/meteor/app/api/server/v1/push.ts` were already migrated to the chained router API pattern on `develop` prior to PR `#39625`. `cleanTokenResult` (which strips `authToken` and returns `PushTokenResult`) and `isPushTokenPOSTProps`/`isPushTokenDELETEProps` validators already exist on `develop`. PR `#39625` only migrates `push.get` and `push.info` to the chained pattern. Do not flag `cleanTokenResult` or `PushTokenResult` as newly introduced behavior-breaking changes when reviewing this PR.
Applied to files:
apps/meteor/app/api/server/v1/settings.ts
📚 Learning: 2025-09-19T15:15:04.642Z
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 36991
File: apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts:219-221
Timestamp: 2025-09-19T15:15:04.642Z
Learning: The Federation_Matrix_homeserver_domain setting in apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts is part of the old federation system and is being deprecated/removed, so configuration issues with this setting should not be flagged for improvement.
Applied to files:
apps/meteor/app/api/server/v1/settings.ts
📚 Learning: 2026-02-25T20:10:16.987Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38913
File: packages/ddp-client/src/legacy/types/SDKLegacy.ts:34-34
Timestamp: 2026-02-25T20:10:16.987Z
Learning: In the RocketChat/Rocket.Chat monorepo, packages/ddp-client and apps/meteor do not use TypeScript project references. Module augmentations in apps/meteor (e.g., declare module 'rocket.chat/rest-typings') are not visible when compiling packages/ddp-client in isolation, which is why legacy SDK methods that depend on OperationResult types for OpenAPI-migrated endpoints must remain commented out.
Applied to files:
apps/meteor/app/api/server/v1/settings.ts
📚 Learning: 2026-03-20T13:51:23.302Z
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 39553
File: apps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.ts:179-181
Timestamp: 2026-03-20T13:51:23.302Z
Learning: In `apps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.ts`, the truthiness guards `...(integration.avatar && { avatar })`, `...(integration.emoji && { emoji })`, `...(integration.alias && { alias })`, and `...(integration.script && { script })` in the `$set` payload of `updateIncomingIntegration` are intentional. Empty-string values for these fields should NOT overwrite the stored value — only truthy values are persisted. Do not flag these as bugs preventing explicit clears.
Applied to files:
apps/meteor/app/api/server/v1/settings.ts
📚 Learning: 2026-03-12T10:26:26.697Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39340
File: apps/meteor/app/api/server/v1/im.ts:1349-1398
Timestamp: 2026-03-12T10:26:26.697Z
Learning: In `apps/meteor/app/api/server/v1/im.ts` (PR `#39340`), the `DmEndpoints` type intentionally includes temporary stub entries for `/v1/im.kick`, `/v1/dm.kick`, `/v1/im.leave`, and `/v1/dm.leave` (using `DmKickProps` and `DmLeaveProps`) even though no route handlers exist for them yet. These stubs were added to preserve type compatibility after removing the original `DmLeaveProps` and related files. They are planned for cleanup in a follow-up PR. Do not flag these as missing implementations when reviewing this file until the follow-up is merged.
Applied to files:
apps/meteor/app/api/server/v1/settings.ts
📚 Learning: 2026-03-20T13:52:29.575Z
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 39553
File: apps/meteor/app/api/server/v1/stats.ts:98-117
Timestamp: 2026-03-20T13:52:29.575Z
Learning: In `apps/meteor/app/api/server/v1/stats.ts`, the `statistics.telemetry` POST endpoint intentionally has no `body` AJV schema in its route options. The proper request body shape (a `params` array of telemetry event objects) has not been formally defined yet, so body validation is deferred to a follow-up. Do not flag the missing body schema for this endpoint during OpenAPI migration reviews.
Applied to files:
apps/meteor/app/api/server/v1/settings.ts
📚 Learning: 2026-02-23T17:53:06.802Z
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 35995
File: apps/meteor/app/api/server/v1/rooms.ts:1107-1112
Timestamp: 2026-02-23T17:53:06.802Z
Learning: During PR reviews that touch endpoint files under apps/meteor/app/api/server/v1, enforce strict scope: if a PR targets a specific endpoint (e.g., rooms.favorite), do not propose changes to unrelated endpoints (e.g., rooms.invite) unless maintainers explicitly request them. Focus feedback on the touched endpoint's behavior, API surface, and related tests; avoid broad cross-endpoint changes in the same PR unless requested.
Applied to files:
apps/meteor/app/api/server/v1/settings.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.
Applied to files:
apps/meteor/app/api/server/v1/settings.tspackages/core-typings/src/ISetting.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Applied to files:
apps/meteor/app/api/server/v1/settings.tspackages/core-typings/src/ISetting.ts
📚 Learning: 2026-03-04T14:16:49.202Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39304
File: packages/ui-contexts/src/ActionManagerContext.ts:26-26
Timestamp: 2026-03-04T14:16:49.202Z
Learning: In `packages/ui-contexts/src/ActionManagerContext.ts` (TypeScript, RocketChat/Rocket.Chat), the `disposeView` method in `IActionManager` uses an intentionally explicit union `UiKit.ModalView['id'] | UiKit.BannerView['viewId'] | UiKit.ContextualBarView['id']` to document which view types are accepted, even though all constituents resolve to the same primitive. The inline `// eslint-disable-next-line typescript-eslint/no-duplicate-type-constituents` comment is intentional and should not be flagged or removed.
Applied to files:
apps/meteor/client/views/admin/settings/Setting/inputs/ActionInputBase.tsx
🔇 Additional comments (5)
apps/meteor/client/views/admin/settings/Setting/inputs/ActionInputBase.tsx (1)
29-40: LGTM!The component structure is clean. The button disabling logic and conditional hint rendering are correctly implemented.
apps/meteor/app/api/server/v1/settings.ts (1)
9-9: LGTM!Import correctly includes the new type guard alongside existing guards.
apps/meteor/client/views/admin/settings/Setting/inputs/ActionSettingInput.tsx (1)
17-23: LGTM!Clean conditional rendering using the type guard. The type narrowing ensures
value.methodandvalue.pathare available in the endpoint branch, whilevalueis properly narrowed tokeyof ServerMethodsin the method branch.packages/core-typings/src/ISetting.ts (1)
167-168: Type guard implementation is correct.The guard properly checks for a non-null object with both
methodandpathproperties, correctly narrowing the type toSettingActionEndpoint.apps/meteor/client/views/admin/settings/Setting/inputs/MethodActionInput.tsx (1)
11-14: Type constraint missing: action methods should enforce return type{ message: TranslationKey; params?: string[] }.
useMethodreturnsPromise<ServerMethodReturn<MethodName>>, which is whatever the server method declares. Currently, there's no type-level enforcement that action-type server methods return{ message: TranslationKey; params?: string[] }.Existing methods like
OEmbedCacheCleanupandsendSMTPTestEmaildo follow this pattern, but new action methods added toServerMethodscould return mismatched shapes (e.g.,void,{ data: ... }) without triggering a compile error. This creates a silent contract violation risk where the toast handler inActionInputBasewould access undefined properties.Consider adding a branded type or separate interface for action methods to enforce the contract at the type level.
sampaiodiego
approved these changes
Mar 31, 2026
aleksandernsilva
approved these changes
Mar 31, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Mar 31, 2026
dionisio-bot
Bot
removed
the
stat: ready to merge
dionisio-bot
Bot
added
the
stat: ready to merge
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Apr 1, 2026
dionisio-bot
Bot
removed
the
stat: ready to merge
dionisio-bot
Bot
added
the
stat: ready to merge
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Apr 1, 2026
dionisio-bot
Bot
removed
the
stat: ready to merge
dionisio-bot
Bot
added
the
stat: ready to merge
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Apr 1, 2026
dionisio-bot
Bot
removed
the
stat: ready to merge
dionisio-bot
Bot
added
the
stat: ready to merge
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
3 participants
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.
Proposed changes (including videos or screenshots)
Issue(s)
https://rocketchat.atlassian.net/browse/CORE-2028
Steps to test or reproduce
Further comments
Summary by CodeRabbit