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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions api/app/controllers/api/v1/projects_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -99,14 +99,15 @@ def unarchive
end

def duplicate
return render_archived_organization_error if @project.organization&.archived?
destination = duplicate_organization
return render_archived_organization_error if destination&.archived?

copy = current_user.projects.new(
title: "#{@project.title} Copy",
title: "#{@project.title.first(115)} Copy",
kind: @project.kind,
entry_path: @project.entry_path,
visibility: "private",
organization: duplicate_organization,
organization: destination,
forked_from: @project
)
@project.project_files.each_with_index do |file, index|
Expand Down Expand Up @@ -173,6 +174,12 @@ def project_organization
end

def duplicate_organization
if params.key?(:organization_id)
return nil if params[:organization_id].blank?

return organization_scope.find(params[:organization_id])
end

return nil unless @project.organization

current_user.organizations.find_by(id: @project.organization_id)
Expand Down
71 changes: 71 additions & 0 deletions api/test/integration/projects_api_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1455,6 +1455,77 @@ class ProjectsApiTest < ActionDispatch::IntegrationTest
assert_equal source_project, copy.forked_from
end

test "duplicates into an explicitly selected valid workspace" do
source_organization = Organization.create!(name: "Source Class", created_by: @user)
destination_organization = Organization.create!(name: "Destination Class", created_by: @user)
source_organization.organization_memberships.create!(user: @user, role: :student)
destination_organization.organization_memberships.create!(user: @user, role: :student)
source_project = @user.projects.create!(
organization: source_organization,
title: "A" * 120,
kind: "ruby",
visibility: "organization",
project_files: [ ProjectFile.new(path: "main.rb", language: "ruby", content: "puts 'copy me'") ]
)

post "/api/v1/projects/#{source_project.id}/duplicate", headers: @headers

assert_response :created
default_copy = Project.find(response.parsed_body.dig("project", "id"))
assert_equal source_organization, default_copy.organization

post "/api/v1/projects/#{source_project.id}/duplicate",
params: { organization_id: destination_organization.id }.to_json,
headers: @headers

assert_response :created
class_copy = Project.find(response.parsed_body.dig("project", "id"))
assert_equal destination_organization, class_copy.organization
assert_equal "private", class_copy.visibility
assert_equal source_project, class_copy.forked_from
assert_equal 120, class_copy.title.length
assert class_copy.title.end_with?(" Copy")

post "/api/v1/projects/#{source_project.id}/duplicate",
params: { organization_id: nil }.to_json,
headers: @headers

assert_response :created
personal_copy = Project.find(response.parsed_body.dig("project", "id"))
assert_nil personal_copy.organization
assert_equal "private", personal_copy.visibility
end

test "rejects unavailable or archived copy destinations" do
source_organization = Organization.create!(name: "Source Class", created_by: @user)
source_organization.organization_memberships.create!(user: @user, role: :student)
source_project = @user.projects.create!(
organization: source_organization,
title: "Class Starter",
kind: "ruby",
visibility: "private",
project_files: [ ProjectFile.new(path: "main.rb", language: "ruby", content: "puts 'copy me'") ]
)
unavailable = Organization.create!(name: "Other Class", created_by: @user)

post "/api/v1/projects/#{source_project.id}/duplicate",
params: { organization_id: unavailable.id }.to_json,
headers: @headers
assert_response :not_found

source_organization.update!(archived_at: Time.current)
post "/api/v1/projects/#{source_project.id}/duplicate",
params: { organization_id: source_organization.id }.to_json,
headers: @headers
assert_response :unprocessable_entity

post "/api/v1/projects/#{source_project.id}/duplicate",
params: { organization_id: nil }.to_json,
headers: @headers
assert_response :created
assert_nil Project.find(response.parsed_body.dig("project", "id")).organization
end

test "destroying an organization keeps formerly organization-visible projects valid" do
organization = Organization.create!(name: "Closing School", created_by: @user)
organization.organization_memberships.create!(user: @user, role: :owner)
Expand Down
23 changes: 12 additions & 11 deletions docs/FDMS_CLASSROOM_LAUNCH_PLAN.md
Original file line number Diff line number Diff line change
Expand Up @@ -217,15 +217,15 @@ For the initial FDMS launch, default every class project to **Teacher only**, of

