From 5f663c3ae7c22156d33d06ad1479facd9eb0311d Mon Sep 17 00:00:00 2001 From: nvms Date: Wed, 5 Jun 2024 11:00:49 -0400 Subject: [PATCH 01/19] initial pass --- frontend/src/fetchers/courses.js | 30 ++- frontend/src/pages/Admin/CourseAdd.js | 44 ++++ frontend/src/pages/Admin/CourseEdit.js | 105 ++++++++++ frontend/src/pages/Admin/CourseList.js | 50 +++++ frontend/src/pages/Admin/Courses.js | 65 +++--- .../src/pages/Admin/components/CsvImport.js | 195 +++++++++--------- frontend/src/pages/Admin/index.js | 5 + src/routes/courses/handlers.ts | 82 ++++++++ src/routes/courses/index.ts | 8 + src/services/course.ts | 8 + 10 files changed, 467 insertions(+), 125 deletions(-) create mode 100644 frontend/src/pages/Admin/CourseAdd.js create mode 100644 frontend/src/pages/Admin/CourseEdit.js create mode 100644 frontend/src/pages/Admin/CourseList.js diff --git a/frontend/src/fetchers/courses.js b/frontend/src/fetchers/courses.js index 4efff9a66f..0d3d55f756 100644 --- a/frontend/src/fetchers/courses.js +++ b/frontend/src/fetchers/courses.js @@ -1,6 +1,11 @@ /* eslint-disable import/prefer-default-export */ import join from 'url-join'; -import { get } from './index'; +import { + destroy, + get, + post, + put, +} from './index'; export async function getCourses() { const courses = await get(join('/', 'api', 'courses')); @@ -16,6 +21,29 @@ export async function getCourses() { // ]; } +export async function getCourseById(id) { + const course = await get(join('/', 'api', 'courses', id)); + return course.json(); + // response like { id: 1, name: 'Guiding Children\'s Behavior (BTS-P)' } +} + +export async function updateCourseById(id, data) { + const course = await put(join('/', 'api', 'courses', id), data); + return course.json(); + // response like { id: 1, name: 'Guiding Children\'s Behavior (BTS-P)' } +} + +export async function deleteCourseById(id) { + return destroy(join('/', 'api', 'courses', id)); + // response like { id: 1, name: 'Guiding Children\'s Behavior (BTS-P)' } +} + +export async function createCourseByName(name) { + const course = await post(join('/', 'api', 'courses'), { name }); + return course.json(); + // response like { id: 1, name: 'Guiding Children\'s Behavior (BTS-P)' } +} + export const fetchCourseDashboardData = async (query) => { const request = join('/', 'api', 'courses', 'dashboard', `?${query}`); const res = await get(request); diff --git a/frontend/src/pages/Admin/CourseAdd.js b/frontend/src/pages/Admin/CourseAdd.js new file mode 100644 index 0000000000..c79cd48ed4 --- /dev/null +++ b/frontend/src/pages/Admin/CourseAdd.js @@ -0,0 +1,44 @@ +import React, { useState } from 'react'; +import PropTypes from 'prop-types'; +import { Button, Label, TextInput } from '@trussworks/react-uswds'; +import { createCourseByName } from '../../fetchers/courses'; + +function CourseAdd({ refresh }) { + const [courseName, setCourseName] = useState(''); + + const create = async () => { + await createCourseByName(courseName); + refresh(); + setCourseName(''); + }; + + return ( +
+

Add a new course

+ + setCourseName(e.target.value)} + value={courseName} + /> + +
+ ); +} + +CourseAdd.propTypes = { + refresh: PropTypes.func.isRequired, +}; + +export default CourseAdd; diff --git a/frontend/src/pages/Admin/CourseEdit.js b/frontend/src/pages/Admin/CourseEdit.js new file mode 100644 index 0000000000..5f964f4222 --- /dev/null +++ b/frontend/src/pages/Admin/CourseEdit.js @@ -0,0 +1,105 @@ +import React, { + useCallback, + useEffect, + useRef, + useState, +} from 'react'; +import ReactRouterPropTypes from 'react-router-prop-types'; +import { + Button, + ButtonGroup, + Label, + TextInput, +} from '@trussworks/react-uswds'; +import { useHistory } from 'react-router-dom'; +import { deleteCourseById, getCourseById, updateCourseById } from '../../fetchers/courses'; +import Modal from '../../components/Modal'; + +function CourseEdit({ match }) { + const { params: { courseId } } = match; + const [course, setCourse] = useState(); + const [newCourse, setNewCourse] = useState(); + const modalRef = useRef(); + const history = useHistory(); + + useEffect(() => { + async function fetchCourse() { + const c = await getCourseById(courseId); + setCourse({ ...c }); + setNewCourse({ ...c }); + } + + if (!course) { + fetchCourse(); + } + }, [course, courseId]); + + const askConfirmDelete = useCallback(() => { + if (modalRef.current) { + modalRef.current.toggleModal(true); + } + }, []); + + if (!course || !newCourse) { + return
Loading course...
; + } + + const onChange = (e) => { + setNewCourse((prevCourse) => ({ ...prevCourse, name: e.target.value })); + }; + + const saveChanges = async () => { + const updated = await updateCourseById(courseId, { name: newCourse.name }); + setCourse({ ...updated }); + }; + + const confirmDelete = async () => { + if (modalRef.current) { + modalRef.current.toggleModal(false); + } + + await deleteCourseById(courseId); + + history.push('/admin/courses'); + }; + + return ( +
+

{course.name}

+ + + + + + + + + + This action cannot be undone. + +
+ ); +} + +CourseEdit.propTypes = { + match: ReactRouterPropTypes.match.isRequired, +}; + +export default CourseEdit; diff --git a/frontend/src/pages/Admin/CourseList.js b/frontend/src/pages/Admin/CourseList.js new file mode 100644 index 0000000000..568bd863ab --- /dev/null +++ b/frontend/src/pages/Admin/CourseList.js @@ -0,0 +1,50 @@ +import React, { useState } from 'react'; +import PropTypes from 'prop-types'; +import { Label, TextInput } from '@trussworks/react-uswds'; + +function CourseList({ courses }) { + const [filter, setFilter] = useState(''); + + if (!courses) { + return null; + } + + const onFilterChange = (e) => { + setFilter(e.target.value); + }; + + const filteredCourses = courses.filter( + (course) => course.name.toLowerCase().includes(filter.toLowerCase()), + ); + + return ( +
+ + + {filteredCourses.map((course) => ( +
+ {course.name} +
+ ))} +
+ ); +} + +CourseList.propTypes = { + courses: PropTypes.arrayOf(PropTypes.shape({ + name: PropTypes.string, + id: PropTypes.number, + })).isRequired, +}; + +export default CourseList; diff --git a/frontend/src/pages/Admin/Courses.js b/frontend/src/pages/Admin/Courses.js index e9a42a3d67..590d9f09d9 100644 --- a/frontend/src/pages/Admin/Courses.js +++ b/frontend/src/pages/Admin/Courses.js @@ -1,7 +1,10 @@ -import React, { useEffect, useState } from 'react'; -import { Button } from '@trussworks/react-uswds'; +import React, { useCallback, useState } from 'react'; +import { Button, Grid, GridContainer } from '@trussworks/react-uswds'; import CsvImport from './components/CsvImport'; import { getCourses } from '../../fetchers/courses'; +import Container from '../../components/Container'; +import CourseList from './CourseList'; +import CourseAdd from './CourseAdd'; function Courses() { const [courses, setCourses] = useState(); @@ -9,16 +12,16 @@ function Courses() { const typeName = 'courses'; const apiPathName = 'courses'; - useEffect(() => { + const refresh = useCallback(async () => { async function get() { const response = await getCourses(); setCourses(response); } - if (!courses) { - get(); - } - }, [courses]); + get(); + }, []); + + refresh(); const exportCourses = () => { // export courses as CSV @@ -47,24 +50,36 @@ function Courses() { return ( <> - - - {courses && ( - - )} - + + + + + + + + + + + + + + + {courses && ( + + + + )} + + + ); } diff --git a/frontend/src/pages/Admin/components/CsvImport.js b/frontend/src/pages/Admin/components/CsvImport.js index 95191a8a55..6cf355d3c4 100644 --- a/frontend/src/pages/Admin/components/CsvImport.js +++ b/frontend/src/pages/Admin/components/CsvImport.js @@ -9,7 +9,6 @@ import { Button, } from '@trussworks/react-uswds'; import languageEncoding from 'detect-file-encoding-and-language'; -import Container from '../../../components/Container'; import { importCsv, } from '../../../fetchers/Admin'; @@ -177,113 +176,111 @@ export default function CsvImport( }; return ( <> - -
-

- { - // Capitalize first letter of each word in typeName. - typeName.split(' ').map((word) => word.charAt(0).toUpperCase() + word.slice(1)).join(' ') - } - {' '} - Import -

- {(success && !error) && ( - - {success} - - )} - {error && ( - - {error} - - )} - {info && ( - - {info} +
+

+ { + // Capitalize first letter of each word in typeName. + typeName.split(' ').map((word) => word.charAt(0).toUpperCase() + word.slice(1)).join(' ') + } + {' '} + Import +

+ {(success && !error) && ( + + {success} - )} -
- - - - {(success) && ( -
-

Import Summary:

-
    - { - created && created.length > 0 && ( -
  • - {`${created.length} created`} - {created.map((c) => ( -
  • {c.name}
    • - ))} - - ) - } - { - skipped && skipped.length > 0 && ( -
  • - {`${skipped.length} skipped`} - {skipped.map((item) => ( -
  • {item}
    • - ))} - - ) + )} + {error && ( + + {error} + + )} + {info && ( + + {info} + + )} +
    + + + + {(success) && ( +
    +

    Import Summary:

    +
      + { + created && created.length > 0 && ( +
    • + {`${created.length} created`} + {created.map((c) => ( +
    • {c.name}
      • + ))} + + ) } - { - errors && errors.length > 0 && ( -
    • - {`${errors.length} errors`} - {errors.map((err) => ( -
    • {err}
      • - ))} - - ) + { + skipped && skipped.length > 0 && ( +
    • + {`${skipped.length} skipped`} + {skipped.map((item) => ( +
    • {item}
      • + ))} + + ) + } + { + errors && errors.length > 0 && ( +
    • + {`${errors.length} errors`} + {errors.map((err) => ( +
    • {err}
      • + ))} + + ) + } + { + replaced && replaced.length > 0 && ( +
    • + {`${replaced.length} replaced`} + {replaced.map((r) => ( +
    • {r.name}
      • + ))} + + ) } - { - replaced && replaced.length > 0 && ( -
    • - {`${replaced.length} replaced`} - {replaced.map((r) => ( -
    • {r.name}
      • - ))} - - ) - } - { - updated && updated.length > 0 && ( -
    • - {`${updated.length} updated`} - {updated.map((u) => ( -
    • {u.name}
      • - ))} - - ) - } - { - deleted && deleted.length > 0 && ( + { + updated && updated.length > 0 && (
    • - {`${deleted.length} deleted`} - {deleted.map((d) => ( -
    • {d.name}
      • + {`${updated.length} updated`} + {updated.map((u) => ( +
    • {u.name}
      • ))} ) + } + { + deleted && deleted.length > 0 && ( +
    • + {`${deleted.length} deleted`} + {deleted.map((d) => ( +
    • {d.name}
      • + ))} + + ) - } -
    -
    - )} - -     -
    - + } +
+
+ )} + +
- + +
); } diff --git a/frontend/src/pages/Admin/index.js b/frontend/src/pages/Admin/index.js index 20d6e914c5..340e92a790 100644 --- a/frontend/src/pages/Admin/index.js +++ b/frontend/src/pages/Admin/index.js @@ -11,6 +11,7 @@ import NationalCenters from './NationalCenters'; import Goals from './Goals'; import TrainingReports from './TrainingReports'; import Courses from './Courses'; +import CourseEdit from './CourseEdit'; function Admin() { return ( @@ -93,6 +94,10 @@ function Admin() { path="/admin/courses/" render={({ match }) => } /> + } + /> ); diff --git a/src/routes/courses/handlers.ts b/src/routes/courses/handlers.ts index d2bac79bd1..36a2f2d5f7 100644 --- a/src/routes/courses/handlers.ts +++ b/src/routes/courses/handlers.ts @@ -5,10 +5,14 @@ import handleErrors from '../../lib/apiErrorHandler'; import { setReadRegions } from '../../services/accessValidation'; import { getAllCourses, + getCourseById as getById, + createCourseByName as createCourse, } from '../../services/course'; import { currentUserId } from '../../services/currentUser'; import getCachedResponse from '../../lib/cache'; import { getCourseUrlWidgetData } from '../../services/dashboards/course'; +import { userById } from '../../services/users'; +import UserPolicy from '../../policies/user'; const COURSE_DATA_CACHE_VERSION = 1.5; @@ -28,6 +32,84 @@ export async function allCourses(req: Request, res: Response) { } } +export async function getCourseById(req: Request, res: Response) { + try { + const { id } = req.params; + const course = await getById(Number(id)); + res.json(course); + } catch (err) { + await handleErrors(err, req, res, logContext); + } +} + +export async function updateCourseById(req: Request, res: Response) { + try { + const { id } = req.params; + const userId = await currentUserId(req, res); + const user = await userById(userId); + const authorization = new UserPolicy(user); + + if (!authorization.isAdmin()) { + res.status(403).send('Forbidden'); + return; + } + + const course = await getById(Number(id)); + if (!course) { + res.status(404).send('Course not found'); + return; + } + const updated = await course.update(req.body); + res.json(updated); + } catch (err) { + await handleErrors(err, req, res, logContext); + } +} + +export async function createCourseByName(req: Request, res: Response) { + try { + const userId = await currentUserId(req, res); + const user = await userById(userId); + const authorization = new UserPolicy(user); + + if (!authorization.isAdmin()) { + res.status(403).send('Forbidden'); + return; + } + + const { name } = req.body; + const course = await createCourse(name); + + res.json(course); + } catch (err) { + await handleErrors(err, req, res, logContext); + } +} + +export async function deleteCourseById(req: Request, res: Response) { + try { + const { id } = req.params; + const userId = await currentUserId(req, res); + const user = await userById(userId); + const authorization = new UserPolicy(user); + + if (!authorization.isAdmin()) { + res.status(403).send('Forbidden'); + return; + } + + const course = await getById(Number(id)); + if (!course) { + res.status(404).send('Course not found'); + return; + } + await course.destroy(); + res.status(204).send(); + } catch (err) { + await handleErrors(err, req, res, logContext); + } +} + export async function getCourseUrlWidgetDataWithCache(req, res) { const userId = await currentUserId(req, res); const query = await setReadRegions(req.query, userId); diff --git a/src/routes/courses/index.ts b/src/routes/courses/index.ts index b448345d27..f23f1ec1f3 100644 --- a/src/routes/courses/index.ts +++ b/src/routes/courses/index.ts @@ -1,11 +1,19 @@ import express from 'express'; import { allCourses, + createCourseByName, + deleteCourseById, + getCourseById, getCourseUrlWidgetDataWithCache, + updateCourseById, } from './handlers'; import transactionWrapper from '../transactionWrapper'; const router = express.Router(); router.get('/', transactionWrapper(allCourses)); +router.get('/:id', transactionWrapper(getCourseById)); +router.put('/:id', transactionWrapper(updateCourseById)); +router.post('/', transactionWrapper(createCourseByName)); +router.delete('/:id', transactionWrapper(deleteCourseById)); router.get('/dashboard', transactionWrapper(getCourseUrlWidgetDataWithCache)); export default router; diff --git a/src/services/course.ts b/src/services/course.ts index 7bbc742846..27fb9a1846 100644 --- a/src/services/course.ts +++ b/src/services/course.ts @@ -23,6 +23,14 @@ export async function getAllCourses(where: WhereOptions = {}) { }); } +export async function getCourseById(id: number) { + return Course.findByPk(id); +} + +export async function createCourseByName(name: string) { + return Course.create({ name }); +} + export async function csvImport(buffer: Buffer | string) { const created = []; const replaced = []; From b89737bd0f4b6a90bfa38d0982e8bd96c3a5c7da Mon Sep 17 00:00:00 2001 From: nvms Date: Wed, 5 Jun 2024 11:07:30 -0400 Subject: [PATCH 02/19] add test for CourseAdd --- frontend/src/pages/Admin/CourseAdd.js | 1 + .../src/pages/Admin/__tests__/CourseAdd.js | 50 +++++++++++++++++++ 2 files changed, 51 insertions(+) create mode 100644 frontend/src/pages/Admin/__tests__/CourseAdd.js diff --git a/frontend/src/pages/Admin/CourseAdd.js b/frontend/src/pages/Admin/CourseAdd.js index c79cd48ed4..dc35e290cc 100644 --- a/frontend/src/pages/Admin/CourseAdd.js +++ b/frontend/src/pages/Admin/CourseAdd.js @@ -28,6 +28,7 @@ function CourseAdd({ refresh }) { /> - + { - const renderIt = () => { - render(); - }; + const mockRefresh = jest.fn(); beforeEach(() => { - - }); - - afterEach(() => { - - }); - - it('renders', async () => { - act(() => { - renderIt(); - }); - - const courseNameInput = document.querySelector('input[name="coursename"]'); - expect(courseNameInput).toBeInTheDocument(); + fetchMock.reset(); + render(); }); - it('calls refresh when submitting', async () => { - act(() => { - renderIt(); - }); - - const courseNameInput = document.querySelector('input[name="coursename"]'); - const addButton = document.querySelector('button[data-testid="add-course"]'); - - expect(courseNameInput).toBeInTheDocument(); - - act(() => { - courseNameInput.value = 'New Course'; - addButton.click(); - }); - - expect(refresh).toHaveBeenCalledTimes(1); + it('renders the CourseAdd component', () => { + expect(screen.getByText('Add a new course')).toBeInTheDocument(); + expect(screen.getByLabelText('Course name')).toBeInTheDocument(); + expect(screen.getByTestId('add-course')).toBeInTheDocument(); }); }); diff --git a/frontend/src/pages/Admin/__tests__/CourseEdit.js b/frontend/src/pages/Admin/__tests__/CourseEdit.js new file mode 100644 index 0000000000..77346d38c7 --- /dev/null +++ b/frontend/src/pages/Admin/__tests__/CourseEdit.js @@ -0,0 +1,53 @@ +import React from 'react'; +import { + render, + screen, + act, +} from '@testing-library/react'; +import fetchMock from 'fetch-mock'; +import userEvent from '@testing-library/user-event'; +import { createMemoryHistory } from 'history'; +import { Router } from 'react-router-dom'; +import CourseEdit from '../CourseEdit'; + +describe('CourseEdit', () => { + const courseId = '1'; + const courseUrl = `/api/courses/${courseId}`; + const course = { + id: courseId, + name: 'Test Course', + }; + const history = createMemoryHistory(); + const renderCourseEdit = () => { + render( + + + , + ); + }; + + beforeEach(() => { + fetchMock.get(courseUrl, course); + fetchMock.put(courseUrl, { ...course, name: 'Updated Course' }); + fetchMock.delete(courseUrl, 204); + }); + + afterEach(() => fetchMock.restore()); + + it('loads and displays the course', async () => { + act(renderCourseEdit); + expect(await screen.findByText('Test Course')).toBeInTheDocument(); + }); + + it('updates the course name', async () => { + act(renderCourseEdit); + const input = await screen.findByPlaceholderText('Test Course'); + act(() => { + userEvent.type(input, 'Updated Course'); + }); + act(() => { + userEvent.click(screen.getByRole('button', { name: 'Save changes' })); + }); + expect(await screen.findByText('Updated Course')).toBeInTheDocument(); + }); +}); From dda57ac2965540576833a225a81fb4778bceb0c8 Mon Sep 17 00:00:00 2001 From: nvms Date: Wed, 5 Jun 2024 11:41:38 -0400 Subject: [PATCH 05/19] update test --- .../src/pages/Admin/__tests__/CourseList.js | 75 +++++++------------ 1 file changed, 28 insertions(+), 47 deletions(-) diff --git a/frontend/src/pages/Admin/__tests__/CourseList.js b/frontend/src/pages/Admin/__tests__/CourseList.js index 36975478ae..61cc9301c3 100644 --- a/frontend/src/pages/Admin/__tests__/CourseList.js +++ b/frontend/src/pages/Admin/__tests__/CourseList.js @@ -1,60 +1,41 @@ -import '@testing-library/jest-dom'; import React from 'react'; import { - act, render, + screen, + fireEvent, } from '@testing-library/react'; import CourseList from '../CourseList'; -const courses = [ - { id: 1, name: 'Course 1' }, - { id: 2, name: 'Course 2' }, -]; - describe('CourseList', () => { - const renderIt = () => { - render(); - }; - - beforeEach(() => { - + const courses = [ + { id: 1, name: 'Introduction to Dogs' }, + { id: 2, name: 'Advanced Penguins' }, + { id: 3, name: 'Data Structures and Kangaroos' }, + ]; + + it('renders without courses', () => { + render(); + expect(screen.queryByLabelText('Filter courses by name')).toBeInTheDocument(); }); - afterEach(() => { - - }); - - it('renders and shows all courses', async () => { - act(() => { - renderIt(); - }); - - const courseFilterInput = document.querySelector('input[name="courses-filter"]'); - expect(courseFilterInput).toBeInTheDocument(); - - const course1Link = document.querySelector('a[href="/admin/course/1"]'); - const course2Link = document.querySelector('a[href="/admin/course/2"]'); - - expect(course1Link).toBeInTheDocument(); - expect(course2Link).toBeInTheDocument(); + it('renders with courses and allows filtering', () => { + render(); + expect(screen.getByLabelText('Filter courses by name')).toBeInTheDocument(); + expect(screen.getByText('Introduction to Dogs')).toBeInTheDocument(); + expect(screen.getByText('Advanced Penguins')).toBeInTheDocument(); + expect(screen.getByText('Data Structures and Kangaroos')).toBeInTheDocument(); + + fireEvent.change(screen.getByLabelText('Filter courses by name'), { target: { value: 'dog' } }); + expect(screen.getByText('Introduction to Dogs')).toBeInTheDocument(); + expect(screen.queryByText('Advanced Penguins')).not.toBeInTheDocument(); + expect(screen.queryByText('Data Structures and Kangaroos')).not.toBeInTheDocument(); }); - it('filters courses', async () => { - act(() => { - renderIt(); - }); - - const courseFilterInput = document.querySelector('input[name="courses-filter"]'); - expect(courseFilterInput).toBeInTheDocument(); - - act(() => { - courseFilterInput.value = '1'; - courseFilterInput.dispatchEvent(new Event('input', { bubbles: true })); - }); - - const course1Link = document.querySelector('a[href="/admin/course/1"]'); - const course2Link = document.querySelector('a[href="/admin/course/2"]'); - expect(course1Link).toBeInTheDocument(); - expect(course2Link).not.toBeInTheDocument(); + it('handles no matching filter results', () => { + render(); + fireEvent.change(screen.getByLabelText('Filter courses by name'), { target: { value: 'asdf' } }); + expect(screen.queryByText('Introduction to Dogs')).not.toBeInTheDocument(); + expect(screen.queryByText('Advanced Penguins')).not.toBeInTheDocument(); + expect(screen.queryByText('Data Structures and Kangaroos')).not.toBeInTheDocument(); }); }); From b205ed98ff49a85b959c9e3ed7309da6d24dfb91 Mon Sep 17 00:00:00 2001 From: nvms Date: Sun, 9 Jun 2024 20:47:25 -0400 Subject: [PATCH 06/19] hopefully fix this test --- frontend/src/pages/Admin/Courses.js | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/frontend/src/pages/Admin/Courses.js b/frontend/src/pages/Admin/Courses.js index 590d9f09d9..f89624f9bc 100644 --- a/frontend/src/pages/Admin/Courses.js +++ b/frontend/src/pages/Admin/Courses.js @@ -1,4 +1,4 @@ -import React, { useCallback, useState } from 'react'; +import React, { useCallback, useEffect, useState } from 'react'; import { Button, Grid, GridContainer } from '@trussworks/react-uswds'; import CsvImport from './components/CsvImport'; import { getCourses } from '../../fetchers/courses'; @@ -13,15 +13,13 @@ function Courses() { const apiPathName = 'courses'; const refresh = useCallback(async () => { - async function get() { - const response = await getCourses(); - setCourses(response); - } - - get(); + const response = await getCourses(); + setCourses(response); }, []); - refresh(); + useEffect(() => { + refresh(); + }, [refresh]); const exportCourses = () => { // export courses as CSV From 841e49359061702bc636389b974c9fd44aa5af82 Mon Sep 17 00:00:00 2001 From: nvms Date: Tue, 11 Jun 2024 12:07:53 -0400 Subject: [PATCH 07/19] wrap in form, expand coverage --- frontend/src/pages/Admin/CourseAdd.js | 55 ++++++++++++------- .../src/pages/Admin/__tests__/CourseAdd.js | 19 ++++++- 2 files changed, 52 insertions(+), 22 deletions(-) diff --git a/frontend/src/pages/Admin/CourseAdd.js b/frontend/src/pages/Admin/CourseAdd.js index dc35e290cc..2fad3e5778 100644 --- a/frontend/src/pages/Admin/CourseAdd.js +++ b/frontend/src/pages/Admin/CourseAdd.js @@ -1,6 +1,13 @@ import React, { useState } from 'react'; import PropTypes from 'prop-types'; -import { Button, Label, TextInput } from '@trussworks/react-uswds'; +import { + Button, + Fieldset, + Form, + FormGroup, + Label, + TextInput, +} from '@trussworks/react-uswds'; import { createCourseByName } from '../../fetchers/courses'; function CourseAdd({ refresh }) { @@ -12,28 +19,36 @@ function CourseAdd({ refresh }) { setCourseName(''); }; + const onSubmit = (e) => { + e.preventDefault(); + create(); + }; + return (

Add a new course

- - setCourseName(e.target.value)} - value={courseName} - /> - +
+ +
+ + setCourseName(e.target.value)} + value={courseName} + /> +
+ + +
); } diff --git a/frontend/src/pages/Admin/__tests__/CourseAdd.js b/frontend/src/pages/Admin/__tests__/CourseAdd.js index 8e090ec7e4..af9061d6e3 100644 --- a/frontend/src/pages/Admin/__tests__/CourseAdd.js +++ b/frontend/src/pages/Admin/__tests__/CourseAdd.js @@ -2,21 +2,36 @@ import React from 'react'; import { render, screen, + fireEvent, + waitFor, } from '@testing-library/react'; import fetchMock from 'fetch-mock'; import CourseAdd from '../CourseAdd'; describe('CourseAdd', () => { const mockRefresh = jest.fn(); + const renderCourseAdd = () => { + render(); + }; beforeEach(() => { fetchMock.reset(); - render(); }); it('renders the CourseAdd component', () => { - expect(screen.getByText('Add a new course')).toBeInTheDocument(); + renderCourseAdd(); expect(screen.getByLabelText('Course name')).toBeInTheDocument(); expect(screen.getByTestId('add-course')).toBeInTheDocument(); }); + + it('calls createCourseByName and refresh on form submit', async () => { + fetchMock.post('/api/courses', {}); + renderCourseAdd(); + + fireEvent.change(screen.getByLabelText('Course name'), { target: { value: 'New Course' } }); + fireEvent.click(screen.getByTestId('add-course')); + + await waitFor(() => expect(fetchMock.called('/api/courses')).toBe(true)); + await waitFor(() => expect(mockRefresh).toHaveBeenCalled()); + }); }); From 2539aa5e83ca7ed4207a11c34127c7af954ad8b9 Mon Sep 17 00:00:00 2001 From: nvms Date: Tue, 11 Jun 2024 13:10:19 -0400 Subject: [PATCH 08/19] catch if destroy throws --- src/routes/courses/handlers.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/routes/courses/handlers.ts b/src/routes/courses/handlers.ts index 36a2f2d5f7..83a52f5bf6 100644 --- a/src/routes/courses/handlers.ts +++ b/src/routes/courses/handlers.ts @@ -103,7 +103,12 @@ export async function deleteCourseById(req: Request, res: Response) { res.status(404).send('Course not found'); return; } - await course.destroy(); + try { + await course.destroy(); + } catch (error) { + res.status(500).send('Could not destroy course.'); + return; + } res.status(204).send(); } catch (err) { await handleErrors(err, req, res, logContext); From c365ee4759ca5e927b9b2b030d5ca4544cc86613 Mon Sep 17 00:00:00 2001 From: nvms Date: Tue, 11 Jun 2024 13:10:31 -0400 Subject: [PATCH 09/19] handler tests --- src/routes/courses/handlers.test.js | 54 +++++++++++++++++++++++++++-- 1 file changed, 52 insertions(+), 2 deletions(-) diff --git a/src/routes/courses/handlers.test.js b/src/routes/courses/handlers.test.js index 4452d74f8d..f2ecbb2a7a 100644 --- a/src/routes/courses/handlers.test.js +++ b/src/routes/courses/handlers.test.js @@ -1,6 +1,16 @@ import db from '../../models'; -import { allCourses, getCourseUrlWidgetDataWithCache } from './handlers'; -import { getAllCourses } from '../../services/course'; +import { + allCourses, + getCourseUrlWidgetDataWithCache, + getCourseById, + updateCourseById, + createCourseByName, +} from './handlers'; +import { + getAllCourses, + getCourseById as getById, + createCourseByName as createCourse, +} from '../../services/course'; import handleErrors from '../../lib/apiErrorHandler'; import { getUserReadRegions } from '../../services/accessValidation'; import { getCourseUrlWidgetData } from '../../services/dashboards/course'; @@ -45,6 +55,46 @@ describe('Courses handlers', () => { expect(handleErrors).toHaveBeenCalled(); }); + describe('getCourseById', () => { + it('should return a course by id', async () => { + const course = { id: 1, name: 'Test Course' }; + getById.mockResolvedValue(course); + const req = { + params: { id: 1 }, + }; + await getCourseById(req, mockResponse); + expect(mockResponse.json).toHaveBeenCalledWith(course); + }); + }); + + describe('updateCourseById', () => { + it('should update a course by id', async () => { + const course = { id: 1, name: 'Test Course', update: jest.fn() }; + getById.mockResolvedValue(course); + const req = { + session: { userId: 1 }, + params: { id: 1 }, + body: { name: 'Updated Course' }, + }; + await updateCourseById(req, mockResponse); + expect(mockResponse.json).toHaveBeenCalled(); + expect(course.update).toHaveBeenCalledWith({ name: 'Updated Course' }); + }); + }); + + describe('createCourseByName', () => { + it('should create a course by name', async () => { + const course = { id: 1, name: 'Test Course' }; + createCourse.mockResolvedValue(course); + const req = { + session: { userId: 1 }, + body: { name: 'Test Course' }, + }; + await createCourseByName(req, mockResponse); + expect(mockResponse.json).toHaveBeenCalledWith(course); + }); + }); + describe('getCourseUrlsWidgetData', () => { it('should return all course url widget data', async () => { const responseData = [ From 9f4ebfbd65c99e85351eded195cea2931cffa400 Mon Sep 17 00:00:00 2001 From: nvms Date: Tue, 11 Jun 2024 14:20:23 -0400 Subject: [PATCH 10/19] use mapsTo --- frontend/src/pages/Admin/CourseEdit.js | 1 + src/routes/courses/handlers.ts | 12 ++++++++++-- src/services/course.ts | 2 +- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/frontend/src/pages/Admin/CourseEdit.js b/frontend/src/pages/Admin/CourseEdit.js index 27b6bc239f..52a7c2cd1f 100644 --- a/frontend/src/pages/Admin/CourseEdit.js +++ b/frontend/src/pages/Admin/CourseEdit.js @@ -51,6 +51,7 @@ function CourseEdit({ match }) { const saveChanges = async () => { const updated = await updateCourseById(courseId, { name: newCourse.name }); setCourse({ ...updated }); + history.replace(`/admin/course/${updated.id}`); }; const confirmDelete = async () => { diff --git a/src/routes/courses/handlers.ts b/src/routes/courses/handlers.ts index 83a52f5bf6..5c97b894fd 100644 --- a/src/routes/courses/handlers.ts +++ b/src/routes/courses/handlers.ts @@ -59,8 +59,16 @@ export async function updateCourseById(req: Request, res: Response) { res.status(404).send('Course not found'); return; } - const updated = await course.update(req.body); - res.json(updated); + + const newCourse = await createCourse(req.body.name); + + await course.update({ + mapsTo: newCourse.id, + }); + + await course.destroy(); + + res.json(newCourse); } catch (err) { await handleErrors(err, req, res, logContext); } diff --git a/src/services/course.ts b/src/services/course.ts index 27fb9a1846..f4b64406a7 100644 --- a/src/services/course.ts +++ b/src/services/course.ts @@ -28,7 +28,7 @@ export async function getCourseById(id: number) { } export async function createCourseByName(name: string) { - return Course.create({ name }); + return Course.create({ name, persistsOnUpload: true }); } export async function csvImport(buffer: Buffer | string) { From 50fca1baa659f1a9eb2c8cf21c23f46556ce5556 Mon Sep 17 00:00:00 2001 From: nvms Date: Tue, 11 Jun 2024 15:47:36 -0400 Subject: [PATCH 11/19] update test --- src/routes/courses/handlers.test.js | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/routes/courses/handlers.test.js b/src/routes/courses/handlers.test.js index f2ecbb2a7a..c03f7dfd11 100644 --- a/src/routes/courses/handlers.test.js +++ b/src/routes/courses/handlers.test.js @@ -69,16 +69,27 @@ describe('Courses handlers', () => { describe('updateCourseById', () => { it('should update a course by id', async () => { - const course = { id: 1, name: 'Test Course', update: jest.fn() }; + const course = { + id: 1, + name: 'Test Course 1', + update: jest.fn(), + destroy: jest.fn(), + }; + const newCourse = { id: 2, name: 'Test Course 2' }; + getById.mockResolvedValue(course); + createCourse.mockResolvedValue(newCourse); + const req = { session: { userId: 1 }, params: { id: 1 }, body: { name: 'Updated Course' }, }; + await updateCourseById(req, mockResponse); - expect(mockResponse.json).toHaveBeenCalled(); - expect(course.update).toHaveBeenCalledWith({ name: 'Updated Course' }); + + expect(course.update).toHaveBeenCalledWith({ mapsTo: newCourse.id }); + expect(course.destroy).toHaveBeenCalled(); }); }); From 284e7cba8d138878c53d7b50ea58970b1771d770 Mon Sep 17 00:00:00 2001 From: Nathan Powell Date: Mon, 17 Jun 2024 07:39:19 -0700 Subject: [PATCH 12/19] Close all r10 goals and complete objectives --- ...0614000000-r10-clear-all-goals-and-objs.js | 106 ++++++++++++++++++ 1 file changed, 106 insertions(+) create mode 100644 src/migrations/20240614000000-r10-clear-all-goals-and-objs.js diff --git a/src/migrations/20240614000000-r10-clear-all-goals-and-objs.js b/src/migrations/20240614000000-r10-clear-all-goals-and-objs.js new file mode 100644 index 0000000000..f48340022c --- /dev/null +++ b/src/migrations/20240614000000-r10-clear-all-goals-and-objs.js @@ -0,0 +1,106 @@ +const { + prepMigration, +} = require('../lib/migration'); + +module.exports = { + up: async (queryInterface) => queryInterface.sequelize.transaction( + async (transaction) => { + await prepMigration(queryInterface, transaction, __filename); + await queryInterface.sequelize.query(/* sql */` + + -- 1. Get all r10 goals that are currently visible. + WITH r_10_active_goals AS ( + SELECT DISTINCT + g.* + FROM "Grants" gr + JOIN "Goals" g + ON gr.id = g."grantId" + WHERE gr."regionId" = 10 + AND g.status != 'Closed' + AND g."deletedAt" IS NULL + AND g."mapsToParentGoalId" IS NULL + ), + -- 2. insert the status changes for the goals and return the important elements + log_status_change AS ( + INSERT INTO "GoalStatusChanges"( + "goalId", + "userId", + "userName", + "userRoles", + "oldStatus", + "newStatus", + "reason", + "context", + "createdAt", + "updatedAt" + ) + SELECT + g.id "goalId", + u.id, + u.name, + ARRAY_AGG(ro.name), + g.status "oldSataus", + 'Closed' "newStatus", + 'TTA completed' "reason", + 'Close all goals to move to new goal language' "context", + now() "createdAt", + now() "updatedAt" + FROM r_10_active_goals g + LEFT JOIN "Users" u + ON u.name = 'Melissa Bandy-Ogden' + LEFT JOIN "UserRoles" ur + ON u.id = ur."userId" + LEFT JOIN "Roles" ro + ON ur."roleId" = ro.id + GROUP BY 1,2,3,5,6,7,8,9,10 + RETURNING + id, + "goalId", + "newStatus", + "updatedAt" + ), + -- 3. Update the actual goals + update_goals AS ( + UPDATE "Goals" g + SET + "status" = lsc."newStatus", + "updatedAt" = lsc."updatedAt" + FROM log_status_change lsc + WHERE g.id = lsc."goalId" + RETURNING + g.id "goalId", + g.status + ), + -- 4. Update the objectives attached to the goals + update_objectives AS ( + UPDATE "Objectives" o + SET + "status" = 'Complete', + "updatedAt" = NOW() + FROM log_status_change lsc + WHERE o."goalId" = lsc."goalId" + RETURNING + o.id "objectiveId", + o.status + ) + -- 5. show stats for what was done when testing. + SELECT + 'goals updated' stat, + COUNT("goalId") statcnt + FROM update_goals + UNION + SELECT + 'objectives updated', + COUNT("objectiveId") + FROM update_objectives; + `, { transaction }); + }, + ), + + down: async (queryInterface) => queryInterface.sequelize.transaction( + async (transaction) => { + await prepMigration(queryInterface, transaction, __filename); + // If we end up needing to revert this, it would be easier to use a separate + // migration using the txid (or a similar identifier) after it's already set + ), +}; From 801ecd7c9394cf578375ef1ddbfb4dd5ed52a8c2 Mon Sep 17 00:00:00 2001 From: Nathan Powell Date: Mon, 17 Jun 2024 07:48:24 -0700 Subject: [PATCH 13/19] restore missing line --- src/migrations/20240614000000-r10-clear-all-goals-and-objs.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/migrations/20240614000000-r10-clear-all-goals-and-objs.js b/src/migrations/20240614000000-r10-clear-all-goals-and-objs.js index f48340022c..2021b86d3d 100644 --- a/src/migrations/20240614000000-r10-clear-all-goals-and-objs.js +++ b/src/migrations/20240614000000-r10-clear-all-goals-and-objs.js @@ -102,5 +102,6 @@ module.exports = { await prepMigration(queryInterface, transaction, __filename); // If we end up needing to revert this, it would be easier to use a separate // migration using the txid (or a similar identifier) after it's already set + }, ), }; From 0e93eab1df6e0581e9d561a7760fdacb500e690d Mon Sep 17 00:00:00 2001 From: Nathan Powell Date: Mon, 17 Jun 2024 07:55:51 -0700 Subject: [PATCH 14/19] remove extraneous whitespace --- src/migrations/20240614000000-r10-clear-all-goals-and-objs.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/migrations/20240614000000-r10-clear-all-goals-and-objs.js b/src/migrations/20240614000000-r10-clear-all-goals-and-objs.js index 2021b86d3d..e4ccff0bf9 100644 --- a/src/migrations/20240614000000-r10-clear-all-goals-and-objs.js +++ b/src/migrations/20240614000000-r10-clear-all-goals-and-objs.js @@ -69,7 +69,7 @@ module.exports = { WHERE g.id = lsc."goalId" RETURNING g.id "goalId", - g.status + g.status ), -- 4. Update the objectives attached to the goals update_objectives AS ( @@ -81,7 +81,7 @@ module.exports = { WHERE o."goalId" = lsc."goalId" RETURNING o.id "objectiveId", - o.status + o.status ) -- 5. show stats for what was done when testing. SELECT From d5071774293deb065a470245bf21a2ff94074aeb Mon Sep 17 00:00:00 2001 From: nvms Date: Thu, 20 Jun 2024 14:25:28 -0400 Subject: [PATCH 15/19] use Link --- frontend/src/pages/Admin/CourseList.js | 3 ++- .../src/pages/Admin/__tests__/CourseList.js | 20 ++++++++++++++++--- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/frontend/src/pages/Admin/CourseList.js b/frontend/src/pages/Admin/CourseList.js index 568bd863ab..ce40a92dda 100644 --- a/frontend/src/pages/Admin/CourseList.js +++ b/frontend/src/pages/Admin/CourseList.js @@ -1,6 +1,7 @@ import React, { useState } from 'react'; import PropTypes from 'prop-types'; import { Label, TextInput } from '@trussworks/react-uswds'; +import { Link } from 'react-router-dom'; function CourseList({ courses }) { const [filter, setFilter] = useState(''); @@ -33,7 +34,7 @@ function CourseList({ courses }) { /> {filteredCourses.map((course) => (
- {course.name} + {course.name}
))}
diff --git a/frontend/src/pages/Admin/__tests__/CourseList.js b/frontend/src/pages/Admin/__tests__/CourseList.js index 61cc9301c3..abe0dbd24c 100644 --- a/frontend/src/pages/Admin/__tests__/CourseList.js +++ b/frontend/src/pages/Admin/__tests__/CourseList.js @@ -1,9 +1,11 @@ +import { Router } from 'react-router'; import React from 'react'; import { render, screen, fireEvent, } from '@testing-library/react'; +import { createMemoryHistory } from 'history'; import CourseList from '../CourseList'; describe('CourseList', () => { @@ -13,13 +15,24 @@ describe('CourseList', () => { { id: 3, name: 'Data Structures and Kangaroos' }, ]; + const renderComponent = (c) => { + const history = createMemoryHistory(); + render( + + + , + ); + }; + it('renders without courses', () => { - render(); + // render(); + renderComponent([]); expect(screen.queryByLabelText('Filter courses by name')).toBeInTheDocument(); }); it('renders with courses and allows filtering', () => { - render(); + // render(); + renderComponent(); expect(screen.getByLabelText('Filter courses by name')).toBeInTheDocument(); expect(screen.getByText('Introduction to Dogs')).toBeInTheDocument(); expect(screen.getByText('Advanced Penguins')).toBeInTheDocument(); @@ -32,7 +45,8 @@ describe('CourseList', () => { }); it('handles no matching filter results', () => { - render(); + // render(); + renderComponent(); fireEvent.change(screen.getByLabelText('Filter courses by name'), { target: { value: 'asdf' } }); expect(screen.queryByText('Introduction to Dogs')).not.toBeInTheDocument(); expect(screen.queryByText('Advanced Penguins')).not.toBeInTheDocument(); From 199fa424835bc1568bd3e849087692563daa9651 Mon Sep 17 00:00:00 2001 From: nvms Date: Thu, 20 Jun 2024 14:52:03 -0400 Subject: [PATCH 16/19] update test --- frontend/src/pages/Admin/__tests__/Courses.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/frontend/src/pages/Admin/__tests__/Courses.js b/frontend/src/pages/Admin/__tests__/Courses.js index 2066027a83..4a4f275e7f 100644 --- a/frontend/src/pages/Admin/__tests__/Courses.js +++ b/frontend/src/pages/Admin/__tests__/Courses.js @@ -1,16 +1,23 @@ import '@testing-library/jest-dom'; +import { Router } from 'react-router'; import React from 'react'; import { render, act, screen, } from '@testing-library/react'; +import { createMemoryHistory } from 'history'; import fetchMock from 'fetch-mock'; import Courses from '../Courses'; describe('Courses', () => { const renderTest = () => { - render(); + const history = createMemoryHistory(); + render( + + + , + ); }; beforeEach(() => { From 7210e5990ed9fc1755523c215be93bfe4c36c68d Mon Sep 17 00:00:00 2001 From: Nathan Powell Date: Fri, 21 Jun 2024 08:35:32 -0700 Subject: [PATCH 17/19] change to close all open r10 objectives --- .../20240614000000-r10-clear-all-goals-and-objs.js | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/migrations/20240614000000-r10-clear-all-goals-and-objs.js b/src/migrations/20240614000000-r10-clear-all-goals-and-objs.js index e4ccff0bf9..1d5f34670b 100644 --- a/src/migrations/20240614000000-r10-clear-all-goals-and-objs.js +++ b/src/migrations/20240614000000-r10-clear-all-goals-and-objs.js @@ -71,14 +71,20 @@ module.exports = { g.id "goalId", g.status ), - -- 4. Update the objectives attached to the goals + -- 4. Update all objectives attached to any R10 goal update_objectives AS ( UPDATE "Objectives" o SET "status" = 'Complete', "updatedAt" = NOW() - FROM log_status_change lsc - WHERE o."goalId" = lsc."goalId" + FROM "Grants" gr + JOIN "Goals" g + ON g."grantId" = gr.id + AND gr."regionId" = 10 + WHERE o."goalId" = g.id + AND o."status" != 'Complete' + AND o."deletedAt" IS NULL + AND o."mapsToParentObjectiveId" IS NULL RETURNING o.id "objectiveId", o.status From acc4a713b01829057b65a69d5aafdebb1c47e32e Mon Sep 17 00:00:00 2001 From: Nathan Powell Date: Fri, 21 Jun 2024 08:42:24 -0700 Subject: [PATCH 18/19] clear tabs for linter --- .../20240614000000-r10-clear-all-goals-and-objs.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/migrations/20240614000000-r10-clear-all-goals-and-objs.js b/src/migrations/20240614000000-r10-clear-all-goals-and-objs.js index 1d5f34670b..f0177ec875 100644 --- a/src/migrations/20240614000000-r10-clear-all-goals-and-objs.js +++ b/src/migrations/20240614000000-r10-clear-all-goals-and-objs.js @@ -80,11 +80,11 @@ module.exports = { FROM "Grants" gr JOIN "Goals" g ON g."grantId" = gr.id - AND gr."regionId" = 10 + AND gr."regionId" = 10 WHERE o."goalId" = g.id AND o."status" != 'Complete' - AND o."deletedAt" IS NULL - AND o."mapsToParentObjectiveId" IS NULL + AND o."deletedAt" IS NULL + AND o."mapsToParentObjectiveId" IS NULL RETURNING o.id "objectiveId", o.status From c0d30c41eafa39f72c156ad32e18e2ce1adbab9d Mon Sep 17 00:00:00 2001 From: Matt Bevilacqua Date: Mon, 24 Jun 2024 15:06:12 -0400 Subject: [PATCH 19/19] Update requested TRs and sessions --- .../20240624185931-unlock-tr-and-sessions.js | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 src/migrations/20240624185931-unlock-tr-and-sessions.js diff --git a/src/migrations/20240624185931-unlock-tr-and-sessions.js b/src/migrations/20240624185931-unlock-tr-and-sessions.js new file mode 100644 index 0000000000..447cdbf5d0 --- /dev/null +++ b/src/migrations/20240624185931-unlock-tr-and-sessions.js @@ -0,0 +1,27 @@ +const { + prepMigration, +} = require('../lib/migration'); + +module.exports = { + up: async (queryInterface) => queryInterface.sequelize.transaction( + async (transaction) => { + await prepMigration(queryInterface, transaction, __filename); + await queryInterface.sequelize.query(/* sql */` + UPDATE "SessionReportPilots" + SET data = jsonb_set(data, '{status}', '"In progress"', true) + WHERE "eventId" = 48; + + UPDATE "EventReportPilots" + SET data = jsonb_set(data, '{status}', '"In progress"', true) + WHERE "id" = 48; + `, { transaction }); + }, + ), + + down: async (queryInterface) => queryInterface.sequelize.transaction( + async (transaction) => { + await prepMigration(queryInterface, transaction, __filename); + // No down migration needed here + }, + ), +};