eops-369-fix(commands): gate generic filter flags per resource - #20
Closed
mykhaylob-de wants to merge 1 commit into
Closed
eops-369-fix(commands): gate generic filter flags per resource#20mykhaylob-de wants to merge 1 commit into
mykhaylob-de wants to merge 1 commit into
Conversation
mykhaylob-de
force-pushed
the
eops-369-fix-generic-filter-gating
branch
from
September 1, 2026 09:56
dffccd5 to
0fd007b
Compare
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.
mykhaylob-de
force-pushed
the
eops-369-fix-generic-filter-gating
branch
from
September 1, 2026 09:56
0fd007b to
7ed526a
Compare
6 tasks
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
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
_build_filter_paramssentsearch,record_status,start_dateandend_dateto every resource. Public API filter schemas declare only their own fields, and django-ninja drops undeclared query params before validation, so unsupported flags came back as unfiltered results with exit code 0 — a wrong answer that looks right.The audit named four resources. Cross-referencing every
make_resource_appregistration against the live/public/v2OpenAPI schema found 13:depreciation-books--search,--status,--start-date,--end-date(all four)classifications,companies,inbox,workflows,recurring/{invoices,bills,journal-entries}--status,--start-date,--end-datecustomers,items,budgets,fixed-assets--start-date,--end-datecontracts--statusTwo corrections to the ticket:
record_statusis declared oncustomers,itemsandbudgets, so only the date flags were broken there; and their date filters do exist, under different names (updated_after/updated_before, fromUpdatedAtRangeFilterMixin).Changes
Each resource declares the filter flags its v2 list endpoint actually exposes. Flags outside that set are stripped from
list, aliases included, so passing one exits non-zero naming the flag and the resource:A genuine typo still gets click's own suggestion:
--stat→No such option: --stat (Possible options: --status).filtersis now a required keyword argument, so a new resource cannot inherit a wrong default.contractsname their status filterstatus, notrecord_status. The CLI mapped--statustorecord_status, which the API ignored while the real filter stayed unreachable. Addedstatus_paramto the factory.fixed-assetsdeclares both, and keeps therecord_statusmapping — that is what--statusmeans everywhere else in this CLI.Exposed
--company/--customer/--vendoron the eleven resources that support them but were not offering them (journal-entries,fixed-assets,paper-checks,customers,vendors,contracts,budgets,workflows,bank-transfers,recurring/*).On
extra="forbid"(acceptance criterion 4)Setting it on filter schemas would be a no-op, so it is not worth doing. django-ninja resolves query params through
__ninja_flatten_map__, which copies only the schema's declared field names out ofrequest.GETbefore pydantic runs. Extras never reach validation.Verified empirically: a
FilterSchemawithmodel_config = ConfigDict(extra="forbid")still returns 200 and ignores?bogus=1, and amodel_validator(mode="before")shows pydantic receiving<DjangoGetter: {'name': 'a'}>with the extras already stripped. Independently confirmed against django-ninja's source (ninja/params/models.py:58-84,ninja/signature/details.py:148-218).Rejecting unknown params server-side needs a different mechanism — middleware diffing
request.GETagainst each operation's declared query params. That is separate work; this PR fixes the CLI only, so other v2 clients remain exposed.Test plan
uv run pytest) — 212 passed, 125 skipped (skips are live-API tests gated onX_API_KEY).uv run ruff check src/ tests/) — all checks pass;ruff format --checkclean.tests/test_filter_gating.pypins the full 35-resource flag matrix, coverscustomers/items/companies/budgetsplusdepreciation-books,classifications,workflows,inboxandrecurring/*, exercises short aliases (-c,-s), and asserts no HTTP call is made when a flag is rejected.customers list --start-date 2025-01-01exits 2;customers list --status posted --company 7sends{"record_status": "posted", "company_id": "7"};contracts list --all --status activesends{"status": "active"}on the pagination path too.