### FDMS-003 — Keep class copies inside the class

**Why:** The frontend duplication helper explicitly sets `organizationId`, owner, and organization to `null`. If a teacher publishes a starter project and a student clicks Duplicate, the student's copy becomes a personal project and disappears from the teacher's class view.
**Why:** A learner must always know where a copy will live. Class starters should default to their source class, while personal or cross-class copies require a deliberate destination choice.

**Work:**

- [x] When duplicating a class project, default the destination to the active class.
- [ ] Allow a destination chooser only when the user belongs to multiple valid contexts.
- [x] Allow a destination chooser when the user belongs to multiple valid contexts.
- [x] Preserve private visibility for the student's new copy.
- [x] Use the server duplicate endpoint for signed-in cloud projects or make the frontend behavior match it.
- [ ] Clearly show the destination before confirmation.
- [x] Clearly show the destination before confirmation.

**Acceptance criteria:**

Expand Down Expand Up @@ -272,7 +272,7 @@ For the initial FDMS launch, default every class project to **Teacher only**, of
- [x] Update affected frontend dependencies, including the DOMPurify and `js-cookie` dependency chains.
- [x] Update affected Ruby dependencies, prioritizing `jwt`, `puma`, and `websocket-driver`, then the remaining advisories.
- [x] Rebuild and rerun all tests after lockfile updates.
- [ ] Make passing CI required before merging to `main`.
- [x] Make passing CI required before merging to `main`.

**Acceptance criteria:**

Expand Down Expand Up @@ -510,11 +510,11 @@ This is an order of operations, not a guaranteed calendar estimate. Each phase m
### Before the pilot

- [ ] Production origin and invitation URLs are correct.
- [ ] High-severity dependency audits are clear.
- [ ] CI runs on every pull request.
- [ ] Save failure and recovery scenarios pass.
- [ ] Class-preserving duplication passes.
- [ ] Teacher feedback workflow is implemented or FDMS accepts the documented LMS fallback.
- [x] High-severity dependency audits are clear.
- [x] CI runs on every pull request.
- [x] Save failure and recovery scenarios pass.
- [x] Class-preserving duplication passes.
- [x] Teacher feedback workflow is implemented or FDMS accepts the documented LMS fallback.
- [ ] Public sharing is disabled or governed by an approved policy.
- [ ] Test accounts for every role exist.
- [ ] Backup restore and monitoring checks pass.
Expand Down Expand Up @@ -588,14 +588,15 @@ Unless FDMS changes the requirements, do not make these launch blockers:
| --- | --- |
| `npm --prefix web run lint` | Pass |
| `npm --prefix web run build` | Pass, with large-chunk warnings |
| `npm --prefix web test` | Pass: 22 files, 203 tests |
| `bundle exec rails test` | Pass: 52 runs, 410 assertions |
| `npm --prefix web test` | Pass: 22 files, 209 tests |
| `bundle exec rails test` | Pass: 54 runs, 425 assertions |
| `bundle exec rubocop` | Pass: 73 files, no offenses |
| `bundle exec brakeman --no-pager` | Pass: 0 warnings |
| `npm audit --audit-level=high` | Pass: 0 vulnerabilities |
| `bundle exec bundler-audit check` | Pass after updating Rails and Active Storage from 8.1.3 to the 8.1.3.1 security patch for CVE-2026-66066 |
| Local multi-role API workflow | Pass: teacher/student/classmate feedback, private isolation, bulk invite, export, archive, audit, CORS, and stale-save conflict |
| Local visible browser smoke test | Pass under Netlify's local CSP: editor loads and the default Ruby program prints all expected output |
| GitHub `main` ruleset | Active: pull requests, resolved review threads, and current-head `frontend` and `backend` checks are required |
| Netlify production page and headers | Reachable; security headers present |
| Render health endpoint | Healthy |
| Production-origin Render CORS preflight | Pass for `https://code.shimizu-technology.com` |
Expand Down
62 changes: 62 additions & 0 deletions web/src/App.css
Original file line number Diff line number Diff line change
Expand Up @@ -3068,6 +3068,68 @@ button.context-chip.active,
width: 100%;
}

