Skip to content

feat: add task comment system with role-based permissions - #2

Merged
obvious-autobuild[bot] merged 3 commits into
masterfrom
obvious/build-task-comment-system-9df889
Jun 3, 2026
Merged

feat: add task comment system with role-based permissions#2
obvious-autobuild[bot] merged 3 commits into
masterfrom
obvious/build-task-comment-system-9df889

Conversation

@obvious-autobuild

@obvious-autobuild obvious-autobuild Bot commented Jun 3, 2026 •
edited
Loading

Copy link
Copy Markdown

Problem

Tasks lacked any commenting capability. The feature requires two distinct permission models: personal task comments (owner-only) and project task comments (role-based: Viewer/Member/Moderator/Admin).

Solution

Adds a full task comment CRUD system as a nested resource under /tasks/{task_pk}/comments/ and /projects/{project_pk}/tasks/{task_pk}/comments/.

Files changed

File Change
tasks/models.py Add TaskComment model
tasks/migrations/0006_add_taskcomment_model.py Auto-generated migration
tasks/serializers.py Add TaskCommentSerializer
tasks/permissions.py Add IsCommentAuthor, ProjectCommentPermission
tasks/services.py Add CommentService
tasks/views.py Add TaskCommentViewSet
tasks/urls.py Wire nested router for /tasks/{id}/comments/
projects/urls.py Wire nested router for /projects/{id}/tasks/{id}/comments/
api/tests/test_comments.py 34 new tests

Permission matrix

Personal tasks:
  ALL operations  ->  task owner only

Project tasks:
  GET/HEAD/OPTIONS  ->  Viewer+
  POST              ->  Member+
  PUT/PATCH         ->  comment author only
  DELETE            ->  comment author OR Moderator+
  (Project owner bypasses all role checks)

Key design choices (per locked decisions)

  • CommentService follows TaskService/CategoryService pattern — static methods, no state
  • TEXT_FIELD_VALIDATOR applied to TaskComment.text (same as Task.title/Task.description)
  • Flat comments only (no parent/reply FK)
  • ProjectCommentPermission.has_object_permission receives the task's project (a Project instance) directly so IsProjectMinRole's membership lookup ProjectMembership.objects.get(project=obj, user=user) works correctly

Implementation note

TaskCommentViewSet.get_object() explicitly calls ProjectCommentPermission().has_object_permission(...) directly rather than relying on DRF's check_object_permissions(). The reason: permission_classes = [IsAuthenticated] controls viewset-level auth; for object-level decisions that vary by HTTP method AND user-role/authorship, calling the permission directly ensures the right logic runs and avoids the dispatcher only finding IsAuthenticated in permission_classes.

Test coverage

34 tests in api/tests/test_comments.py:

  • Personal task CRUD: owner access, unauthenticated rejection, non-owner rejection at list/detail/create, empty text validation, ordering
  • Project task CRUD: Viewer reads but cannot create/delete; Member creates and deletes own; Moderator deletes any; author edits own; non-author cannot edit; owner has full access
  • Edge cases: wrong project -> 404, nonexistent task -> 404, validator rejects special chars, task isolation

All 108 tests pass (74 pre-existing + 34 new).

Human author: Dasha Shifrina dasha@flatfile.io

Obvious Agent @dashashifrina
feat: add TaskComment model, migration, serializer, permissions, serv…
57bd7e7 
…ice, viewset, and URL wiring

- TaskComment model in tasks/models.py: task FK, author FK, text (TextField + TEXT_FIELD_VALIDATOR),
  created_at (auto_now_add), updated_at (auto_now). Ordered by created_at asc. Index on task.
- Auto-generated Django migration 0006_add_taskcomment_model.py
- TaskCommentSerializer: read-only author_name, author ID, timestamps; writable text
- ProjectCommentPermission: GET/HEAD/OPTIONS → Viewer+, POST → Member+, PUT/PATCH → author only,
  DELETE → author OR Moderator+
