Skip to content

fix(cli): reject filter flags the resource does not support - #26

Merged
Warkanlock merged 2 commits into
mainfrom
eops-369-filter-gating
Sep 1, 2026
Merged

fix(cli): reject filter flags the resource does not support#26
Warkanlock merged 2 commits into
mainfrom
eops-369-filter-gating

Conversation

@Warkanlock

Copy link
Copy Markdown
Contributor

Summary

  • Generic --start-date / --end-date / --status were sent on every list. The API drops unknown query params, so customers list --start-date 2025-01-01 returned unfiltered results with exit 0.
  • Gate those flags per resource. Unsupported flags now exit 2 and name the resource plus the flag.
  • Recreates eops-369-fix(commands): gate generic filter flags per resource #20 (closed unmerged as draft). Same commit, new PR.

Fixes EOPS-369

Changes

  • Per-resource filter set at registration
  • Unknown generic flags rejected before the request
  • Tests in tests/test_filter_gating.py

Test plan

  • Unit tests pass (uv run pytest)
  • Linter passes (uv run ruff check .)
  • Live: customers list --start-date 2025-01-01 exits 2 and names the flag
  • Live: companies list --status posted exits 2
  • Live: invoices list --start-date 2025-01-01 still filters
  • Live: customers list --status posted still works

The list factory sent search, record_status, start_date and end_date to
every resource. Filter schemas in the public API declare only their own
fields and django-ninja drops the rest before validation, so unsupported
flags returned unfiltered results with exit code 0.

Each resource now declares the filter flags its /public/v2 list endpoint
actually exposes, verified against the v2 OpenAPI schema. Flags outside
that set are removed from the command, aliases included, so passing one
exits non-zero and names the flag and the resource. The argument is
required, so a new resource cannot inherit a wrong default.

Also:
- contracts name their status filter `status`, not `record_status`; the
  CLI mapped --status to record_status, which the API ignored while the
  real filter stayed unreachable. Fixed assets declare both and keep the
  record_status mapping, which is what --status means everywhere else.
- expose company/customer/vendor on the eleven resources that support
  them but were not offering them.

@Warkanlock Warkanlock left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additional Comments

The following comments reference lines outside the diff and are included here:

💡 src/dualentry_cli/main.py:20
Verdict: Filter gating is correctly assigned to all 30+ resources and matches test expectations in test_filter_gating.py. The new status_param override for contracts (line 88) correctly maps --status to the status query param instead of record_status. All make_resource_app() calls now supply a filters set. Rejection of unsupported flags is tested (lines 97–127 of the test file). Residual: the diff cites the v2 OpenAPI schema as the source of truth for each resource's filters set, but does not link to a live schema check—manual updates could drift if the API changes.

Comment thread src/dualentry_cli/commands/__init__.py
Comment thread src/dualentry_cli/commands/__init__.py
Comment thread src/dualentry_cli/cli.py
Keep both make_list_command_cls (EOPS-369) and _MAX_PAGES
(EOPS-378) after #23 landed.
@Warkanlock
Warkanlock merged commit 00a0222 into main Sep 1, 2026
4 checks passed
Warkanlock added a commit that referenced this pull request Sep 1, 2026
Keep EOPS-368 has_* flags with EOPS-369 filter sets after
#25 and #26 landed. Paper-checks stay list/get only.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants