From 16c0ce0992ea6473f9b5a9c5bb9b3b582d734d9f Mon Sep 17 00:00:00 2001 From: Kevin Buffardi Date: Thu, 20 Aug 2026 21:43:29 -0700 Subject: [PATCH 1/2] feat(create): persist ordered module resources Store zero-based ModuleResources ordering, return it across module and course reads, and make resource selection sortable and removable. Order learner Demo attempts by the existing exercise order.\n\nCloses #182 --- codewit/api/src/controllers/course.ts | 26 ++- codewit/api/src/controllers/module.ts | 31 ++- ...0820120000-add-module-resource-ordering.js | 37 ++++ codewit/api/src/models/index.ts | 7 +- codewit/api/src/models/moduleResources.ts | 46 +++++ codewit/api/src/routes/course.ts | 6 +- codewit/api/src/routes/demo.ts | 10 +- codewit/client-e2e/src/e2e/app.cy.ts | 32 +++ .../src/components/form/ResourceSelect.tsx | 182 +++++++++++++----- codewit/client/src/pages/ModuleForm.tsx | 43 +++-- .../shared/interfaces/src/lib/interfaces.ts | 4 +- .../shared/validations/src/lib/module.spec.ts | 19 ++ .../lib/shared/validations/src/lib/module.ts | 11 +- 13 files changed, 380 insertions(+), 74 deletions(-) create mode 100644 codewit/api/src/migrations/20260820120000-add-module-resource-ordering.js create mode 100644 codewit/api/src/models/moduleResources.ts create mode 100644 codewit/lib/shared/validations/src/lib/module.spec.ts diff --git a/codewit/api/src/controllers/course.ts b/codewit/api/src/controllers/course.ts index 6d5b4a58..581a4670 100644 --- a/codewit/api/src/controllers/course.ts +++ b/codewit/api/src/controllers/course.ts @@ -12,6 +12,7 @@ import { CourseModules, Language, Module, + ModuleResources, Demo, Resource, UserDemoCompletion, @@ -223,7 +224,10 @@ async function getAllCourses(): Promise { { association: Course.associations.instructors }, { association: Course.associations.roster }, ], - order: [[Course.associations.modules, CourseModules, 'ordering', 'ASC']], + order: [ + [Course.associations.modules, CourseModules, 'ordering', 'ASC'], + [Course.associations.modules, Module.associations.resources, ModuleResources, 'ordering', 'ASC'], + ], }); return formatCourseResponse(courses); } @@ -241,7 +245,10 @@ async function getTeacherCourses(teacherId: string): Promise { { association: Course.associations.instructors, where: { googleId: teacherId } }, { association: Course.associations.roster }, ], - order: [[Course.associations.modules, CourseModules, 'ordering', 'ASC']], + order: [ + [Course.associations.modules, CourseModules, 'ordering', 'ASC'], + [Course.associations.modules, Module.associations.resources, ModuleResources, 'ordering', 'ASC'], + ], }); return formatCourseResponse(courses); @@ -259,7 +266,10 @@ async function getStudentCourses(studentId: string): Promise { { association: Course.associations.instructors }, { association: Course.associations.roster, where: { googleId: studentId } }, ], - order: [[Course.associations.modules, CourseModules, 'ordering', 'ASC']], + order: [ + [Course.associations.modules, CourseModules, 'ordering', 'ASC'], + [Course.associations.modules, Module.associations.resources, ModuleResources, 'ordering', 'ASC'], + ], }); return formatCourseResponse(courses, true); @@ -277,7 +287,10 @@ async function getStudentCoursesByUid(userUid: number): Promise ({ + moduleUid: module.uid, + resourceUid, + ordering, + })), + { transaction }, + ); + await module.reload({ + include: [Language, Demo, Resource], + order: [[Module.associations.resources, ModuleResources, 'ordering', 'ASC']], + transaction, + }); return formatModuleResponse(module); }); @@ -40,6 +51,7 @@ async function createModule( async function getModule(uid: number): Promise { const module = await Module.findByPk(uid, { include: [Language, Demo, Resource], + order: [[Module.associations.resources, ModuleResources, 'ordering', 'ASC']], }); return formatModuleResponse(module); @@ -72,7 +84,15 @@ async function updateModule( await module.setLanguage(languageInstance, { transaction }); } if (resources) { - await module.setResources(resources, { transaction }); + await ModuleResources.destroy({ where: { moduleUid: module.uid }, transaction }); + await ModuleResources.bulkCreate( + resources.map((resourceUid, ordering) => ({ + moduleUid: module.uid, + resourceUid, + ordering, + })), + { transaction }, + ); } await module.save({ transaction }); @@ -94,6 +114,7 @@ async function updateModule( await module.reload({ include: [Language, Demo, Resource], + order: [[Module.associations.resources, ModuleResources, 'ordering', 'ASC']], transaction, }); @@ -104,6 +125,7 @@ async function updateModule( async function getModules(): Promise { const modules = await Module.findAll({ include: [Language, Demo, Resource], + order: [[Module.associations.resources, ModuleResources, 'ordering', 'ASC']], }); return formatModuleResponse(modules); @@ -112,6 +134,7 @@ async function getModules(): Promise { async function deleteModule(uid: number): Promise { const module = await Module.findByPk(uid, { include: [Language, Demo, Resource], + order: [[Module.associations.resources, ModuleResources, 'ordering', 'ASC']], }); if (!module) { return null; diff --git a/codewit/api/src/migrations/20260820120000-add-module-resource-ordering.js b/codewit/api/src/migrations/20260820120000-add-module-resource-ordering.js new file mode 100644 index 00000000..dc9a986f --- /dev/null +++ b/codewit/api/src/migrations/20260820120000-add-module-resource-ordering.js @@ -0,0 +1,37 @@ +/** @type {import('sequelize-cli').Migration} */ +module.exports = { + up: (queryInterface, Sequelize) => queryInterface.sequelize.transaction(async transaction => { + await queryInterface.addColumn( + 'ModuleResources', + 'ordering', + { + type: Sequelize.INTEGER, + allowNull: false, + defaultValue: 0, + }, + { transaction }, + ); + + await queryInterface.sequelize.query( + ` + with ordered_resources as ( + select "moduleUid" as module_uid, + "resourceUid" as resource_uid, + row_number() over ( + partition by "moduleUid" + order by "createdAt", "resourceUid" + ) - 1 as ordering + from "ModuleResources" + ) + update "ModuleResources" + set ordering = ordered_resources.ordering + from ordered_resources + where "ModuleResources"."moduleUid" = ordered_resources.module_uid + and "ModuleResources"."resourceUid" = ordered_resources.resource_uid`, + { type: Sequelize.QueryTypes.RAW, transaction }, + ); + }), + down: (queryInterface, Sequelize) => queryInterface.sequelize.transaction(async transaction => { + await queryInterface.removeColumn('ModuleResources', 'ordering', { transaction }); + }), +}; diff --git a/codewit/api/src/models/index.ts b/codewit/api/src/models/index.ts index e37affb7..0f44e34c 100644 --- a/codewit/api/src/models/index.ts +++ b/codewit/api/src/models/index.ts @@ -14,6 +14,7 @@ import { UserExerciseCompletion } from './userExerciseCompletion'; import { UserModuleCompletion } from './userModuleCompletion'; import { DemoExercises } from './demoExercises'; import { ModuleDemos } from './moduleDemos'; +import { ModuleResources } from './moduleResources'; require('dotenv').config(); @@ -60,6 +61,7 @@ const sequelize = new Sequelize({ UserModuleCompletion, DemoExercises, ModuleDemos, + ModuleResources, ].forEach((model) => model.initialize(sequelize)); Demo.belongsToMany(Exercise, { through: DemoExercises }); @@ -96,8 +98,8 @@ Language.hasMany(Exercise, { foreignKey: 'languageUid' }); Demo.belongsToMany(Module, { through: ModuleDemos }); Module.belongsToMany(Demo, { through: ModuleDemos }); -Resource.belongsToMany(Module, { through: 'ModuleResources' }); -Module.belongsToMany(Resource, { through: 'ModuleResources' }); +Resource.belongsToMany(Module, { through: ModuleResources }); +Module.belongsToMany(Resource, { through: ModuleResources }); Module.belongsTo(Language); Language.hasMany(Module); @@ -175,6 +177,7 @@ export { Course, Module, ModuleDemos, + ModuleResources, CourseModules, CourseRegistration, Resource, diff --git a/codewit/api/src/models/moduleResources.ts b/codewit/api/src/models/moduleResources.ts new file mode 100644 index 00000000..82e17749 --- /dev/null +++ b/codewit/api/src/models/moduleResources.ts @@ -0,0 +1,46 @@ +import { + DataTypes, + InferAttributes, + InferCreationAttributes, + Model, + Sequelize, +} from 'sequelize'; + +class ModuleResources extends Model< + InferAttributes, + InferCreationAttributes +> { + declare moduleUid: number; + declare resourceUid: number; + declare ordering: number; + + static initialize(sequelize: Sequelize) { + this.init( + { + moduleUid: { + type: DataTypes.INTEGER, + primaryKey: true, + references: { model: 'modules', key: 'uid' }, + }, + resourceUid: { + type: DataTypes.INTEGER, + primaryKey: true, + references: { model: 'resources', key: 'uid' }, + }, + ordering: { + type: DataTypes.INTEGER, + allowNull: false, + defaultValue: 0, + }, + }, + { + sequelize, + modelName: 'ModuleResources', + tableName: 'ModuleResources', + timestamps: true, + }, + ); + } +} + +export { ModuleResources }; diff --git a/codewit/api/src/routes/course.ts b/codewit/api/src/routes/course.ts index 206ec67f..d7dee558 100644 --- a/codewit/api/src/routes/course.ts +++ b/codewit/api/src/routes/course.ts @@ -29,6 +29,7 @@ import { CourseRegistration, Resource, Module, + ModuleResources, } from '../models'; import { asyncHandle } from "../middleware/catch"; import { } from "../models"; @@ -275,7 +276,10 @@ courseRouter.get('/:uid', asyncHandle(async (req, res) => { { association: Course.associations.instructors, where: { uid: req.user.uid } }, { association: Course.associations.roster }, ], - order: [[Course.associations.modules, CourseModules, 'ordering', 'ASC']], + order: [ + [Course.associations.modules, CourseModules, 'ordering', 'ASC'], + [Course.associations.modules, Module.associations.resources, ModuleResources, 'ordering', 'ASC'], + ], }); let result = formatCourseResponse(course, true); diff --git a/codewit/api/src/routes/demo.ts b/codewit/api/src/routes/demo.ts index 4f06d78c..4c517529 100644 --- a/codewit/api/src/routes/demo.ts +++ b/codewit/api/src/routes/demo.ts @@ -22,7 +22,7 @@ import { DemoAttempt } from "@codewit/interfaces"; import { fromZodError } from 'zod-validation-error'; import { checkAdmin } from '../middleware/auth'; import { asyncHandle } from '../middleware/catch'; -import { Attempt, Demo, DemoTags, Language, sequelize, Tag, UserExerciseCompletion } from '../models'; +import { Attempt, Demo, DemoExercises, DemoTags, Language, sequelize, Tag, UserExerciseCompletion } from '../models'; import { Op, QueryTypes } from 'sequelize'; const demoRouter = Router(); @@ -84,7 +84,10 @@ demoRouter.get("/:uid/attempt", asyncHandle(async (req, res) => { Tag, Language ], - order: [[Tag, DemoTags, "ordering", "ASC"]] + order: [ + [Tag, DemoTags, "ordering", "ASC"], + [Demo.associations.exercises, DemoExercises, "order", "ASC"], + ] }); if (demo_record == null) { @@ -166,7 +169,8 @@ demoRouter.get("/:uid/attempt", asyncHandle(async (req, res) => { mod_resc."moduleUid" = $1 left join "ResourceLikes" as resc_likes on resources.uid = resc_likes."resourceUid" and - resc_likes."userUid" = $2`, + resc_likes."userUid" = $2 + order by mod_resc.ordering asc`, { type: QueryTypes.SELECT, bind: [maybe_module_id, req.user.uid] diff --git a/codewit/client-e2e/src/e2e/app.cy.ts b/codewit/client-e2e/src/e2e/app.cy.ts index 39b28dbe..6e33e6fa 100644 --- a/codewit/client-e2e/src/e2e/app.cy.ts +++ b/codewit/client-e2e/src/e2e/app.cy.ts @@ -613,9 +613,17 @@ describe("Module creations functionality", () => { statusCode: 200, body: [] }).as('getModules'); + cy.intercept('GET', '/resources', { + statusCode: 200, + body: [ + { uid: 1, title: 'First resource', url: 'https://example.com/first', source: 'Example', likes: 0 }, + { uid: 2, title: 'Second resource', url: 'https://example.com/second', source: 'Example', likes: 0 }, + ], + }).as('getResources'); cy.visit('/create/module'); cy.wait('@getUserInfo'); cy.wait('@getModules'); + cy.wait('@getResources'); }) it('should render successfully', () => { @@ -642,6 +650,30 @@ describe("Module creations functionality", () => { getSubmitButton().click(); cy.wait('@createModule'); }) + + it('adds, reorders, and removes selected resources before saving', () => { + cy.intercept('POST', '/modules', (req) => { + expect(req.body).to.deep.equal({ + language: 'cpp', + resources: [2], + topic: 'operation', + }); + }).as('createOrderedModule'); + + cy.contains('Create Module').click(); + getTopicSelect().type('operation{enter}'); + getLanguageSelect().type('cpp{enter}'); + cy.get('#resource-select').type('First resource{enter}'); + cy.get('#resource-select').type('Second resource{enter}'); + + cy.get('[data-testid="selected-resources"]').should('contain.text', 'First resource'); + cy.get('[data-testid="selected-resources"]').should('contain.text', 'Second resource'); + cy.get('[aria-label="Drag Second resource"]').focus().type('{space}{uparrow}{space}'); + cy.get('[aria-label="Remove First resource"]').click(); + + getSubmitButton().click(); + cy.wait('@createOrderedModule'); + }); }) diff --git a/codewit/client/src/components/form/ResourceSelect.tsx b/codewit/client/src/components/form/ResourceSelect.tsx index b3603a31..c10dee44 100644 --- a/codewit/client/src/components/form/ResourceSelect.tsx +++ b/codewit/client/src/components/form/ResourceSelect.tsx @@ -1,43 +1,139 @@ -// codewit/client/src/components/form/ResourceSelect.tsx -import Select, { MultiValue } from 'react-select'; -import { SelectedTag, Resource } from '@codewit/interfaces'; -import { SelectStyles } from '../../utils/styles'; - -interface ResourceSelectProps { - resourceOptions: SelectedTag[]; - selectedResources: Resource[]; - handleResourceChange: (selectedOptions: MultiValue) => void; -} - -const ResourceSelect = ({ - resourceOptions, - selectedResources, - handleResourceChange -}: ResourceSelectProps) => { - return ( -
- - { + if (option != null) { + onAddResource(option.value); + } + }} + options={availableResources} + isSearchable + placeholder="Search resources" + styles={SelectStyles} + /> + + +
+ {selectedResources.map(resource => ( + onRemoveResource(resource.value)} + /> + ))} +
+ + +
+ ); +}; + +export default ResourceSelect; diff --git a/codewit/client/src/pages/ModuleForm.tsx b/codewit/client/src/pages/ModuleForm.tsx index cf2a751d..2813a36c 100644 --- a/codewit/client/src/pages/ModuleForm.tsx +++ b/codewit/client/src/pages/ModuleForm.tsx @@ -1,6 +1,5 @@ // codewit/client/src/pages/ModuleForm.tsx import React, { useState, useEffect } from "react"; -import Select, { MultiValue } from "react-select"; import LanguageSelect from "../components/form/LanguageSelect"; import TopicSelect from "../components/form/TagSelect"; import ResourceSelect from "../components/form/ResourceSelect"; @@ -8,7 +7,7 @@ import CreateButton from "../components/form/CreateButton"; import ReusableTable, { Column } from "../components/form/ReusableTable"; import ReusableModal from "../components/form/ReusableModal"; import { toast } from "react-toastify"; -import { SelectedTag, Module } from "@codewit/interfaces"; +import { SelectedTag, Module, ModuleDraft } from "@codewit/interfaces"; import { isFormValid } from "../utils/formValidationUtils"; import { useFetchResources } from "../hooks/useResource"; import { @@ -18,8 +17,6 @@ import { usePatchModule, } from "../hooks/useModule"; -type ModuleDraft = Omit; - const ModuleForm = (): JSX.Element => { const { data: existingResources } = useFetchResources(); const { data: existingModules, setData: setExistingModules } = useFetchModules(); @@ -48,10 +45,29 @@ const ModuleForm = (): JSX.Element => { setResourceOptions(options); }, [existingResources]); - const handleResourceChange = (selectedOptions: MultiValue) => { - const resources = selectedOptions.map((option) => option.value); - // @ts-ignore - setFormData((prev) => ({ ...prev, resources })); + const addResource = (resourceId: number) => { + setFormData((prev) => ( + prev.resources.includes(resourceId) + ? prev + : { ...prev, resources: [...prev.resources, resourceId] } + )); + }; + + const moveResource = (fromIndex: number, toIndex: number) => { + setFormData((prev) => { + const resources = [...prev.resources]; + const [resource] = resources.splice(fromIndex, 1); + resources.splice(toIndex, 0, resource); + + return { ...prev, resources }; + }); + }; + + const removeResource = (resourceId: number) => { + setFormData((prev) => ({ + ...prev, + resources: prev.resources.filter(id => id !== resourceId), + })); }; const handleTopicSelect = (topics: SelectedTag | SelectedTag[]) => { @@ -64,8 +80,7 @@ const ModuleForm = (): JSX.Element => { ...module, language: module.language, topic: module.topic, - // @ts-ignore - resources: module.resources.map((resource) => resource.uid), + resources: module.resources.flatMap(resource => resource.uid == null ? [] : [resource.uid]), }); setIsEditing(true); setModalOpen(true); @@ -188,8 +203,10 @@ const ModuleForm = (): JSX.Element => { @@ -197,4 +214,4 @@ const ModuleForm = (): JSX.Element => { ); }; -export default ModuleForm; \ No newline at end of file +export default ModuleForm; diff --git a/codewit/lib/shared/interfaces/src/lib/interfaces.ts b/codewit/lib/shared/interfaces/src/lib/interfaces.ts index 9fa38d5e..d5dc4dcc 100644 --- a/codewit/lib/shared/interfaces/src/lib/interfaces.ts +++ b/codewit/lib/shared/interfaces/src/lib/interfaces.ts @@ -247,7 +247,9 @@ interface AttemptResult { // Payload sent when creating a NEW exercise (no uid yet) export type ExerciseInput = Omit; -export type ModuleDraft = Omit; +export type ModuleDraft = Omit & { + resources: number[]; +}; export type { AttemptDTO, diff --git a/codewit/lib/shared/validations/src/lib/module.spec.ts b/codewit/lib/shared/validations/src/lib/module.spec.ts new file mode 100644 index 00000000..81f594d9 --- /dev/null +++ b/codewit/lib/shared/validations/src/lib/module.spec.ts @@ -0,0 +1,19 @@ +import { createModuleSchema, updateModuleSchema } from './module'; + +describe('module resource validation', () => { + it('rejects duplicate resource IDs when creating a module', () => { + const result = createModuleSchema.safeParse({ + topic: 'operation', + language: 'cpp', + resources: [1, 1], + }); + + expect(result.success).toBe(false); + }); + + it('rejects duplicate resource IDs when updating a module', () => { + const result = updateModuleSchema.safeParse({ resources: [2, 2] }); + + expect(result.success).toBe(false); + }); +}); diff --git a/codewit/lib/shared/validations/src/lib/module.ts b/codewit/lib/shared/validations/src/lib/module.ts index ce062cf5..d753fbb1 100644 --- a/codewit/lib/shared/validations/src/lib/module.ts +++ b/codewit/lib/shared/validations/src/lib/module.ts @@ -1,12 +1,17 @@ import { z } from 'zod'; import { validateTopic } from './topic'; +const uniqueResourceIds = (resources: number[]) => + new Set(resources).size === resources.length; + const createModuleSchema = z.object({ topic: z .string() .refine((t) => validateTopic(t), { message: 'Invalid topic' }), language: z.string(), - resources: z.number().array(), + resources: z.number().array().refine(uniqueResourceIds, { + message: 'Resources must not contain duplicate IDs', + }), }); const updateModuleSchema = z.object({ @@ -15,7 +20,9 @@ const updateModuleSchema = z.object({ .refine((t) => validateTopic(t), { message: 'Invalid topic' }) .optional(), language: z.string().optional(), - resources: z.number().array().optional(), + resources: z.number().array().refine(uniqueResourceIds, { + message: 'Resources must not contain duplicate IDs', + }).optional(), }); export { createModuleSchema, updateModuleSchema }; From bca7cdd2da9c208238dad23af9edde8a83868e96 Mon Sep 17 00:00:00 2001 From: Kevin Buffardi Date: Mon, 14 Sep 2026 00:10:48 -0700 Subject: [PATCH 2/2] test(ordering): cover persisted module and demo order Add API and browser regression coverage for module resource ordering and learner-facing demo exercise order. Correct the merged demo drag operation so the dragged item moves to the target position, and align Cypress intercepts with current API routes.\n\nRefs #182\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- codewit/api/src/controllers/module.spec.ts | 112 +++++++++++++++++ codewit/api/src/routes/demo.spec.ts | 85 +++++++++++++ codewit/api/src/routes/demo.ts | 6 +- codewit/client-e2e/src/e2e/app.cy.ts | 127 +++++++++++++++----- codewit/client-e2e/src/support/app.po.ts | 4 +- codewit/client/src/pages/DemoForm.tsx | 16 ++- codewit/client/src/utils/arrayOrder.spec.ts | 10 ++ codewit/client/src/utils/arrayOrder.ts | 11 ++ 8 files changed, 329 insertions(+), 42 deletions(-) create mode 100644 codewit/api/src/controllers/module.spec.ts create mode 100644 codewit/api/src/routes/demo.spec.ts create mode 100644 codewit/client/src/utils/arrayOrder.spec.ts create mode 100644 codewit/client/src/utils/arrayOrder.ts diff --git a/codewit/api/src/controllers/module.spec.ts b/codewit/api/src/controllers/module.spec.ts new file mode 100644 index 00000000..97308feb --- /dev/null +++ b/codewit/api/src/controllers/module.spec.ts @@ -0,0 +1,112 @@ +import { + Demo, + Language, + Module, + ModuleResources, + Resource, + sequelize, +} from '../models'; +import { createModule, getModule, updateModule } from './module'; + +jest.mock('../models', () => ({ + Demo: { findAll: jest.fn() }, + Language: { findOrCreate: jest.fn() }, + Module: { + associations: { resources: 'resources' }, + create: jest.fn(), + findByPk: jest.fn(), + }, + ModuleResources: { + bulkCreate: jest.fn(), + destroy: jest.fn(), + }, + Resource: {}, + sequelize: { + transaction: jest.fn(async ( + callback: (transaction: object) => Promise, + ) => callback({})), + }, +})); + +const resource = (uid: number) => ({ + get: () => ({ + uid, + title: `Resource ${uid}`, + url: `https://example.com/${uid}`, + source: 'Example', + likes: 0, + }), +}); + +const makeModule = () => ({ + uid: 12, + topic: 'operation', + language: { name: 'cpp' }, + demos: [], + resources: [resource(3), resource(1)], + setDemos: jest.fn(), + setLanguage: jest.fn(), + reload: jest.fn(), + save: jest.fn(), +}); + +describe('module resource ordering', () => { + const transaction = {}; + + beforeEach(() => { + jest.clearAllMocks(); + jest.mocked(Language.findOrCreate).mockResolvedValue([{ uid: 4 }] as never); + jest.mocked(Demo.findAll).mockResolvedValue([]); + }); + + it('persists submitted resource order when creating a module', async () => { + const module = makeModule(); + jest.mocked(Module.create).mockResolvedValue(module as never); + + const created = await createModule('operation', 'cpp', [3, 1]); + + expect(ModuleResources.bulkCreate).toHaveBeenCalledWith( + [ + { moduleUid: 12, resourceUid: 3, ordering: 0 }, + { moduleUid: 12, resourceUid: 1, ordering: 1 }, + ], + { transaction }, + ); + expect(module.reload).toHaveBeenCalledWith(expect.objectContaining({ + order: [['resources', ModuleResources, 'ordering', 'ASC']], + })); + expect(created.resources.map(({ uid }) => uid)).toEqual([3, 1]); + }); + + it('replaces persisted resource order when updating a module', async () => { + const module = makeModule(); + jest.mocked(Module.findByPk).mockResolvedValue(module as never); + + const updated = await updateModule(12, undefined, undefined, [3, 1]); + + expect(ModuleResources.destroy).toHaveBeenCalledWith({ + where: { moduleUid: 12 }, + transaction, + }); + expect(ModuleResources.bulkCreate).toHaveBeenCalledWith( + [ + { moduleUid: 12, resourceUid: 3, ordering: 0 }, + { moduleUid: 12, resourceUid: 1, ordering: 1 }, + ], + { transaction }, + ); + expect(updated.resources.map(({ uid }) => uid)).toEqual([3, 1]); + }); + + it('requests persisted resource order when retrieving a module', async () => { + const module = makeModule(); + jest.mocked(Module.findByPk).mockResolvedValue(module as never); + + const retrieved = await getModule(12); + + expect(Module.findByPk).toHaveBeenCalledWith(12, expect.objectContaining({ + order: [['resources', ModuleResources, 'ordering', 'ASC']], + })); + expect(retrieved?.resources.map(({ uid }) => uid)).toEqual([3, 1]); + }); +}); diff --git a/codewit/api/src/routes/demo.spec.ts b/codewit/api/src/routes/demo.spec.ts new file mode 100644 index 00000000..9d02b02f --- /dev/null +++ b/codewit/api/src/routes/demo.spec.ts @@ -0,0 +1,85 @@ +import 'passport'; +import { Attempt, Demo, DemoExercises } from '../models'; +import { getDemoAttempt } from './demo'; + +jest.mock('../models', () => ({ + Attempt: { findAll: jest.fn() }, + Demo: { + associations: { exercises: 'exercises' }, + findByPk: jest.fn(), + }, + DemoExercises: 'DemoExercises', + DemoTags: 'DemoTags', + Language: 'Language', + Tag: 'Tag', + UserExerciseCompletion: 'UserExerciseCompletion', + User: {}, + sequelize: { query: jest.fn(), transaction: jest.fn() }, +})); + +describe('GET /demos/:uid/attempt exercise ordering', () => { + it('returns exercises in their persisted DemoExercises order', async () => { + jest.mocked(Demo.findByPk).mockImplementation(async (_uid, options) => { + const order = options?.order as unknown[]; + const hasExerciseOrder = order.some(entry => + Array.isArray(entry) + && entry[0] === 'exercises' + && entry[1] === DemoExercises + && entry[2] === 'order' + && entry[3] === 'ASC' + ); + const exercises = [ + { + uid: 1, + prompt: 'First', + language: { name: 'cpp' }, + starterCode: '', + DemoExercises: { order: 1 }, + }, + { + uid: 2, + prompt: 'Second', + language: { name: 'cpp' }, + starterCode: '', + DemoExercises: { order: 0 }, + }, + ]; + + return { + uid: 9, + title: 'Ordered demo', + topic: 'operation', + language: { name: 'cpp' }, + youtube_id: 'video', + youtube_thumbnail: 'thumbnail', + tags: [], + exercises: hasExerciseOrder + ? exercises.sort((left, right) => + left.DemoExercises.order - right.DemoExercises.order + ) + : exercises, + hasLikedBy: jest.fn().mockResolvedValue(false), + } as never; + }); + jest.mocked(Attempt.findAll).mockResolvedValue([]); + const json = jest.fn(); + const response = { + json, + status: jest.fn().mockReturnThis(), + }; + + await getDemoAttempt( + { + params: { uid: '9' }, + query: {}, + user: { uid: 4 }, + } as never, + response as never, + jest.fn(), + ); + + expect(json.mock.calls[0][0].demo.exercises.map( + (exercise: { uid: number }) => exercise.uid + )).toEqual([2, 1]); + }); +}); diff --git a/codewit/api/src/routes/demo.ts b/codewit/api/src/routes/demo.ts index 4c517529..47354681 100644 --- a/codewit/api/src/routes/demo.ts +++ b/codewit/api/src/routes/demo.ts @@ -61,7 +61,7 @@ function parse_non_zero_int(given: string): number | null { return parsed; } -demoRouter.get("/:uid/attempt", asyncHandle(async (req, res) => { +export const getDemoAttempt = asyncHandle(async (req, res) => { let maybe_module_id = typeof req.query.module_id === "string" ? parse_non_zero_int(req.query.module_id) : null; @@ -244,7 +244,9 @@ demoRouter.get("/:uid/attempt", asyncHandle(async (req, res) => { resources, related_demos, } as DemoAttempt); -})); +}); + +demoRouter.get("/:uid/attempt", getDemoAttempt); demoRouter.post('/', checkAdmin, async (req, res) => { try { diff --git a/codewit/client-e2e/src/e2e/app.cy.ts b/codewit/client-e2e/src/e2e/app.cy.ts index 6e33e6fa..fe56a14c 100644 --- a/codewit/client-e2e/src/e2e/app.cy.ts +++ b/codewit/client-e2e/src/e2e/app.cy.ts @@ -609,11 +609,11 @@ describe("Course creations functionality", () => { describe("Module creations functionality", () => { beforeEach(() => { mockAdminUser(); - cy.intercept('GET', '/modules', { + cy.intercept('GET', '/api/modules', { statusCode: 200, body: [] }).as('getModules'); - cy.intercept('GET', '/resources', { + cy.intercept('GET', '/api/resources', { statusCode: 200, body: [ { uid: 1, title: 'First resource', url: 'https://example.com/first', source: 'Example', likes: 0 }, @@ -636,7 +636,7 @@ describe("Module creations functionality", () => { }) it("should allow a user to creat a new moduele", () => { - cy.intercept('POST', '/modules', (req) => { + cy.intercept('POST', '/api/modules', (req) => { expect(req.body).to.deep.equal({ language: "cpp", resources: [], @@ -651,14 +651,10 @@ describe("Module creations functionality", () => { cy.wait('@createModule'); }) - it('adds, reorders, and removes selected resources before saving', () => { - cy.intercept('POST', '/modules', (req) => { - expect(req.body).to.deep.equal({ - language: 'cpp', - resources: [2], - topic: 'operation', - }); - }).as('createOrderedModule'); + it('submits selected resources in their reordered sequence', () => { + cy.intercept('POST', '/api/modules', (req) => { + expect(req.body.resources).to.deep.equal([2, 1]); + }).as('createReorderedModule'); cy.contains('Create Module').click(); getTopicSelect().type('operation{enter}'); @@ -668,11 +664,26 @@ describe("Module creations functionality", () => { cy.get('[data-testid="selected-resources"]').should('contain.text', 'First resource'); cy.get('[data-testid="selected-resources"]').should('contain.text', 'Second resource'); - cy.get('[aria-label="Drag Second resource"]').focus().type('{space}{uparrow}{space}'); + cy.get('[aria-label="Drag Second resource"]').focus().type('{enter}{uparrow}{enter}'); + + getSubmitButton().click(); + cy.wait('@createReorderedModule'); + }); + + it('removes a selected resource before saving', () => { + cy.intercept('POST', '/api/modules', (req) => { + expect(req.body.resources).to.deep.equal([2]); + }).as('createModuleWithoutRemovedResource'); + + cy.contains('Create Module').click(); + getTopicSelect().type('operation{enter}'); + getLanguageSelect().type('cpp{enter}'); + cy.get('#resource-select').type('First resource{enter}'); + cy.get('#resource-select').type('Second resource{enter}'); cy.get('[aria-label="Remove First resource"]').click(); getSubmitButton().click(); - cy.wait('@createOrderedModule'); + cy.wait('@createModuleWithoutRemovedResource'); }); }) @@ -680,7 +691,7 @@ describe("Module creations functionality", () => { describe('Demo creation functionality', () => { beforeEach(() => { mockAdminUser(); - cy.intercept('GET', '/exercises', { + cy.intercept('GET', '/api/exercises', { statusCode: 200, body: [{ uid: 1, @@ -707,7 +718,7 @@ describe('Demo creation functionality', () => { }); it('allows a user to create a new demo', () => { - cy.intercept('POST', '/demos', (req) => { + cy.intercept('POST', '/api/demos', (req) => { expect(req.body).to.deep.equal({ title: 'New Demo Title', youtube_id: '8bc-VU3V7lU', @@ -750,7 +761,7 @@ describe('Demo creation functionality', () => { describe('Demo Editing/Deleting functionality', () => { beforeEach(() => { mockAdminUser(); - cy.intercept('GET', '/demos', { + cy.intercept('GET', '/api/demos', { statusCode: 200, body: [ { @@ -761,23 +772,34 @@ describe('Demo Editing/Deleting functionality', () => { topic: 'operation', language: 'cpp', tags: ['console io'], - exercises: [1], + exercises: [ + { uid: 1, prompt: 'First Exercise Prompt' }, + { uid: 2, prompt: 'Second Exercise Prompt' }, + ], }, ], }).as('getDemos'); - cy.intercept('GET', '/exercises', { + cy.intercept('GET', '/api/exercises', { statusCode: 200, - body: [{ - uid: 1, - prompt: 'New Exercise Prompt', - referenceTest: 'console.log("Hello World");', - tags: ['console io', 'customtag'], - topic: 'console io', - language: { - name: 'cpp' - } - }] + body: [ + { + uid: 1, + prompt: 'First Exercise Prompt', + referenceTest: 'console.log("First");', + tags: ['console io', 'customtag'], + topic: 'console io', + language: { name: 'cpp' }, + }, + { + uid: 2, + prompt: 'Second Exercise Prompt', + referenceTest: 'console.log("Second");', + tags: ['console io'], + topic: 'console io', + language: { name: 'cpp' }, + }, + ] }).as('getExercise'); cy.visit('/create/demo'); @@ -801,7 +823,7 @@ describe('Demo Editing/Deleting functionality', () => { }); it('edit should send updated demo and verify body', () => { - cy.intercept('PATCH', '/demos/99', (req) => { + cy.intercept('PATCH', '/api/demos/99', (req) => { expect(req.body).to.deep.equal({ uid: 99, title: 'Updated Demo Title', @@ -810,7 +832,7 @@ describe('Demo Editing/Deleting functionality', () => { topic: 'operation', language: 'cpp', tags: ['console io', 'updated tag'], - exercises: [1], + exercises: [1, 2], }); }).as('patchDemo'); @@ -821,14 +843,55 @@ describe('Demo Editing/Deleting functionality', () => { cy.wait('@patchDemo'); }); + it('saves and restores reordered exercises', () => { + cy.intercept('PATCH', '/api/demos/99', (req) => { + expect(req.body.exercises).to.deep.equal([2, 1]); + req.reply({ + ...req.body, + exercises: [ + { uid: 2, prompt: 'Second Exercise Prompt' }, + { uid: 1, prompt: 'First Exercise Prompt' }, + ], + }); + }).as('reorderDemoExercises'); + cy.intercept('GET', '/api/demos', { + statusCode: 200, + body: [{ + uid: 99, + title: 'New Demo Title', + youtube_id: '8bc-VU3V7lU', + youtube_thumbnail: 'https://i.ytimg.com/vi/8bc-VU3V7lU/hqdefault.jpg', + topic: 'operation', + language: 'cpp', + tags: ['console io'], + exercises: [ + { uid: 2, prompt: 'Second Exercise Prompt' }, + { uid: 1, prompt: 'First Exercise Prompt' }, + ], + }], + }).as('getReorderedDemos'); + + cy.contains('Edit').click(); + cy.get('[aria-label="Drag Second Exercise Prompt"]').focus().type('{enter}{uparrow}{enter}'); + cy.contains('button', 'Update').click(); + cy.wait('@reorderDemoExercises'); + cy.wait('@getReorderedDemos'); + + cy.contains('Edit').click(); + cy.get('[aria-label^="Drag"]').then(handles => { + expect(handles.eq(0)).to.have.attr('aria-label', 'Drag Second Exercise Prompt'); + expect(handles.eq(1)).to.have.attr('aria-label', 'Drag First Exercise Prompt'); + }); + }); + it('delete should call endpoint with correct UID', () => { - cy.intercept('DELETE', '/demos/99').as('deleteDemo'); + cy.intercept('DELETE', '/api/demos/99').as('deleteDemo'); cy.contains('Delete').click(); cy.wait('@deleteDemo').its('request.url').should('include', '/demos/99'); }); it('edit should update the title visually on table', () => { - cy.intercept('PATCH', '/demos/99', { + cy.intercept('PATCH', '/api/demos/99', { statusCode: 200, body: { uid: 99, diff --git a/codewit/client-e2e/src/support/app.po.ts b/codewit/client-e2e/src/support/app.po.ts index 52fcabda..c9d52d22 100644 --- a/codewit/client-e2e/src/support/app.po.ts +++ b/codewit/client-e2e/src/support/app.po.ts @@ -30,7 +30,7 @@ export const getCheckList = () => cy.get('[data-testid="check-list"]'); export const getHomeModule = () => cy.get('[data-testid="module"]'); export const mockNonAdminUser = () => { - cy.intercept('GET', '/oauth2/google/userinfo', { + cy.intercept('GET', '/api/oauth2/google/userInfo', { statusCode: 200, body: { user: { @@ -44,7 +44,7 @@ export const mockNonAdminUser = () => { }).as('getUserInfo'); }; export const mockAdminUser = () => { - cy.intercept('GET', '/oauth2/google/userinfo', { + cy.intercept('GET', '/api/oauth2/google/userInfo', { statusCode: 200, body: { user: { diff --git a/codewit/client/src/pages/DemoForm.tsx b/codewit/client/src/pages/DemoForm.tsx index edb45171..f903f19d 100644 --- a/codewit/client/src/pages/DemoForm.tsx +++ b/codewit/client/src/pages/DemoForm.tsx @@ -37,6 +37,7 @@ import { language_options, get_language_option } from "../components/form/Langua import CreateButton from "../components/form/CreateButton"; import ReusableTable, { Column } from "../components/form/ReusableTable"; import { VideoOption, use_yt_videos } from "../hooks/yt_videos"; +import { moveArrayItem } from "../utils/arrayOrder"; interface DemoForm { uid?: number, @@ -387,10 +388,7 @@ function DemoForm({view, demo, on_cancel, on_created, on_updated}: DemoFormProps { - let next = [...field.state.value]; - next.splice(a_index, 0, next.splice(b_index, 1)[0]); - - field.setValue(next); + field.setValue(moveArrayItem(field.state.value, a_index, b_index)); }} on_remove={(index, uid) => { let next = [...field.state.value]; @@ -554,9 +552,15 @@ function SortableExerciseItem({uid, prompt, on_remove}: SortableExerciseItemProp className="border rounded-lg p-2 gap-x-2 flex flex-row items-center bg-[rgb(55,65,81)] border-[rgb(75,85,99)]" style={style} > -
+
+

{prompt}

uid: {uid} diff --git a/codewit/client/src/utils/arrayOrder.spec.ts b/codewit/client/src/utils/arrayOrder.spec.ts new file mode 100644 index 00000000..d340d3f7 --- /dev/null +++ b/codewit/client/src/utils/arrayOrder.spec.ts @@ -0,0 +1,10 @@ +import { moveArrayItem } from './arrayOrder'; + +describe('moveArrayItem', () => { + it('moves the dragged item to the target position without mutating the input', () => { + const original = ['first', 'second', 'third']; + + expect(moveArrayItem(original, 2, 0)).toEqual(['third', 'first', 'second']); + expect(original).toEqual(['first', 'second', 'third']); + }); +}); diff --git a/codewit/client/src/utils/arrayOrder.ts b/codewit/client/src/utils/arrayOrder.ts new file mode 100644 index 00000000..6ffecc6f --- /dev/null +++ b/codewit/client/src/utils/arrayOrder.ts @@ -0,0 +1,11 @@ +export function moveArrayItem( + values: readonly T[], + fromIndex: number, + toIndex: number, +): T[] { + const reordered = [...values]; + const [item] = reordered.splice(fromIndex, 1); + reordered.splice(toIndex, 0, item); + + return reordered; +}