.copy-project-sheet {
width: min(520px, 100%);
max-height: calc(100dvh - 2rem);
overflow: auto;
}

.copy-destination-list {
display: grid;
gap: 0.65rem;
margin: 1rem 0 0;
padding: 0;
border: 0;
}

.copy-destination-list legend {
margin-bottom: 0.5rem;
color: var(--muted);
font-size: 0.76rem;
font-weight: 900;
letter-spacing: 0.08em;
text-transform: uppercase;
}

.copy-destination {
display: grid;
grid-template-columns: auto minmax(0, 1fr);
align-items: start;
gap: 0.75rem;
min-height: 58px;
padding: 0.75rem 0.85rem;
border: 1px solid var(--line);
border-radius: 0.9rem;
background: rgba(255, 255, 255, 0.58);
cursor: pointer;
}

.copy-destination.active {
border-color: var(--brand);
box-shadow: 0 0 0 2px color-mix(in srgb, var(--brand) 18%, transparent);
}

.copy-destination input {
width: 1.1rem;
height: 1.1rem;
margin: 0.1rem 0 0;
accent-color: var(--brand);
}

.copy-destination span {
display: grid;
gap: 0.2rem;
}

.copy-destination small {
color: var(--muted);
line-height: 1.35;
}

.app-shell[data-theme="dark"] .copy-destination {
background: rgba(255, 255, 255, 0.07);
}

.workspace-transfer-sheet {
width: min(590px, 100%);
}
Expand Down
98 changes: 97 additions & 1 deletion web/src/App.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@ import type { ErrorCoachContext } from './lib/errorCoach'
import { languageGuideFor } from './lib/languageGuides'
import { practiceChallengeById, practiceChallengesFor } from './lib/practiceLab'
import { completePracticeChallenge, completedPracticeChallengeIds, practiceChallengeIdForProject, preservePracticeConflictLinks } from './lib/practiceProgress'
import { createConflictCopy } from './lib/projectStorage'
import { createConflictCopy, createProject } from './lib/projectStorage'
import type { ProjectLibrary } from './lib/projectStorage'
import type { RunnerOutcome } from './lib/runnerOutcome'
import { api } from './lib/api'
import type { useAuthContext } from './contexts/AuthContext'

vi.mock('@monaco-editor/react', () => ({
default: ({ value, onChange }: { value?: string; onChange?: (value: string) => void }) => (
Expand All @@ -24,6 +26,16 @@ const runnerHarness = vi.hoisted(() => ({
onRunComplete: undefined as undefined | ((outcome: RunnerOutcome) => void),
}))

type AppAuthContext = ReturnType<typeof useAuthContext>

const authHarness = vi.hoisted(() => ({
value: null as unknown as AppAuthContext,
}))

vi.mock('./contexts/AuthContext', () => ({
useAuthContext: () => authHarness.value,
}))

vi.mock('./components/RunnerPanel', () => ({
RunnerPanel: ({ onErrorAdviceChange, onRunCancel, onRunComplete }: {
onErrorAdviceChange?: typeof runnerHarness.onErrorAdviceChange
Expand All @@ -46,6 +58,13 @@ function storedLibrary() {
describe('App language guide practice projects', () => {
beforeEach(() => {
localStorage.clear()
authHarness.value = {
isSignedIn: false,
isLoading: true,
user: null,
organizations: [],
syncSession: async () => {},
}
runnerHarness.onErrorAdviceChange = undefined
runnerHarness.onRunCancel = undefined
runnerHarness.onRunComplete = undefined
Expand All @@ -71,6 +90,7 @@ describe('App language guide practice projects', () => {
afterEach(() => {
cleanup()
vi.useRealTimers()
vi.restoreAllMocks()
})

it('opens a complete example in a new private project without changing the original', async () => {
Expand Down Expand Up @@ -426,4 +446,80 @@ describe('App language guide practice projects', () => {
expect(screen.getByRole('button', { name: 'Check my work' })).toBeTruthy()
expect(screen.getByRole('status').textContent).toMatch(/ended before a result arrived/i)
})

it('shows and confirms the destination before duplicating a project', async () => {
const user = userEvent.setup()
render(<App />)
expect(storedLibrary().projects).toHaveLength(1)

await user.click(screen.getAllByRole('button', { name: 'Duplicate' })[0])

const dialog = screen.getByRole('dialog', { name: 'Where should the copy live?' })
expect(within(dialog).getByRole('radio', { name: /Personal projects/ })).toHaveProperty('checked', true)
expect(storedLibrary().projects).toHaveLength(1)

await user.click(within(dialog).getByRole('button', { name: 'Duplicate here' }))

await waitFor(() => expect(storedLibrary().projects).toHaveLength(2))
expect(screen.getByRole('status').textContent).toMatch(/duplicated into Personal projects/i)
expect(screen.queryByRole('dialog', { name: 'Where should the copy live?' })).toBeNull()
})

it('keeps a failed cloud copy retryable and sends the selected classroom', async () => {
const user = userEvent.setup()
const cloudProject = {
...createConflictCopy(createProject('ruby')),
id: '42',
title: 'Cloud starter',
owner: { id: 7, fullName: 'Student One' },
organizationId: null,
organization: null,
lockVersion: 3,
}
const cloudCopy = {
...cloudProject,
id: '43',
title: 'Cloud starter Copy',
organizationId: '20',
organization: { id: 20, name: 'Robotics', slug: 'robotics' },
lockVersion: 0,
}
authHarness.value = {
isSignedIn: true,
isLoading: false,
user: { id: 7, email: 'student@example.com', first_name: 'Student', last_name: 'One', full_name: 'Student One', role: 'user' },
organizations: [{ id: 20, name: 'Robotics', slug: 'robotics', role: 'student' }],
syncSession: vi.fn(),
}
vi.spyOn(api, 'getProjects').mockResolvedValue({ data: [cloudProject], error: null })
vi.spyOn(api, 'getProjectComments').mockResolvedValue({ data: { comments: [], unread_count: 0 }, error: null })
const updateProject = vi.spyOn(api, 'updateProject').mockResolvedValue({
data: cloudProject,
error: null,
status: 200,
code: null,
conflictProject: null,
})
const duplicateCloudProject = vi.spyOn(api, 'duplicateProject')
.mockResolvedValueOnce({ data: null, error: 'Classroom service unavailable' })
.mockResolvedValueOnce({ data: cloudCopy, error: null })

render(<App />)
await user.click((await screen.findAllByRole('button', { name: 'Cloud starter Ruby' }))[0])
expect(screen.getByLabelText('Project name')).toHaveProperty('value', 'Cloud starter')
await user.click(screen.getAllByRole('button', { name: 'Duplicate' })[0])
const dialog = screen.getByRole('dialog', { name: 'Where should the copy live?' })
await user.click(within(dialog).getByRole('radio', { name: /Robotics/ }))
await user.click(within(dialog).getByRole('button', { name: 'Duplicate here' }))

await waitFor(() => expect(screen.getByRole('status').textContent).toMatch(/Classroom service unavailable/i))
expect(duplicateCloudProject).toHaveBeenLastCalledWith('42', '20')
expect(within(dialog).getByRole('button', { name: 'Duplicate here' })).toHaveProperty('disabled', false)
expect(updateProject).toHaveBeenCalledWith(expect.objectContaining({ id: '42' }))

await user.click(within(dialog).getByRole('button', { name: 'Duplicate here' }))
await waitFor(() => expect(screen.queryByRole('dialog', { name: 'Where should the copy live?' })).toBeNull())
expect(duplicateCloudProject).toHaveBeenCalledTimes(2)
expect(screen.getByRole('status').textContent).toMatch(/duplicated into Robotics/i)
})
})
Loading