Skip to content

fix(user): list active org members by default#21

Merged
xergioalex merged 1 commit into
mainfrom
fix/user-list-active-only
May 26, 2026
Merged

fix(user): list active org members by default#21
xergioalex merged 1 commit into
mainfrom
fix/user-list-active-only

Conversation

@xergioalex
Copy link
Copy Markdown
Member

Summary

dailybot user list was returning every user the server exposed, including deactivated accounts — confusing for the human-facing table and a correctness hazard for downstream resolvers (e.g. kudos give --to "Old Employee" matching against a deactivated user record).

This change filters out is_active: false rows by default at the DailyBotClient.list_users layer. A missing is_active field is treated as active for forward-compatibility with older server payloads. The legacy unfiltered output is still available via dailybot user list --include-inactive.

Change Log

  • api_client.py::DailyBotClient.list_users now accepts include_inactive (keyword-only, default False). When False, drops rows with is_active falsy.
  • commands/user.py: new --include-inactive flag (default off). Updated help text + examples.
  • commands/user_scoped_actions.py::execute_user_list plumbed through.
  • Tests:
    • 2 new list_users tests — filter-by-default + opt-out behavior.
    • 1 new command-level test — asserts --include-inactive is forwarded.
    • Existing list_users paginated fixtures updated with is_active: True.

Risks

  • Behavior change in the default response of dailybot user list — but the change reflects user intent ("members of the organization" → current/active members), and --include-inactive preserves the legacy output for admin / audit flows.
  • No payload change to outgoing requests; no schema migrations; no new dependencies.
  • Downstream impact: kudos give --to "<name>" resolves against list_users(), so deactivated accounts are now correctly excluded from name-matching. This is the desired behavior.

Local validation

  • ruff check, ruff format --check, mypy dailybot_cli — all clean.
  • pytest tests/api_client_test.py tests/public_api_commands_test.py — 78/78 pass.
  • Smoke-tested against the local api-services server: dailybot user list returns 4 active members.

Test plan

  • dailybot user list shows only active members by default.
  • dailybot user list --include-inactive shows the full server response.
  • dailybot kudos give --to "<deactivated-user-name>" returns "No user found matching X".
  • dailybot user list --json matches the table contents (both filtered identically by default).

🤖 Generated with Claude Code

`dailybot user list` (and any internal caller of `DailyBotClient.list_users`)
was returning every user the server exposed, including deactivated
accounts. That's both confusing for the human-facing table and a
correctness hazard for downstream resolvers (e.g. `kudos give --to "X"`
matching against a deactivated user record).

This change filters out `is_active: false` rows by default. A missing
`is_active` field is treated as active for forward-compatibility with
older server payloads.

## Change Log
- `api_client.py::DailyBotClient.list_users` now accepts `include_inactive`
  (keyword-only, default False). When False, drops rows with
  `is_active` falsy.
- `commands/user.py`: new `--include-inactive` flag on `dailybot user list`
  (default off). Updated help text + examples.
- `commands/user_scoped_actions.py::execute_user_list` plumbed through.
- Tests: two new `list_users` tests (filter-by-default + opt-out) and one
  new command-level test asserting `--include-inactive` is forwarded.
- Existing `list_users` tests updated to include `is_active: True` on
  the fixture rows so the default-filter path behaves correctly.

## Risks
- Behavior change in the default response of `dailybot user list` — but
  the change reflects user intent ("members of the organization" means
  current/active members), and the opt-out flag preserves the legacy
  output for admin / audit flows.
- No payload change to outgoing requests; no schema migrations.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@xergioalex xergioalex merged commit 32e2c17 into main May 26, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant