Repository navigation
refactor(actions): move action folders to Action/Helper/Service layout - #248
Open
RishadAlam wants to merge 288 commits into
Open
RishadAlam wants to merge 288 commits into
RishadAlam wants to merge 288 commits into
Conversation
The snapshot kept only parameter counts, static-ness and a hash of $authConfig, so diff.php called a handler that gained an unsatisfiable type, or an $authConfig that became protected, equivalent. Record each parameter's type, by-ref flag and default, the return type, and the visibility, static-ness and declared type of $authConfig (an error when it is not public static). public-contract.php takes --root so it can check a moved tree.
seams.php hashed only the second argument, so reordering or replacing the arguments passed to Pro callbacks went unnoticed. Record a hash of all argument expressions (class names mapped through the rename rules) and resolve hook names built from self::/static:: constants and static properties in the same folder to their declared values (BitCrm, NinjaTables, ZendeskSupport). Moving the six sandbox integrations leaves the output byte-identical.
- PHP warnings, notices and deprecations raised in a run are now part
of the compared result, so a property left on the wrong side of a
split ("Undefined property") fails the comparison.
- T3: when an integration has $authConfig but no saved connection, the
run inserts a fixture btcbi_connections row inside the rolled-back
transaction and exercises the connection_id path instead of
recording "skipped".
- Each route gets a coverage verdict (reached HTTP, stopped at a guard,
no HTTP in handler); a handler that calls HTTP but records no request
is reported. Hostnames resolve through a stub so the SSRF check no
longer depends on live DNS, and param fixtures cover integrations
whose params build URLs (CapsuleCRM, Insightly, Zendesk).
…eline Add a dispatch-snapshot job: install WordPress with MySQL, snapshot the base and head trees with the head's tool, and diff them allowing only the Controller/RecordApiHelper renames; every route handler must exist and every $authConfig must be readable. The seams step now also fails when a PR rewrites baseline/seams.tsv unless it carries the seams-change label.
Helpers that bi apply inserts members into, and edited Routes.php, are modified rather than added, so they were skipped. Hold them to the same subset rule against their base file plus the manifest's member sources.
…ools The snapshot now records parameter and return types and $authConfig access, seams hashes every hook argument and resolves constant hook names, and the smoke recordings carry coverage verdicts, T3 connection runs and PHP diagnostics. The backend is unchanged since 0e132f3, so these describe the same F0 tree.
MailChimp, FluentCrm, Asana, GiveWp, WpErp: 0 refusals. Signed-off flags: MailChimp widens resolveTokenDetails from private static to public static (called from execute and the routes); Asana copies its no-argument constructor into the Helper for apiEndpoint; GiveWp's unreferenced pluginActive moves to the Helper for a later dead-code PR. T2 (Pro hooks) on MailChimp, FluentCrm and WpErp; T3 ($authConfig) on MailChimp and Asana.
Each commit only moves whole members between files, so blame should look through it to the change that last touched the line. Use git config blame.ignoreRevsFile .git-blame-ignore-revs locally.
bi verify --tests needs a T2 result for every integration with a Pro Hooks.php, but no harness step wrote one. A flow-smoke --compare with --pro=on,off now records T2 when every flow matched the baseline in both states. F1 verifies with full evidence and no --allow-missing-tests.
List the Controller/RecordApiHelper to Action/Helper/Service renames, the ActionResolver fallback that keeps old <Name>Controller classes dispatchable, and the two names kept permanently for older Pro (SalesforceController, MoosendHelper::formatPhoneNumber).
- params/<Integration>.json keys the deriver did not register (a handler
that reads $params->{$key}) are applied at the top level, and an
override may be an object (WebHooks test_webhook needs hookDetails).
- connections/<Integration>.json adds auth_details to the fixture
connection when $authConfig['fields'] is empty and the client reads
the site URL or token straight off the connection (BrilliantDirectories,
SureContact).
- A forced-expiry run without a write-back is no longer a failure on
its own: the whole-record compare already fails when it changes.
T1 evidence still needs at least one write-back per integration,
unless t1-exempt.json names the integration with a reason.
Fixture flows for the 130 remaining integrations the local site has no saved flow for, HTTP fixtures where a branch needs a specific response, route param and connection fixtures for the 12 routes that stopped at a guard, coverage exemptions for the three MoxieCRM routes that cannot reach HTTP (their $apiEndpoint is never set), and T1 exemptions with reasons for the five OAuth integrations whose refresh never writes the token back today.
Recorded on the tree before any of these integrations move: all 474 routes (with and without a fixture connection), 582 flow replays under Pro on and Pro off (stored flows plus the new fixtures), and forced token-expiry replays for the 21 T1 integrations. Tokens are redacted by the harness. Each batch compares against these.
A full Pro-on/off replay an hour after recording showed 49 differences, none from code: every one came from the clock or randomness. - Hashed content (request bodies, headers, URLs, mail, hook arguments, field data) scrubs every time value to a bare <kind> token before hashing. Visible fields keep <kind+offset@absolute>, and Canon treats two tokens as equal when either the absolute time (a stored value) or the offset from the run clock (a value computed from now) matches. - Clock-based and random smart tags the flow maps are pre-filled with fixed values; Flow::execute keeps existing keys when it merges them. - random_password is stubbed, so WooCommerce account mails with a generated password or reset key hash the same every run. - Quoted multipart boundaries, fractional epochs (wp-cron spawn URLs), month-name dates and clock times are normalised; time-based digests in secret headers (KirimEmail Auth-Token) redact to <redacted:digest>. The previously unstable integrations compare clean after an 8-minute gap.
The first authoring pass could not write these files (the permission classifier timed out), so these three integrations had no flow to replay.
main brought eight new action folders in the Controller + RecordApiHelper layout (BookingsAndAppointmentsForWoocommerce, Charitable, ElementsKit, Flodesk, QuizAndSurveyMaster, QuizMaker, RoxAppointmentBooking, WpSimpleBookingCalendar). Allowlist their 16 files so actions-shape passes until they move, and give them their own batch; bi analyze plans all eight with no refusals.
run-batch.sh moves one batch and stops at the first failing gate: analyze with no refusals, reviewed manifests, apply with one commit per unit, strict autoload dump, route/flow/T1 smoke compare, bi verify with harness evidence, the contract snapshot, seams, counts, public contract and Pro bindings, PHPStan, the php-cs-fixer subset, actions-shape and an untouched frontend. pro-rename-bound.php renames the one class a Pro Hooks.php binds to <Folder>Service and updates its references.
main added eight action folders, changed PerfexCRM, Zoom, GoogleSheet and BuddyBoss, and Pro added ten action folders (renamed to <Folder>Service in Pro). Snapshot: 217 folders, 505 routes, 109 $authConfig, one unresolved stored type (Vimeo, no folder). Seams 713 fire points; counts 113/0/626/593; PHPStan 879 errors; Pro bindings 744 (37 FreshBooks warnings: its free side is not on main yet).
…BookingCalendar Both came in from main with no stored flow. Neither target plugin is active on the reference site, so every replay stops at the plugin check; that still catches a broken class lookup or a guard that moved after a hook or log call. create_calendar is left out because Pro stores a rand()-based ical_hash in the log row.
Flodesk reads its API key off the connection and declares no $authConfig fields, so the fixture connection carried no username and all four list routes stopped at the 'Select a connection' guard.
Recorded with the time-stable harness after merging main and before any F2 to F11 folder moves: 505 routes with and without a fixture connection, 615 flow replays under Pro on and Pro off (every one of the 217 folders has a stored or fixture flow), and 60 forced token-expiry replays for the 21 T1 integrations. Tokens are redacted.
Autonami Mailster Memberpress Newsletter PaidMembershipPro
RestrictContent SliceWp SureCart SureMembers TheEventsCalendar
WPCourseware Airtable BenchMark CampaignMonitor Clickup CompanyHub
Demio DirectIq ElasticEmail EmailOctopus Encharge Flowlu Getgist
Gravitec Lemlist Livestorm MailBluster MailRelay Mailercloud Mailify
Mailjet MoxieCRM OneHashCRM PerfexCRM Salesflare SendFox SendGrid Sendy
Smaily SuiteDash SystemeIO Vbout Woodpecker
bi analyze summary:
integrations=43 refused=0
test T3: 33
flag T3: 33
flag sharedMembers: 8
flag unreachable: 15
layout Action (git mv) + Helper (created): 10
layout Action only: 4
layout Helper (git mv) + Action (created): 29
StoreInCache keeps the active trigger entities in a day-long transient. Once it expires, every smoke process rebuilds it at boot and rolls the write back, so Model is compiled before Probe::begin() installs the error handler and its compile-time deprecations drop out of the record. F9's route smoke ran past the expiry and reported them as removed for every route. Each session now rebuilds the transient once, committed, before discovery and the DB fingerprint.
headerDigest scrubbed header values with the visible Scrub, which renders a timestamp as <ts@seconds>, and hashing could not collapse it after that. KirimEmail signs every request with Timestamp: time(), so its header hash changed every second and F9's route smoke failed on it. The baseline only matched when both runs landed in the same second. Headers are now digested with the collapsed Scrub, which leaves the hash of every header set without a time value unchanged. The KirimEmail route and flow baselines are re-recorded from the pre-move code (F9 base 5641eeb) with only the header hashes and datetime labels differing.
Salesforce Moosend
bi analyze summary:
integrations=2 refused=0
test T1: 1
test T2: 2
test T3: 2
test T5: 1
test T6: 1
flag T1: 1
flag T2: 2
flag T3: 2
flag T5: 1
flag T6: 1
flag getClassStrings: 1
flag visibilityWidenings: 1
layout Action (+ insert into existing Helper): 1
layout Helper (git mv) + Action (created): 1
BookingsAndAppointmentsForWoocommerce Charitable ElementsKit Flodesk
QuizAndSurveyMaster QuizMaker RoxAppointmentBooking
WpSimpleBookingCalendar
bi analyze summary:
integrations=8 refused=0
test T2: 8
test T3: 1
flag T2: 8
flag T3: 1
flag visibilityWidenings: 1
layout Helper (git mv) + Action (created): 8
…on/Service/Helper layout
…sses The cache-key source list in deprecated.md and the actionType normalizer docblock in Flow still named Controller and RecordApiHelper files that the F2 to F9 moves renamed.
Removes file-name header labels ("X Integration", "X Record Api"), section
banners, commented-out code and comments that repeat the line below them,
across the action folders the restructure moved, plus the Flow class
docblock. WHY comments, pragmas (phpcs, translators), @deprecated tags and
typed docblocks stay. Every file's tokens are unchanged apart from comments
and whitespace.
The action moves are finished, so the codemod, smoke harness, gate scripts, recorded baselines, their CI workflow and the blame-ignore list for the move commits go. They stay in this branch's history if a later branch needs them.
✅ WordPress Plugin Check Report
📊 ReportAll checks passed! No errors or warnings found. 🤖 Generated by WordPress Plugin Check Action • Learn more about Plugin Check |
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.
Description
Moves every action folder in
backend/Actions(217 integrations) from<Name>Controller+RecordApiHelperto the bit-pi layout:<Name>Action(what Flow dispatches),<Name>Helper(route handlers) and<Name>Service(field mapping, API calls, hook fire points, logs). A newActionResolverlets Flow and the credential injector find the new classes and still accepts the old<Name>Controllernames. Code moved verbatim, so integrations behave the same.Motivation & Context
Controllers mixed three jobs: dispatch, admin route handlers and record execution. That made large integrations hard to read and every new integration took a different shape. The bit-pi layout splits those jobs into one class each, so Free, Pro and bit-pi now follow the same structure and new integrations have one shape to copy.
Related Links: (if applicable)
<Name>Service)Type of Change
Key Changes
Dispatch
Core/Integration/ActionResolver: tries<Name>Action, then<Name>Controller, then the Pro namespaces; requires anexecutemethod; a broken class file is treated as unresolved instead of a fatal error.Flow::isActionExists()to use the resolver, and log "Integration handler not found" (at most once per flow per hour) when a saved flow's action cannot be resolved instead of failing silently.CredentialInjectorto read$authConfigfrom the sibling<Name>Actionwhen a route points at a Helper. The same 310 routes and 109 actions get credentials as before.Actions
Actions\Salesforce\SalesforceController(empty subclass ofSalesforceHelper) andActions\Moosend\MoosendHelper::formatPhoneNumber()permanently: Pro 2.6.11 to 2.8.6 call them by name.sync_all_sitesfilter, built the BuddyBoss action name throughConfig::withPrefix(), and madeGoogleCalendarHelper::saveRefreshedToken()static.MailChimp Integration, section banners, commented-out code). WHY comments,phpcs/translatorspragmas and@deprecatedtags stay.Docs
deprecated.mdand updated the cache-key source paths there.Verification
Brevo(Sendinblue),Kit(ConvertKit),Monday.Com,GoHighLevel, …).bf3dd0ca5~1).Checklist
Changelog