Docs: note that the status filters are now enforced - #18
Open
HrDelwar wants to merge 1 commit into
Open
Conversation
Both status vocabularies are validated at the write path from the next release (current stable is 6.2.13), so a status that is not registered through its filter can no longer be applied. Previously either endpoint would write whatever string it was handed. fluentform/entry_statuses_core - entry status writes are checked against the per-form list. Add-ons that gate their statuses on a feature being active only contribute while it is enabled, so a row can hold a status that cannot be re-applied once the feature is off; the base statuses stay available. fluentform/available_payment_statuses - the same for the admin transaction endpoint, with one difference worth stating: validation runs only when the status actually changes, so a row already holding an unregistered status keeps it and its other fields stay editable. Also corrects the location note. Two classes define the payment list and both apply the filter - the free PaymentHelper as well as Pro's - and the example array shown is Pro's. 'requires_review' is Pro-only and absent from the free list, which is exactly why a status should be registered through the filter rather than assumed from a hardcoded default. Refs: https://lounge.authlab.io/projects#/boards/16/tasks/22737-Security%3A-form-scope-bypass-an
Deploying fluentform-developers with
|
| Latest commit: |
990dc94
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://20145e85.fluentform-developers.pages.dev |
| Branch Preview URL: | https://docs-status-vocabulary-filte.fluentform-developers.pages.dev |
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
1 participant
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
Two status filters are documented as "you can modify or add statuses". From the next plugin release that becomes a requirement rather than an option: both vocabularies are validated at the write path, so a status not registered through its filter can no longer be applied. Previously either endpoint wrote whatever string it was handed.
This documents the new constraint and corrects a factual error found while writing it.
Related: ticket #22737 · code PRs fluentform-dev#1156 (free) and fluentformpro#273 (Pro)
Changes
fluentform/entry_statuses_core— entry status writes are checked against the per-form list. The note covers the part that isn't obvious from the filter signature: add-ons gate their statuses on the feature being active, so a row can hold a status that cannot be re-applied once the feature is switched off. The base statuses stay available, so an orphaned entry can still be triaged.fluentform/available_payment_statuses— the same for the admin transaction endpoint, with one difference stated explicitly: validation runs only when the status changes, so a row already holding an unregistered status keeps it and its other fields stay editable.Corrected the payment filter's location note. It read:
That is incomplete in a way that misleads. Two classes define this list and both apply the filter —
FluentForm\App\Modules\Payments\PaymentHelper(free) andFluentFormPro\Payments\PaymentHelper(Pro). The example array shown on the page is Pro's, and therequires_reviewentry in it is Pro-only — it is absent from the free list, which is the one the admin transaction endpoint validates against.A developer trusting that example would conclude
requires_reviewis a generally valid status. It is written by Pro's PayPal, Mollie, Paystack and RazorPay processors on an amount mismatch, but cannot be selected in the admin UI, and the status badge falls back to printing the raw slug. The note now says so and points readers at the filter rather than at a hardcoded default.How to verify
npm install && npm run dev, then open Hooks → Filters → Submission and Hooks → Filters → Payment.fluentform/entry_statuses_coreandfluentform/available_payment_statuses— these blocks are collapsible, so the body is hidden until clicked. Each renders a warning callout after its Reference line.requires_reviewas Pro-only.Verified locally both ways:
npm run devandnpm run buildboth compile clean, and the callouts are present in the generateddist/hooks/filters/{submission,payment}/index.html. Uses the repo's existing::: warning Title…:::container, as insrc/hooks/filters/miscellaneous.md:1747.Anything the reviewer should know?
Timing. This describes behaviour that ships with the two code PRs linked below. It reads as a statement of fact, so it should land with or after them, not before.
The divergence is a real defect, documented here only as a workaround. Pro hardcodes
requires_reviewinto its own copy ofgetPaymentStatuses()instead of registering it viafluentform/available_payment_statuses, which is why the two lists differ at all. The correct fix is a one-lineadd_filterin Pro, after which core's list would be complete and this caveat could be deleted. That is deliberately not in scope here — it would add a newly selectable option to the admin status radio group, a visible change on a money path. Worth its own ticket.Heads-up for anyone building locally:
npm run builddeletes the trackedsrc/.vuepress/dist/.gitkeep, which.gitignoreexplicitly un-ignores. Restore it before committing or the diff carries a spurious deletion.Scope. Docs only; no code, no config, no navigation changes.