Phase 5 — The restructure (Plugin Modules + composition + alias trick + code fixes)#14
Open
edpittol wants to merge 10 commits into
Open
Conversation
Restore the missing comma between `allow-plugins` and `sort-packages` so the manifest parses as valid JSON. Without this fix `composer install` and the static-analysis CI gates cannot run.
…ubnamespaces Move every WooCommerce concern (Method Traits, Page Objects, Config, storage hierarchies) under `src/WooCommerce/` and rename the namespace to `Aztec\WPBrowser\WooCommerce\*`. Move the Action Scheduler Method Trait under `src/ActionScheduler/Method/` and rename `ActionSchedulerMethods` to `ActionMethods` since the Plugin Subnamespace now carries the "Action Scheduler" qualifier (ADR-0003). `src/AztecWPBrowser.php` is updated to import from the new locations so the codebase remains buildable while the new Plugin Modules are introduced in the follow-up commit. Anchor the `woocommerce/`, `woocommerce-subscriptions/` and `storefront/` `.gitignore` rules to the repo root so the new `src/WooCommerce/` directory stops being matched case-insensitively on HFS+/APFS filesystems.
`amOnMyAccountPage` lived inside `CustomerMethods` and `amOnAdminOrderPage` inside `OrderMethods` even though both touch the browser only. With the upcoming split into `WooCommerceDb` (DB-layer) and `WooCommerceWebDriver` (browser-layer) Plugin Modules, the orphan methods need to live on the browser-layer side so the DB module stays free of `WPWebDriver` and `WooCommerceConfig` dependencies. Move them into `CustomerBrowserMethods` and `OrderBrowserMethods`. The new `OrderBrowserMethods` resolves the appropriate `OrderStorageInterface` itself (reading `woocommerce_custom_orders_table_enabled` via `WPDb`) so the browser module never has to call back into `WooCommerceDb` for HPOS state (ADR-0005). Drop the now-unused `wpWebDriver`/`wooCommerceConfig` abstract dependency-getters from the DB-only traits.
… with class alias trick Replace the monolithic `Aztec\WPBrowser\AztecWPBrowser` module with a pair of Plugin Modules under `Aztec\WPBrowser\WooCommerce\Module\`: - `WooCommerceDb` composes the WooCommerce DB-layer Method Traits and the `ActionScheduler` Method Trait. It validates that `WPDb` is enabled in the suite and throws `ModuleException` with a clear message otherwise. HPOS detection lives in a `protected isHposEnabled()` helper that reads `woocommerce_custom_orders_table_enabled` from `wp_options` on every call (no cache, no public actor method — ADR-0005). - `WooCommerceWebDriver` composes the WooCommerce browser-layer Method Traits including the new `CustomerBrowserMethods` and `OrderBrowserMethods`. It owns the `PageObjectProvider` (page objects are browser-side) and validates that both `WPWebDriver` and `WPDb` are enabled in the suite (`WPDb` is needed for slug/option lookups; the dependency never goes through `WooCommerceDb`). Both modules extend `\Codeception\Module` and reach wp-browser via `getModule()` (composition over extension — ADR-0002). Method Traits keep their abstract dependency-getter declarations because PHPStan level-max cannot infer `getModule()` return types without either docblocks (forbidden by the project style) or per-call `assert()` casts that would bloat every trait method body. The abstract decls give PHPStan the typing it needs while keeping trait call sites identical to the legacy code. Add `src/aliases.php` to register `Codeception\Module\WooCommerceDb` and `Codeception\Module\WooCommerceWebDriver` aliases via composer's `autoload.files` directive so consumers can enable the modules by short name in `suite.yml` (ADR-0004). The registration guards against namespace squatting with `class_exists($alias, false)` and emits an `E_USER_WARNING` when an alias is already registered, recommending the FQN as the documented escape hatch. The whole file runs inside an IIFE to keep helper variables out of the global scope. Update `tests/acceptance.suite.yml` to enable both Plugin Modules by short name and move the `pageObjects` configuration onto `WooCommerceWebDriver`. Delete the legacy `src/AztecWPBrowser.php`. Storage abstractions audit (Conservative outcome from the audit step of the PRD): the three storage hierarchies (`Storage/`, `OrderStorage/`, `SubscriptionStorage/`) were already relocated under `src/WooCommerce/` in the previous commit; no further consolidation in this phase. The parallel `Storage/` + `*Storage/` structure stays as-is to avoid expanding the surface of an already-large restructure; revisit in a future phase if duplication becomes painful.
…>assert* Three quality improvements that were tagged P2 in the planning sweep and were small enough to fold into this restructure: 1. Rename `array $data` to `array $overrides` on every `have*InDatabase` method that takes an overrides bag (traits, storage classes, and their interfaces). The name now matches the semantics — callers pass overrides, not a complete representation. 2. Rewrite `seeOrderStatus` and `seeSubscriptionStatus` so they call `seeOrderInDatabase` / `seeSubscriptionInDatabase` instead of grabbing the status and calling `assertEquals`. Failures now surface as a Codeception DB-step in the report rather than a bare assertion miss. 3. Replace every raw `assert(...)` in Cests with the equivalent `$I->assert*(...)` call so failure messages are rich and the assertion shows up in Codeception's step log. Conversions cover the standard patterns: `is_int && > 0` → `assertIsInt` + `assertGreaterThan`, `=== false` → `assertFalse`, `=== <literal>` → `assertSame`, `count(...) === N` → `assertCount`, `in_array(..., true)` → `assertContains`, etc.
Add a Codeception unit suite plus three targeted test classes for the deep modules called out in the PRD's Testing Decisions section: - `WooCommerceDbHposDetectorTest` exercises the `protected isHposEnabled()` helper through reflection. It verifies the `'yes' → true`, `'no' → false`, and missing-option-returns-false contract, and explicitly asserts that the option is re-read on every call (no caching at module level — ADR-0005). - `AliasRegistrarTest` proves both short-name aliases exist after composer's `autoload.files` runs, that the alias resolves to the FQN class, and that a pre-existing alias causes the registrar to skip with a `E_USER_WARNING` that mentions the conflicting alias and recommends the FQN as the documented fallback (ADR-0004 S2 safeguard). - `SiblingModuleCheckTest` mocks `hasModule()` on each Plugin Module to prove `_initialize()` throws `ModuleException` with a helpful message when its required Sibling Module is missing, and returns cleanly when present. To make the alias registrar testable, extract the registration loop out of the IIFE into `Aztec\WPBrowser\registerAliases()`. The function remains a private detail of the autoload step (no public API yet) but unit tests can now exercise the conflict path without re-evaluating the autoload-time IIFE. Wire the unit suite via `tests/unit.suite.yml` (no actor — tests extend `Codeception\Test\Unit` directly) and add the `Aztec\WPBrowser\Tests\Unit\` PSR-4 mapping to `composer.json`'s `autoload-dev`. Also fix the implicit-nullable `string $hook = null` deprecation on `ActionMethods::runScheduledActions()` while in the neighbourhood; PHP 8.4 warns on the implicit form.
Both Plugin Modules detect HPOS independently (ADR-0005 forbids the browser module from calling the DB module), but until now they each owned the option name and the comparison literal — two sources of truth that could drift if WooCommerce ever renamed the flag. Introduce `HposState` (a `final` class with the option name as a `const` and a single static `isEnabled(WPDb $wpDb): bool` method). Both `WooCommerceDb::isHposEnabled()` and `OrderBrowserMethods::resolveOrderStorage()` now delegate to it. The modules still detect HPOS independently — they share a class, not a call path. Add `HposStateTest` covering the `yes`/`no`/missing branches and asserting that the option key is the canonical `woocommerce_custom_orders_table_enabled`.
The three public methods (`cartPageSlug`, `checkoutPageSlug`, `myAccountPageSlug`) differed only in the `wp_options` row they queried. They each owned a private nullable cache field and a copy of the option-id → post-name lookup body — three sources of truth for the same algorithm. Move the body into a single `pageSlug(string $pageIdOption)` helper keyed by option name and back it with one `slugCache` array. Promote the constructor parameter while passing through. Public method names and signatures stay unchanged so no caller needs to move.
`AbstractHPOSStorage` and `AbstractLegacyStorage` carried byte-identical copies of the same handful of helpers: the `WPDb` constructor, the `getMetaTableName` lookup, `mapMetaCriteria`, `grabEntityMeta`, and `haveEntityMetaInDatabase`. All four methods are post-meta-flavoured even in HPOS mode (HPOS keeps order metadata in `wp_postmeta`), so there is no storage-mode reason to keep them split. Introduce `AbstractStorage` as the shared parent: it owns the constructor, the `getEntityIdKey()` abstract, the four post-meta helpers, and `implements WooCommerceStorageInterface`. The two storage-mode abstracts now `extend AbstractStorage` and keep only the methods that genuinely differ — `getTableName`, `getIdColumnName`, `mapCriteria`, `grabEntityStatus`, `haveEntityStatus`. The HPOS one also keeps `use HPOSStorageTrait` for the `wc_orders` table-name and ID-generation helpers. Net code reduction: ~36 lines, and the next storage entity to land (e.g. EDD downloads) inherits the post-meta helpers for free.
…rt trait
`WooCommerceDb` and `WooCommerceWebDriver` carried byte-identical
`wpDb()` helpers (`getModule('WPDb')` + `assert` cast) and the same
lazy-initialised `wooCommerceConfig()` factory. Two sources of truth
for code that has to behave identically across both modules.
Move both helpers into a new `WooCommerceModuleSupport` trait. The
modules `use` it alongside their Method Traits and drop the inline
copies. The trait is named for its current scope: when a future
plugin's modules want the same shape (e.g. an `EddDb` reaching for
`getModule('WPDb')`), they can either copy this trait into their own
Plugin Subnamespace or, if the duplication recurs, we promote the
generic `wpDb()` helper into a shared `Support` subnamespace.
`WooCommerceWebDriver` keeps `wpWebDriver()` inline since it is the
only module that needs a `WPWebDriver` accessor today.
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.
Closes #6.
What changed
This PR is the Phase 5 restructure: replace the monolithic
Aztec\WPBrowser\AztecWPBrowsermodule with a pair of Plugin Modules (WooCommerceDb,WooCommerceWebDriver) and reorganise every WooCommerce concern under theAztec\WPBrowser\WooCommerce\Plugin Subnamespace. Action Scheduler moves to its ownAztec\WPBrowser\ActionScheduler\Subnamespace and its Method Trait is renamedActionMethods. Consumers reference the new modules by short name insuite.ymlvia the Class Alias Trick (src/aliases.php).Reviewed commit-by-commit, the PR is small per layer:
chore: fix composer.json syntaxrefactor: relocate WooCommerce and ActionScheduler code into Plugin Subnamespacesrefactor: split orphan browser methods into dedicated browser traitsfeat: introduce WooCommerceDb and WooCommerceWebDriver Plugin Modules with class alias trickrefactor: P2 fold-ins — overrides parameter, see*Status via WPDb, $I->assert*test: cover HPOS detector, alias registrar, and sibling-module checksHow HPOS-aware browser code works without depending on
WooCommerceDbADR-0005 forbids cross-module calls for HPOS state.
WooCommerceWebDrivertherefore re-implements detection inOrderBrowserMethods::resolveOrderStorage()by readingwoocommerce_custom_orders_table_enableddirectly through itsWPDbsibling. The two modules detect HPOS independently; neither calls the other.Storage-abstractions audit outcome
Per the PRD's audit step I went with the Conservative outcome: relocate
Storage/,OrderStorage/, andSubscriptionStorage/wholesale undersrc/WooCommerce/with their existing structure intact. Justification:Storage/(shared abstract base) +*Storage/(entity-specific concrete) split is intentional — it letsOrderandSubscriptionshare the HPOS/Legacy table-name detection without duplicating it.If duplication starts to bite in a later phase, a dedicated audit ticket can revisit the moderate/aggressive options.
Intentional deviation from the PRD
The PRD's "Deletions" subsection says abstract dependency-getter declarations should be removed from each trait. I kept them.
Reason: PHPStan level-max cannot infer the return type of
\Codeception\Module::getModule(string $name)— it's typed as\Codeception\Module. Dropping the abstract decls would force one of three bad options at every call site: docblock@varcasts (project style forbids docblocks), per-callassert($x instanceof WPDb)(massive noise in every method body), or@phpstan-ignorecomments (anti-pattern).The abstract decls are a small, well-understood boilerplate that gives PHPStan the typing it needs. ADR-0002 already documents them as the chosen pattern, and the trait call sites stay identical to the legacy code.
If the project later adopts a generic
getModule()stub via a PHPStan extension this can be revisited.What was deferred
composer checklocally (no PHP vendor in the worktree; the project's tooling runs throughdocker compose). CI will be the source of truth — if it surfaces violations I'll address them here in a follow-up commit. The new code follows PSR-12 + slevomat conventions throughout, so I expect findings to be minor.main. Listed in the PRD as a wrap-up admin task; happens after this PR lands and CI is green.Test plan
composer install && composer check— phpstan level max + phpcs clean.make test-up && docker compose -f docker-compose.test.yml exec php vendor/bin/codecept build— actor regenerates with the new module method surface.docker compose -f docker-compose.test.yml exec php vendor/bin/codecept run unit— all three new unit suites green.make hpos-disable && docker compose -f docker-compose.test.yml exec php vendor/bin/codecept run acceptance OrderCest SubscriptionCest— legacy storage paths still pass.make hpos-enable && docker compose -f docker-compose.test.yml exec php vendor/bin/codecept run acceptance OrderHPOSCest SubscriptionHPOSCest— HPOS storage paths still pass.docker compose -f docker-compose.test.yml exec php vendor/bin/codecept run acceptance— full suite green.