feat: move email templates list to mui#936
Conversation
|
Warning Review limit reached
More reviews will be available in 51 minutes and 14 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe email template list feature adds pagination support and migrates the UI from React-Bootstrap to MUI. The action layer now passes ChangesEmail Template Pagination and UI Migration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 `@src/pages/emails/email-template-list-page.js`:
- Around line 81-83: The delete handler expects a row object but the MuiTable
onDelete callback passes the row id primitive; change the handler signature from
(row) to (id) and call removeEmailTemplate(id) instead of
removeEmailTemplate(row.id). Update every place that currently reads row.id
(including the other delete handler instance that mirrors
handleDeleteEmailTemplate) to accept the id primitive and forward it to
removeEmailTemplate.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0943f768-e6dd-473b-9212-324d2a42b809
📒 Files selected for processing (3)
src/actions/email-actions.jssrc/pages/emails/email-template-list-page.jssrc/reducers/emails/email-template-list-reducer.js
| const handleDeleteEmailTemplate = (row) => { | ||
| removeEmailTemplate(row.id); | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate the MuiTable component
fd -t f "table" -p | grep -i mui | head -20Repository: fntechgit/summit-admin
Length of output: 162
🏁 Script executed:
# Search for MuiTable component in the codebase
rg "MuiTable|export.*Table" --type ts --type tsx --type js --type jsx | grep -i component | head -20Repository: fntechgit/summit-admin
Length of output: 94
🏁 Script executed:
# Search for the openstack-uicore-foundation import path mentioned in the review
rg "openstack-uicore-foundation" --type ts --type tsx --type js --type jsxRepository: fntechgit/summit-admin
Length of output: 94
🏁 Script executed:
# Read the email template list page to see how MuiTable is imported and used
cat -n src/pages/emails/email-template-list-page.js | head -100Repository: fntechgit/summit-admin
Length of output: 3510
🏁 Script executed:
# Search for MuiTable import without tsx filter
rg "MuiTable|from.*table" src/pages/emails/email-template-list-page.jsRepository: fntechgit/summit-admin
Length of output: 163
🏁 Script executed:
# Look for the component being used in the file
rg "const.*MuiTable|import.*MuiTable" --type jsRepository: fntechgit/summit-admin
Length of output: 4412
🏁 Script executed:
# Read the MuiTable usage in the email template page around line 168
cat -n src/pages/emails/email-template-list-page.js | sed -n '150,180p'Repository: fntechgit/summit-admin
Length of output: 1129
🏁 Script executed:
# Check similar usage patterns in other files with onDelete
rg -A 3 "onDelete=" src/pages/sponsors/sponsor-list-page.jsRepository: fntechgit/summit-admin
Length of output: 255
🏁 Script executed:
# Look at another similar file to understand the pattern
cat -n src/pages/sponsors/sponsor-list-page.js | grep -A 30 "const handleDelete"Repository: fntechgit/summit-admin
Length of output: 1157
🏁 Script executed:
# Check another file to confirm the pattern
cat -n src/pages/sponsors/sponsor-forms-list-page/index.js | grep -A 2 "const handleDelete"Repository: fntechgit/summit-admin
Length of output: 48
🏁 Script executed:
# Check one more for consistency
rg -B 2 -A 2 "const handleDelete.*\(.*Id\)" src/pages/ | head -30Repository: fntechgit/summit-admin
Length of output: 2178
Fix MuiTable delete callback parameter handling.
The onDelete callback receives the row identifier as a primitive value, not the full row object. With the current handler expecting row.id, the delete call will pass undefined as the template id.
Proposed fix
- const handleDeleteEmailTemplate = (row) => {
- removeEmailTemplate(row.id);
+ const handleDeleteEmailTemplate = (templateId) => {
+ removeEmailTemplate(templateId);
};Also applies to: Line 168
🤖 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 `@src/pages/emails/email-template-list-page.js` around lines 81 - 83, The
delete handler expects a row object but the MuiTable onDelete callback passes
the row id primitive; change the handler signature from (row) to (id) and call
removeEmailTemplate(id) instead of removeEmailTemplate(row.id). Update every
place that currently reads row.id (including the other delete handler instance
that mirrors handleDeleteEmailTemplate) to accept the id primitive and forward
it to removeEmailTemplate.
priscila-moneo
force-pushed
the
feature/move-email-templates-list-to-mui
branch
from
a6f996a to
cdcb06e
Compare
| } | ||
| )(params)(dispatch) | ||
| .then(() => { | ||
| dispatch(showSuccessMessage(T.translate("emails.template_created"))); |
There was a problem hiding this comment.
The original POST path called showMessage with a callback that navigated to the new template after the user dismissed the modal:
dispatch(showMessage(success_message, () => {
history.push(`/app/emails/templates/${payload.response.id}`);
}));showSuccessMessage here is a toast — no callback, no navigation. edit-email-template-page.js passes saveEmailTemplate directly as onSubmit with no .then() in the component, so there is no fallback navigation either. After creating a template the user sees the toast and stays on the blank create form with no path to the new entity.
Either restore history.push in this action, or add a .then(({ response }) => history.push(/app/emails/templates/${response.id})) in the page component.
| }); | ||
| }; | ||
|
|
||
| describe("saveEmailTemplate", () => { |
There was a problem hiding this comment.
This file covers saveEmailTemplate only. Two other actions changed in this PR have no test coverage:
getEmailTemplates — the REQUEST payload now includes page and perPage for the first time (previously only order, orderDir, term). The reducer reads those fields to set currentPage and perPage state. Worth a test asserting the REQUEST_TEMPLATES action carries { page, perPage }:
describe('getEmailTemplates', () => {
it('dispatches REQUEST_TEMPLATES with page and perPage', async () => {
const store = mockStore({});
store.dispatch(getEmailTemplates('foo', 2, 25));
await flushPromises();
const req = store.getActions().find(a => a.type === 'REQUEST_TEMPLATES');
expect(req.payload).toMatchObject({ page: 2, perPage: 25 });
});
});deleteEmailTemplate — no test verifies that TEMPLATE_DELETED is dispatched with the correct templateId on a successful delete.
Signed-off-by: Priscila Moneo <priscila_moneo@hotmail.com.ar>
priscila-moneo
force-pushed
the
feature/move-email-templates-list-to-mui
branch
from
cdcb06e to
729f875
Compare
3 participants
ref: https://app.clickup.com/t/86b9xbm8u
Summary by CodeRabbit
Release Notes