Skip to content

fix(projects): delete chapter assignments across all project units#204

Merged
henrique221 merged 2 commits into
mainfrom
fix/delete-project-assignments-all-units
Jun 29, 2026
Merged

fix(projects): delete chapter assignments across all project units#204
henrique221 merged 2 commits into
mainfrom
fix/delete-project-assignments-all-units

Conversation

@henrique221

@henrique221 henrique221 commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

From the code audit (untracked follow-up).

Problem

deleteByProject selected a single project unit with .limit(1) and deleted only that unit's chapter assignments. If a project ever has more than one project_unit, the other units' assignments are silently orphaned.

createProject currently makes exactly one unit, so this is latent today — but the cleanup shouldn't assume a 1:1 project↔unit relationship.

Fix

Replace the select-one + delete-by-id (two round-trips, in a transaction) with a single statement that deletes assignments for all of the project's units via a subquery — effectively:

DELETE FROM chapter_assignments
WHERE project_unit_id IN (SELECT id FROM project_units WHERE project_id = $1)
RETURNING id

One round-trip, no transaction needed (single atomic statement), and the empty case returns deletedCount: 0 naturally.

Test plan

  • npm run typecheck
  • npm run lint
  • npm run test — 80/80 pass

Summary by CodeRabbit

  • Bug Fixes
    • Deleting chapter assignments for a project now removes assignments across all units in that project, not just one.
    • The number of removed assignments is now reported more accurately after deletion.

deleteByProject selected a single project_unit with limit(1) and deleted
only that unit's chapter assignments, so a project with more than one unit
would orphan the others. createProject currently makes exactly one unit, so
this is latent — but the cleanup shouldn't assume a 1:1 project<->unit
relationship.

Replace the select-one + delete-by-id (two round-trips in a transaction)
with a single DELETE whose WHERE uses a project_units subquery, covering all
of the project's units. One round-trip, no transaction needed, and the empty
case returns deletedCount: 0 naturally.

From the code audit (untracked follow-up).
@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 37013cd5-07c7-42a4-82d2-51ce92d6bc92

📥 Commits

Reviewing files that changed from the base of the PR and between bffc4c8 and 5199e0e.

📒 Files selected for processing (1)
  • src/domains/projects/chapter-assignments/project-chapter-assignments.repository.ts

📝 Walkthrough

Walkthrough

deleteByProject now removes chapter assignments for every project unit with one delete query and returns the deleted count from that query.

Changes

Project chapter assignment deletion

Layer / File(s) Summary
Bulk delete by project
src/domains/projects/chapter-assignments/project-chapter-assignments.repository.ts
deleteByProject replaces the single-unit transactional flow with a bulk DELETE over all matching project units and uses the deleted rows to compute the return value.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: deleting chapter assignments for all project units in a project.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/delete-project-assignments-all-units

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@henrique221

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.

@henrique221 henrique221 self-assigned this Jun 25, 2026
@henrique221 henrique221 added the bug Something isn't working label Jun 25, 2026

Copilot AI left a comment

Copy link
Copy Markdown

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 fixes a latent data-integrity issue in the projects domain by ensuring chapter assignments are deleted for all project units under a project, rather than only one unit.

Changes:

  • Replaced a transaction-based “select one project unit + delete by that unit id” flow with a single DELETE ... WHERE project_unit_id IN (subquery) statement.
  • Ensures multi-unit projects don’t leave orphaned chapter assignments and returns an accurate deletedCount.

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

@henrique221
henrique221 merged commit 997cce8 into main Jun 29, 2026
3 checks passed
@github-actions
github-actions Bot deleted the fix/delete-project-assignments-all-units branch June 29, 2026 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants