From dd811a213f285ab493e8251fa23a9a0d586fa138 Mon Sep 17 00:00:00 2001 From: Abhirve Munipalle Date: Fri, 23 Oct 2026 13:17:57 -0400 Subject: [PATCH 1/2] Migrated tag and group modals to functional React components --- Changelog.md | 1 + .../Components/Modals/create_group_modal.jsx | 79 +++++++-------- .../Components/Modals/create_tag_modal.jsx | 74 ++++++-------- .../Components/Modals/edit_tag_modal.jsx | 70 ++++++------- .../Components/Modals/rename_group_modal.jsx | 99 +++++++++---------- .../__tests__/create_group_modal.test.jsx | 4 +- .../__tests__/create_tag_modal.test.jsx | 47 ++++++--- .../__tests__/edit_tag_modal.test.jsx | 38 +++++-- .../__tests__/rename_group_modal.test.jsx | 17 +++- 9 files changed, 223 insertions(+), 206 deletions(-) diff --git a/Changelog.md b/Changelog.md index 408a74baf1..79be2d51e6 100644 --- a/Changelog.md +++ b/Changelog.md @@ -40,6 +40,7 @@ ### 🔧 Internal changes - Added the passing `active_record_doctor` checks to the pre-commit hooks and CI (#8259) - Added indexes on the 22 foreign key columns reported by `rails active_record_doctor:unindexed_foreign_keys` (#8245) +- Converted `CreateTagModal`, `EditTagModal`, `CreateGroupModal`, and `RenameGroupModal` to functional React components (#8232) - Moved the RuboCop, erb_lint and i18n-tasks dependencies into the Gemfile and run them in pre-commit hooks using the `rails` Docker Compose service (#8224) - Moved the Prettier and stylelint dependencies into `package.json` and run them in pre-commit hooks using the `rails` Docker Compose service (#8225) - Converted `AssignmentSummary`, `AssessmentChart`, `CoreStatistics`, and `FractionStat` to functional React components (#8220) diff --git a/app/javascript/Components/Modals/create_group_modal.jsx b/app/javascript/Components/Modals/create_group_modal.jsx index 3ecc57ecde..a8e8fc5c09 100644 --- a/app/javascript/Components/Modals/create_group_modal.jsx +++ b/app/javascript/Components/Modals/create_group_modal.jsx @@ -1,59 +1,46 @@ -import React from "react"; +import {useEffect, useState} from "react"; import Modal from "react-modal"; -export default class CreateGroupModal extends React.Component { - constructor(props) { - super(props); - this.state = { - groupName: "", - }; - } +export default function CreateGroupModal({isOpen, onRequestClose, onSubmit}) { + const [groupName, setGroupName] = useState(""); - componentDidMount() { + useEffect(() => { Modal.setAppElement("body"); - } + }, []); - handleChange = event => { - this.setState({groupName: event.target.value}); + const handleChange = event => { + setGroupName(event.target.value); }; - handleSubmit = event => { + const handleSubmit = event => { event.preventDefault(); - if (!this.state.groupName) { + if (!groupName) { return; } - this.props.onSubmit(this.state.groupName); - this.setState({groupName: ""}); + onSubmit(groupName); + setGroupName(""); }; - render() { - return ( - -

{I18n.t("helpers.submit.create", {model: I18n.t("activerecord.models.group.one")})}

