fa/breadcrumb-the-shire
1
0
Fork 1
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

refactor(action-context): remove unused fragment aggregator + building block

Browse Source 
actionFragmentContext was built spec-driven in step 1 to handle the
anticipated drawer-fragment pattern: authorize + finder → status DTO.
The action-context rollout (steps 2-11) found that none of the three
real drawer fragments (users/view-fragment, addressbook/view-fragment,
helpdesk/ticket-fragment) match that pattern — all three delegate to
domain services that own their own status models. Step 11 declared
them structural exceptions; the helper code remained unused.

This commit removes the dead spec:
* core/Support/helpers/action_context.php drops actionFragmentContext
  (~9 LOC) and its building block actionFragmentResolveOrStatus
  (~30 LOC) plus their docblocks. The MUST-call-actionRequireCsrf
  warning, present in three aggregator docblocks before, now appears
  twice (one per remaining aggregator).
* tests/Support/Helpers/ActionContextHelperTest.php drops the six
  unit tests that exercised these functions (~83 LOC).
* tests/Architecture/ActionContextHelperContractTest.php drops the
  fragment building block from the buildingBlocks() data provider
  (5 entries instead of 6) and removes testFragmentResolveReturnDocblockIsFrozen.
  The CSRF-warning expectation is updated from 3 to 2 with a code
  comment explaining the rollback.

Verified:
* No production caller exists in pages/ or modules/.
* All 9 aggregator callers (5 actionEditContext + 4 actionCreateContext)
  remain unchanged.
* ActionContextCsrfPairingContractTest and DetailDrawerFragmentContractTest
  are deliberately left untouched: their allowlists/recognizer regexes
  still mention actionFragmentContext, but the patterns now match an
  empty set — harmless dead text. Documented as open items in the run
  report; future cleanup is optional and orthogonal to this removal.
* QGs all green (PHPUnit 2088 tests, PHPStan level 5, CS-fixer 0 diffs).

Net: 3 files, +31/-198 LOC, behavior unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Felix Schneider
2026-04-26 17:34:53 +02:00
parent 95723cebd9
commit 9ec10f5c02
3 changed files with 31 additions and 198 deletions
Download Patch File Download Diff File Expand all files Collapse all files

51
tests/Architecture/ActionContextHelperContractTest.php
View File

@@ -7,22 +7,27 @@ use ReflectionFunction;
use ReflectionNamedType;
/**
* Freezes the API of the 6 orthogonal building blocks in
* Freezes the API of the 5 orthogonal building blocks in
* core/Support/helpers/action_context.php.
*
* Aggregators (actionEditContext, actionCreateContext, actionFragmentContext)
* are intentionally NOT frozen — they may evolve additively in Step 2 of the
* rollout.
* Aggregators (actionEditContext, actionCreateContext) are intentionally NOT
* frozen — they may evolve additively across the rollout.
*
* This test also asserts that the Tenant-Scope and Fragment-Resolver helpers
* carry their PHPStan array-shape return docblocks. Acceptance check SC-004
* relies on the shape staying stable.
* This test also asserts that the Tenant-Scope helper carries its PHPStan
* array-shape return docblock. Acceptance check SC-004 relies on the shape
* staying stable.
*
* Step 2 (Pilot-Migration Departments-Edit, Run
* 2026-04-25-action-context-rollout-step2-departments) is the first
* production caller. The previous testNoProductionCallSitesYet assertion
* has been removed; the CSRF↔Aggregator pairing is enforced by
* tests/Architecture/ActionContextCsrfPairingContractTest.php instead.
*
* Run 2026-04-25-action-fragment-context-removal pruned the dead
* Cluster-10 Drawer-Fragment helpers (actionFragmentResolveOrStatus building
* block + actionFragmentContext aggregator) after Step 11 declared all three
* fragment actions structural exceptions that delegate to domain-services
* with their own status models.
*/
class ActionContextHelperContractTest extends TestCase
{
@@ -60,12 +65,6 @@ class ActionContextHelperContractTest extends TestCase
['capabilities', null, false],
['whitelistedFlags', null, false],
]],
['actionFragmentResolveOrStatus', [
['finder', null, false],
['rawId', null, false],
['abilityKey', null, false],
['context', null, false],
]],
];
}
@@ -132,18 +131,6 @@ class ActionContextHelperContractTest extends TestCase
);
}
public function testFragmentResolveReturnDocblockIsFrozen(): void
{
$reflection = new ReflectionFunction('actionFragmentResolveOrStatus');
$doc = (string) $reflection->getDocComment();
$this->assertMatchesRegularExpression(
"/@return array\{status:[^}]*'ok'[^}]*'forbidden'[^}]*'not_found'[^}]*'invalid_id'/",
$doc,
"actionFragmentResolveOrStatus must declare PHPStan array-shape return covering all 4 status values."
);
}
public function testEnforceCanViewPageDocblockHasStrictComparisonNote(): void
{
$reflection = new ReflectionFunction('actionEnforceCanViewPage');
@@ -181,19 +168,25 @@ class ActionContextHelperContractTest extends TestCase
'/MUST call actionRequireCsrf\(\) BEFORE this aggregator/',
$contents
);
// Expected count is 2: actionEditContext + actionCreateContext.
// Run 2026-04-25-action-fragment-context-removal removed the
// actionFragmentContext aggregator (Cluster 10 has no aggregator —
// drawer-fragments use domain-service status models instead), reducing
// the warning count from 3 to 2.
$this->assertSame(
3,
2,
$count,
'Each of the 3 aggregators (actionEditContext, actionCreateContext, actionFragmentContext) must carry the explicit CSRF-warning in its docblock (GR-SEC-001).'
'Each of the 2 aggregators (actionEditContext, actionCreateContext) must carry the explicit CSRF-warning in its docblock (GR-SEC-001).'
);
}
public function testAggregatorsExistButAreNotFrozen(): void
{
// We assert existence only — signatures may evolve in Step 2.
// We assert existence only — signatures may evolve.
// Run 2026-04-25-action-fragment-context-removal removed
// actionFragmentContext; only EditContext and CreateContext remain.
$this->assertTrue(function_exists('actionEditContext'));
$this->assertTrue(function_exists('actionCreateContext'));
$this->assertTrue(function_exists('actionFragmentContext'));
}
/**

87
tests/Support/Helpers/ActionContextHelperTest.php
View File

@@ -234,57 +234,6 @@ class ActionContextHelperTest extends TestCase
$this->assertSame(['page' => ['can_view_page' => true]], $result);
}
/* ----------------------------------------------------------------
* actionFragmentResolveOrStatus
* ---------------------------------------------------------------- */
public function testFragmentResolveReturnsOkWithModelAndCapabilities(): void
{
$this->stubAuthorizationService(static fn (): AuthorizationDecision => AuthorizationDecision::allow([
'capabilities' => ['can_view' => true],
]));
$finder = static fn (string $id): array => ['id' => $id];
$r = actionFragmentResolveOrStatus($finder, 'abc-123', 'ABILITY_VIEW', []);
$this->assertSame('ok', $r['status']);
$this->assertSame(['id' => 'abc-123'], $r['model'] ?? null);
$this->assertSame(['can_view' => true], $r['capabilities'] ?? null);
}
public function testFragmentResolveReturnsForbidden(): void
{
$this->stubAuthorizationService(static fn (): AuthorizationDecision => AuthorizationDecision::deny());
$finder = static fn (string $id): array => ['id' => $id];
$r = actionFragmentResolveOrStatus($finder, 'abc', 'ABILITY_VIEW', []);
$this->assertSame('forbidden', $r['status']);
$this->assertSame(403, $r['http_code'] ?? null);
}
public function testFragmentResolveReturnsNotFound(): void
{
$this->stubAuthorizationService(static fn (): AuthorizationDecision => AuthorizationDecision::allow());
$finder = static fn (): mixed => null;
$r = actionFragmentResolveOrStatus($finder, 'abc', 'ABILITY_VIEW', []);
$this->assertSame('not_found', $r['status']);
$this->assertSame(404, $r['http_code'] ?? null);
}
public function testFragmentResolveReturnsInvalidIdForBlankRawId(): void
{
$this->stubAuthorizationService(static fn (): AuthorizationDecision => AuthorizationDecision::allow());
$finder = static fn (): mixed => null;
$r = actionFragmentResolveOrStatus($finder, ' ', 'ABILITY_VIEW', []);
$this->assertSame('invalid_id', $r['status']);
$this->assertSame(400, $r['http_code'] ?? null);
}
/* ----------------------------------------------------------------
* Aggregator: actionEditContext
* ---------------------------------------------------------------- */
@@ -449,42 +398,6 @@ class ActionContextHelperTest extends TestCase
$this->assertStringContainsString('error/forbidden', $this->capturedRedirect());
}
/* ----------------------------------------------------------------
* Aggregator: actionFragmentContext
* ---------------------------------------------------------------- */
public function testFragmentContextOk(): void
{
$this->stubAuthorizationService(static fn (): AuthorizationDecision => AuthorizationDecision::allow([
'capabilities' => ['can_view' => true],
]));
$result = actionFragmentContext([
'finder' => static fn (string $id): array => ['uuid' => $id],
'rawId' => 'u-1',
'abilityKey' => 'ABILITY_VIEW',
'context' => [],
]);
$this->assertSame('ok', $result['status']);
$this->assertSame(['uuid' => 'u-1'], $result['model'] ?? null);
}
public function testFragmentContextForbidden(): void
{
$this->stubAuthorizationService(static fn (): AuthorizationDecision => AuthorizationDecision::deny());
$result = actionFragmentContext([
'finder' => static fn (string $id): array => ['uuid' => $id],
'rawId' => 'u-1',
'abilityKey' => 'ABILITY_VIEW',
'context' => [],
]);
$this->assertSame('forbidden', $result['status']);
$this->assertSame(403, $result['http_code'] ?? null);
}
/* ----------------------------------------------------------------
* helpers
* ---------------------------------------------------------------- */