Skip to content

Record source project when adding ExCS projects - #989

Merged
cocomarine merged 5 commits into
mainfrom
1724-add-source-project-id-to-ExCS-projects
Sep 2, 2026
Merged

Record source project when adding ExCS projects#989
cocomarine merged 5 commits into
mainfrom
1724-add-source-project-id-to-ExCS-projects

Conversation

@cocomarine

@cocomarine cocomarine commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Status

Why not reuse remixed_from_id ?

  • Although they are created by similar processes, ExCS projects added to editor-api are different from remixes.
  • A lesson project copied from an ExCS source would have a non-nil remixed_from_id -> can't be teacher_project_ids with current logic in ability -> regression in teacher's ability such as teachers losing read/show_context on their students' remixes of that lesson etc.
  • Api::Projects::RemixesController#index and RemixSelection both query Project.where(remixed_from_id: ), so the ExCS source project would start reporting lesson projects as its remixes, which is not correct.
  • For these reasons, decided to go with new column specifically for recording source_project.

What's changed?

  • Added source_project_id column
  • Updated attributes_for_copy of lesson create.rb to add source project
    • Also cleared remix columns for above reasons (and for the unlikely situation where source projects have these columns populated)
  • Added tests for asserting source_projects are copied during lesson copy and project remix.

@cla-bot cla-bot Bot added the cla-signed label Aug 28, 2026
@github-actions

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown

Test coverage

93.51% line coverage reported by SimpleCov.
Run: https://github.com/RaspberryPiFoundation/editor-api/actions/runs/33613004125

@raspberrypiherokubot
raspberrypiherokubot temporarily deployed to editor-api-p-1724-add-s-m22az1 August 28, 2026 10:31 Inactive
@cocomarine
cocomarine temporarily deployed to editor-api-p-1724-add-s-m22az1 August 28, 2026 10:58 Inactive
@cocomarine
cocomarine temporarily deployed to editor-api-p-1724-add-s-m22az1 August 28, 2026 11:30 Inactive
@cocomarine
cocomarine temporarily deployed to editor-api-p-1724-add-s-m22az1 August 28, 2026 11:49 Inactive
@cocomarine
cocomarine marked this pull request as ready for review August 28, 2026 12:54
Copilot AI lite review requested due to automatic review settings August 28, 2026 12:54
@cocomarine cocomarine changed the title Track source project Keep source project when adding ExCS projects Aug 28, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces a dedicated source_project_id lineage field on Project to track Experience CS “source” projects separately from remix lineage, avoiding behavior/permission regressions tied to remixed_from_id.

Changes:

  • Add projects.source_project_id with a self-referential FK (nullified on source deletion) and corresponding Project associations (source_project / derived_projects).
  • Ensure lesson project creation records source_project_id and explicitly clears remix fields during ExCS-to-lesson copying.
  • Add specs covering association behavior, lesson copy behavior, and remix behavior when a project already has a source_project.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
app/models/project.rb Adds self-referential associations for source_project and derived_projects.
lib/concepts/lesson/operations/create.rb Sets source_project_id for ExCS-based lesson project copies and clears remix fields.
db/migrate/20260828095502_add_source_project_to_projects.rb Adds source_project reference to projects with FK nullify-on-delete.
db/schema.rb Reflects the new column, index, and FK.
spec/models/project_spec.rb Adds association coverage and verifies nullification when source project is destroyed.
spec/concepts/project/create_remix_spec.rb Verifies remixes preserve source_project_id while remixed_from_id points to the immediate parent.
spec/concepts/lesson/create_spec.rb Asserts lesson creation records source_project_id and does not inherit remix fields from the source.
spec/concepts/lesson/create_copy_spec.rb Ensures lesson copy preserves source_project_id when copying a lesson whose project already has a source.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread lib/concepts/lesson/operations/create.rb
@cocomarine cocomarine changed the title Keep source project when adding ExCS projects Record source project when adding ExCS projects Aug 28, 2026
@zetter-rpf
zetter-rpf self-requested a review September 1, 2026 10:19
@zetter-rpf

Copy link
Copy Markdown
Contributor

Thanks for investigating. I want to think about this a bit more - I'm worried adding another similar column to projects might be confusing so want to think through if there any any alternatives or ways to make this clearer.

@zetter-rpf zetter-rpf left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great,

I've had a longer think about this, the thing I don't like about the existing code is how the permissions rely on remixed_from_id to determine what students and teachers should do with a project. I think if a project has been remixed should be a separate concern to if a student or teacher should be able to see it. Currently checking remixes does add complexity to the abilities as we're doing additional database loads for this.

But, even if we changed the abilities, I don't think it would be right to populate the remixed_from_id as part of this - remixing is a different action to what we're doing here (as you said in the description).

@cocomarine
cocomarine temporarily deployed to editor-api-p-1724-add-s-m22az1 September 2, 2026 09:14 Inactive
@cocomarine
cocomarine merged commit c8d3ede into main Sep 2, 2026
8 checks passed
@cocomarine
cocomarine deleted the 1724-add-source-project-id-to-ExCS-projects branch September 2, 2026 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants