Skip to content

fix(tasks): keep assignees when tasks/edit omits users - #494

Merged
vermorag merged 1 commit into
mainfrom
fix/task-edit-without-users
Nov 4, 2026
Merged

vermorag merged 1 commit into
mainfrom
fix/task-edit-without-users

Conversation

@amazing-cat

@amazing-cat amazing-cat commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Closes #474

The assignee sync in the TaskController::edit after-action listener now runs only when users is present in the request. An omitted users leaves assignees unchanged and no longer returns 500; an explicit users: [] still unassigns everyone.

Verification

  • Added EditTest::test_edit_without_users_field_keeps_assignees (fails on main with the count() TypeError) and test_edit_with_empty_users_unassigns_everyone.
  • Full PHPUnit suite: 592 tests, 3018 assertions, all passing.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Editing a task without submitting an assignee list now preserves its existing assignees.
    • Submitting an empty assignee list removes all assignees from the task.
    • Assignee change history is recorded only when the assigned users actually change.

An omitted users field made the after-action listener call sync(null),
detaching all assignees, and then whereIn(null) failed with HTTP 500.
Only sync (and record history) when users is present in the request.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

Task edits now leave assignees unchanged when the request omits users. When the request includes users, the controller synchronizes assignees and records user-change history only when synchronization reports changes.

Changes: Task edit assignees

Layer / File(s) Summary
Conditional assignee synchronization
app/Http/Controllers/Api/TaskController.php, tests/Feature/Tasks/EditTest.php
The edit action synchronizes users only when the request includes users. Tests verify that omitting the field preserves an assignee and submitting an empty array removes assignments.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: vermorag

Merge Risk: Minimal · up to 8f4f6

Edits preserve assignees when omitted and remove them when an empty list is submitted. No merge-blocking risk was identified.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: preserving task assignees when the edit request omits the users field.
Linked Issues check Passed Issue #474 requires three things: (1) an omitted users field leaves assignees unchanged, (2) the request does not return HTTP 500, and (3) an explicit users: [] still unassigns everyone. Also, regress…
Out of Scope Changes check Passed The whole-PR diff (merge base e0e9789 to head 8f4f607) changes two files only: TaskController.php and tests/Feature/Tasks/EditTest.php. The controller change is limited to the assignee sync and its hi…
Docstring Coverage Passed No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Full details: Linked Issues check

Explanation

Issue #474 requires three things: (1) an omitted users field leaves assignees unchanged, (2) the request does not return HTTP 500, and (3) an explicit users: [] still unassigns everyone. Also, regression tests must cover edits with and without users on a task with assignees.

Code at head (app/Http/Controllers/Api/TaskController.php, after-action listener in edit): the users sync and the users history dispatch now run only inside if ($request-&gt;has('users')). When users is omitted, sync() is not called, so assignees are kept. The whereIn('id', $userIds) call that caused the count(null) TypeError is only reached when users is present, so the 500 path is removed. When users is [], has() is true, sync([]) detaches all assignees, and the existing behavior is preserved.

Validation check: app/Http/Requests/Task/EditTaskRequest.php declares 'users' => 'sometimes|array' with no nullable rule. An explicit users: null is rejected with 422 before the listener runs, so the has() guard does not leave a null path open.

Tests (tests/Feature/Tasks/EditTest.php): test_edit_without_users_field_keeps_assignees attaches an assignee, omits users, and asserts HTTP OK and that tasks_users still holds the assignee row. test_edit_with_empty_users_unassigns_everyone attaches an assignee, sends users: [], and asserts HTTP OK and no tasks_users rows. Together these cover both required cases. I did not execute the test suite. The result relies on the reviewed source and the PR's reported test run.

Full details: Out of Scope Changes check

Explanation

The whole-PR diff (merge base e0e9789 to head 8f4f607) changes two files only: TaskController.php and tests/Feature/Tasks/EditTest.php. The controller change is limited to the assignee sync and its history dispatch in the edit after-action listener. It replaces get('users') with input('users') and stores the value in $userIds. The inner history dispatch remains conditional on sync changes, and the unconditional SaveTaskEditHistory dispatch is unchanged. The test change adds the Arr import and the two tests required by #474. No unrelated refactoring, feature, or configuration change appears in the diff.

Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 2 files.

Finishing Touches
Generate docstrings
  • Commit to this branch
  • Create a new PR
Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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
Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between ed67358 and 8f4f607.

Files selected for processing: 2
  • app/Http/Controllers/Api/TaskController.php
  • tests/Feature/Tasks/EditTest.php

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

@cattr-cla

cattr-cla Bot commented Nov 4, 2026

Copy link
Copy Markdown

Contributor License Agreement

✅ Every human contributor has accepted Cattr CLA version 1.

CLA SHA-256: 68cb254dc87c899f320d5a8977e7e380fd8f6cf6f9388e51d07d936f3a8ce38b

@vermorag
vermorag marked this pull request as ready for review November 4, 2026 07:35
@vermorag
vermorag requested a review from a team November 4, 2026 07:35
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-04T07:36:48.985894Z 8f4f607 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@vermorag
vermorag merged commit c7427ef into main Nov 4, 2026
36 checks passed
@vermorag
vermorag deleted the fix/task-edit-without-users branch November 4, 2026 07:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

tasks/edit without users returns 500 and silently unassigns all users

2 participants