Changed members filter state to preserve unknown NQL filter properties#28479
Draft
kevinansfield wants to merge 2 commits into
Draft
Changed members filter state to preserve unknown NQL filter properties#28479kevinansfield wants to merge 2 commits into
kevinansfield wants to merge 2 commits into
Conversation
no issue This adds a labs-gated warning and exact members filter for identifying members with active subscriptions across multiple Stripe customers.
ref #28232 - the multiple-active-subscriptions banner needed its filter to survive the members URL writeback, which was handled with a one-off `preserveMultipleActiveSubscriptionsFilter` mode bolted onto the filter state hook - generalised the underlying filter state handling instead: the filter param is parsed once, split into its top-level AND clauses via the AST, and any clause the filter UI can't represent (unknown fields, OR groups, operators the field map doesn't advertise) is preserved through the URL, the `nql` output, and chip edits rather than silently dropped - added an AST→NQL serializer as the inverse of nql-lang's parser, since the library has no serializer of its own; it returns undefined for anything it can't faithfully round-trip so clauses are dropped rather than corrupted
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
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.
ref #28232
The multiple-active-subscriptions filter in #28232 needed to survive the members URL writeback, which was handled with a one-off
preserveMultipleActiveSubscriptionsFiltermode bolted ontouseMembersFilterState. That special case meant any other NQL the filter UI can't represent — unknown fields, OR groups, operators the field map doesn't advertise — was still silently scrubbed from the URL, breaking hand-crafted and legacy Ember filter links.This generalises the underlying filter state handling so unknown NQL filter properties are always preserved, and removes the special-case mode entirely:
filterparam is parsed once and split into its top-level AND clauses via the AST. Clauses that map to filter chips behave as before; every other valid clause is carried through the URL writeback, thenqloutput (so it still filters the member list via the API), and chip edits. Only "clear" actions remove them.serializeAstToNql) as the inverse of the parser for every shape its grammar can emit, including the~/~^/~$regex forms and preserved relative dates. It's deliberately conservative: anything it can't faithfully round-trip returns undefined and the clause is dropped rather than risk writing corrupted NQL back to the URL.The banner behaviour from #28232 is unchanged — it still exact-matches its filter in
nql— but adding a chip on top of an unknown filter now ANDs the two together instead of dropping the unknown part, and unknown filters now work regardless of the labs flag (the flag only gates the banner UI).Stacked on #28232 so only the refactor is in the diff; it'll retarget to
mainwhen that merges.