Conversation
Closes #1787. The Jobs class was an 18/19 passthrough facade delegating to JobsOperations and JobsStatus. With 29 external callers using `new Jobs(...)`, the cleanest fix is to absorb the satellites back into Jobs rather than push delegation out to every call site. Changes: - Jobs now extends BaseRepository directly and owns CRUD, status transitions, flow health bookkeeping, and schema migrations in one place. - JobsOperations and JobsStatus deleted. - JobCompleteHandler switched from `new JobsOperations()` to `new Jobs()` (update_flow_health_cache and friends are now first-class on Jobs). - Removed an unused `use ... JobsOperations` import in inc/Api/System/System.php. - PostTrackingTest fixed: previously did `new JobsOperations($jobs_db)` against a 0-arg constructor (PHP swallowed the extra arg). Now uses Jobs directly. - Updated docblock references in tests/jobs-get-children-smoke.php. Verified: - `php -l` clean on all touched files. - `composer dump-autoload -o` regenerates clean (1033 classes). - `php tests/jobs-get-children-smoke.php` → all pass. - `homeboy audit` shows facade_passthrough findings dropped from 1 → 0. No behavior change. External `new Jobs()` API preserved.
Contributor
Homeboy Results —
|
… prepare in Jobs::get_children The single-line phpcs:ignore at the top of the get_results() call only suppressed the first line, leaving the inner $wpdb->prepare() args still flagged by WordPress.DB.PreparedSQL.NotPrepared in CI. Switched to a disable/enable block so all 5 lines of the prepared call are covered.
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.
Summary
Jobsfacade passthrough flagged in chubes4/data-machine#1787 by absorbingJobsOperationsandJobsStatusback intoJobs. Net result: 1107 lines deleted, 969 added — one class instead of three, no delegation layer.homeboy audit --path .showsfacade_passthroughfindings drop from 1 → 0 on this branch. No new findings introduced.Why collapse instead of inline
The facade wrapped 18 of 19 public methods as one-liner delegates to
$this->operationsor$this->status. With 29 external call sites all usingnew Jobs(...), two options were on the table:new JobsOperations()everywhere).Jobs.Option 2 has lower blast radius — preserves the public API every caller already depends on, and the two satellites had no independent reason to exist (no separate constructor args, no shared state outside the table, both extended
BaseRepository).Changes
inc/Core/Database/Jobs/Jobs.php— now extendsBaseRepositorydirectly and owns CRUD, status transitions, flow health bookkeeping, and schema migrations in one place.inc/Core/Database/Jobs/JobsOperations.php— deleted.inc/Core/Database/Jobs/JobsStatus.php— deleted.inc/Engine/Actions/Handlers/JobCompleteHandler.php— switched fromnew JobsOperations()tonew Jobs().update_flow_health_cache()is now a first-class method onJobs.inc/Api/System/System.php— removed an unuseduse ... JobsOperationsimport.tests/Unit/Core/WordPress/PostTrackingTest.php— previously callednew JobsOperations($jobs_db)against a 0-arg constructor (PHP silently ignored the extra arg). Now usesJobsdirectly.tests/jobs-get-children-smoke.php— updated docblock references.Verification
php -lclean on every touched file.composer dump-autoload -oregenerates clean (1033 classes).php tests/jobs-get-children-smoke.php→ all assertions pass.homeboy audit --path .→facade_passthrough: 0(was 1).No behavior change. External
new Jobs()API preserved.Related audit issues — closed as upstream false positives
While triaging the open audit issues that produced this fix, three sibling issues were closed as detector false positives and filed upstream against the homeboy / homeboy-extensions audit rules rather than worked around in DM:
option scope drift(54) — fires on every*_optioncall in any file whose comments mention "multisite" or "network" in passing. DM is intentionally single-site (SITE.md: multisite: false). Upstream: Extra-Chill/homeboy-extensions#424.constant backed slug literal(25) —has::GROUPis a parser glitch (the word "has" in a docblock matched as a class name); the remaining findings are semantic conflations (plugin slug vs Action Scheduler group, bundle JSON keys vs internal class constants, test stubs deliberately inlining option names to verify production code uses them). Upstream: Extra-Chill/homeboy-extensions#425.command output policy(3) — fires on idiomatic WP-CLI usage (WP_CLI::success/error/log). No canonical output layer exists for WP-CLI consumers without inventing one. Upstream: Extra-Chill/homeboy#2335.Per the
RULES.md"fix upstream first, never paper over" rule — these are documented upstream rather than papered over withhomeboy:ignoremarkers in DM.AI assistance