- IsCommentAuthor permission helper
- CommentService with create_comment, update_comment, delete_comment static methods
- TaskCommentViewSet nested under TaskViewSet; works for both personal and project tasks
- tasks/urls.py: NestedSimpleRouter wires /tasks/{task_pk}/comments/
- projects/urls.py: NestedDefaultRouter wires /projects/{project_pk}/tasks/{task_pk}/comments/

Co-authored-by: Dasha Shifrina <dasha@flatfile.io>
Obvious Agent @dashashifrina
feat: add TaskCommentViewSet, permissions, and tests for comment system
488df63 
- Fix ProjectCommentPermission to call has_object_permission directly in
  get_object() rather than relying on check_object_permissions (which uses
  permission_classes, not ProjectCommentPermission)
- TaskCommentViewSet.get_object: explicitly calls ProjectCommentPermission for
  project tasks instead of using DRF's check_object_permissions dispatcher
- Add IsCommentAuthor and ProjectCommentPermission classes in tasks/permissions.py
- Add 34 tests in api/tests/test_comments.py covering:
  - Personal task CRUD: owner-only access, empty text, non-owner rejection
  - Project task CRUD: Viewer read, Member create, author edit, Moderator delete
  - Edge cases: wrong project, nonexistent task, validator rejection, ordering
- All 108 tests pass (74 existing + 34 new)

Co-authored-by: Dasha Shifrina <dasha@flatfile.io>
@obvious-autobuild
obvious-autobuild Bot marked this pull request as ready for review June 3, 2026 18:22
Obvious Agent @dashashifrina
ci: trigger CI after retargeting PR to master
52ceb24 
Co-authored-by: Dasha Shifrina <dasha@flatfile.io>
@coveralls

coveralls commented Jun 3, 2026 •
edited
Loading

Copy link
Copy Markdown

Coverage Report for CI Build 26904747932

Warning

No base build found for commit 66c300c on master.
Coverage changes can't be calculated without a base build.
If a base build is processing, this comment will update automatically when it completes.

Coverage: 93.53%

Details

  • Patch coverage: 17 uncovered changes across 4 files (411 of 428 lines covered, 96.03%).

Uncovered Changes

File Changed Covered %
tasks/permissions.py 26 18 69.23%
tasks/views.py 84 77 91.67%
tasks/models.py 11 10 90.91%
tasks/serializers.py 11 10 90.91%
Total (9 files) 428 411 96.03%

Coverage Regressions

Requires a base build to compare against. How to fix this →

Coverage Stats

Coverage Status
Relevant Lines: 2164
Covered Lines: 2024
Line Coverage: 93.53%
Coverage Strength: 3.74 hits per line
💛 - Coveralls

1 similar comment
@coveralls

coveralls commented Jun 3, 2026

Copy link
Copy Markdown

Coverage Report for CI Build 26904747932

Warning

No base build found for commit 66c300c on master.
Coverage changes can't be calculated without a base build.
If a base build is processing, this comment will update automatically when it completes.

Coverage: 93.53%

Details

  • Patch coverage: 17 uncovered changes across 4 files (411 of 428 lines covered, 96.03%).

Uncovered Changes

File Changed Covered %
tasks/permissions.py 26 18 69.23%
tasks/views.py 84 77 91.67%
tasks/models.py 11 10 90.91%
tasks/serializers.py 11 10 90.91%
Total (9 files) 428 411 96.03%

Coverage Regressions

Requires a base build to compare against. How to fix this →

Coverage Stats

Coverage Status
Relevant Lines: 2164
Covered Lines: 2024
Line Coverage: 93.53%
Coverage Strength: 3.74 hits per line
💛 - Coveralls

@obvious-autobuild
obvious-autobuild Bot merged commit dce6169 into master Jun 3, 2026
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@dashashifrina dashashifrina

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@coveralls @dashashifrina