diff --git a/Sources/APIServer/Routes/Web/AdminRoutes+Courses.swift b/Sources/APIServer/Routes/Web/AdminRoutes+Courses.swift index c6393999..be1353b6 100644 --- a/Sources/APIServer/Routes/Web/AdminRoutes+Courses.swift +++ b/Sources/APIServer/Routes/Web/AdminRoutes+Courses.swift @@ -111,6 +111,10 @@ extension AdminRoutes { .filter(\.$courseID == sourceID) .sort(\.$sortOrder) .all() + let sections = try await APICourseSection.query(on: req.db) + .filter(\.$courseID == sourceID) + .sort(\.$sortOrder) + .all() let newCourseID = try await req.db.transaction { db -> UUID in // 1. Create the new course. @@ -118,7 +122,21 @@ extension AdminRoutes { try await newCourse.save(on: db) let newCourseID = try newCourse.requireID() - // 2. Copy each test setup (zip + optional notebook) to a new ID. + // 2. Copy sections, building an old→new UUID map. + var sectionIDMap: [UUID: UUID] = [:] + for section in sections { + guard let oldSectionID = section.id else { continue } + let newSection = APICourseSection( + name: section.name, + defaultGradingMode: section.defaultGradingMode, + sortOrder: section.sortOrder, + courseID: newCourseID + ) + try await newSection.save(on: db) + sectionIDMap[oldSectionID] = try newSection.requireID() + } + + // 3. Copy each test setup (zip + optional notebook) to a new ID. var setupIDMap: [String: String] = [:] for setup in setups { guard let oldID = setup.id else { continue } @@ -129,11 +147,16 @@ extension AdminRoutes { let dstZip = URL(fileURLWithPath: setupsDir + "\(newID).zip") try FileManager.default.copyItem(at: srcZip, to: dstZip) + // Copy the notebook using the actual stored path, not a reconstructed one. var newNotebookPath: String? = nil - if setup.notebookPath != nil { - let srcNb = URL(fileURLWithPath: setupsDir + "\(oldID).ipynb") + if let srcPath = setup.notebookPath { + let srcNb = URL(fileURLWithPath: srcPath) if FileManager.default.fileExists(atPath: srcNb.path) { - let dstNb = URL(fileURLWithPath: setupsDir + "\(newID).ipynb") + let filename = srcNb.lastPathComponent + let nbDir = setupsDir + "notebooks/\(newID)/" + try? FileManager.default.createDirectory(atPath: nbDir, + withIntermediateDirectories: true) + let dstNb = URL(fileURLWithPath: nbDir + filename) try FileManager.default.copyItem(at: srcNb, to: dstNb) newNotebookPath = dstNb.path } @@ -149,10 +172,11 @@ extension AdminRoutes { try await newSetup.save(on: db) } - // 3. Copy each assignment, remapping to the new test setup IDs. + // 4. Copy each assignment, remapping test setup IDs and section IDs. // Validation state is reset so the instructor re-validates before opening. for (idx, a) in assignments.enumerated() { guard let newSetupID = setupIDMap[a.testSetupID] else { continue } + let newSectionID = a.sectionID.flatMap { sectionIDMap[$0] } let newAssignment = APIAssignment( testSetupID: newSetupID, title: a.title, @@ -161,6 +185,7 @@ extension AdminRoutes { sortOrder: a.sortOrder ?? idx, validationStatus: nil, validationSubmissionID: nil, + sectionID: newSectionID, courseID: newCourseID ) try await newAssignment.save(on: db) diff --git a/Sources/APIServer/Routes/Web/CourseBundleRoutes.swift b/Sources/APIServer/Routes/Web/CourseBundleRoutes.swift index 551814d1..2f4991d2 100644 --- a/Sources/APIServer/Routes/Web/CourseBundleRoutes.swift +++ b/Sources/APIServer/Routes/Web/CourseBundleRoutes.swift @@ -54,6 +54,11 @@ struct CourseBundleRoutes: RouteCollection { .filter(\.$courseID == courseUUID) .all() + let sections = try await APICourseSection.query(on: req.db) + .filter(\.$courseID == courseUUID) + .sort(\.$sortOrder) + .all() + let enrollments = try await APICourseEnrollment.query(on: req.db) .filter(\.$course.$id == courseUUID) .all() @@ -100,10 +105,11 @@ struct CourseBundleRoutes: RouteCollection { // ── 2. Assign bundleIDs ──────────────────────────────────────────── - var userBundleIDByUUID: [UUID: String] = [:] - var setupBundleIDByID: [String: String] = [:] - var assignBundleIDByID: [UUID: String] = [:] - var subBundleIDByID: [String: String] = [:] + var userBundleIDByUUID: [UUID: String] = [:] + var setupBundleIDByID: [String: String] = [:] + var assignBundleIDByID: [UUID: String] = [:] + var subBundleIDByID: [String: String] = [:] + var sectionBundleIDByUUID: [UUID: String] = [:] for (i, u) in allUsers.enumerated() { guard let uid = u.id else { continue } @@ -121,6 +127,10 @@ struct CourseBundleRoutes: RouteCollection { guard let sid = s.id else { continue } subBundleIDByID[sid] = "sub_\(i + 1)" } + for (i, sec) in sections.enumerated() { + guard let secID = sec.id else { continue } + sectionBundleIDByUUID[secID] = "section_\(i + 1)" + } // ── 3. Build manifest ────────────────────────────────────────────── @@ -143,17 +153,29 @@ struct CourseBundleRoutes: RouteCollection { ) } + let bundledSections = sections.compactMap { sec -> BundledSection? in + guard let secID = sec.id, let bid = sectionBundleIDByUUID[secID] else { return nil } + return BundledSection( + bundleID: bid, + name: sec.name, + defaultGradingMode: sec.defaultGradingMode, + sortOrder: sec.sortOrder + ) + } + let bundledAssignments = assignments.compactMap { a -> BundledAssignment? in guard let aid = a.id, let bid = assignBundleIDByID[aid], let setupBid = setupBundleIDByID[a.testSetupID] else { return nil } + let sectionBid = a.sectionID.flatMap { sectionBundleIDByUUID[$0] } return BundledAssignment( bundleID: bid, title: a.title, dueAt: a.dueAt, isOpen: a.isOpen, sortOrder: a.sortOrder, - testSetupBundleID: setupBid + testSetupBundleID: setupBid, + sectionBundleID: sectionBid ) } @@ -192,6 +214,7 @@ struct CourseBundleRoutes: RouteCollection { enrollmentMode: course.enrollmentMode), users: bundledUsers, enrolledUserBundleIDs: enrolledBundleIDs, + sections: bundledSections, assignments: bundledAssignments, testSetups: bundledSetups, submissions: bundledSubmissions, @@ -435,7 +458,20 @@ struct CourseBundleRoutes: RouteCollection { } } - // 6e. Create test setups → setupIDMap[bundleID] = new live ID + // 6e. Create sections → sectionIDMap[bundleID] = new live UUID + var sectionIDMap: [String: UUID] = [:] + for bundledSection in manifest.sections ?? [] { + let newSection = APICourseSection( + name: bundledSection.name, + defaultGradingMode: bundledSection.defaultGradingMode, + sortOrder: bundledSection.sortOrder, + courseID: t.courseID + ) + try await newSection.save(on: db) + sectionIDMap[bundledSection.bundleID] = try newSection.requireID() + } + + // 6f. Create test setups → setupIDMap[bundleID] = new live ID var setupIDMap: [String: String] = [:] for bundledSetup in manifest.testSetups { let newSetupID = "setup_\(UUID().uuidString.lowercased().prefix(8))" @@ -466,9 +502,10 @@ struct CourseBundleRoutes: RouteCollection { t.testSetupsImported += 1 } - // 6f. Create assignments + // 6g. Create assignments for bundledAssign in manifest.assignments { guard let setupID = setupIDMap[bundledAssign.testSetupBundleID] else { continue } + let sectionID = bundledAssign.sectionBundleID.flatMap { sectionIDMap[$0] } let newAssign = APIAssignment( testSetupID: setupID, title: bundledAssign.title, @@ -476,13 +513,14 @@ struct CourseBundleRoutes: RouteCollection { isOpen: bundledAssign.isOpen, sortOrder: bundledAssign.sortOrder, validationStatus: nil, // not imported — requires re-validation + sectionID: sectionID, courseID: t.courseID ) try await newAssign.save(on: db) t.assignmentsImported += 1 } - // 6g. Create submissions → subIDMap[bundleID] = new live ID + // 6h. Create submissions → subIDMap[bundleID] = new live ID var subIDMap: [String: String] = [:] for bundledSub in manifest.submissions { guard let setupID = setupIDMap[bundledSub.testSetupBundleID] else { continue } @@ -511,7 +549,7 @@ struct CourseBundleRoutes: RouteCollection { t.submissionsImported += 1 } - // 6h. Create results + // 6i. Create results for bundledResult in manifest.results { guard let subID = subIDMap[bundledResult.submissionBundleID] else { continue } let newResultID = "res_\(UUID().uuidString.lowercased().prefix(8))" diff --git a/Sources/Core/CourseBundleManifest.swift b/Sources/Core/CourseBundleManifest.swift index 32bf97fb..1c355a45 100644 --- a/Sources/Core/CourseBundleManifest.swift +++ b/Sources/Core/CourseBundleManifest.swift @@ -31,6 +31,8 @@ public struct CourseBundleManifest: Codable, Sendable { public let users: [BundledUser] /// bundleIDs of users enrolled in the course. public let enrolledUserBundleIDs: [String] + /// Course sections (nil in bundles exported before this field was added). + public let sections: [BundledSection]? public let assignments: [BundledAssignment] public let testSetups: [BundledTestSetup] /// Student submissions only (kind == "student"); validation runs excluded. @@ -46,6 +48,7 @@ public struct CourseBundleManifest: Codable, Sendable { course: BundledCourse, users: [BundledUser], enrolledUserBundleIDs: [String], + sections: [BundledSection] = [], assignments: [BundledAssignment], testSetups: [BundledTestSetup], submissions: [BundledSubmission], @@ -58,6 +61,7 @@ public struct CourseBundleManifest: Codable, Sendable { self.course = course self.users = users self.enrolledUserBundleIDs = enrolledUserBundleIDs + self.sections = sections self.assignments = assignments self.testSetups = testSetups self.submissions = submissions @@ -106,6 +110,21 @@ public struct BundledUser: Codable, Sendable { } } +public struct BundledSection: Codable, Sendable { + public let bundleID: String + public let name: String + /// "browser" | "worker" + public let defaultGradingMode: String + public let sortOrder: Int + + public init(bundleID: String, name: String, defaultGradingMode: String, sortOrder: Int) { + self.bundleID = bundleID + self.name = name + self.defaultGradingMode = defaultGradingMode + self.sortOrder = sortOrder + } +} + public struct BundledAssignment: Codable, Sendable { public let bundleID: String public let title: String @@ -114,15 +133,18 @@ public struct BundledAssignment: Codable, Sendable { public let sortOrder: Int? /// References BundledTestSetup.bundleID. public let testSetupBundleID: String + /// References BundledSection.bundleID; nil when assignment is ungrouped. + public let sectionBundleID: String? public init(bundleID: String, title: String, dueAt: Date?, isOpen: Bool, - sortOrder: Int?, testSetupBundleID: String) { + sortOrder: Int?, testSetupBundleID: String, sectionBundleID: String? = nil) { self.bundleID = bundleID self.title = title self.dueAt = dueAt self.isOpen = isOpen self.sortOrder = sortOrder self.testSetupBundleID = testSetupBundleID + self.sectionBundleID = sectionBundleID } } diff --git a/Tests/APITests/AdminRoutesTests.swift b/Tests/APITests/AdminRoutesTests.swift index 3fb3f93f..7def231e 100644 --- a/Tests/APITests/AdminRoutesTests.swift +++ b/Tests/APITests/AdminRoutesTests.swift @@ -530,4 +530,131 @@ final class AdminRoutesTests: XCTestCase { XCTAssertFalse(body.contains(">Available<")) }) } + + // MARK: - POST /admin/courses/:courseID/copy + + func testCourseCopySectionsArePreserved() async throws { + let cookie = try await loginAsAdmin() + let course = try await makeCourse(code: "CPSECT") + let courseID = try course.requireID() + + // Two sections with distinct grading modes + let browserSec = APICourseSection(name: "Browser Labs", defaultGradingMode: "browser", + sortOrder: 1, courseID: courseID) + try await browserSec.save(on: app.db) + let workerSec = APICourseSection(name: "Worker Labs", defaultGradingMode: "worker", + sortOrder: 2, courseID: courseID) + try await workerSec.save(on: app.db) + + let setup1 = try await makeSetup(id: "cpsect_s1", courseID: courseID, withNotebook: false) + let setup2 = try await makeSetup(id: "cpsect_s2", courseID: courseID, withNotebook: false) + + let a1 = APIAssignment(testSetupID: setup1.id!, title: "Browser Lab", + isOpen: false, sectionID: try browserSec.requireID(), + courseID: courseID) + try await a1.save(on: app.db) + let a2 = APIAssignment(testSetupID: setup2.id!, title: "Worker Lab", + isOpen: false, sectionID: try workerSec.requireID(), + courseID: courseID) + try await a2.save(on: app.db) + + let (boundCookie, token) = try await csrfCookieAndToken(cookie) + try await app.asyncTest(.POST, "/admin/courses/\(courseID.uuidString)/copy", + beforeRequest: { req in + req.headers.add(name: .cookie, value: boundCookie) + try req.content.encode(["_csrf": token], as: .urlEncodedForm) + }, afterResponse: { res in + XCTAssertEqual(res.status, .seeOther) + } + ) + + guard let copied = try await APICourse.query(on: app.db) + .filter(\.$code == "CPSECT-COPY").first() + else { XCTFail("Copied course not found"); return } + let copiedID = try copied.requireID() + + // Sections should be recreated with matching names and modes + let copiedSections = try await APICourseSection.query(on: app.db) + .filter(\.$courseID == copiedID) + .sort(\.$sortOrder) + .all() + XCTAssertEqual(copiedSections.count, 2, "Copy should have 2 sections") + XCTAssertEqual(copiedSections.first?.name, "Browser Labs") + XCTAssertEqual(copiedSections.first?.defaultGradingMode, "browser") + XCTAssertEqual(copiedSections.last?.name, "Worker Labs") + XCTAssertEqual(copiedSections.last?.defaultGradingMode, "worker") + + // Every copied assignment should be placed in a section, not ungrouped + let copiedAssignments = try await APIAssignment.query(on: app.db) + .filter(\.$courseID == copiedID) + .all() + XCTAssertEqual(copiedAssignments.count, 2) + XCTAssertTrue(copiedAssignments.allSatisfy { $0.sectionID != nil }, + "All copied assignments should have a sectionID") + + // Each assignment should land in the correct new section + if let copiedBrowserSec = copiedSections.first(where: { $0.defaultGradingMode == "browser" }), + let browserLab = copiedAssignments.first(where: { $0.title == "Browser Lab" }) { + XCTAssertEqual(browserLab.sectionID, try copiedBrowserSec.requireID()) + } else { + XCTFail("Missing browser section or Browser Lab assignment in copied course") + } + if let copiedWorkerSec = copiedSections.first(where: { $0.defaultGradingMode == "worker" }), + let workerLab = copiedAssignments.first(where: { $0.title == "Worker Lab" }) { + XCTAssertEqual(workerLab.sectionID, try copiedWorkerSec.requireID()) + } else { + XCTFail("Missing worker section or Worker Lab assignment in copied course") + } + } + + func testCourseCopyNotebookPathIsPreserved() async throws { + let cookie = try await loginAsAdmin() + let course = try await makeCourse(code: "CPNB") + let courseID = try course.requireID() + + // Store notebook in the new-style subdirectory (notebooks//filename.ipynb), + // not the legacy flat path — this was the path the old copy code couldn't find. + let setupID = "cpnb_s1" + let zipPath = app.testSetupsDirectory + "\(setupID).zip" + let nbDir = app.testSetupsDirectory + "notebooks/\(setupID)/" + let nbPath = nbDir + "assignment.ipynb" + try FileManager.default.createDirectory(atPath: nbDir, withIntermediateDirectories: true) + try Data([0x50, 0x4B, 0x05, 0x06] + [UInt8](repeating: 0, count: 18)) + .write(to: URL(fileURLWithPath: zipPath)) + try #"{"nbformat":4,"nbformat_minor":5,"metadata":{},"cells":[]}"# + .write(to: URL(fileURLWithPath: nbPath), atomically: true, encoding: .utf8) + + let manifest = """ + {"schemaVersion":1,"gradingMode":"browser","requiredFiles":[],"testSuites":[],"timeLimitSeconds":10,"makefile":null} + """ + let setup = APITestSetup(id: setupID, manifest: manifest, zipPath: zipPath, + notebookPath: nbPath, courseID: courseID) + try await setup.save(on: app.db) + try await makeAssignment(testSetupID: setupID, courseID: courseID) + + let (boundCookie, token) = try await csrfCookieAndToken(cookie) + try await app.asyncTest(.POST, "/admin/courses/\(courseID.uuidString)/copy", + beforeRequest: { req in + req.headers.add(name: .cookie, value: boundCookie) + try req.content.encode(["_csrf": token], as: .urlEncodedForm) + }, afterResponse: { res in + XCTAssertEqual(res.status, .seeOther) + } + ) + + guard let copied = try await APICourse.query(on: app.db) + .filter(\.$code == "CPNB-COPY").first() + else { XCTFail("Copied course not found"); return } + let copiedCourseID = try copied.requireID() + + let copiedSetup = try await APITestSetup.query(on: app.db) + .filter(\.$courseID == copiedCourseID) + .first() + XCTAssertNotNil(copiedSetup?.notebookPath, + "Copied setup should have a notebookPath set") + if let path = copiedSetup?.notebookPath { + XCTAssertTrue(FileManager.default.fileExists(atPath: path), + "Notebook file should exist on disk at the copied path") + } + } } diff --git a/Tests/APITests/CourseBundleTests.swift b/Tests/APITests/CourseBundleTests.swift index 95db61e1..7bb66da7 100644 --- a/Tests/APITests/CourseBundleTests.swift +++ b/Tests/APITests/CourseBundleTests.swift @@ -520,6 +520,98 @@ final class CourseBundleTests: XCTestCase { XCTAssertEqual(importedAssignments.first?.title, "RT Lab") } + // MARK: - Round-trip: sections preserved through export → import + + func testRoundTripPreservesSections() async throws { + let cookie = try await loginAsAdmin() + + let course = try await makeTestCourse(code: "RT_SECTS") + let courseID = try course.requireID() + + // Two sections with distinct grading modes + let browserSec = APICourseSection(name: "Browser Labs", defaultGradingMode: "browser", + sortOrder: 1, courseID: courseID) + try await browserSec.save(on: app.db) + let workerSec = APICourseSection(name: "Worker Labs", defaultGradingMode: "worker", + sortOrder: 2, courseID: courseID) + try await workerSec.save(on: app.db) + + let setup1 = try await insertSetupWithZip(id: "rt_sects_s1", courseID: courseID) + let setup2 = try await insertSetupWithZip(id: "rt_sects_s2", courseID: courseID) + + let a1 = APIAssignment(testSetupID: setup1.id!, title: "Browser Lab", + isOpen: false, sectionID: try browserSec.requireID(), + courseID: courseID) + try await a1.save(on: app.db) + let a2 = APIAssignment(testSetupID: setup2.id!, title: "Worker Lab", + isOpen: false, sectionID: try workerSec.requireID(), + courseID: courseID) + try await a2.save(on: app.db) + + // Export + var exportedZip = Data() + try await app.asyncTest(.GET, "/admin/courses/\(courseID.uuidString)/export", + beforeRequest: { req in req.headers.add(name: .cookie, value: cookie) }, + afterResponse: { res in + XCTAssertEqual(res.status, .ok) + exportedZip = Data(res.body.readableBytesView) + } + ) + XCTAssertFalse(exportedZip.isEmpty) + + // Inspect the raw manifest to confirm sections and sectionBundleIDs are present + let zipVerifyPath = tmpDir + "testsetups/rt_sects_verify.zip" + let extractDir = FileManager.default.temporaryDirectory + .appendingPathComponent("rt-sects-ext-\(UUID().uuidString)", isDirectory: true) + defer { try? FileManager.default.removeItem(at: extractDir) } + try exportedZip.write(to: URL(fileURLWithPath: zipVerifyPath)) + try await extractZipArchive(zipPath: zipVerifyPath, into: extractDir) + + let manifestData = try Data(contentsOf: extractDir.appendingPathComponent("bundle.json")) + let decoder = JSONDecoder() + decoder.dateDecodingStrategy = .iso8601 + let manifest = try decoder.decode(CourseBundleManifest.self, from: manifestData) + + XCTAssertEqual(manifest.sections?.count, 2, "Manifest should include 2 sections") + XCTAssertTrue(manifest.assignments.allSatisfy { $0.sectionBundleID != nil }, + "Every assignment in the manifest should carry a sectionBundleID") + + // Archive original so import doesn't collide + course.isArchived = true + try await course.save(on: app.db) + + // Import + let (status, body) = try await postImport(cookie: cookie, zipData: exportedZip) + XCTAssertNotEqual(status, .badRequest, "Import should not fail: \(body.prefix(300))") + XCTAssertNotEqual(status, .conflict, body) + + guard let imported = try await APICourse.query(on: app.db) + .filter(\.$code == "RT_SECTS") + .filter(\.$isArchived == false) + .first() + else { XCTFail("Imported course not found"); return } + let importedID = try imported.requireID() + + // Sections should be recreated with correct names and grading modes + let importedSections = try await APICourseSection.query(on: app.db) + .filter(\.$courseID == importedID) + .sort(\.$sortOrder) + .all() + XCTAssertEqual(importedSections.count, 2, "Imported course should have 2 sections") + XCTAssertEqual(importedSections.first?.name, "Browser Labs") + XCTAssertEqual(importedSections.first?.defaultGradingMode, "browser") + XCTAssertEqual(importedSections.last?.name, "Worker Labs") + XCTAssertEqual(importedSections.last?.defaultGradingMode, "worker") + + // All assignments should be placed in a section, not ungrouped + let importedAssignments = try await APIAssignment.query(on: app.db) + .filter(\.$courseID == importedID) + .all() + XCTAssertEqual(importedAssignments.count, 2) + XCTAssertTrue(importedAssignments.allSatisfy { $0.sectionID != nil }, + "All imported assignments should have a sectionID") + } + // MARK: - Bundle builder with a user (used by user-matching tests) private func makeBundleZipWithUser(courseCode: String, username: String) async throws -> Data {