-
- - this.handleChange(event)} - autoFocus - /> -
- - -
-
- - ); - } + return ( + +

{I18n.t("helpers.submit.create", {model: I18n.t("activerecord.models.group.one")})}

+
+ + +
+ + +
+
+ + ); } diff --git a/app/javascript/Components/Modals/create_tag_modal.jsx b/app/javascript/Components/Modals/create_tag_modal.jsx index de27aad214..9a4c98d584 100644 --- a/app/javascript/Components/Modals/create_tag_modal.jsx +++ b/app/javascript/Components/Modals/create_tag_modal.jsx @@ -1,32 +1,26 @@ -import React from "react"; +import {useContext, useEffect, useState} from "react"; import TagModal from "../Helpers/tag_modal"; import Modal from "react-modal"; import PropTypes from "prop-types"; import {ResultContext} from "../Result/result_context"; -export default class CreateTagModal extends React.Component { - constructor(props) { - super(props); - this.state = { - name: "", - description: "", - }; - } - - static contextType = ResultContext; +export default function CreateTagModal({isOpen, onRequestClose}) { + const [name, setName] = useState(""); + const [description, setDescription] = useState(""); + const context = useContext(ResultContext); - componentDidMount() { + useEffect(() => { Modal.setAppElement("body"); - } + }, []); - onSubmit = event => { + const onSubmit = event => { event.preventDefault(); const data = { tag: { - name: this.state.name, - description: this.state.description, + name, + description, }, - grouping_id: this.context.grouping_id, + grouping_id: context.grouping_id, }; const options = { method: "POST", @@ -37,44 +31,36 @@ export default class CreateTagModal extends React.Component { body: JSON.stringify(data), }; fetch( - Routes.course_tags_path(this.context.course_id, {assignment_id: this.context.assignment_id}), + Routes.course_tags_path(context.course_id, {assignment_id: context.assignment_id}), options ) .catch(error => { console.error(`Error submitting form: ${error.message}`); }) .finally(() => { - this.setState( - { - name: "", - description: "", - }, - () => { - this.props.onRequestClose(); - } - ); + setName(""); + setDescription(""); + onRequestClose(); }); }; - render() { - return ( - this.setState({name: event.target.value})} - handleDescriptionChange={event => this.setState({description: event.target.value})} - isOpen={this.props.isOpen} - onRequestClose={this.props.onRequestClose} - tagModalHeading={I18n.t("helpers.submit.create", { - model: I18n.t("activerecord.models.tag.one"), - })} - onSubmit={this.onSubmit} - /> - ); - } + return ( + setName(event.target.value)} + handleDescriptionChange={event => setDescription(event.target.value)} + isOpen={isOpen} + onRequestClose={onRequestClose} + tagModalHeading={I18n.t("helpers.submit.create", { + model: I18n.t("activerecord.models.tag.one"), + })} + onSubmit={onSubmit} + /> + ); } -CreateTagModal.propType = { +CreateTagModal.propTypes = { isOpen: PropTypes.bool.isRequired, onRequestClose: PropTypes.func.isRequired, }; diff --git a/app/javascript/Components/Modals/edit_tag_modal.jsx b/app/javascript/Components/Modals/edit_tag_modal.jsx index 816e56c65e..1d979a19f6 100644 --- a/app/javascript/Components/Modals/edit_tag_modal.jsx +++ b/app/javascript/Components/Modals/edit_tag_modal.jsx @@ -1,30 +1,32 @@ -import React from "react"; +import {useEffect, useState} from "react"; import Modal from "react-modal"; import TagModal from "../Helpers/tag_modal"; import PropTypes from "prop-types"; -export default class EditTagModal extends React.Component { - constructor(props) { - super(props); - this.state = { - name: this.props.currentTagName, - description: this.props.currentTagDescription, - }; - } +export default function EditTagModal({ + course_id, + currentTagDescription, + currentTagName, + isOpen, + onRequestClose, + tag_id, +}) { + const [name, setName] = useState(currentTagName); + const [description, setDescription] = useState(currentTagDescription); - componentDidMount() { + useEffect(() => { Modal.setAppElement("body"); - } + }, []); - onSubmit = event => { + const onSubmit = event => { event.preventDefault(); const data = { tag: { - name: this.state.name, - description: this.state.description, + name, + description, }, - course_id: this.props.course_id, - id: this.props.tag_id, + course_id, + id: tag_id, submit: "Save", }; const options = { @@ -35,34 +37,32 @@ export default class EditTagModal extends React.Component { }, body: JSON.stringify(data), }; - fetch(Routes.course_tag_path(this.props.course_id, this.props.tag_id), options) + fetch(Routes.course_tag_path(course_id, tag_id), options) .then(() => { - this.props.onRequestClose(); + onRequestClose(); }) .catch(error => { console.error(error.message); }); }; - render() { - return ( - this.setState({name: event.target.value})} - handleDescriptionChange={event => this.setState({description: event.target.value})} - isOpen={this.props.isOpen} - onRequestClose={this.props.onRequestClose} - tagModalHeading={I18n.t("helpers.submit.update", { - model: I18n.t("activerecord.models.tag.one"), - })} - onSubmit={this.onSubmit} - /> - ); - } + return ( + setName(event.target.value)} + handleDescriptionChange={event => setDescription(event.target.value)} + isOpen={isOpen} + onRequestClose={onRequestClose} + tagModalHeading={I18n.t("helpers.submit.update", { + model: I18n.t("activerecord.models.tag.one"), + })} + onSubmit={onSubmit} + /> + ); } -EditTagModal.propType = { +EditTagModal.propTypes = { isOpen: PropTypes.bool.isRequired, onRequestClose: PropTypes.func.isRequired, tag_id: PropTypes.number.isRequired, diff --git a/app/javascript/Components/Modals/rename_group_modal.jsx b/app/javascript/Components/Modals/rename_group_modal.jsx index 51873ff8ae..94dd91308e 100644 --- a/app/javascript/Components/Modals/rename_group_modal.jsx +++ b/app/javascript/Components/Modals/rename_group_modal.jsx @@ -1,70 +1,59 @@ -import React from "react"; +import {useEffect, useRef, useState} from "react"; import Modal from "react-modal"; -export default class RenameGroupModal extends React.Component { - constructor(props) { - super(props); - this.state = { - groupName: props.initialGroupName || "", - }; - } +export default function RenameGroupModal({initialGroupName, isOpen, onRequestClose, onSubmit}) { + const [groupName, setGroupName] = useState(initialGroupName || ""); + const previousIsOpen = useRef(isOpen); - componentDidMount() { + useEffect(() => { Modal.setAppElement("body"); - } + }, []); - componentDidUpdate(prevProps) { - if (this.props.isOpen && !prevProps.isOpen && this.props.initialGroupName) { - this.setState({groupName: this.props.initialGroupName}); + useEffect(() => { + if (isOpen && !previousIsOpen.current && initialGroupName) { + setGroupName(initialGroupName); } - } + previousIsOpen.current = isOpen; + }, [initialGroupName, isOpen]); - handleChange = event => { - this.setState({groupName: event.target.value}); + const handleChange = event => { + setGroupName(event.target.value); }; - handleSubmit = event => { + const handleSubmit = event => { event.preventDefault(); - if (!this.state.groupName) { + if (!groupName) { return; } - this.props.onSubmit(this.state.groupName); - this.setState({groupName: ""}); + onSubmit(groupName); + setGroupName(""); }; - render() { - return ( - -

{I18n.t("groups.rename_group")}

-
- - this.handleChange(event)} - autoFocus - /> -
- - -
-
- - ); - } + return ( + +

{I18n.t("groups.rename_group")}

+
+ + +
+ + +
+
+ + ); } diff --git a/app/javascript/Components/__tests__/create_group_modal.test.jsx b/app/javascript/Components/__tests__/create_group_modal.test.jsx index ddac53815a..f97fa2609c 100644 --- a/app/javascript/Components/__tests__/create_group_modal.test.jsx +++ b/app/javascript/Components/__tests__/create_group_modal.test.jsx @@ -19,7 +19,8 @@ describe("CreateGroupModal", () => { it("should add the correct group name on successful submit", async () => { const groupName = "Test Group"; - fireEvent.change(screen.getByLabelText(I18n.t("activerecord.models.group.one")), { + const groupNameInput = screen.getByLabelText(I18n.t("activerecord.models.group.one")); + fireEvent.change(groupNameInput, { target: {value: groupName}, }); @@ -34,6 +35,7 @@ describe("CreateGroupModal", () => { await waitFor(() => { expect(props.onSubmit).toHaveBeenCalledTimes(1); expect(props.onSubmit).toHaveBeenCalledWith(groupName); + expect(groupNameInput).toHaveValue(""); }); }); diff --git a/app/javascript/Components/__tests__/create_tag_modal.test.jsx b/app/javascript/Components/__tests__/create_tag_modal.test.jsx index ee9a8eaf8a..afb5d79081 100644 --- a/app/javascript/Components/__tests__/create_tag_modal.test.jsx +++ b/app/javascript/Components/__tests__/create_tag_modal.test.jsx @@ -1,22 +1,20 @@ -import React from "react"; import {render, screen, fireEvent, waitFor} from "@testing-library/react"; import CreateTagModal from "../Modals/create_tag_modal"; import Modal from "react-modal"; import fetchMock from "jest-fetch-mock"; -import {ResultContext} from "../Result/result_context"; import {DEFAULT_RESULT_CONTEXT_VALUE, renderInResultContext} from "./result_context_renderer"; describe("CreateTagModal", () => { let props; - let component; let mockToken; - let tagName; + let csrfToken; beforeAll(() => { mockToken = "mockToken"; - document.querySelector = jest.fn(() => ({ - content: mockToken, // Return a mock value for the content property - })); + csrfToken = document.createElement("meta"); + csrfToken.name = "csrf-token"; + csrfToken.content = mockToken; + document.head.appendChild(csrfToken); }); beforeEach(() => { @@ -28,23 +26,29 @@ describe("CreateTagModal", () => { // Set the app element for React Modal Modal.setAppElement("body"); - component = renderInResultContext(, {role: "Student"}); - - // Enable submit - tagName = "Name"; - fireEvent.change(screen.getByTestId("tag_name_input"), {target: {value: tagName}}); + renderInResultContext(, {role: "Student"}); }); afterEach(() => { fetchMock.resetMocks(); + jest.restoreAllMocks(); + }); + + afterAll(() => { + csrfToken.remove(); }); it("should be called with correct params on successful submit", async () => { + const tagName = "Name"; + const tagDescription = "Description"; fireEvent.change(screen.getByTestId("tag_name_input"), {target: {value: tagName}}); + fireEvent.change(screen.getByTestId("tag_description_input"), { + target: {value: tagDescription}, + }); const data = { tag: { name: tagName, - description: "", + description: tagDescription, }, grouping_id: DEFAULT_RESULT_CONTEXT_VALUE.grouping_id, }; @@ -65,6 +69,7 @@ describe("CreateTagModal", () => { expect(req.method).toBe(options.method); expect(req.headers.get("Content-Type")).toEqual(options.headers["Content-Type"]); expect(req.headers.get("X-CSRF-Token")).toEqual(options.headers["X-CSRF-Token"]); + expect(await req.json()).toEqual(data); return JSON.stringify({status: "success"}); }); @@ -76,17 +81,33 @@ describe("CreateTagModal", () => { }); it("should call onRequestClose on successful submit", async () => { + fireEvent.change(screen.getByTestId("tag_name_input"), {target: {value: "Name"}}); fireEvent.click(screen.getByText(I18n.t("save"))); await waitFor(() => { expect(props.onRequestClose).toHaveBeenCalledTimes(1); }); }); + it("should clear both inputs after submitting", async () => { + fireEvent.change(screen.getByTestId("tag_name_input"), {target: {value: "Name"}}); + fireEvent.change(screen.getByTestId("tag_description_input"), { + target: {value: "Description"}, + }); + + fireEvent.click(screen.getByText(I18n.t("save"))); + + await waitFor(() => { + expect(screen.getByTestId("tag_name_input")).toHaveValue(""); + expect(screen.getByTestId("tag_description_input")).toHaveValue(""); + }); + }); + it("should console error on error", async () => { const customError = new Error(); customError.message = "msg"; fetchMock.mockRejectOnce(customError); const mockedConsoleError = jest.spyOn(global.console, "error").mockImplementation(() => {}); + fireEvent.change(screen.getByTestId("tag_name_input"), {target: {value: "Name"}}); fireEvent.click(screen.getByText(I18n.t("save"))); await waitFor(() => { expect(fetchMock).toHaveBeenCalledTimes(1); diff --git a/app/javascript/Components/__tests__/edit_tag_modal.test.jsx b/app/javascript/Components/__tests__/edit_tag_modal.test.jsx index 65845cd744..c8a2c24b33 100644 --- a/app/javascript/Components/__tests__/edit_tag_modal.test.jsx +++ b/app/javascript/Components/__tests__/edit_tag_modal.test.jsx @@ -1,4 +1,3 @@ -import React from "react"; import {render, screen, fireEvent, waitFor} from "@testing-library/react"; import EditTagModal from "../Modals/edit_tag_modal"; import Modal from "react-modal"; @@ -6,14 +5,15 @@ import fetchMock from "jest-fetch-mock"; describe("EditTagModal", () => { let props; - let component; let mockToken; + let csrfToken; beforeAll(() => { mockToken = "mockToken"; - document.querySelector = jest.fn(() => ({ - content: mockToken, // Return a mock value for the content property - })); + csrfToken = document.createElement("meta"); + csrfToken.name = "csrf-token"; + csrfToken.content = mockToken; + document.head.appendChild(csrfToken); }); beforeEach(() => { @@ -29,20 +29,33 @@ describe("EditTagModal", () => { // Set the app element for React Modal Modal.setAppElement("body"); - component = render(); + render(); }); afterEach(() => { fetchMock.resetMocks(); + jest.restoreAllMocks(); + }); + + afterAll(() => { + csrfToken.remove(); }); it("should be called with correct params on successful submit", async () => { + const updatedName = "updated tag"; + const updatedDescription = "updated description"; + fireEvent.change(screen.getByTestId("tag_name_input"), {target: {value: updatedName}}); + fireEvent.change(screen.getByTestId("tag_description_input"), { + target: {value: updatedDescription}, + }); const data = { tag: { - name: props.currentTagName, - description: props.currentTagDescription, + name: updatedName, + description: updatedDescription, }, - grouping_id: props.grouping_id, + course_id: props.course_id, + id: props.tag_id, + submit: "Save", }; const options = { method: "PUT", @@ -57,6 +70,7 @@ describe("EditTagModal", () => { expect(req.method).toBe(options.method); expect(req.headers.get("Content-Type")).toEqual(options.headers["Content-Type"]); expect(req.headers.get("X-CSRF-Token")).toEqual(options.headers["X-CSRF-Token"]); + expect(await req.json()).toEqual(data); return JSON.stringify({status: "success"}); }); @@ -74,6 +88,11 @@ describe("EditTagModal", () => { }); }); + it("should initialize both inputs from the current tag", () => { + expect(screen.getByTestId("tag_name_input")).toHaveValue(props.currentTagName); + expect(screen.getByTestId("tag_description_input")).toHaveValue(props.currentTagDescription); + }); + it("should console error on error", async () => { const customError = new Error(); customError.message = "msg"; @@ -83,6 +102,7 @@ describe("EditTagModal", () => { await waitFor(() => { expect(fetchMock).toHaveBeenCalledTimes(1); expect(mockedConsoleError).toHaveBeenNthCalledWith(1, customError.message); + expect(props.onRequestClose).not.toHaveBeenCalled(); }); }); }); diff --git a/app/javascript/Components/__tests__/rename_group_modal.test.jsx b/app/javascript/Components/__tests__/rename_group_modal.test.jsx index 23373994eb..02affdb761 100644 --- a/app/javascript/Components/__tests__/rename_group_modal.test.jsx +++ b/app/javascript/Components/__tests__/rename_group_modal.test.jsx @@ -14,12 +14,15 @@ describe("RenameGroupModal", () => { }; Modal.setAppElement("body"); - render(); }); it("should rename the group on successful submit", async () => { + render(); const groupName = "new_group"; - fireEvent.change(screen.getByLabelText(I18n.t("activerecord.attributes.group.group_name")), { + const groupNameInput = screen.getByLabelText( + I18n.t("activerecord.attributes.group.group_name") + ); + fireEvent.change(groupNameInput, { target: {value: groupName}, }); @@ -29,10 +32,12 @@ describe("RenameGroupModal", () => { await waitFor(() => { expect(props.onSubmit).toHaveBeenCalledTimes(1); expect(props.onSubmit).toHaveBeenCalledWith(groupName); + expect(groupNameInput).toHaveValue(""); }); }); it("should call onRequestClose on successful call", async () => { + render(); fireEvent.click(screen.getByText(I18n.t("cancel"))); await waitFor(() => { expect(props.onRequestClose).toHaveBeenCalledTimes(1); @@ -40,6 +45,7 @@ describe("RenameGroupModal", () => { }); it("should not add the new group when the inputted group name is empty", async () => { + render(); const renameGroupButton = screen.getByTestId("rename-submit-button"); fireEvent.click(renameGroupButton); @@ -56,13 +62,18 @@ describe("RenameGroupModal", () => { initialGroupName: "Original Group Name", }; - const {rerender} = render(); + const {rerender} = render(); + fireEvent.change(screen.getByLabelText(I18n.t("activerecord.attributes.group.group_name")), { + target: {value: "Unsaved Group Name"}, + }); + rerender(); rerender(); expect(screen.getByDisplayValue("Original Group Name")).toBeInTheDocument(); }); it("should not submit when group name is empty", async () => { + render(); fireEvent.change(screen.getByLabelText(I18n.t("activerecord.attributes.group.group_name")), { target: {value: ""}, }); From 61845ea197ca7c6e48646c21256b743974c26222 Mon Sep 17 00:00:00 2001 From: Abhirve Munipalle Date: Thu, 5 Nov 2026 12:14:44 -0500 Subject: [PATCH 2/2] Simplified rename group modal state reset --- .../Components/Modals/rename_group_modal.jsx | 6 ++---- .../__tests__/rename_group_modal.test.jsx | 20 +++++++++++++------ 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/app/javascript/Components/Modals/rename_group_modal.jsx b/app/javascript/Components/Modals/rename_group_modal.jsx index 94dd91308e..328d426bf5 100644 --- a/app/javascript/Components/Modals/rename_group_modal.jsx +++ b/app/javascript/Components/Modals/rename_group_modal.jsx @@ -1,19 +1,17 @@ -import {useEffect, useRef, useState} from "react"; +import {useEffect, useState} from "react"; import Modal from "react-modal"; export default function RenameGroupModal({initialGroupName, isOpen, onRequestClose, onSubmit}) { const [groupName, setGroupName] = useState(initialGroupName || ""); - const previousIsOpen = useRef(isOpen); useEffect(() => { Modal.setAppElement("body"); }, []); useEffect(() => { - if (isOpen && !previousIsOpen.current && initialGroupName) { + if (isOpen && initialGroupName) { setGroupName(initialGroupName); } - previousIsOpen.current = isOpen; }, [initialGroupName, isOpen]); const handleChange = event => { diff --git a/app/javascript/Components/__tests__/rename_group_modal.test.jsx b/app/javascript/Components/__tests__/rename_group_modal.test.jsx index 02affdb761..3bb69a4108 100644 --- a/app/javascript/Components/__tests__/rename_group_modal.test.jsx +++ b/app/javascript/Components/__tests__/rename_group_modal.test.jsx @@ -54,22 +54,30 @@ describe("RenameGroupModal", () => { }); }); - it("should populate input with initialGroupName when modal opens", async () => { + it("should populate input with the selected group name each time the modal opens", () => { const propsWithInitialName = { isOpen: true, onRequestClose: jest.fn(), onSubmit: jest.fn(), - initialGroupName: "Original Group Name", + initialGroupName: "First Group Name", }; const {rerender} = render(); + expect(screen.getByDisplayValue("First Group Name")).toBeInTheDocument(); + fireEvent.change(screen.getByLabelText(I18n.t("activerecord.attributes.group.group_name")), { - target: {value: "Unsaved Group Name"}, + target: {value: "Unsaved First Group Name"}, }); - rerender(); - rerender(); + rerender(); + rerender( + + ); - expect(screen.getByDisplayValue("Original Group Name")).toBeInTheDocument(); + expect(screen.getByDisplayValue("Second Group Name")).toBeInTheDocument(); }); it("should not submit when group name is empty", async () => {