06: Conflict policy constants - #6
Open
nikolaystrikhar wants to merge 3 commits into
Open
Conversation
Nothing validated a policy string. The resolver switches on it with a default branch that deactivates, so a typo or a stale filter return would turn off a plugin the site owner deliberately activated -- the most surprising of the three outcomes to arrive at by accident. all() and is_valid() give callers a way to tell unknown from DEACTIVATE. Also pin the constant set by reflection, so a fourth policy cannot be added without the resolver's switch being revisited, and correct the DEACTIVATE description: the bundled copy loads on the next request, not this one, since the standalone has already defined the guard constant and the request ends at the redirect.
Open
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.
What: the three conflict-policy string constants, plus
all()andis_valid().Stacked on #5.
Usage:
Why this way: string constants rather than an enum because the PHP floor is 7.4, and rather than bare strings because a host may persist one in an option. The test asserts the literal values for that reason — they are a public contract, not an implementation detail.
The review added the two helpers, and they are the substantive change here. The plan had nothing validating a policy:
Sub_Plugin::get_conflict_policy()returns whatever the config or the…/conflict_policyfilter hands back, and the resolver switches on that string withdefault:falling intodeactivate(). So"defered"— a typo, or a stale return from someone else's filter — would deactivate a plugin the site owner deliberately turned on. Of the three branches that is the destructive one and the least recoverable, and it was the one reached by accident.is_valid()lets the resolver treat unknown as its own case; the plan is updated so it does, falling back toNOTICE_ONLY.The constant set is now pinned by reflection rather than by a distinctness check that the literal-value test already implied. That catches an added fourth policy, which would otherwise be swallowed silently by the same
default:branch.Also corrected the
DEACTIVATEdescription in both the README and the docblock. It read "deactivate the standalone, load the bundled copy, notify, redirect", but the bundled copy does not load on that request — the standalone has already defined the guard constant beforeplugins_loadedfires, so the load path skips it, and the request ends at the redirect anyway. It loads on the next request. The old wording had the causality backwards.Verify:
slic run unit— 29 tests, 40 assertions, green.composer test:analysis→[OK] No errors, exit 0.