feat(projects): add projects module with kanban board and backlog#386
Merged
Conversation
When add_member encounters an active member, it now delegates to change_member_role to apply the role change with guards. This prevents unconditionally demoting the last active admin to member, a state that change_member_role and remove_member already prevent for direct calls. Separated paths: - New member or reactivating departed member: direct assignment to role/left_at - Active member: delegated to change_member_role with last-admin guard Regression test: test_add_member_on_active_last_admin_cannot_demote now ensures add_member(project, sole_admin_user, role=MEMBER) raises LastAdminError. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The registry kept _by_id to index actions by id, but action ids are not unique
across target types. Both DeleteProjectAction and DeleteTaskAction register
id="delete", and since task.py loads last, get("delete") silently returned the
task action, breaking project callers. Verified no external callers of get() or
_by_id exist. Removed dead API while keeping register(), get_available_actions(),
all(), _ensure_loaded(), _reset() working exactly as today.
…r archived guard Route create and partial_update responses through get_serializer to include project context. Replace malformed test with two specific tests for missing user and invalid user ID. Add comprehensive test coverage for archived project mutation blocking. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Task.status used PROTECT, which raises even when the referencing TaskStatus rows are being deleted in the same cascade (project delete -> cascade statuses -> protect trips on tasks). RESTRICT keeps direct status deletion blocked while letting a same-cascade delete proceed.
MemberViewSet.destroy applied the archived-project write guard to the self-leave path, trapping members in archived projects. The guard now only applies when an admin removes someone else; the last-admin rule still blocks self-leave at the service level.
LabelSerializer.validate_name was check-then-act; a concurrent insert of the same name could still hit the database unique constraint and surface as a 500. The viewset now catches that IntegrityError inside a savepoint and returns a 400 with the same validation message.
saveTask had no in-flight guard and the modal's Save button was never bound to the saving flag, so a double click (or a slow network) could fire two create requests and produce duplicate tasks.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughSummary by CodeRabbit
WalkthroughChangesAdds a complete projects and tasks feature with Django models, migrations, access rules, services, REST APIs, action permissions, board UI, routing, and automated tests. CI now enforces 85% coverage for the projects module. Projects and tasks — API and permissions — Board UI — Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant BoardUI
participant ActionsAPI
participant ProjectsAPI
participant Database
User->>BoardUI: Open project or task
BoardUI->>ActionsAPI: Request available actions
ActionsAPI->>Database: Resolve project, task, and role
Database-->>ActionsAPI: Matching records and access role
ActionsAPI-->>BoardUI: Action descriptors
BoardUI->>ProjectsAPI: Create, update, delete, or reorder task
ProjectsAPI->>Database: Persist task changes
Database-->>ProjectsAPI: Updated state
ProjectsAPI-->>BoardUI: API response
BoardUI->>ProjectsAPI: Refresh board and backlog
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
workspace/projects/tests/test_queries.py (1)
12-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate setUp fixture across two test classes.
UserProjectIdsTests.setUp(Lines 13-27) andGetProjectRoleTests.setUp(Lines 66-80) build the identical admin/member/outsider/project/membership fixture. Consider extracting a shared local mixin/helper for this file to avoid the duplication.♻️ Proposed refactor
+class _QueriesFixtureMixin: + def setUp(self): + self.admin = User.objects.create_user( + username="admin1", email="admin1@test.com", password="pass123" + ) + self.member = User.objects.create_user( + username="member1", email="member1@test.com", password="pass123" + ) + self.outsider = User.objects.create_user( + username="outsider1", email="outsider1@test.com", password="pass123" + ) + self.project = Project.objects.create(name="Website", created_by=self.admin) + ProjectMember.objects.create( + project=self.project, user=self.admin, role=ProjectMember.Role.ADMIN + ) + ProjectMember.objects.create(project=self.project, user=self.member) + + -class UserProjectIdsTests(TestCase): - def setUp(self): - self.admin = User.objects.create_user( - username="admin1", email="admin1@test.com", password="pass123" - ) - self.member = User.objects.create_user( - username="member1", email="member1@test.com", password="pass123" - ) - self.outsider = User.objects.create_user( - username="outsider1", email="outsider1@test.com", password="pass123" - ) - self.project = Project.objects.create(name="Website", created_by=self.admin) - ProjectMember.objects.create( - project=self.project, user=self.admin, role=ProjectMember.Role.ADMIN - ) - ProjectMember.objects.create(project=self.project, user=self.member) +class UserProjectIdsTests(_QueriesFixtureMixin, TestCase):Apply the analogous change to
GetProjectRoleTests.Also applies to: 65-80
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workspace/projects/tests/test_queries.py` around lines 12 - 27, Extract the identical fixture setup from UserProjectIdsTests.setUp and GetProjectRoleTests.setUp into a shared local mixin or helper in this test module. Have both test classes reuse it while preserving the existing admin, member, outsider, project, and membership attributes and setup behavior.workspace/projects/ui/static/projects/ui/js/board.js (1)
4-9: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueTest the drag path, not just
moveUuidmoveUuidis only exercised throughwindow.projectBoardHelpers; the production reorder flow inonDropuses direct DOM moves pluslistOrderand never calls it. Add coverage foronDrop/reorder behavior, or drop the helper export if it is only for unit tests.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workspace/projects/ui/static/projects/ui/js/board.js` around lines 4 - 9, The current coverage targets moveUuid through window.projectBoardHelpers but does not validate the production reorder flow in onDrop, which directly updates the DOM and listOrder. Add tests that exercise onDrop and verify the resulting order, or remove the projectBoardHelpers export if moveUuid is only retained for unit tests.workspace/projects/serializers.py (1)
147-154: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueN+1 query pattern in
validate_assignees.Each assignee triggers a separate
get_project_rolecall (a membership query, plus a groups query on fallback). For tasks with several assignees this is avoidable with a single batched membership/group lookup.♻️ Suggested batched validation
def validate_assignees(self, users): project = self.context["project"] - for user in users: - if get_project_role(user, project) is None: - raise serializers.ValidationError( - f"{user.username} is not a member of this project." - ) + member_ids = set( + ProjectMember.objects.filter( + project=project, user__in=users, left_at__isnull=True + ).values_list("user_id", flat=True) + ) + group_member_ids = set() + if project.group_id is not None: + group_member_ids = { + u.pk for u in users if u.groups.filter(pk=project.group_id).exists() + } + for user in users: + if user.pk not in member_ids and user.pk not in group_member_ids: + raise serializers.ValidationError( + f"{user.username} is not a member of this project." + ) return users🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workspace/projects/serializers.py` around lines 147 - 154, Update validate_assignees to replace the per-user get_project_role calls with one batched membership/group lookup for all assignees in the project. Validate each user against the batched result and preserve the existing serializers.ValidationError message for non-members.workspace/projects/services/tasks.py (1)
14-50: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winWrap
create_taskmulti-write operations intransaction.atomic()
Task.objects.create(),task.assignees.set(),task.labels.set(), and the conditionaltask.save()are separate DB writes. If any M2M operation fails after the task is created, the task persists in a partial state (e.g., created but missing assignees or labels). Thereorder_tasksfunction already usestransaction.atomic()for consistency, so this function should follow the same pattern.♻️ Proposed fix
def create_task( project, user, *, title, description="", status=None, priority=Task.Priority.MEDIUM, due_date=None, assignees=(), labels=(), ): """Create a task; defaults to the end of the project's backlog column.""" if status is None: status = ( project.statuses.filter(category=TaskStatus.Category.BACKLOG) .order_by("position", "created_at") .first() ) or project.statuses.order_by("position", "created_at").first() - task = Task.objects.create( - project=project, - title=title, - description=description, - status=status, - priority=priority, - due_date=due_date, - created_by=user, - position=next_position(project, status), - ) - if assignees: - task.assignees.set(assignees) - if labels: - task.labels.set(labels) - if status.category == TaskStatus.Category.DONE: - task.completed_at = timezone.now() - task.save(update_fields=["completed_at"]) - return task + with transaction.atomic(): + task = Task.objects.create( + project=project, + title=title, + description=description, + status=status, + priority=priority, + due_date=due_date, + created_by=user, + position=next_position(project, status), + ) + if assignees: + task.assignees.set(assignees) + if labels: + task.labels.set(labels) + if status.category == TaskStatus.Category.DONE: + task.completed_at = timezone.now() + task.save(update_fields=["completed_at"]) + return task🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workspace/projects/services/tasks.py` around lines 14 - 50, Wrap the full multi-write body of create_task—including Task.objects.create(), optional assignees.set(), labels.set(), and the conditional completed_at save—in transaction.atomic(). Keep status selection and the existing task field values unchanged, and ensure any failure rolls back all writes together.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@workspace/projects/services/members.py`:
- Around line 14-19: Serialize admin-affecting operations per project to
eliminate the TOCTOU race. Update change_member_role and remove_member to lock
the project row (or otherwise lock the full active-admin set) before checking
_other_active_admins(member).exists() and saving, ensuring concurrent role
changes or removals on the same project queue and always preserve at least one
active admin.
In `@workspace/projects/services/tasks.py`:
- Around line 77-83: Update the transaction logic around the in_status and
listed queries to use one select_for_update() query filtered by Q(status=status)
| Q(uuid__in=ordered_uuids), ensuring all relevant tasks are locked in a single
pass. Preserve the existing status-based and UUID-based collections needed by
the reorder logic, deriving them from the combined locked result rather than
issuing two locking queries.
In `@workspace/projects/ui/static/projects/ui/js/board.js`:
- Around line 147-167: Wrap the fetch-driven bodies of openTask, saveTask, and
deleteTask in try/catch blocks, matching the existing boundary-handling pattern
used by saveOrder, sendToBoard, and fetchActions. Preserve their current success
and generation-check behavior, and route network or other request failures into
the existing formError feedback path instead of allowing unhandled rejections;
retain any necessary finally cleanup in saveTask.
---
Nitpick comments:
In `@workspace/projects/serializers.py`:
- Around line 147-154: Update validate_assignees to replace the per-user
get_project_role calls with one batched membership/group lookup for all
assignees in the project. Validate each user against the batched result and
preserve the existing serializers.ValidationError message for non-members.
In `@workspace/projects/services/tasks.py`:
- Around line 14-50: Wrap the full multi-write body of create_task—including
Task.objects.create(), optional assignees.set(), labels.set(), and the
conditional completed_at save—in transaction.atomic(). Keep status selection and
the existing task field values unchanged, and ensure any failure rolls back all
writes together.
In `@workspace/projects/tests/test_queries.py`:
- Around line 12-27: Extract the identical fixture setup from
UserProjectIdsTests.setUp and GetProjectRoleTests.setUp into a shared local
mixin or helper in this test module. Have both test classes reuse it while
preserving the existing admin, member, outsider, project, and membership
attributes and setup behavior.
In `@workspace/projects/ui/static/projects/ui/js/board.js`:
- Around line 4-9: The current coverage targets moveUuid through
window.projectBoardHelpers but does not validate the production reorder flow in
onDrop, which directly updates the DOM and listOrder. Add tests that exercise
onDrop and verify the resulting order, or remove the projectBoardHelpers export
if moveUuid is only retained for unit tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e14d61e5-ae52-46df-8289-3f264da608f7
📒 Files selected for processing (51)
.github/workflows/tests.ymlworkspace/common/static/css/app.cssworkspace/projects/__init__.pyworkspace/projects/actions/__init__.pyworkspace/projects/actions/base.pyworkspace/projects/actions/project.pyworkspace/projects/actions/task.pyworkspace/projects/apps.pyworkspace/projects/migrations/0001_initial.pyworkspace/projects/migrations/0002_alter_task_status.pyworkspace/projects/migrations/__init__.pyworkspace/projects/models.pyworkspace/projects/queries.pyworkspace/projects/serializers.pyworkspace/projects/services/__init__.pyworkspace/projects/services/members.pyworkspace/projects/services/projects.pyworkspace/projects/services/tasks.pyworkspace/projects/tests/__init__.pyworkspace/projects/tests/base.pyworkspace/projects/tests/js/board.test.jsworkspace/projects/tests/test_actions.pyworkspace/projects/tests/test_api_actions.pyworkspace/projects/tests/test_api_labels.pyworkspace/projects/tests/test_api_members.pyworkspace/projects/tests/test_api_projects.pyworkspace/projects/tests/test_api_reorder.pyworkspace/projects/tests/test_api_tasks.pyworkspace/projects/tests/test_apps.pyworkspace/projects/tests/test_models.pyworkspace/projects/tests/test_queries.pyworkspace/projects/tests/test_services.pyworkspace/projects/tests/test_services_tasks.pyworkspace/projects/tests/test_ui.pyworkspace/projects/ui/__init__.pyworkspace/projects/ui/apps.pyworkspace/projects/ui/static/projects/ui/js/board.jsworkspace/projects/ui/templates/projects/ui/index.htmlworkspace/projects/ui/templates/projects/ui/partials/backlog.htmlworkspace/projects/ui/templates/projects/ui/partials/board.htmlworkspace/projects/ui/templates/projects/ui/partials/project_list.htmlworkspace/projects/ui/templates/projects/ui/partials/task_card.htmlworkspace/projects/ui/templates/projects/ui/partials/task_modal.htmlworkspace/projects/ui/templates/projects/ui/project.htmlworkspace/projects/ui/urls.pyworkspace/projects/ui/views.pyworkspace/projects/urls.pyworkspace/projects/views_actions.pyworkspace/projects/viewsets.pyworkspace/settings.pyworkspace/urls.py
4 tasks
pchopinet
added a commit
that referenced
this pull request
Jul 12, 2026
…tion (#387) ## Summary Follow-ups logged during the projects module final review (#386): - **Membership removal**: `remove_member` is now a no-op on already-departed rows, preserving the original departure timestamp instead of overwriting it on replay. - **Reorder timestamps**: `reorder_tasks` stamps `updated_at` on moved tasks; `bulk_update` bypasses `save()` so `auto_now` left the timestamp stale. - **Task deletion**: deleting a task from the board modal now asks for confirmation through `AppDialog.confirm`, matching the convention used by every other module. - **`members_data` rename**: key `uuid` -> `id`; the value is the stock User integer pk, not a UUID. - **Pinning tests**: departed member with group access keeps `member` role and project visibility (group access is independent of membership rows, files precedent). Two follow-ups from the review list needed no action: the `ProjectRuleError` -> 400 helper already exists (`_rule_error_response`) and `moveUuid` was already removed. ## Test plan - [x] Each behavioral fix has a regression test verified to fail against the pre-fix code (including the JS confirm tests, checked via stash/restore) - [x] `uv run python manage.py test workspace.projects`: 124/124 OK - [x] `node --test "workspace/*/tests/js/**/*.test.js"`: 186/186 OK - [x] ruff format + check clean
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
New
workspace/projects/module: collaborative task management with shared projects, a kanban board and a hand-ordered backlog.Data model
Project(typepersonal/kanban, optional attachedauth.Group, archived flag, no owner column) with a partial unique constraint guaranteeing one personal project per userProjectMemberwithadmin/memberroles andleft_at(chat pattern); service-level guard keeps at least one active admin per projectTaskStatusas a per-project table with a semanticcategory(backlog/active/done), seeded with 4 fixed statuses at creation (schema ready for custom columns later)Task(priority, due date, assignees, labels, manualposition) andLabelAccess control and actions
queries.py(user_project_ids(user, role=None),get_project_role) used by every endpoint and viewfiles/actions/(min_role,available_when_archived, personal-project rules) plus a bulkPOST /api/v1/projects/actionsendpoint driving the UIAPI (DRF ViewSets + serializers)
/api/v1/projects/<uuid>POST .../tasks/reorderendpoint covering backlog ordering, in-column sort and cross-column kanban movesUI (data-visualization first)
/projectslist with lazy-created personal project and a creation modal/projects/<uuid>board + backlog tabs, native HTML5 drag and drop (pinned-folders pattern), action-gated task modal, alpine-ajax partial refreshespreview: truein the module registry, so the dashboard shows the "Preview" badgeTests and CI
projectsadded to the CI matrix with a 85% floorTest plan
uv run python manage.py test workspace.projects(118 tests OK)node --test "workspace/projects/tests/js/**/*.test.js"(4/4)uv run coverage report --include='workspace/projects/*' --fail-under=85(97.3%)