Skip to content

Foreign-key dependent fix Project assets, causing issue in reseed task and endpoint (integration)#826

Merged
abcampo-iry merged 2 commits into
mainfrom
issues/1419
May 20, 2026
Merged

Foreign-key dependent fix Project assets, causing issue in reseed task and endpoint (integration)#826
abcampo-iry merged 2 commits into
mainfrom
issues/1419

Conversation

@abcampo-iry
Copy link
Copy Markdown
Contributor

@abcampo-iry abcampo-iry commented May 20, 2026

Status

Points for consideration:

  • Security
  • Performance

What's changed?

Fix /test/reseed failures caused by Scratch project assets blocking seeded project cleanup.
The test seed destroy task removes seeded lesson projects with Project.destroy_all, but project-scoped ScratchAsset records still referenced those projects.

Added a test and found is red with same error as sentry.

I only wonder here maybe if we want to make dependent :destroy or dependent nullify. (I guess this doesn't happen manually but it's worht to think if we would like to solve it in other way)

Copilot AI review requested due to automatic review settings May 20, 2026 08:15
@cla-bot cla-bot Bot added the cla-signed label May 20, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 20, 2026

Test coverage

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

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes /test/reseed cleanup failures by ensuring Scratch project assets don’t block seeded Project destruction via foreign-key constraints.

Changes:

  • Add Project -> scratch_assets association with dependent: :destroy so associated assets are removed when a project is destroyed.
  • Extend the test_seeds:destroy spec to cover the ScratchAsset cleanup case.
  • Add a model association spec to lock in the new dependency behavior.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
app/models/project.rb Ensures ScratchAsset rows tied to a project are destroyed when the project is destroyed, preventing FK violations during reseed cleanup.
spec/models/project_spec.rb Verifies the new has_many :scratch_assets, dependent: :destroy association behavior.
spec/lib/test_seeds_spec.rb Reproduces the reseed failure scenario by creating a project-scoped ScratchAsset and asserting it’s removed by the destroy task.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@abcampo-iry abcampo-iry changed the title Foreign-key dependent fix Project assets Foreign-key dependent fix Project assets, causing issue in reseed task and endpoint (integration) May 20, 2026
Copy link
Copy Markdown
Contributor

@zetter-rpf zetter-rpf left a comment

Choose a reason for hiding this comment

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

Nice one

@zetter-rpf
Copy link
Copy Markdown
Contributor

I only wonder here maybe if we want to make dependent :destroy or dependent nullify. (I guess this doesn't happen manually but it's worht to think if we would like to solve it in other way)

I think it's hard to decide because we don't have any use cases for deleting scratch projects in production and we're only doing it in test. Think this is a great approach for now.

@abcampo-iry abcampo-iry merged commit 84f36ee into main May 20, 2026
5 checks passed
@abcampo-iry abcampo-iry deleted the issues/1419 branch May 20, 2026 10:17
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.

3 participants