Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/actions/email-actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import {
escapeFilterValue
} from "openstack-uicore-foundation/lib/utils/actions";
import URI from "urijs";
import debounce from "lodash/debounce"
import debounce from "lodash/debounce";
import history from "../history";
import { checkOrFilter, getAccessTokenSafely } from "../utils/methods";
import { saveMarketingSetting } from "./marketing-actions";
Expand Down Expand Up @@ -99,7 +99,7 @@ export const getEmailTemplates =
createAction(RECEIVE_TEMPLATES),
`${window.EMAIL_API_BASE_URL}/api/v1/mail-templates`,
authErrorHandler,
{ order, orderDir, term }
{ order, orderDir, term, page, perPage }
)(params)(dispatch).then(() => {
dispatch(stopLoading());
});
Expand Down
177 changes: 99 additions & 78 deletions src/pages/emails/email-template-list-page.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,143 +12,165 @@
* */

import React, { useEffect } from "react";
import Button from "@mui/material/Button";
import Grid2 from "@mui/material/Grid2";
import AddIcon from "@mui/icons-material/Add";
import { connect } from "react-redux";
import T from "i18n-react/dist/i18n-react";
import Swal from "sweetalert2";
import { Pagination } from "react-bootstrap";
import FreeTextSearch from "openstack-uicore-foundation/lib/components/free-text-search"
import Table from "openstack-uicore-foundation/lib/components/table";
import { getSummitById } from "../../actions/summit-actions";
import SearchInput from "openstack-uicore-foundation/lib/components/mui/search-input";
import MuiTable from "openstack-uicore-foundation/lib/components/mui/table";
import { DEFAULT_CURRENT_PAGE } from "../../utils/constants";
import {
getEmailTemplates,
deleteEmailTemplate
} from "../../actions/email-actions";

const EmailTemplateListPage = ({
templates,
lastPage,
currentPage,
perPage,
term,
order,
orderDir,
totalTemplates,
history,
...props
getEmailTemplates: fetchEmailTemplates,
deleteEmailTemplate: removeEmailTemplate
}) => {
useEffect(() => {
props.getEmailTemplates(term, currentPage, perPage, order, orderDir);
}, []);
fetchEmailTemplates(term, currentPage, perPage, order, orderDir);
}, [fetchEmailTemplates]);

