Conversation
Every writer of stock (product_update.sql, product_create.sql, product_copy.sql) announces product-corpus through ProductCacheInvalidator::invalidateCorpus(), but Stock only carried its own URI tag. An admin stock edit left app://self/product/stock?productCode=... serving the stale value for up to 30s, and App\Product's rebuild after its own purge still re-embedded that stale Stock read because the child's cache was never invalidated. Stock now sets Header::SURROGATE_KEY to Products::SURROGATE_KEY, the same tag every other writer of the corpus already announces. Added a gated oracle flow (product-stock-corpus) that purges product-corpus and asserts Stock's own entry falls with it - it reproduces violations 4/5 before this fix and passes after. Registered in verify-all.sh's flow loop so the gate runs it going forward.
LinkHeaderRenderer.render() called HtmlLinkAuditor::audit() unconditionally, so every HTML response - production included, where a SilentHtmlLinkAuditLogger only discarded the result - paid for the regex scan of the rendered body. HtmlLinkAuditor is final with no interface, so the renderer required it as a hard constructor dependency. Split the renderer and its module in two: - LinkHeaderRenderer (production): only the Link: header contract. No HtmlLinkAuditor dependency at all - the audit type does not appear in the production dependency graph. links() is now public so a decorator can reuse the same computation. - AuditedLinkHeaderRenderer / AuditedLinkHeaderModule (test-only, new): wraps the production renderer and adds the audit call. install()ed explicitly by the two tests that judge the audit result (HtmlLinkAuditLedgerTest, LinkHeaderRendererTest); never referenced by HtmlModule or any production context. HtmlLinkAuditSuppressionTest pinned the old shape (default context resolves HtmlLinkAuditLoggerInterface to a Silent instance); updated to assert the stronger post-#132 fact - the default context has no binding for it at all, so nothing in that graph can warn anywhere. Verified: full suite green (2805 tests), psalm clean, HtmlLinkAuditLedgerTest still reconciles its full ledger, LinkHeaderRendererTest's Link: header contract passes on the audit-free production renderer, and `composer page -- get '/'` still emits the Link: header in a real SQL-backed render.
Three Auth adapters (EccubeSharedSessionAdapter, HtmlAdminSessionAdapter, EccubeSharedCsrfTokenAdapter) each carried an identical private ensureSessionStarted() - same CLI/active/headers-sent guards, same session_name() + session_start() options - and EccubeSharedCsrfTokenAdapter's copy hardcoded a cross-reference to EccubeSharedSessionAdapter::COOKIE_NAME regardless of which session adapter a context actually bound. Which cookie a request's session lived under was a fact only recoverable by reading Auth/ adapter internals, not the context module that wires them. Introduced SessionStarterInterface (ensureStarted(): void) with two implementations: - CookieSessionStarter: the production policy, extracted verbatim from the three duplicates, parametrized by cookie name. - NullSessionStarter: test-null, for a context that must never touch session machinery. Each adapter now takes a SessionStarterInterface with a default value (new CookieSessionStarter(EccubeSharedSessionAdapter::COOKIE_NAME)) so every call across the existing PHPUnit fixtures keeps working unchanged. HtmlModule and EccubeModule each bind SessionStarterInterface explicitly to the same cookie, so the choice is now a context/DI fact rather than adapter-internal. Also added AdminLoginChallengeInterface, extracted from the concrete, final HtmlAdminLoginChallengeAdapter per the issue's follow-up comment: Login, TwoFactorAuth, and TwoFactorAuthSet now depend on the interface. AppModule binds both the interface and the concrete class (the class has no constructor state - is a shared superglobal - so both bindings resolving to separate instances is safe), so existing tests that fetch the concrete class via the injector are unaffected; a future test whose subject is not the challenge machinery itself can now bind a fake. Verified: full suite green (2805 tests), psalm clean, all 35 Auth adapter tests and the admin login/2FA resource/ExcludedResponseBodyStore tests pass, and `composer page -- get '/admin/login'` still renders against a real SQL-backed context.
37 pages rendered a hidden csrfToken field with value="" because their GET resources never published the field: 9 nullByDesign markers left waiting on an EC-CUBE-side EventListener mirror that was never implemented, and 28 notPublished cases where the resource simply never set the key. Both resolve into a production HTTP POST to any of these forms failing CSRF validation (403) - the empty-token-ledger regression guard (#140) is what kept the set from growing further, but never shrank it. EccubeSharedCsrfTokenAdapter::issue() already self-issues and stores a fresh token when none exists (documented but unexercised until now), so closing these out is mechanical: inject Ray\Csrf\CsrfTokenInterface, publish 'csrfToken' => $this->csrf->issue() in each onGet body, and - since every affected schema is additionalProperties: false - add csrfToken to that schema's top-level properties/required (some already declared it under $defs without wiring it to properties; some declared it nullable for the old null marker; most didn't have it at all - each was checked individually, never assumed from a bare grep, since JsonSchemaInterceptor validates every annotated response and throws on an undeclared property). Also closed the blindspot the guard test's own docblock named: three pages (admin/category/category-list, admin/product/csv-category, admin/product/csv-class-name) filled the field via the csrf_token() Twig function instead, which reads $_SESSION directly and renders non-empty while the resource publishes nothing - invisible to the empty-field regex sweep. Fixed the resources to publish the field and simplified the templates to `{{ csrfToken }}`. Added a new guard (testNoTemplateFallsBackToTheSessionReadingCsrfHelper) that fails if any template reaches for csrf_token() again, since the existing sweep structurally cannot detect that regression. AbstractCsvUpload::onGet() is shared by four CSV-upload screens; fixing it once covers CsvCategory, CsvClassCategory, CsvProduct, and CsvClassName. ClassCategoryExport/ClassNameExport previously set $this->body to a raw CSV string, which TwigRenderer::buildBody() silently drops to [] for non-array bodies - that's why their upload-form template always rendered empty regardless of any csrfToken fix; converted their body to an array (content + csrfToken) and renamed the corresponding schemas' `value` property to `content` to match. DownloadResponder already special-cases a 'content' array key for the real CSV byte-stream download path, so the production download response is unchanged. Emptied tests/Html/csrf-empty-token-ledger.json now that all 37 entries are fixed, and fixed three sibling tests (ShoppingShippingResourceTest x2, WithdrawResourceTest, WithdrawResourceSqlTest) that pinned the old assertNull(csrfToken) marker behavior - updated to assert the real FakeCsrfToken::TOKEN value, the correct post-fix contract, not deleted. Delivered as three parallel subagent batches (customer-priority 13 pages, admin 12, admin 12 + the shared base class + the 3 twig-fallback pages) per a shared mechanical pattern and per-file schema-verification procedure, then reconciled centrally: ledger trim, guard test, full suite, psalm. Verified: full suite green (2806 tests, 33061 assertions), psalm clean, CsrfTokenRenderedTest fully green (empty ledger, new Twig-fallback guard passing), zero remaining `csrf_token(` calls in var/templates, zero remaining hardcoded `'csrfToken' => null` in src/Resource. (WithdrawResourceSqlTest's fix is unverifiable, and not because of this worktree: be/tests/Sql/ contains only LoginAttemptGateSqlTest.php, PreOrderClaimSqlTest.php, and bootstrap.php on origin/1.x itself - SqlFixturesTrait.php was deleted by commit 47016e6 "Remove SQL PHPUnit suite" but tests/Resource/Sql/AbstractResourceSqlTestCase.php still does `use SqlFixturesTrait;`. tests/Resource/Sql/* cannot load for anyone on this branch; it is excluded from phpunit.xml's default suite for that reason. The assertion change was made to match the proven Fake-variant pattern and is correct by inspection, but cannot be run until that repo-level gap is fixed separately.)
BlockList and PaymentList declared doUpdate*/doDelete* on their own onGet -
transitions ALPS assigns to the single-row Block/Payment states, not the
list. Both resources' own docblocks already said as much ("Single-row
affordances ... live at page://self/admin/payment/payment"). The surplus
declarations advertised a Link: header target that can't actually be
followed as declared either - a bare href with no bound row identifier,
since the real per-row delete/update forms are rendered inline in the list
HTML, unrelated to this resource-level attribute.
Per-resource resolution (the two options the issue posed have different
right answers depending on whether ALPS's assigned target resource is
GET-reachable at all):
- Block/Payment: a real single-row GET view exists (Block::onGet,
Payment::onGet), so moved the declaration there - matching ALPS exactly.
Payment::onGet already declared doUpdatePayment; Block::onGet was missing
doUpdateBlock entirely (an oversight, not by design) and Payment::onGet
was missing doDeletePayment the same way - filled both in, or removing
the list's declaration would have made the transition undiscoverable via
hypermedia anywhere, not just corrected.
- TaxRuleList: alps.json's TaxRule state lists doDeleteTaxRule, but there is
no goTaxRule transition and no onGet on TaxRule.php - "intentionally no
onPut... edits flow as delete + create" per its own docblock. There is no
GET-reachable resource to move the declaration to without inventing one,
so this is the one case where the resource is right and ALPS was
incomplete: connected #doDeleteTaxRule into TaxRuleList's descriptor set
instead. Regenerated alps.json.html/alps.svg and synced the docs/ copies
per AGENTS.md.
Updated the two Hypermedia workflow tests that exercised the old (wrong)
navigation - extracting doUpdateBlock/doUpdatePayment/doDeletePayment from
the list response, the only place they used to be declared. Now:
Block: testUpdatesBlock's own response already carries doUpdateBlock (no
extra hop needed); testDeletesBlock re-fetches the single-row Block GET
(Block::onPut doesn't redirect, so there's no Location to follow - a real
client returning to a bookmarked item page would do the same).
Payment: both testUpdatesPayment and testDeletesPayment now follow the
Location header (Payment's create/update both redirect to the single-row
GET) instead of detouring through the list.
Adding doUpdateBlock/doDeletePayment to the single-row GETs surfaced two new
html-link-audit-ledger entries, symmetric to the doDeleteBlock/doUpdatePayment
entries already there: the "new blank form" mode (blockId/paymentId absent)
doesn't render an update/delete form, so the declared link has no matching
affordance (target-missing, resourceOnly - same classification as their
siblings). Also removed two ledger entries that are no longer observed now
that the surplus links are gone from the list resources
(block-list doUpdateBlock, payment-list doUpdatePayment method-mismatch).
Verified: full suite green (2806 tests, 33061 assertions), psalm clean,
asd --validate alps.json clean, HtmlLinkAuditLedgerTest and both Hypermedia
workflow test files pass, all four affected HTML render tests
(AdminBlockHtmlRenderTest, AdminBlockListHtmlRenderTest,
AdminPaymentListHtmlRenderTest, AdminTaxRuleListHtmlRenderTest) pass
unchanged - the rendered per-row forms were never driven by this PHP
attribute, only the Link: HTTP header and the audit ledger were.
…code, weak assertions, and a second CSRF Twig-helper bypass Several concerns surfaced against already-committed work; verified each against actual behavior (not assumption) before acting - some were already resolved, some were real. Confirmed already correct (verified, no change): - #132's audit wiring is bound from the test side (AuditedLinkHeaderModule installed by HtmlLinkAuditLedgerTest's own override module), never from a new production context module. - No rename() reliance in that override module - it install()s instead, which is why it works across the override() boundary. - toInstance(), not toConstructor(), for SessionStarterInterface - already fixed before commit. - docs/alps.json.html and docs/alps.svg never carried Jekyll front matter; the plain `cp` sync in #136 didn't strip anything. - The hypermedia suite (350 tests) and WithdrawResourceSqlTest were checked directly: the former genuinely executes (no DB/HTTP-server skip), the latter genuinely fatals on a missing trait (pre-existing, excluded from the default suite in phpunit.xml, not silently skipped). Real gaps found and fixed: - SessionStarterInterface had no test proving a context actually binds it - removing the bind() line stayed green. Added tests/Module/SessionStarterBindingTest.php and proved it catches the regression (temporarily commented out HtmlModule's binding, watched the test fail with Unbound, restored it). - NullSessionStarter was speculative dead code - nothing in the app ever binds a real session adapter without also wanting a real session start (Fake contexts replace CustomerSession/AdminSession entirely; the one path that keeps HtmlSessionAdapter under cli-server SAPI is always reached via public/page.php, which unconditionally starts the session first). Deleted rather than ship an unbound speculative implementation. - HtmlSessionAdapter never depended on SessionStarterInterface at all and its docblock wrongly credited public/index.php with starting the session (it doesn't - only public/page.php does). Wired it through the same port for architectural consistency and corrected the docblock. - Two tests asserted RecordingHtmlLinkAuditLogger::drain() === [] using a fake renderer that already carries the correct rel/class token - passes identically whether the audit runs or not. Added tests/Fake/Html/NonSemanticAnchorRenderer.php (renders the same anchor without the token) and a positive-detection test plus rewrote the module-wiring test to use it, asserting the actual semantic-token-missing warning fires. - TaxRuleList's docblock still claimed doDeleteTaxRule "lives at" the single-row resource, contradicting #136's actual resolution (ALPS now connects it to TaxRuleList directly, since TaxRule has no GET state). Corrected. - The #139 blindspot (Twig csrf_token() reading $_SESSION directly instead of the CsrfTokenInterface port) was not fully closed: csrf_token_for_anchor() delegates to the same session read and ProductList.html.twig called it at 5 sites, none of which matched the guard test's 'csrf_token(' substring check. Published csrfToken from ProductList's resource (it had none), updated its schema, ported all 5 template call sites to {{ csrfToken }}, and deleted both Twig functions from BeMartTwigExtension entirely - structurally closing the bypass rather than only guarding against its text reappearing. Deleted the now-dead tests/Module/BeMartTwigExtensionTest.php, which only tested the removed methods. Verified end-to-end, not just via is_string(): ran the class-name/class-category CSV export resources through a real injector and confirmed actual CSV bytes (header row + data row, correct Content-Disposition) - the value->content schema rename from the earlier #139 batch is genuinely correct, not just type-compatible. docs/api/alps-gap-ledger.md's `value` property references for the two CSV export schemas are now stale (renamed to `content`), and its csrfToken count predates all of #139's schema additions - left alone. No script in composer.json regenerates it and its csrfToken entry was already stale before this session touched anything, establishing it as a periodic manual audit snapshot, not a live-generated or test-enforced artifact. Verified: full suite green (2806 tests, 33061 assertions, same count as before - deleted 3 dead tests, added 3 real ones), psalm clean, HtmlLinkAuditLedgerTest/AlpsReferenceTest/CsrfTokenRenderedTest all green with cleared Twig caches, SessionStarterBindingTest proven to catch its own regression before being trusted.
#143) Admin `/admin/customer`, `/admin/customer-delivery-edit`, and `/admin/product/product-class` rendered `<form method="post">` with no onPost/onPut/onDelete to answer it (405). The reference implementation lives on the unmerged `origin/post-redirect-get-303` branch (same merge-base as this branch, `4beee6a7`); cherry-picked `894a92df` and `5e05cb19` for exactly these three resources and reverted everything else those commits touched that was out of #143's scope: - TwoFactorAuthEdit.php / FileManager.php / CustomerList.php: scope decisions #143 explicitly defers to a separate issue. - A whole admin "flash message" feature (AdminFlash.php, admin-base.html.twig calling `admin_flashes()`): the Twig extension never defined that function, so every admin page render would have thrown. Reverted in full rather than half-wire it. - All three template conflicts (Customer/CustomerDeliveryEdit/ ProductClass) resolved to the current idea-admin-* design already on this branch, not the older Bootstrap-table markup the reference branch had — this branch's templates are the newer ones. Each write path got its own admin-specific Be Input/Final (AdminUpdateCustomerInput → AdminCustomerUpdated, Admin{Create,Update,Delete}CustomerDeliveryAddressInput, RegisterProductClassInput → ProductClassRegistered) rather than reusing the storefront transitions: `UpdateCustomerAddressInput` derives the owner from the customer session by design (a customer cannot reassign someone else's address by tampering with the body), and an admin editing a different customer's data is the opposite authorization model. This also resolves #143's actor-naming question in favor of option (B) — separate ids for the admin path (doUpdateCustomerProfile, doUpdateCustomerDeliveryAddress, doRegisterProductClass) rather than widening the storefront ids' docs. ProductClass.php's onGet was never publishing `csrfToken` (same gap #139 fixed elsewhere) — wired `CsrfTokenInterface` and added it to the GET JSON schema. Its template had two bugs: the hidden parent-id field was named `parentProductCode` but onPost reads `productCode`, and the CSRF token had no hidden field to carry it — both fixed. Added 8 new alps.json descriptors for the ids these fixes introduced (goCustomerDeliveryEdit, doUpdateCustomerDeliveryAddress, doDeleteCustomerDeliveryAddress, doUpdateCustomerProfile, goProductClass, doRegisterProductClass, goLog, goOrderPdf) plus the CustomerDeliveryEdit/Log/OrderPdf states they return to; validated with `asd --validate` and regenerated alps.json.html/alps.svg (root + docs/ copies). Dropped `#[Alps]` from the 8 route-gate/fallback ids (doActionRedirect, doAdminActionRedirect, doAdminUnsupportedRoute, doUnsupportedRoute, goActionRedirect, goAdminActionRedirect, goAdminUnsupportedRoute, goUnsupportedRoute) and the goAdminEmptyPage placeholder — these answer URLs EC-CUBE has that BeMart deliberately doesn't model as application transitions, so a client can't discover them anyway. goAdminLog/goAdminOrderOrderPdf/goAdminOrderMailConfirm/ goAdminTemplateTemplateAdd were renamed in the cherry-picked commits (the last two to already-existing profile ids goOrderMailConfirm/ goTemplateInstall; the first two are new — goAdminOrderOrderPdf is NOT a rename of goExportOrderPdf, it's the distinct options-form screen that links to it). Net: 19 drifted references down to 4 (doCreateMailTemplate, goAdminContentFileManager, goAdminTwoFactorAuthEdit, goShoppingShippingMultipleEdit) — updated `AlpsReferenceTest`'s SCREEN_GAP/ROUTE_GATE/PLACEHOLDER lists and `TemplateFormActionTest`'s KNOWN_DEAD list to match, and refreshed `docs/migration-status.md` §2.1/§2.2 and the feature matrix. Fixes made during verification, beyond the cherry-pick: - Customer::onPost($password), AdminUpdateCustomerInput::$password, and AdminCustomerUpdated::$passwordHash were missing #[SensitiveParameter] (CredentialParameterSensitiveParameterTest / CredentialConstructorParameterSensitiveParameterTest caught this). - Password/PasswordHash Semantic validators made nullable: the admin edit flow passes null to mean "leave the current hash untouched" (EC-CUBE's default-password sentinel); a supplied empty string still fails the length floor. - ExportOrderPdf gained an onPost (EC-CUBE's admin_order_pdf_download route) so OrderPdf.php's submit button reaches a real handler instead of 405 — reuses the existing goExportOrderPdf id since both verbs perform the same transition. The cherry-picked smoke-test fixture expected this to return 200, but OrderPdfCompatibilityService::export() unconditionally throws OrderPdfNotSupportedException (a Phase-A stub, tracked in docs/migration-status.md §4 item 5) — fixed the fixture to expect 501, and dropped an unrelated `?category_id=` the cherry-pick had added to the GET /products fixture line (broke JSON Schema validation; unrelated to this issue). - Removed the stale `POST admin/two-factor-auth-edit` smoke-test fixture entry the cherry-pick added — that resource still has no onPost since TwoFactorAuthEdit stays deferred. - HttpSqlAdminProductClassFormTest (a new real-browser/real-MySQL regression from the cherry-pick) assumed a pre-existing `admin-active-001` product/class row that nothing in this environment seeds; added setUp/tearDown that inserts and cleans up the parent dtb_product + dtb_product_class rows directly, since the fixture helper it was written against (SqlFixturesTrait) doesn't exist in this repo (removed by `47016e64`, already documented in #139's commit). - HttpSqlAdminProductClassFormTest's two markup assertions pinned the reverted Bootstrap-table template's `id="product_class_new_row"` and "登録" button text; updated to the kept idea-admin-* template's actual id and "追加" label. - The new #[Link] declarations on Customer/CustomerDeliveryEdit/ ProductClass introduced 5 HTML Link Audit warnings (the templates don't render a matching `rel="..."` token); classified all 5 in html-link-audit-ledger.json — 4 as semantic-token-missing/fail (real markup gaps, tracked), 1 (doDeleteCustomerDeliveryAddress) as method-mismatch/resourceOnly (the edit form has no delete affordance at all; deleting a delivery address is out of this editor's scope). Deferred, per #143: `/admin/content/file-manager` and `/admin/two-factor-auth-edit` stay dead forms (multipart file I/O and admin-editing-another-member 2FA semantics both need their own scope decision first). Verified: full `vendor/bin/phpunit` suite green (2836 tests, 0 failures/errors; the one pre-existing risky test — CsrfTokenRenderedTest::testLedgerEntriesAreWellFormed — is #139's closed-ledger side effect, unrelated), `vendor/bin/psalm` clean (no errors), `asd --validate alps.json` clean.
`tests/Html/html-link-audit-ledger.json` tracked 152 "fail" entries across
three reasons: 134 semantic-token-missing (an `<a>`/`<form>` reaches the
right href+method but the rendered markup carries no `rel`/`class` token
matching the resource's `#[Link]`), 12 method-mismatch, and 6 target-missing.
All 152 are now fixed or reclassified with an evidence-based note; the
ledger holds 52 entries (49 resourceOnly, 3 targetOut), zero fail.
Three subagents fixed 105 of the 134 semantic-token-missing entries in
parallel (one `rel="<name>"` addition per page, verified per-page against
the real audit wiring — never against ledger removal, to avoid concurrent
edits on the shared ledger file). Before dispatching them, 29 more
semantic-token-missing entries were resolved by editing two site-wide
shared partials once each: `admin-base.html.twig`'s sidebar (12 admin
`go*List` links spanning ~20 pages) and `IdeaStore/component/header.html.twig`
(goTop / goCart spanning 9 pages) — these had to be excluded from the
subagents' per-page assignments and fixed centrally, since three writers
adding the same token to the same shared `<a>` would have collided.
The remaining 18 (12 method-mismatch, 6 target-missing) needed individual
judgment, done directly:
- `admin/delivery/delivery-list` / `admin/news/news-list` doDelete*:
the row-level delete affordance was a GET `<a href="...&_method=delete">`
with JS rewriting it into a POST on click — the auditor (correctly)
treats an `<a>` as GET regardless of `_method`, so this never worked
without JS. Converted both to real per-row `<form method="post">` with
a hidden `_method`/csrfToken field, matching the pattern #130 already
established for Block/Payment/TaxRule and `Mypage/AddressList.html.twig`.
`admin/news/news-list`'s resource never published `csrfToken` at all
(wired now, matching #139). `admin/tag/tag-list`'s delete was worse —
an `href="#"` anchor with no working submit path at all even with JS —
replaced with the same real-form pattern.
- `admin/delivery/delivery-list` / `admin/news/news-list` doUpdate*:
a false positive — the list's own `goDelivery`/`goNews` edit-icon link
coincidentally shares the PUT target's path, so the auditor offers it
as a method-mismatch candidate. No PUT affordance belongs on a list
page; reclassified `resourceOnly`, same call #130 made for
`doUpdateBlock`/`doUpdatePayment`.
- `admin/layout/layout` doUpdateLayout / `admin/order/edit` doUpdateOrder:
both forms only add their `_method` override (query-string or hidden
field) when a real id is present; the smoke-test's "new" render has
none. Added `rel=` to the real edit-mode form (verified working with a
real id) and reclassified the "new"-mode ledger entry `resourceOnly`,
matching the established "new mode doesn't render X" precedent used
throughout this ledger.
- `admin/index` / `admin/system` doAdminLogout: neither page had a
logout affordance. Added a real per-page logout form to each (not a
shared-frame one — tried that first, reverted it: 12 *other* admin
pages don't publish csrfToken, so a topbar-wide form would render an
empty token on all of them and fail CSRF in production). Both
resources gained `CsrfTokenInterface` + `csrfToken` in body/schema
(the same gap #139 fixed elsewhere, just missed on these two).
`Index.php` also declared a `goAdminLogout` POST Link to `/admin/login`
that exists nowhere in ALPS or anywhere else in the codebase and
pointed at the wrong verb for a "go" transition — a duplicate of what
`doAdminLogout`'s own `rt: #AdminLogin` already implies; removed.
- `admin/mail-template` goOrderMail: genuinely missing UI. Added a
"受注メール送信" link next to the existing toolbar back-link (both
template branches).
- `mypage` goMypageHistory / goOrderHistory: not the duplicate `#[Link]`
the issue guessed — alps.json's `Mypage` state descriptor lists
`#goMypageHistory` and not `goOrderHistory`, so `goMypageHistory` is
canonical, not a mistake. The real bug was in the template: the
"すべての履歴を見る" link pointed at `/mypage/history` (the per-order
detail page, which requires `orderNo` — wrong target) instead of
`/mypage/order-history` (the list page). Fixed the href and added
`rel="goOrderHistory"` there and `rel="goMypageHistory"` on the
per-order "詳細" links already using the correct
`/mypage/history?orderNo=...` — both reclassified `resourceOnly`
regardless, since the audited fixture customer (Alice) has zero
seeded orders, so the panel containing both links never renders in
that specific scenario; a customer with real history sees them.
- `shopping/shipping-multiple` goShoppingShipping: genuinely missing —
added a "配送先選択に戻る" link back to `/shopping/shipping` next to
the existing "ご注文手続きに戻る" link, in both the item-list and
empty-cart branches.
- `index` goHelpAbout/goHelpAgreement/goHelpPrivacy: the storefront
top page's footer already linked goHelpGuide/goHelpTradeLaw but was
missing three of the five `#[Link]`s `Index.php` declares. Added all
three.
Caught during verification, not part of the ledger but broken by these
changes:
- This environment's compiled Twig cache does not reliably invalidate
on template mtime — several fixes silently appeared to fail until
`var/tmp/<context>/twig` was cleared. Cleared before every full-suite
run in this session; not a code change, noted for whoever hits it next.
- `HttpSqlAdminNewsFormTest` pinned the literal string
`/admin/news/news?newsId=X&_method=delete` in the rendered list page —
the exact query-string pattern the doDeleteNews form fix replaced with
a hidden field. Updated the assertion to check the new form's `action`
attribute instead; the test's actual functional delete call (a direct
HTTP POST with `_method=delete` in the query string, unrelated to the
list page's markup) was untouched and still passes.
Verified: full `vendor/bin/phpunit` suite green (2836 tests, 0
failures/errors; the one pre-existing risky test —
`CsrfTokenRenderedTest::testLedgerEntriesAreWellFormed` — is #139's
closed-ledger side effect, unrelated), `vendor/bin/psalm` clean (no
errors). Every individual template/resource change was also verified
against its own `tests/Resource/*HtmlRenderTest.php` /
`*ResourceTest.php` / `tests/Hypermedia/*Test.php` before moving on.
…unts, UI-reachability limitation Post-commit review of `716a8a44`/`a0245c36` surfaced one real SSOT gap and a few stale numbers that `AlpsReferenceTest` can't catch on its own: - `CustomerDeliveryEdit.php` declares `#[Link(rel: 'doCreateCustomerDeliveryAddress', ...)]` on its onGet (for HATEOAS discoverability — the create/update split only exists at the Link level; `#[Alps]` uses a single `doUpdateCustomerDeliveryAddress` id for both branches). No alps.json descriptor backed it. `AlpsReferenceTest` only scans `#[Alps]` attributes, not `#[Link]` rels, so this drift was invisible to every test in the suite. Added the descriptor and swept the other 7 files #143 touched (Customer/CustomerDeliveryEdit/ProductClass/Log/OrderPdf/MailConfirm/ TemplateAdd/ExportOrderPdf) for the same gap — none found. - `OrderPdf` state was missing the `src-template` tag every other screen state in this profile carries (Security, ChangePassword, Customer, CustomerAddress, CustomerDeliveryEdit, Log all have it). - `docs/migration-status.md`: the ALPS transition count (216, now 217 after the descriptor above) and three feature-matrix row counts (flow-manage-product 24→26, flow-manage-order 13→14, flow-manage-customer 6→11) were left at their pre-#143 values. - Documented a real, previously-silent completeness gap in `CustomerDeliveryEdit.php`: the rendered form has no `addressId` hidden field and no existing-address list, so `onPost` can only ever take the create branch and `onDelete` has no click path at all — both are exercised directly by `AdminCustomerDeliveryEditResourceTest` but unreachable from the page today. Not a regression from #143 (the cherry-picked template never had this either), and building the edit/delete affordance (an address list on `/admin/customer`, or addressId-aware `onGet`) is a materially larger UI slice than the dead-form fix #143 scoped — documented rather than silently left, so it isn't mistaken for done. Re-verified (independently reviewed and confirmed already correct, no change needed): Customer.php/CustomerDeliveryEdit.php/ProductClass.php constructors carry every dependency from both merge sides with no duplicate imports; TwoFactorAuthEdit.php's only diff from origin/1.x is #139's csrfToken publish (not #143 write-handler scope creep); FileManager.php/CustomerList.php/AdminTwoFactorAuthEditResourceTest.php are byte-identical to origin/1.x (cleanly reverted); the news-list delete dialog's real submission form (`#news-delete-form`) already carries a csrfToken hidden field; `Page/Mypage/navi.html.twig` is unused dead code, not a live shared partial. Verified: `asd --validate alps.json` clean, regenerated alps.json.html / alps.svg (root + docs/ copies), full `vendor/bin/phpunit` suite green (2836 tests, 0 failures/errors; same pre-existing risky test as before), `vendor/bin/psalm` clean.
…ality The feature-matrix row said customer-delivery-edit was simply 'done' alongside customer edit. Per eaabac5's finding (and the docblock it added to CustomerDeliveryEdit.php), the rendered form only ever posts an empty addressId, so update/delete are unreachable from the UI — create-only. Matches the same honesty already applied to the doDeleteCustomerDeliveryAddress ledger note in a0245c3.
|
Important Review skippedToo many files! This PR contains 206 files, which is 106 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: Repository: be-framework/BeMart/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (60)
📒 Files selected for processing (206)
You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
対応issue
#135は対応不要（指示によりスコープ外）。#130は本ブランチの起点時点で既にクローズ済み（PR #137で対応済み）。
検証
各コミットは独立して以下を満たす:
vendor/bin/phpunit: full suite green（2836テスト、0 failure/error）CsrfTokenRenderedTest::testLedgerEntriesAreWellFormedがrisky（#139でledgerを完全解消した副作用、既知・無害）vendor/bin/psalm: clean（no errors）asd --validateclean、alps.json.html/alps.svg再生成、docs/コピー同期済み既知の残課題（別issue/別判断として明示的にスコープ外）
/admin/content/file-manager、/admin/two-factor-auth-editは死んだフォームのまま（multipartファイルI/O設計、管理者による他メンバー2FA意味論の整理が先に必要）CustomerDeliveryEdit.html.twigは新規作成のみ対応（更新・削除はリソース層では実装・テスト済みだが、UIからは到達不可 —CustomerDeliveryEdit.phpのdocblockに明記）doCreateMailTemplate,goAdminContentFileManager,goAdminTwoFactorAuthEdit,goShoppingShippingMultipleEdit）コミット一覧