fix: close UIKit modal when viewSubmit returns success with no type f… - #7537
fix: close UIKit modal when viewSubmit returns success with no type f…#7537erikurt9 wants to merge 3 commits into
Conversation
…ield
When a Rocket.Chat App responds to a viewSubmit interaction with a bare
success response ({ success: true }, no type field), the modal now closes
correctly instead of throwing an unhandled error that gets swallowed silently.
Fixes RocketChat#7503
|
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Walkthrough
ChangesModal response handling
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/lib/methods/actions.ts`:
- Line 177: Add the missing trailing newline after the final closing brace in
the file. Then run pnpm prettier-lint and TZ=UTC pnpm test for the modified
files.
🪄 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 Plus
Run ID: fad29f0c-e4f0-4c63-9f84-6f1fe8a60610
📒 Files selected for processing (1)
app/lib/methods/actions.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions
Files:
app/lib/methods/actions.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript for type safety; add explicit type annotations to function parameters and return types
Prefer interfaces over type aliases for defining object shapes in TypeScript
Use enums for sets of related constants rather than magic strings or numbers
Files:
app/lib/methods/actions.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{js,jsx,ts,tsx}: Before committing changes to JavaScript or TypeScript files, runpnpm prettier-lintandTZ=UTC pnpm testfor the modified files.
Use the local-first data flow: the UI reads from WatermelonDB, while sagas synchronize data with the server.
Use Redux and Redux-Saga for global or server state, and use Zustand for feature-local stores; do not assume all state is in Redux.
Files:
app/lib/methods/actions.ts
🧠 Learnings (1)
📚 Learning: 2026-04-30T17:07:51.020Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7274
File: app/lib/services/voip/MediaCallEvents.ts:0-0
Timestamp: 2026-04-30T17:07:51.020Z
Learning: In this Rocket.Chat React Native codebase, the ESLint rule `no-void: error` is enforced. When you see a promise returned from an async call that is not awaited (a “floating promise”), do not silence it with the `void somePromise()` pattern. Instead, handle the promise explicitly by attaching `.catch(...)` (or otherwise awaiting/handling the error) so unhandled-rejection risks are addressed in a way that satisfies the existing ESLint configuration.
Applied to files:
app/lib/methods/actions.ts
🪛 ESLint
app/lib/methods/actions.ts
[error] 177-177: Insert ⏎
(prettier/prettier)
🔇 Additional comments (1)
app/lib/methods/actions.ts (1)
158-176: LGTM!
| invalidateTriggerId(triggerId); | ||
| } | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add the missing final newline.
Prettier reports Insert ⏎ at Line 177. Add a newline after the closing brace.
As per coding guidelines, run pnpm prettier-lint and TZ=UTC pnpm test for the modified files before committing.
🧰 Tools
🪛 ESLint
[error] 177-177: Insert ⏎
(prettier/prettier)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/lib/methods/actions.ts` at line 177, Add the missing trailing newline
after the final closing brace in the file. Then run pnpm prettier-lint and
TZ=UTC pnpm test for the modified files.
Sources: Coding guidelines, Linters/SAST tools
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
Please review already signed the CLA |
2 participants
Summary
Fixes #7503
When a Rocket.Chat App responds to a
viewSubmitUIKit interaction with a bare success response ({ "success": true }, notypefield), the modal now closes correctly instead of staying open silently.Root Cause
In
app/lib/methods/actions.ts,triggerAction()was passinginteractionType ?? ''totoServerModalInteractionType(), which returnsnullfor an empty string. This caused an error to be thrown and caught silently byModalBlockView.submit()'s outer catch block, preventingNavigation.back()from ever being called.Fix
Added an early return of
ModalActions.CLOSEwheninteractionTypeisundefined, before consulting the allow-list. This restores the behavior that existed before PR #7057 for this specific case, without weakening the error thrown for genuinely unrecognized type strings.const { type: interactionType, ...data } = parsed; + if (interactionType === undefined) { + return ModalActions.CLOSE; + } const modalType = toServerModalInteractionType(interactionType); if (!modalType) { - throw new Error(`Unknown modal interaction type: ${interactionType ?? 'undefined'}`); + throw new Error(`Unknown modal interaction type: ${interactionType}`); }Summary by CodeRabbit