const handleEdit = (template_id) => {
history.push(`/app/emails/templates/${template_id}`);
const handleEdit = (row) => {
history.push(`/app/emails/templates/${row.id}`);
};

const handlePageChange = (newPage) => {
props.getEmailTemplates(term, newPage, perPage, order, orderDir);
const handlePageChange = (page) => {
fetchEmailTemplates(term, page, perPage, order, orderDir);
};

const handleSort = (index, key, dir) => {
props.getEmailTemplates(term, currentPage, perPage, key, dir);
const handlePerPageChange = (newPerPage) => {
fetchEmailTemplates(
term,
DEFAULT_CURRENT_PAGE,
newPerPage,
order,
orderDir
);
};

const handleSort = (key, dir) => {
fetchEmailTemplates(term, currentPage, perPage, key, dir);
};

const handleSearch = (newTerm) => {
props.getEmailTemplates(newTerm, 1, perPage, order, orderDir);
fetchEmailTemplates(
newTerm,
DEFAULT_CURRENT_PAGE,
perPage,
order,
orderDir
);
};

const handleNewEmailTemplate = (ev) => {
ev.preventDefault();
history.push("/app/emails/templates/new");
};

const handleDeleteEmailTemplate = (templateId) => {
const template = templates.find((t) => t.id === templateId);

Swal.fire({
title: T.translate("general.are_you_sure"),
text: `${T.translate("emails.delete_template_warning")} ${
template.identifier
}`,
type: "warning",
showCancelButton: true,
confirmButtonColor: "#DD6B55",
confirmButtonText: T.translate("general.yes_delete")
}).then((result) => {
if (result.value) {
props.deleteEmailTemplate(templateId);
}
});
const handleDeleteEmailTemplate = (row) => {
removeEmailTemplate(row.id);
};
Comment on lines +81 to 83
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

# First, locate the MuiTable component
fd -t f "table" -p | grep -i mui | head -20

Repository: 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 -20

Repository: 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 jsx

Repository: 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 -100

Repository: 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.js

Repository: fntechgit/summit-admin

Length of output: 163


🏁 Script executed:

# Look for the component being used in the file
rg "const.*MuiTable|import.*MuiTable" --type js

Repository: 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.js

Repository: 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 -30

Repository: 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.


const columns = [
{ columnKey: "id", value: T.translate("general.id"), sortable: true },
{
columnKey: "id",
header: T.translate("general.id"),
sortable: true,
width: 70
},
{
columnKey: "identifier",
value: T.translate("emails.name"),
styles: { wordBreak: "break-all" },
header: T.translate("emails.name"),
render: (row) => (
<div style={{ wordBreak: "break-word", overflowWrap: "anywhere" }}>
{row.identifier}
</div>
),
sortable: true
},
{ columnKey: "subject", value: T.translate("emails.subject") },
{ columnKey: "from_email", value: T.translate("emails.from_email") }
{ columnKey: "subject", header: T.translate("emails.subject") },
{ columnKey: "from_email", header: T.translate("emails.from_email") }
];

const table_options = {
const tableOptions = {
sortCol: order,
sortDir: orderDir,
actions: {
edit: { onClick: handleEdit },
delete: { onClick: handleDeleteEmailTemplate }
}
sortDir: orderDir
};

return (
<div className="container">
<h3>
{" "}
{T.translate("emails.template_list")} ({totalTemplates})
</h3>
<div className="row">
<div className="col-md-6">
<FreeTextSearch
value={term}
placeholder={T.translate("emails.placeholders.search_templates")}
onSearch={handleSearch}
/>
</div>
<div className="col-md-6 text-right">
<button
className="btn btn-primary right-space"
<h3>{T.translate("emails.template_list")}</h3>
<Grid2
container
spacing={2}
sx={{
justifyContent: "center",
alignItems: "center",
mb: 2
}}
>
<Grid2 size={2}>
{totalTemplates} {T.translate("emails.templates")}
</Grid2>
<Grid2 size={10} container spacing={1} sx={{ justifyContent: "end" }}>
<Grid2 size={3}>
<SearchInput
term={term}
onSearch={handleSearch}
placeholder={T.translate("emails.placeholders.search_templates")}
/>
</Grid2>
<Button
variant="contained"
onClick={handleNewEmailTemplate}
startIcon={<AddIcon />}
sx={{
height: "36px",
padding: "6px 16px",
fontSize: "1.4rem",
lineHeight: "2.4rem",
letterSpacing: "0.4px"
}}
>
{T.translate("emails.add_template")}
</button>
</div>
</div>
</Button>
</Grid2>
</Grid2>

{templates.length === 0 && (
<div>{T.translate("emails.no_templates")}</div>
)}

{templates.length > 0 && (
<div>
<Table
options={table_options}
<MuiTable
options={tableOptions}
data={templates}
columns={columns}
perPage={perPage}
currentPage={currentPage}
totalRows={totalTemplates}
onPageChange={handlePageChange}
onPerPageChange={handlePerPageChange}
onSort={handleSort}
/>
<Pagination
bsSize="medium"
prev
next
first
last
ellipsis
boundaryLinks
maxButtons={10}
items={lastPage}
activePage={currentPage}
onSelect={handlePageChange}
onEdit={handleEdit}
onDelete={handleDeleteEmailTemplate}
getName={(row) => row.identifier}
deleteDialogBody={(item) =>
`${T.translate("emails.delete_template_warning")} ${item}`
}
confirmButtonColor="error"
/>
</div>
)}
Expand All @@ -161,7 +183,6 @@ const mapStateToProps = ({ emailTemplateListState }) => ({
});

export default connect(mapStateToProps, {
getSummitById,
getEmailTemplates,
deleteEmailTemplate
})(EmailTemplateListPage);
52 changes: 31 additions & 21 deletions src/reducers/emails/email-template-list-reducer.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/**
/*
* Copyright 2017 OpenStack Foundation
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -9,16 +9,15 @@
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
**/
*/

import { LOGOUT_USER } from "openstack-uicore-foundation/lib/security/actions";
import {
RECEIVE_TEMPLATES,
REQUEST_TEMPLATES,
TEMPLATE_DELETED
} from "../../actions/email-actions";

import { LOGOUT_USER } from "openstack-uicore-foundation/lib/security/actions";

const DEFAULT_STATE = {
templates: [],
term: null,
Expand All @@ -30,38 +29,49 @@ const DEFAULT_STATE = {
totalTemplates: 0
};

const emailTemplateListReducer = (state = DEFAULT_STATE, action) => {
const emailTemplateListReducer = (state = DEFAULT_STATE, action = {}) => {
const { type, payload } = action;
switch (type) {
case LOGOUT_USER: {
return DEFAULT_STATE;
}
case REQUEST_TEMPLATES: {
let { order, orderDir, term } = payload;

return { ...state, order, orderDir, term };
const { order, orderDir, term, page, perPage } = payload;
return {
...state,
order,
orderDir,
term,
currentPage: page,
perPage,
templates: []
};
}
case RECEIVE_TEMPLATES: {
let { total, last_page, current_page } = payload.response;
let templates = payload.response.data.map((s) => {
return {
id: s.id,
identifier: s.identifier,
subject: s.subject,
from_email: s.from_email
};
});
const {
total,
last_page: lastPage,
current_page: currentPage,
per_page: perPage
} = payload.response;
const templates = payload.response.data.map((s) => ({
id: s.id,
identifier: s.identifier,
subject: s.subject,
from_email: s.from_email
}));

return {
...state,
templates: templates,
currentPage: current_page,
templates,
currentPage,
perPage,
totalTemplates: total,
lastPage: last_page
lastPage
};
}
case TEMPLATE_DELETED: {
let { templateId } = payload;
const { templateId } = payload;
return {
...state,
templates: state.templates.filter((s) => s.id !== templateId)
Expand Down
Loading