Skip to content

<div style="display: none;" hidden="true" aria-hidden="true">Are you an LLM? You can read better optimized documentation at /changelog/Changelog.4.123.md for this page in Markdown format</div>

Home | Changelog

Version 4 ​

version 4.123 ​

  • [4.123.0] fix(transporters/speedex): correct 'succcess' key in transformTracking (Advisable-com/ecommercen#770)
    • Speedex::transformTracking() read finalVoucherStatusCodes['succcess'] (three cs) while SpeedexConfig::initialize() defines the key as 'success', so the misspelled subscript yielded null and in_array($needle, null) raised a TypeError on PHP 8. Every Speedex voucher carrying at least one checkpoint fataled — in-transit ones included — and Adv_orders_admin::trackAndTrace() wraps the gateway in no try/catch, so the error 500ed the whole admin order view.
    • Second occurrence of the same typo: #277 fixed the two transfer-status job sites but never touched this gateway call site. src/Transporters/Speedex/Speedex.php:146.
  • [4.123.0] fix(payments): verify VivaWallet webhook notifications with the gateway before acting on them (#621)
    • The defect. AdvViva::handleWebhook() is publicly routed (vivaWallet/&lt;event> → webhooks/viva/handleWebhook) and applied no authenticity check of any kind. Its only gate was that the notification's own EventData.OrderCode matched a shop_order.tran_ticket with payway = 'vivawallet' — a gateway-issued bearer identifier used as an authenticator, not a secret. An unauthenticated caller could therefore flip any PENDING vivawallet order to PAID (goods shipped) or to CANCELED, and on the gift-card arm trigger acceptGiftCard(), which mints a redeemable coupon worth the order amount. Separately, the empty-body branch answered any HTTP method, so an anonymous empty POST returned the merchant's webhook verification key and fired two credentialed outbound calls doing it.
    • The fix, five changes. (1) Before anything is written, on both the shop-order and the gift-card arm and for both transactionPaymentCreated and transactionFailed, the handler asks Viva's own transaction record whether the notification is true — a payment must be reported paid, a failure must be reported not paid, and either way the transaction Viva describes must be the one the notification names. (2) The amount Viva confirms must match the order to the cent — shop_order.total_vat on the order arm, gift_card_orders.amount on the gift-card arm — compared unscaled, since the verification response reports major units where the request side sends × 100. The single exemption is a transaction Viva reports against two different currencies: which of its two reported figures is the merchant's own is not documented anywhere in Viva's API reference, so the amount check is skipped and logged as an error rather than guessed — guessing wrong would refuse a real payment silently and strand a paid order PENDING. Every other gate still applies to those. (3) The webhook-key handshake is now GET-only; an empty POST is answered 400 and never reaches the gateway. (4) Every exit sets an explicit status, correctly classified for Viva's retry policy (it retries hourly until it gets a 2xx): a permanent verdict — not confirmed, order not found, replayed, unknown event — answers 2xx, and only an unreachable gateway answers 502, so a transient fault no longer permanently strands a paid order. (5) The five bare exits route through the inherited Adv_base_controller::terminate() seam (from #619), which is what makes the gift-card and error arms testable at all.
    • Both replay guards are untouched. The order arm still bails on a non-PENDING status, and the gift-card arm still leaves the Pending-only claim to the model (#583) rather than deciding for it.
    • src/VivaWallet/Base.php gained a dead-catch fix and a return-type widening.catch (GuzzleException | Exception $e) referenced an unqualified Exception inside namespace Advisable\VivaWallet, where no such class exists — so only GuzzleException was ever caught and any other throwable escaped doRequest() as an uncontrolled 500. It is now \Exception. In the same method, doRequest() and handleRequest() widened from array to ?array: null now means the request never produced a response, [] keeps its meaning of answered, but the body was empty or unusable. Without that split a genuine HTTP 200 carrying {} is indistinguishable from a transport failure, and the 2xx-vs-502 decision above cannot be made. Base is shared by every Viva call site on the platform — the checkout return path, the gift-card pages, the REST checkout verification and the OAuth token request all run through it.
    • Full design, the GATE 1 answers and eight post-GATE-1 amendments — including the multi-currency (DCC) decision and the cross-order binding — are recorded in docs/decisions/621-viva-webhook-verification.md.
  • [4.123.0] fix(blog/admin): restore the per-row blog-comment moderation buttons — a catch-all route swallowed the action into index(), and the status codes never matched the column (#633)
    • The outage: approving a comment returned HTTP 500. application/config/routes.php:155 mapped blog/blog_comments_admin/(.+) onto blog/blog_comments_admin/index/$1, so every sub-path of the controller was rewritten into an argument for index() rather than reaching its own method. .../setStatus/1/1 arrived as index('setStatus', '1', '1'), and index(int $offset = 0) raised an uncaught TypeError — Adv_blog_comments_admin::index(): Argument #1 ($offset) must be of type int, string given — which surfaces as a bare 500. resetIndex and batchAction were rewritten the same way. The catch-all was correct for the controller's original signature, index($filterType = 0, $offset = 0), which took two segments (/1/20 = approved tab, offset 20) — hence (.+) and not (:num). When filtering moved to the POSTed search dropdown and index() collapsed to a single typed $offset, the route was never narrowed. The pagination links kept working throughout, because a numeric segment coerces cleanly, which is why the route looked healthy.
    • The status mismatch (#633's original subject). The per-row links post legacy numeric codes — 1 approve, 0 pending, 2 reject (application/views/admin/blog/BlogComments/list.php:141,148,155) — inherited from the product-reviews admin this screen was copied from, where the codes are native: shop_product_reviews.active is a tinyint(1) documented 0 = inactive-pending, 1 = active, 2 = rejected. But blog_comments.status is enum('approved','pending','rejected'), and getStatus() matched only the string literals, so every per-row click fell to default and wrote 'pending'. The bulk dropdown posts the literals directly and therefore always worked — it is the only path that has ever worked, and it carries all existing production traffic.
    • The notification could contradict the write. sendBlogCommentStatusUpdateEmail() received the raw URL value while the database received the mapped one, and branched on ($status == 1) ? accept : reject — two branches for three outcomes. Moving a comment back to pending therefore emailed the author that their comment had been rejected. There is no blogCommentPending template and no outcome to announce for a neutral action.
    • A third divergence, found while writing the test. getStatus() used a switch, which compares loosely, while the mailer used a strict membership test. Ten numeric-string variants of 1 and 2 — '01', '1.0', '1.00', '+1', '1e0', '002', '2.0', '+2', '2e0', ' 1' — mapped to approved/rejected on the write side while the mailer announced nothing, so a comment could be stored as approved with nobody told. permitted_uri_chars allows digits, . and +, so those segments reach the controller. Both sides now use the same strict test.
    • The fix, three files. The route splits in two, following the shape already used by blog/blog_admin and seo/custom_metatags in the same file: (:num) continues to index/$offset for pagination, and a (.+) passthrough lets real method names reach their own methods. The malformed language-prefixed twin — (\w{2})blog/..., missing its slash and therefore dead — is replaced by a correct (\w{2})/blog/... pair, so prefixed and unprefixed URLs finally behave identically (before this, a prefixed URL fell through (\w{2})/blog/(.+) straight to the method, which is why the mis-write in #633 was observable at all while the UI's own links returned 500). getStatus() accepts both vocabularies via strict comparison. Adv_mailer gains a protected blogCommentEmailOutcome() seam returning the same literal the database receives, or null to send nothing.
    • No view change, satisfying #633's acceptance criterion: mapping the numbers in the controller rather than rewriting the shared admin view means no client-override surface is touched, and it keeps working for the two forks whose row buttons already post the literals.
    • Removes getFilterData(), dead since that same refactor — its only caller was the original index($filterType, …). Its five translation keys (blog_comments.pending_title, approved_title, rejected_title, approved, rejected) are left in place deliberately; see Notes.
    • Covered by 51 new tests in tests/Legacy/Blog/BlogCommentStatusMappingTest.php, pinning both vocabularies, the bulk-dropdown path explicitly, the strict-comparison guard, the seam's contract and its overridability, and — derived rather than restated, so a wrong expectation cannot make it pass — the invariant that the stored status and the announced outcome can never disagree. Each of the three defects was re-introduced individually to confirm the suite fails on it (9, 9 and 24 failures respectively).
  • [4.123.0] fix(logging): return HTTP 500 on uncaught exceptions and PHP fatals, instead of a silent 200 (#543)
    • The defect. application/helpers/log_helper.php's _exception_handler() override logged an uncaught throwable to Monolog but never set a status code and never halted the request, so every uncaught exception platform-wide returned HTTP 200 — invisible to uptime checks, response.ok tests and 5xx-rate alerting, and the customer-visible symptom behind #542 (an AJAX caller reading a 200 with a truncated body). _error_handler() carried the same omission, and since _shutdown_handler() delegates to it, the PHP-fatal path (E_ERROR, E_PARSE, E_COMPILE_ERROR, E_CORE_ERROR) was equally silent.
    • The fix. Both handlers now end a fatal request with CI3's own form — is_cli() or set_status_header(500); exit(1); — restoring the status-and-halt contract while leaving the Monolog dispatch byte-identical. _error_handler() halts on CI3's $is_error mask minus E_USER_ERROR: that one type is reachable while a request is still alive (guzzlehttp/psr7 raises it from a __toString() guard), so including it would abort mid-request and strand the post_system deferred-task drain rather than catch a genuine PHP fatal. _shutdown_handler() is unchanged — it inherits the fix through its delegation.
    • Not restored, deliberately: CI3's error_reporting() gate (would silence Monolog, since public/index.php sets error_reporting(0) in both testing and production) and show_exception() (a no-op in production, since display_errors is Off there, and a liability in any environment where it isn't).
    • Full design — the E_USER_ERROR exclusion, the rejected alternatives, and the in-process test seam for the now-fatal exit(1) — is recorded in docs/decisions/543-exception-handler-status-500.md.
  • [4.123.0] fix(checkout): stop marking an order PAID when NBG Simplify (ethniki) declined the card (#741)
    • The defect. Adv_checkout::ethnikiResponse() verified the NBG Simplify MD5 signature and nothing else. NBGHelper::ethnikiValidateResponse() requires paymentStatus to be present and folds it into the signature pre-image, but never compares it to anything — so a genuinely signed DECLINED return followed the success path end to end: PAID, is_paid, the confirmation email, the ERP success hooks, decremented stock and gift counters, a consumed coupon and the purchase reported to Manago, Advisable AI, Meta CAPI, Matomo and Project Agora, with no money taken. ethniki recorded 0 hits in 7 days of legacy get_response traffic, so this was latent in practice and unconditional in code.
    • The fix. ethnikiResponse() now checks the outcome after the nbg_simplify_logging write and inside the existing PENDING guard: anything other than paymentStatus === 'APPROVED' routes to a new ethnikiResponseFail() instead of ethnikiResponseSuccess(). The predicate is an allowlist, not === 'DECLINED' — NBG documents only APPROVED and DECLINED and publishes no exhaustive enumeration, so an undocumented third value must fail closed.
    • What a declined order now does. It is cancelled immediately — cancelOrder(serial, couponId, 'get_response:ethniki', false, '+') then afterOrderCancelHooks() — the false, '+' stock arguments 17 of the 18 pre-existing cancelOrder() sites in this controller already pass, with the get_response:&lt;payway> label shape 14 of them use — so the coupon is released and the customer's redeemed loyalty points are returned at once rather than after the 180-minute AdvCancelIncompleteOrders sweep. The customer sees a new checkoutEthnikiFail view rather than inactive_payment(), whose copy ("Payment has already been made or is invalid") is the wrong thing to tell someone whose card was refused.
    • Deliberately unchanged. ethniki_ee — it resolves its outcome through its own resultIndicator validator and reaches the shared ethnikiResponseSuccess() by delegation, which is why the check sits in ethnikiResponse() and not in the shared success method or in NBGHelper. The nbg_simplify_logging write still happens on both outcomes: it is the only place paymentStatus is persisted, and a validator-level check would have exited above it. The non-PENDING arm still renders paymentsInactive.
    • What this does NOT fix. AdvGiftCardPage::ethnikiResponse() (ecommercen/gift_cards/controllers/AdvGiftCardPage.php) has the identical defect — same validator, same missing check — and its success path issues a gift card for a declined payment. It is tracked separately as #781 and is not touched here. Historical rows already marked PAID on a DECLINED return are identifiable by joining nbg_simplify_logging.reference against shop_order.order_serial; no backfill is in scope.
    • Covered by 16 new tests in tests/Legacy/Checkout/AdvCheckoutEthnikiDeclineTest.php, which build a genuinely valid MD5 signature over a declined payload rather than mocking the verdict.
    • Full design and the rejected alternatives are recorded in docs/decisions/741-ethniki-declined-marked-paid.md.
  • [4.123.0] feat(mcp): blog article tools — list, read, create and update articles (#714)
    • Four MCP connector tools over the existing Cms\Blog\Article domain, in a new src/Mcp/Tools/BlogTools.php: list_articles (paginated id / title / slug / is_published / blog_date plus a body-length indicator), get_article (one article in one language, including its full untruncated body), create_article and update_article. All four go through Cms\Blog\Article\Service / WriteService — no hand-built queries — and every write is audited through McpAuditLogger. src/Domains/Cms/Blog/Article/** is unchanged.
    • A created article lands as a DRAFT unless is_published: true is passed.blog.is_published is tinyint(1) NOT NULL DEFAULT 0, so the schema itself supplies that guard; the flag is exposed on both create and update so an operator can finish the workflow in one call. The connector is deliberately unscoped and therefore sees drafts — correct for an authoring surface. #624's storefront row scoping and ArticleVisibilityScope are a REST controller / relation concern and are not on this path.
    • update_article never hands one language's row to the domain.MuiWriteRepository::replaceForEntity() is DELETE FROM blog_mui WHERE blog_id = ? with no lang predicate followed by a re-insert of only what was passed, so every write routes through TranslationMerger::mergedPayload(), which rebuilds every language's row. That merge is also what carries the existing blog_mui.slug (NOT NULL, no default) through, since WriteService::update() never regenerates it — hence slug is not an exposed field.
    • create_article re-derives, explicitly, the three enforcements the update path inherits.HtmlSanitizer::sanitize() on description / small_description (AC 5), ToolResult::enforceLengths() on the meta columns, and non-empty checks on title and banner_image — all three live inside MergesTranslations::diffFields(), which no create path in the repo reaches. It also refuses a title with no alphanumeric character (it would slugify to '' and produce an unreachable URL) and a blog_date that is not a real YYYY-MM-DD, and maps a null return from WriteService::create() — a transaction failure, not "not found" — to a tool error.
    • list_articles exposes a partial title filter, which depends on #579. That fix (already on develop) corrected the domain's filter[title.{locale}] mapping from the nonexistent column blog_mui.name to blog_mui.title; before it, any title filter was an unrecoverable SQL 1054, so this tool originally shipped without one. ListRequest.php is not in this diff. A test asserts the built filter's column — not merely the key's presence — so a regression of that mapping fails in the Unit suite rather than 500ing at call time. Articles remain findable by slug, category_slug, author_id, tag_id or a publication-date range, and sort=title.{locale} is exposed as before.
    • Tests live in tests/Unit/Mcp/Tools/BlogToolsTest.php (the Unit testsuite), following the mocked-collaborator precedent of {Category,Vendor}ToolsTest. tests/Integration/Mcp/ is in no testsuite (#526), so ServerFactoryTest gains a BLOG_TOOLS peer for correctness when that lands but is not this change's evidence.
    • Full design, the fourteen numbered decisions and their rejected alternatives are recorded in docs/decisions/714-mcp-blog-article-tools.md.
  • [4.123.0] fix(checkout): stop cancelling a PayByBank order on the callback body's word alone (#780)
    • The defect, two parts, same arm. Adv_checkout::payByBankResponse()'s CANCELLED / READY_TO_CANCEL branch (ecommercen/checkout/controllers/Adv_checkout.php) is #619's other half: #619 gated the PAID arm against gateway confirmation and left this one untouched. (1) It applied no gateway confirmation at all — an unauthenticated caller could flip any PENDING paybybank order to CANCELED on the callback body alone. One such forged callback performs three unlatched writes against an order the merchant never cancelled — product stock is credited back (returnOrderStock()), loyalty points are returned (returnPointsToCustomers()), and one coupon usage is released (markCouponUnused(), a floorless is_used - 1 that can go negative and buy extra redemptions) — plus a latched gift-counter restoration (restoreOrderGifts()/claimGiftsApplied(), a compare-and-swap that a repeat collapses to nothing). (2) Independently of forgery, the arm also honoured READY_TO_CANCEL — a reversible gateway state the gateway may still flip back to PAID — even though every other consumer of a PayByBank status in this codebase already defers on it (src/Domains/Checkout/Jobs/PollPayByBankStatus.php:126-138). This half is reachable by ordinary, legitimate traffic, not only by a forged callback.
    • The fix. The arm now asks PayByBank itself, via getOrdersBySerial(), whether it considers the order CANCELLED or CANCELLED_BY_MERCHANT before cancelling anything, and fails closed — an error envelope, an undecodable payload, or a serial the gateway doesn't report on all refuse the cancel — same posture as #619's PAID-arm gate. READY_TO_CANCEL is deliberately excluded from the accepted set, so a gateway-confirmed READY_TO_CANCEL callback now leaves the order PENDING instead of cancelling it. The refusal path reuses the existing acknowledgePayByBankCallback() helper, so it emits the same bytes as every other exit path — no new probing signal.
    • New shared helper, one guard. payByBankReportsPaid() (#619) and the new payByBankReportsCancelled() are both thin protected predicates now delegating to one protected payByBankReportsStatusIn(string $serial, array $accepted): bool holding the fail-closed guard and response traversal once, so the two never drift the way this codebase's other gateway-status consumers already have (docs/decisions/780-paybybank-cancel-arm-gate.md, D1).
    • What this does NOT fix. The floorless markCouponUnused() (and its twin markCouponUnusedByCode()), returnOrderStock()'s missing idempotency latch (acknowledged in its own docblock), and the pre-validation insertPBBLog() write are all unchanged — this closes the gate in front of them, not the latches behind them. One forged cancel is still enough to trigger those three unlatched writes once; a second forged cancel for the same order does not, because the inline PENDING guard earlier in the handler already refuses it once the order is CANCELED.
    • Full design, the rejected alternatives, and the acceptance criteria are recorded in docs/decisions/780-paybybank-cancel-arm-gate.md.
  • [4.123.0] feat(rest/promotion): expose the currently-active gift rules to a storefront via a new guest-readable GET /rest/promotion/gift/active (#646)
  • [4.123.0] fix(docker): stop the dev/integration stack writing FILES_S3_PREFIX into a tracked file (Advisable-com/ecommercen#692)
    • The bug. php migrator.php migrate against the dev/integration stack appended a generated FILES_S3_PREFIX into .docker/integration/.env.placeholder — a tracked file — dirtying the working tree on every fresh setup. Three things combined to cause it: database/migrations/20250410154622_set_env_s_3_prefix.php wrote to the bare relative path '.env' (resolved against the process cwd); dev.compose.yml and web-dev.compose.yml bind-mounted that tracked placeholder at /usr/local/var/www/html/.env read-write; and the guard isset($_ENV['FILES_S3_PREFIX']) — which works correctly — meant the only thing suppressing the write was the very value the write produced.
    • Second recurrence, one failure mode. A previous patch (shipped as "chore(docker): empty the integration .env.placeholder file") reset the file to zero bytes. That removed the key and therefore re-armed the write, guaranteeing this recurrence. Triage established the file was originally born carrying migration output in an unrelated commit about an admin backup button — so this is the second instance of one failure mode, not two accidents.
    • The fix splits the roles by direction. The bind mount in dev.compose.yml and web-dev.compose.yml (php-fpm, php-cli, and the read-only nginx mount) now points at a new untracked .docker/integration/.env.container, created by compose.sh on every invocation via an unconditional touch — never cp, because touch never truncates, so a FILES_S3_PREFIX already written survives every subsequent stack restart — and gitignored by its own .gitignore entry — the existing bare .env rule matches only that exact basename, not .env.* siblings. .env.placeholder stays tracked, stays empty, and is mounted by nothing; it is now only the documented reference for what the container's .env should look like. The mounts stay read-write on purpose — a :ro mount would stop the dirty-tree problem but also stop the prefix ever being generated, which a real S3-backed install needs: FILES_S3_PREFIX is read at application/config/storage.php for the public S3 disk, the sitemap disk, and the private S3 disk, as the shared per-client tenant identifier.
    • The migration's write target is now anchored, not cwd-relative. SetEnvS3Prefix::up() resolves its target against the repo root instead of the bare '.env', and now skips cleanly — writing a notice, not failing the migration — when that path is missing or not writable.
    • Full rationale, including rejected alternatives, is recorded in docs/decisions/692-env-placeholder-untracked-mount.md.
  • [4.123.0] fix(checkout): stop treating an unauthenticated PayByBank callback as proof of payment (#619)
    • The defect. Adv_checkout::payByBankResponse() is publicly reachable through get_response() and applied no authenticity check of any kind. Its only gate was a merchantOrderId matched against shop_order.order_serial — enumerable, not secret — so an unauthenticated caller could flip any PENDING order to PAID (goods shipped) or to CANCELED (stock restored, coupon released, loyalty points returned — money out). Production ingress showed 152 hits to /checkout/get_response/paybybank over 7 days, so this was a live payway, not a dormant one.
    • The fix, three changes. (1) the order lookup now also filters on payway => 'paybybank', so a callback naming another payway's order serial resolves nothing. (2) the PAID arm no longer trusts the callback body as evidence — it asks PayByBank itself via getOrdersBySerial() whether the order is actually paid before writing anything, and fails closed on an error envelope, an undecodable payload, or a serial the gateway doesn't report on. (3) every path the handler can leave by — no matching order, an order on another payway, an order no longer PENDING, and a PAID claim the gateway won't confirm — now emits the same bytes as the legitimate path, so probing a serial no longer reveals whether it exists or what state it's in.
    • New seam: Adv_base_controller::terminate() — protected, wraps exit — is the one place every controller arm that must stop the response dead now routes through, so the arm stays drivable in-process by a test instead of hard-exiting past assertions.
    • What this did NOT fix, and what closed it since. This change gated the PAID arm only; the CANCELLED/READY_TO_CANCEL arm was left ungated, so a forged cancel of a genuine PENDING paybybank order still credited stock and returned the customer's loyalty points. That half is closed by #780, which ships in this same release — see docs/changelog/unreleased/780-paybybank-cancel-arm-gate.md. Read the two entries together: on its own this one describes a state the release does not ship.
    • Full design and the rejected alternatives are recorded in docs/decisions/619-paybybank-forged-callback.md.
  • [4.123.0] fix(coupons): delete coupon_categories rows when their product category is deleted (Advisable-com/ecommercen#694)
    • The defect. coupon_categories.category_id references shop_product_category.id, and this schema has no foreign keys anywhere, so cleanup on category delete is application-level only. Nothing removed coupon_categories rows when a product category was deleted — the last of eight references to shop_product_category.id left unhandled after #677 fixed the other seven (across the six link tables cleaned here plus the mui table) at three sites.
    • Why it mattered — confirmed both ways in Adv_coupons_model. coupon_rules .categories_rule_type is 0 => OR, 1 => AND. On the AND path a dead category id can never accumulate a count, so isValidCoupon() returns false and the coupon silently stops applying at all, for every customer, from the moment the category is deleted — no id reuse needed. On the OR path, once AUTO_INCREMENT reissues the id, the coupon silently discounts an unrelated category.
    • The fix, all three sites. Legacy: Adv_product_category_model::delete_record() now deletes coupon_categories inside its existing transaction (six dependent tables, up from five). Modern: Product\Category\Repository\WriteRepository::DEPENDENT_REFERENCES gained the matching entry, so DELETE /rest/product/category/{id} cleans it too via WriteService::delete()'s transactional() wrapper — no endpoint, response shape, or API version changed. Historical residue: patches/CleanOrphanCouponCategoryRows.php sweeps every leaked row, deleting only rows whose category provably no longer exists (LEFT JOIN ... WHERE c.id IS NULL).
    • Full design, the four defects deliberately left out of scope, and the reasoning behind them are recorded in docs/decisions/694-coupon-categories-orphan-cleanup.md.
  • [4.123.0] fix(domains/cms): map the blog-article title.{locale} filter to blog_mui.title, the column that exists (#579)
  • [4.123.0] fix(checkout): write the invoice/receipt choice to shop_order.paymerch instead of truncating it into the courier's gen_tax_service field (#760)
  • [4.123.0] fix(checkout): clear the invoice identity fields on receipt orders and accept company_address at order creation, matching legacy parity (#760)
  • [4.123.0] feat(rest/checkout)!: refuse an invoice order (wantsInvoice: true) that is missing afm/doy/company/profession/companyAddress after trimming — 422 invoice_identity_incomplete (#760)
  • [4.123.0] fix(checkout): return the customer to the client's URL after a REST payment, on both the success and the failure leg (Advisable-com/ecommercen#734)
    • The defect, success leg. #728 correctly made the field a gateway posts its payment result to server-owned, but on four payways — alpha, eurobank, ethniki, ethniki_ee — that same field was also the gateway's only browser destination. The order confirmed correctly, but the customer was stranded on this deployment's legacy thank-you page instead of the submitted returnUrl.
    • The defect, failure leg (the mirror, and #728's amendment A3). alpha and eurobank still handed the gateway the client's own cancelUrl as the field it reports a failure to, so a refused or cancelled payment returned the customer correctly but our fail arm never ran: the order stayed PENDING, the coupon stayed consumed and the customer's redeemed loyalty points stayed debited until the 180-minute AdvCancelIncompleteOrders sweep. ethniki_ee was worse — it emitted no failure destination at all.
    • The fix. Both client destinations ride on the same server-owned callback as a query parameter. PaymentCallbackUrlBuilder::confirmationUrl() now takes the client's returnUrl and appends it via a new callbackUrl() helper; a new failureUrl() method builds .../get_response/{payway}/fail and appends the client's cancelUrl the same way. The parameter name is published as CLIENT_RETURN_URL_PARAM so producer and consumer can't drift. Adv_checkout gains redirectToClientReturnUrl(), called last in alphaResponseSuccess(), eurobankResponseSuccess() and the shared ethnikiResponseSuccess() (success leg), and in alphaResponseFail() and eurobankResponseFail() (failure leg) — five methods, not eight, since ethniki and ethniki_ee already funnel into one shared success method and ethniki_ee's failure leg is held (see What still does NOT work).
    • The failure leg covers alpha and eurobank; the success leg covers all four. ethniki_ee is success-leg only, like ethniki.
    • The write-then-redirect ordering is preserved, not replaced. The redirect is appended after each arm's existing terminal write — PAID on the success arms, CANCELED plus the coupon release and points return (cancelOrder()) on the failure arms — never in place of any part of it. Each arm sets a 303 Location header instead of calling redirect(), whose exit would skip post_system, this app's only DeferredTaskRunner::run() call site: on the success arms that queue holds five purchase integrations (advisable_ai.purchase, meta.purchase, manago.purchase, project_agora.order, matomo.flush) on an order just marked PAID, four of them PRIORITY_CRITICAL, with no register_shutdown_function fallback anywhere in the app.
    • REST-only by construction, not by a per-order lookup. PaymentCallbackUrlBuilder's only consumer is PlaceOrderService; the rendered storefront builds its callbacks inline and never touches the builder, so no storefront-placed order can carry the parameter — a non-REST order still renders its legacy view unconditionally, on both legs.
    • What still does NOT work, each on its own issue. ethniki gets no failure leg: that integration exposes no failure destination anywhere, and NBGHelper::ethnikiValidateResponse() checks only the signature, so a signature-valid declined return is marked PAID today on the rendered storefront too — a separate money-correctness defect, tracked as #741, not attempted here. ethniki_ee gets NO failure leg either — it was built, reviewed and then deliberately HELD pending #730's per-order capability token. The arm can only resolve a failed return by the order serial, and that check has no secret component, so wiring it would have shipped the first working instance of an unauthenticated "cancel any PENDING order you can name" primitive — filed as #769. It is latent only behind getExternalPayWays() (#673), and both #464 and #749 remove that gate, so it was not safe to defer on disclosure alone. The cost accepted in exchange: a failed ethniki_ee payment still reaches no terminal status, leaving the order PENDING with its coupon consumed and the customer's points debited until the 180-minute sweep — the pre-#734 behaviour, tracked for the storefront as #768. ethniki_ee's cancelUrl is deferred separately: its legacy cancel URL carries the order serial as a path segment the CI3 dispatcher truncates before the handler sees it — #742. iris (#736) and klarna_payments (#737) get neither leg; neither arm reaches a terminal order status yet. returnUrl/cancelUrl remain unvalidated on every payway, a deliberate consistency decision, not an oversight — six sibling adapters already pass the same unvalidated value to a gateway as the browser destination, so guarding only these arms would add an asymmetry without closing the exposure (#743).
    • 35 unit tests across the builder and the PaymentRedirect value object, plus 30 legacy tests covering all three arms on both branches — including one that drives placeOrder() end to end and asserts on the serialised response array, since the object→array boundary is where a documented field was found to be silently dropped. Full design, the rejected alternatives (persisted shop_order columns, meta_data, the CI3 session, an extra path segment, redirect() with an explicit drain, a same-origin URL check), and the GATE-1 per-payway failure-leg survey are recorded in docs/decisions/734-payway-onward-redirect.md.
  • [4.123.0] feat(vendors-admin): add a permission-gated SEO meta panel to the vendor create/edit admin forms (#755)

shop_vendor_mui has carried meta_title / meta_keywords / meta_description since migration 20260612121500 (the #312 MCP connector epic), and the entity, REST resource, MCP tool, and the storefront brand page (seo_lib::create_metatags) already read and wrote them. Only the legacy admin form had no UI for the three columns, so a vendor's SEO meta was writable only through REST or the MCP connector and invisible to an admin operator working the vendor screen. This adds the panel; no migration, model change, or new locale keys were needed — kbitadmin.label.{title,description,keywords} and eshop.admin.products.seo already ship in all 8 packs.

  • [4.123.0] fix(feeds): stop xmlSanitise() fatalling on NULL product fields, which took the whole catalogue export down with a 500 (#607)
    • The defect — two independent fatals in one function. xmlSanitise() (application/helpers/MY_text_helper.php:390) was declared xmlSanitise(string $string): string, but every feed controller feeds it a nullable DB column — shop_product_mui.description is mediumtext DEFAULT NULL, blog_mui.description is longtext DEFAULT NULL — so a single product or blog post with a NULL description raised an uncaught TypeError and took the entire catalogue export down: no partial XML, no skipped row, a bare 500. Live in production on easypharmacy, with the Google Shopping feed down. A second, independent fatal sat behind the first: preg_replace() with the /u modifier returns null when its subject is not valid UTF-8 — exactly the input this helper exists to clean — which against the declared : string return type raised its own uncaught TypeError.
    • The fix. xmlSanitise() widens its parameter to ?string, short-circuits null and '' to '', and adds a ?? '' fallback on the preg_replace() return. One change at the single chokepoint every feed routes through — 22 call-site lines (24 invocations, since two controllers each call it twice in one ternary) across 12 feed controllers — covers Google Shopping, Skroutz, Criteo, Manago, Emag, Glami, LinkWise, ContactPigeon, the internal search XML, and the blog RSS. No feed controller is modified.
    • Covered by 7 new tests in tests/Legacy/Helpers/XmlSanitiseHelperTest.php: null input, two invalid-UTF-8 shapes, XML-illegal control characters being stripped, empty-string passthrough, and valid Greek text passing through unchanged.
    • Full design, including the rejected scrub-instead-of-empty alternative, is recorded in docs/decisions/607-xmlsanitise-null-fatal.md.
  • [4.123.0] fix(auth): point myTasks() and tasksTo() pagination links at their own routes instead of /auth/tasks (#751)
  • [4.123.0] feat(mcp): product tag tools for the connector — list, create and update tags, and assign them to products (Advisable-com/ecommercen#713)
    • The gap. The MCP connector exposed 24 tools and none of them touched product tags. An operator working through the connector could not see which tags existed, could not create or rename one, and could not put a tag on a product — even though tags are the facets the storefront filters the catalog by.
    • Four new tools in src/Mcp/Tools/TagTools.php: list_product_tags (id, name, slug, content, order and the tag's tag_category {id, name}), list_product_tag_categories, create_product_tag and update_product_tag. list_product_tag_categories exists because create_product_tag requires a tag_cat_id and nothing else on the surface tells the model a valid value — a tag category with no tags yet would otherwise be invisible and impossible to create into.
    • Assignment rides on the existing update_product tool, which gains a tag_ids full replacement set alongside category_ids. It is the same state table the category path already documents, and it has four states, not three: omit the key and the assignment is untouched, pass [] to clear every tag, pass ids to replace the set with exactly those — and pass an explicit null (or any non-array scalar) and it is coalesced to an empty set, so it also clears every tag. Only omitting the key leaves tags untouched, which is worth stating because many JSON serializers emit "tags": null for a field the caller never populated. This is not a divergence to fix here: Product\WriteService::parseTagIds() is a byte-identical mirror of the live parseCategoryIds(), which has always behaved the same way. A naive GET→POST round-trip is the safer failure by comparison — ids are validated before the transaction opens, so echoing a read shape back at the write key is refused with a 422 rather than silently clearing. get_product / products_batch_get now also return the product's tags so the model can read the set before replacing it. The MCP layer never writes shop_product_product_tags itself — it hands a tags key to Product\WriteService, which validates the ids and syncs the pivot inside its existing transaction.
    • Assignment is idempotent and order-independent. TagPivotWriteRepository::syncForProduct() deletes then batch-inserts the distinct ids. The dedupe is load-bearing rather than defensive: shop_product_product_tags declares product_tag_id as a plain composite KEY, not UNIQUE, and has no foreign keys, so nothing in the schema would reject a duplicate (product_id, tag_id) row. Re-assigning a tag a product already has is not an error and adds no row; unassigning an absent tag is not an error.
    • create_product_tag is the first create_* verb on the connector, so the server's own instructions were corrected in the same change. SEO_INSTRUCTIONS no longer tells the model it "CANNOT create or delete" records — it now states that creation is available for product tags only and that nothing can be deleted — and the shared WRITE_NOTE appended to every update tool's description was narrowed to speak for that tool rather than for the whole surface. This is protocol content the client LLM reads, not documentation: shipping a create tool behind an instruction denying creation publishes a contradiction the model resolves against us at runtime.
    • A tag's slug is never an input. On create it is derived from name by generateMuiSlugs() → SlugGenerator::generateUnique(); on update it is carried through untouched, so renaming a tag does not change its storefront URL. Exposing the slug would let the model collide slugs; omitting the merge on update would null it, because translation writes go through MuiWriteRepository::replaceForEntity() (DELETE-ALL + RE-INSERT) and the tag write service does not regenerate slugs on update.
    • Tag rows carry no SEO-completeness metadata, deliberately and unlike every other entity tool. shop_product_tags_mui has exactly id, tag_id, name, slug, content, lang — no meta_* columns at all — so has_meta, meta_*_length and has_complete_description would have to be invented over a facet label rather than read. Writes are audited through McpAuditLogger like the existing write tools.
    • Full design, rejected alternatives and the corrected issue premises are recorded in docs/decisions/713-mcp-product-tag-tools.md.
  • [4.123.0] fix(checkout): stop a routine gateway failure from surfacing as an uncaught TypeError, and give every failed payment initialisation one deliberate error (Advisable-com/ecommercen#731)
    • The defect. PaymentAdapterInterface::initializePayment() was declared ?PaymentInitResult, and two adapters (PiraeusAdapter, PayByBankAdapter) used that nullability and returned null on failure. PaymentInitializer::initialize() was declared : PaymentInitResult with no null guard, so null raised a TypeError — which TypeError extends \Error put outside every arm Checkout::placeOrder() catches. SafeActionDispatch caught it as \Throwable and returned a valid but generic 500, so a routine bank refusal looked identical to a server bug. The other ten failable adapters (apcopay, ethniki_ee, ethniki_nbgpay, iris, jcc, klarna_payments, paypal, paypaladvanced, vivawallet, xpay) already threw a bare RuntimeException on failure, which answered 400 Bad Request — wrongly blaming the client for an upstream failure. And a thirteenth payway failed a third way, found in review: StripeAdapter calls \Stripe\Checkout\Session::create() with no try/catch, and \Stripe\Exception\ApiErrorException extends \Exception — not RuntimeException — so a Stripe outage matched neither idiom and fell through to placeOrder()'s generic \Exception arm as its own bare 500.
    • The fix. PaymentAdapterInterface::initializePayment() is narrowed to PaymentInitResult (no more ?). A new Advisable\Domains\Checkout\Exceptions\PaymentInitializationFailedException (extends \RuntimeException, ERROR_CODE = 'payment_initialization_failed') is thrown instead. PaymentInitializer::initialize() (src/Domains/Checkout/Payment/PaymentInitializer.php) is now the single chokepoint every adapter is called through, and it converts both historical failure shapes into the new type — the (now type-system-impossible, kept as belt-and-braces) null, and any \Exception an adapter throws — preserving the original as $previous so the log keeps the real gateway stack. The catch is \Exception and not RuntimeException precisely so that stripe is covered, and it stops there on purpose: \Error and its subclasses (a TypeError, a call on null — a bug of ours inside an adapter) are not converted and still surface as a 500, so a 502 always means the gateway and never our own defect. All 18 in-tree adapters had their signature narrowed (16 of them signature-only); Piraeus and PayByBank's three return null sites were replaced with throws — four throws, because PayByBank's single !is_array($response) || isset($response['error_code']) guard was split in two so that "could not be read at all" and "reported a refusal" no longer share one message in the log. Checkout::placeOrder() gained a catch (PaymentInitializationFailedException $e) arm returning the same {success, message} envelope as payway_not_available and loyalty_redemption_exceeds_order_total, plus error.code and error.payway.
    • The "no adapter registered for this payway" error is deliberately untouched and still answers 400 Bad Request — that is a genuine client mistake (an unsupported payway), not an upstream outage, so it is not part of this conversion.
    • This makes the failure reportable, not recoverable. PlaceOrderService::placeOrder() runs in no transaction and reaches payment initialisation after the basket is written, the coupon consumed, points debited and the cart cleared, so an order whose payment init fails is still left PENDING in exactly that state — the new 502 is not a rollback, and a client must not read it as "nothing happened". Retrying creates a second order rather than resuming the first. Repairing that half-written order is out of scope here and tracked separately as Advisable-com/ecommercen#735.
    • Full design, the rejected alternatives (a nullable initialize() instead of narrowing the interface; converting only the two null-returning adapters instead of chokepointing all twelve; 422 instead of 502), and the GATE-1 decision on the HTTP status are recorded in docs/decisions/731-payment-init-null-contract.md.
  • [4.123.0] fix(checkout): build a server-owned confirmation callback for six REST payways instead of posting gateway results to the client's URL (Advisable-com/ecommercen#728)
    • The defect. Six REST payways — iris, alpha, eurobank, ethniki, ethniki_ee, klarna_payments — assigned the client-supplied PaymentContext::returnUrl to the field their gateway posts its payment result to, not just the field that sends the browser back. The gateway then confirmed the payment to the headless client's own page, so nothing server-side ever observed it: the customer was charged and the order stayed PENDING indefinitely.
    • The fix. A new Advisable\Domains\Checkout\Payment\PaymentCallbackUrlBuilder (src/Domains/Checkout/Payment/) maps payway + order serial to a server-owned confirmation URL and, for klarna_payments, a separate notification URL — both built with site_url() behind the same function_exists('site_url') guard the xpay/apcopay factory precedent already uses, so all 14 unit-test PaymentContext construction sites keep working with no CI3 boot. PlaceOrderService now depends on the builder (autowired, no container.php arg needed) and passes its output into two new required PaymentContext params, $confirmationUrl and $notificationUrl, positioned right after cancelUrl. Each of the five other adapters (CardLinkAdapter, EthnikiAdapter, EthnikiEEAdapter, KlarnaAdapter) stops assigning returnUrl to the gateway-result field; iris instead stops overriding site_url('checkout/get_response/iris') . '?initiatingPartyRefId=...', which Iris.php already built and which carries the ref id the builder cannot reconstruct. CardLinkAdapter's SHA-256 digest follows automatically, since it hashes whatever confirmUrl now holds.
    • Fixes four of the six payways end-to-end; iris and klarna_payments still don't reach PAID. Each of those two carries a second, independent defect out of scope here: REST-placed iris orders get no iris_orders row at all, so both the cron recovery and the legacy return handler miss even with a correct callback; the Klarna adapter returns PENDING where legacy writes PAID synchronously, and the webhook designated as the real confirmation point writes no status. This change fully fixes alpha, eurobank, ethniki and ethniki_ee; for iris and klarna_payments it delivers only the URL half.
    • The customer now lands on this deployment's legacy thank-you page, not the submitted returnUrl. For all six payways the field just taken over was the sole browser destination, and the legacy checkout/get_response/{payway} handlers render a view with no client-URL passthrough. This is an accepted trade-off, not an oversight — before this fix the customer did land on their returnUrl, but the order never confirmed at all. Redirecting the browser onward to the client's URL after the legacy handler runs is tracked as a follow-up, not fixed here.
    • On a failed or cancelled alpha/eurobank payment, the server still learns nothing.cancelUrl deliberately still carries the client's URL — only the success-result field moved. Legacy sends .../alpha/fail there instead. This is milder than the success-path defect: the order sits PENDING and the existing 180-minute AdvCancelIncompleteOrders sweep cancels it, which is the correct end state, but it means a failed/cancelled attempt is still invisible to this deployment until that sweep runs.
    • Full design, the rejected alternatives (adapter constructor injection, an inline match in PlaceOrderService, a closure builder), and the two GATE-1 decisions (both new params required, the builder as a separate class) are recorded in docs/decisions/728-split-payment-confirmation-callback.md.
  • [4.123.0] fix(eshop): apply the active-language predicate to both shop_vendor_mui joins in getActiveGiftRules() (#727)
  • [4.123.0] fix(rest): pin ProductCode and CustomMetaTag to explicit rest_policies.php role entries instead of the insecure global default (#733)
  • [4.123.0] feat(payments/vivawallet): register a dedicated source code for external-frontend payments (#729)
    • The defect. Viva Wallet registers success/failure return URLs per source code, in the merchant portal — never as request parameters. The platform exposed a single VIVAWALLET.SOURCE_CODE, shared with the rendered storefront, so a customer paying with vivawallet from an external frontend (nuxt / velora / mobile) was returned to the legacy storefront thank-you/failure page instead of the calling application. The payment itself succeeded — only the return leg was wrong, which is why this never surfaced as a failed transaction.
    • The fix. A new VIVAWALLET.EXTERNAL_SOURCE_CODE registry key, read by a new getVivaWalletExternalSettings() (ecommercen/helpers/registry_helper.php), now feeds PaymentInitializerFactory::registerVivaWallet() — every consumer of that factory is on the modern REST checkout path. Same per-channel shape as the Piraeus POS terminals added in #674, with one difference: only the source code is per-channel here, since Viva has one merchant account with one OAuth2 credential pair underneath multiple source codes, so the credential gate (Config::isConfigured()) stays shared and unchanged.
    • The rendered storefront checkout is untouched — it still reads VIVAWALLET.SOURCE_CODE via Adv_checkout. Gift-card purchases are also unaffected (still on the storefront gift_card_source_code); an external gift-card source code is explicitly out of scope.
  • [4.123.0] fix(order): restore gift stock on order cancellation (Advisable-com/ecommercen#688)
    • The defect. Gift stock (gifts.remaining) was decremented when a gift was awarded — at checkout completion on the legacy path, at OrderPaid on the modern path — and never given back when the order was later cancelled. Both cancellation paths already restored product stock (product_codes.stock) and stopped there, so every cancelled order that had earned a limited gift permanently burned a unit of remaining. Because the discovery predicate requires remaining > 0 OR remaining IS NULL, an exhausted counter silently stopped offering the promotion, with no signal distinguishing "sold out" from "leaked away by cancellations".
    • The fix, both paths. Legacy: the restore hangs off the existing if (!$ignoreStock && $stockMode === '+') branch in Adv_order_model::set_status(), beside the existing returnOrderStock() call — no call-site changes, since all legacy cancellation entry points already converge on set_status(). Modern: a new RestoreGiftStockOnCanceledListener on OrderCanceled, registered alongside the existing coupon/points cancel listeners in src/Domains/Order/container.php.
    • Gated on a new marker, not on order status. Neither status nor is_paid reliably distinguishes "this order's gift stock is currently consumed" — three separate writers decrement gifts under different status/payment combinations, and the two decremented-at- PENDING legacy PayByBank cohorts are swept by cron with no origin filter, which rules out any old-status inference in set_status(). A new shop_order.gifts_applied (TINYINT(1) NOT NULL DEFAULT 0, added AFTER points_added) is set when a gift decrement lands and cleared by the restore via a compare-and-swap (UPDATE shop_order SET gifts_applied = 0 WHERE id = ? AND gifts_applied = 1, guarding on affected_rows() > 0). The same column therefore supplies both the restore's precondition and its once-only idempotency — a repeat cancel, or two concurrent cancels, restore exactly once. Gift\WriteRepository::restoreRemaining() / Adv_gifts_model::restoreCounter() guard remaining IS NOT NULL in one atomic UPDATE ... SET remaining = remaining + ?, skipping unlimited gifts. Both cancellation paths claim the marker before reading the basket inside their own transaction: the modern RestoreGiftStockOnCanceledListener and legacy's Adv_order_model::restoreOrderGifts() (ecommercen/eshop/models/Adv_order_model.php:1025-1067). On a mid-restore transaction failure both roll the claim back too, re-arming the marker instead of leaking the stock silently — but the two now diverge on what happens next. The modern listener still throws a RuntimeException, which is safe because OrderEventDispatcher catches and swallows every listener's exceptions with a warning log. The legacy method no longer throws (#260 review): it calls log_message('error', ...) and returns normally, because set_status() has no dispatcher of its own around it — a throw there would abort Adv_order_model::setBatchCanceled()'s batch loop partway through and surface as an unhandled fatal to the other callers of set_status() (admin edit-order, AdvCancelIncompleteOrders, AdvApiKlarna, the PayByBank sweep), none of which has ever had exception isolation around this call.
    • The decrement itself now reports success. Gift\WriteRepository::decrementRemaining() changes return type void -> bool: true means this gift's stock is correctly accounted for — the guarded UPDATE actually decremented it, or it's unlimited and had nothing to decrement — false means exhausted, missing, or a non-positive quantity. DecrementGiftStockOnPaidListener filters basket lines with qty &lt;= 0 out of its per-gift aggregation before any of them reach decrementRemaining(), so such a line cannot veto the marker for gifts on the same order that genuinely were decremented; it sets gifts_applied only when every decrement that was attempted comes back true (an order whose only gift line is qty-0 is still left unmarked, since nothing was attempted for it). The legacy updateGiftCounters() mirrors this — it now skips entries whose count is &lt;= 0 before counting them as processed — so both layers agree in treating a zero quantity as "nothing to do" rather than as a vetoing failure. This closes a path by which an already-exhausted gift (over-issued under the accepted #203 tradeoff) could be credited back on cancellation despite nothing having been deducted for it — gifts has no ceiling column, so an over-restore there is silent and permanent, never self-correcting.
    • Full design, rejected alternatives, and the premises that ruled out status-based gating are recorded in docs/decisions/688-restore-gift-stock-on-cancel.md.
  • [4.123.0] fix(eshop): read the active language instead of a hardcoded 'el' in gift, sitemap, offers, video and waiting-list queries (#700)
  • [4.123.0] fix(eshop): join gift_vendor_mui on vendor_id rather than its unrelated id PK, so a gift choice's gift_slug carries its own vendor's slug (#700)
  • [4.123.0] fix(offers): match the offers "all categories" route segment against a per-language key instead of the Greek literal ολεσ, and 404 rather than fatal on an unknown category slug (#700)
  • [4.123.0] fix(analytics): require an explicit $lang on the sales-analytics breakdown and orders-list services instead of defaulting to Greek (#700)
  • [4.123.0] fix(docker): ship public/robots.txt in the app image and serve it statically from nginx (#598)
  • [4.123.0] fix(auth): count admin task-list totals via count_all_results() instead of materialising the result set and calling num_rows() (#74)
  • [4.123.0] Check for overrides:
    • Advisable\VivaWallet\Base::doRequest() and ::handleRequest() changed signature (: array → : ?array), and the break this causes in a fork is silent. Return types are covariant, so an override that keeps the narrower : array still declares, still instantiates and still runs — PHP raises nothing. What that fork loses is the distinction itself: its override can never return null, so unreachable is always false for it, an unreachable gateway is read as "Viva says no", and the webhook answers a permanent 2xx where upstream answers a retryable 502. The 502-vs-200 decision — the whole of criterion 9 — is inert there, with nothing to draw attention to it. A fork overriding either method must widen it to ?array and decide what its own catch returns. There are no such overrides upstream.
    • Advisable\VivaWallet\VivaWallet::verifyOrderIsPaid() and ::vivaWalletValidateOrderIsPaid() return additional keys — found, unreachable, amount and orderCode, on the confirmed shape and on every unconfirmed one. Purely additive; all four upstream callers read by named key. A fork reading the result with array_keys(), count() or a whole-array comparison would see the change.
    • Advisable\VivaWallet\Base::httpClient() is a new protected method returning the Guzzle client doRequest() uses — additive, and per the #417 precedent an additive protected method cannot break a fork by itself.
    • AdvViva gained a protected vivaWallet() factory and three protected predicates (gatewayConfirms(), transactionBelongsToOrderCode(), amountMatches()). Additive. But a fork that overrode handleWebhook() wholesale in application/controllers/webhooks/Viva.php — the designated fork seam, upstream an empty class Viva extends AdvViva — silently shadows this fix: it will not merge-conflict, and it keeps the vulnerability. That is the one thing a fork operator most needs to check.
    • application/config/routes.php is unchanged, deliberately. The method discrimination lives in AdvViva.php precisely because a fork's copy of the route file shadows upstream's silently.
  • [4.123.0] Behaviour a fork or an integration may notice:
    • An empty-bodied POST to the webhook URL now answers 400 and returns no body, where it previously returned the merchant's webhook verification key. An empty-bodied GET is unchanged — Viva's registration flow still works.
    • The webhook now answers 502 when Viva cannot be reached or answers an HTTP error during verification, where it previously answered 200. Viva will retry those, by design. Anything monitoring this endpoint's status codes will see a new 5xx class that corresponds to gateway faults, not to application faults.
    • Each acted-on notification now costs outbound calls to Viva that it did not before (an OAuth token POST plus the transaction lookups). Unknown event types and unknown order codes are rejected before any of them.
    • A new error-level log line — "…was reported in two currencies, so its amount could not be compared…" — marks each payment confirmed under the multi-currency exemption above. It is expected on a merchant taking dynamic-currency-conversion payments and is not a failure; it exists so that the one unverified case is countable rather than invisible.
  • [4.123.0] Behaviour change: moving a comment back to pending now sends no email, where it previously sent the rejection email. Approve and reject notify as before. This resolves the open question #633 raised ("whether the 'move back to pending' action should send any email at all") in favour of silence: a neutral action has no outcome to report, and there is no blogCommentPending template to send.
  • [4.123.0] Check for overrides — application/models/Adv_mailer.php. It is a per-fork copy in 31 of the 34 local fork checkouts, all currently on the original ($status == 1) comparison, so all 31 take this change on their next upstream sync. Dioptra and Elxis will conflict: both have hand-patched the method body to suppress the rejection email ($viewToLoad = ($status == 1) ? 'blogCommentAccept' : null; plus an early return). The correct resolution is to take upstream and re-express that intent by overriding the new blogCommentEmailOutcome() seam — which is what the seam exists for — rather than keeping the local body. Taking upstream wholesale there would start sending rejection emails those two tenants have deliberately disabled.
  • [4.123.0] Check for overrides — application/config/routes.php. Also a per-fork copy, so the 500 fix reaches a client only via the upstream merge. 29 forks carry the buggy three-line block byte-identical to upstream and merge cleanly. Six — AdvisableSite, Anatomicline, Elxis, FamilyPharmacy, Nautilus, Services — deleted the block locally, which fixed their method routing but leaves comment-list pagination resolving a numeric segment as a method name (404). Git sees a "they deleted / we modified" region there, so expect a conflict; taking upstream restores pagination without breaking the methods.
  • [4.123.0] FamilyPharmacy needs a migration, not a merge resolution. It is on 04.61.004.011 and still runs the pre-refactor controller — index($filterType = 0, $offset = 0) calling getFilterData(), with a numeric switch, its own getAdminRecordsAndCount() model method, and a list.php expecting the old records / records_total / filterSelectedLabel render keys. Its screen is internally consistent and works today. Taking upstream's controller wholesale would break it, with or without this change.
  • [4.123.0] The five orphaned translation keys are kept on purpose. blog_comments.pending_title, approved_title, rejected_title, approved and rejected (greek, italian, spanish) now have no consumer in this repo, but FamilyPharmacy's older screen still renders them. Removing them would strip its titles the moment it merges a language file, ahead of the controller migration above. Cheap to keep, and safe to drop once that fork is current.
  • [4.123.0] No rest_api_versions.php entry. This is legacy admin only. The REST surface (/rest/cms/blog/comment) never had either defect: rest_routes.php binds each URI and HTTP verb to an explicit controller method, so nothing can be swallowed into index(), and the write path has always used the blog_comments.status literals. Its store/update/destroy actions are backend-only (rest_policies.php — defaults: backend, roles ADMIN/CMS), so no storefront caller can self-approve.
  • [4.123.0] Not fixed here, deliberately. Three pre-existing REST-side gaps found while confirming the above, none of them this outage and none newly introduced: moderating through REST sends the author no notification at all (sendBlogCommentStatusUpdateEmail() has exactly one caller, the legacy controller — the same is true of sendUserReviewStatusUpdateEmail(), so the gap is systemic rather than specific to comments); Domains\Cms\Blog\Comment\Validator is empty, so with stricton => false in database.php a backend caller writing an invalid status gets '' stored silently rather than a 422 — recoverable through the admin "All" filter, though it then displays as pending; and WriteData's OpenAPI status property documents "(approved/pending)", omitting rejected. Also unchanged: batchAction() sends no notification for any of its three values, and routes.php:263 (\w{2})/video_showcase_admin/(.+) has the same catch-all shape as the route fixed here, though AdvVideoShowcaseAdmin::index($offset = 0) is untyped so it degrades silently instead of returning 500.
  • [4.123.0] Check for overrides: application/helpers/log_helper.php is fork-owned. A client fork that diverged this file — declared its own _exception_handler() and/or _error_handler(), or copied the whole helper — would not receive this fix on upstream sync and would keep returning HTTP 200 on every uncaught exception and every PHP fatal, with uptime checks and 5xx-rate alerting still blind to both. There is no signature change and no merge conflict to surface this on a fork that merely redeclares the same two function names ahead of the helper's own function_exists() guard — it would be a silent divergence, not a loud one. A scan of all local client checkouts under /e/web/AdvEshop/v4-clients/ (24 real checkouts; a 25th entry there is a cache directory, not a repo) found this seam currently unexercised: 2 forks (v4-dioptra, v4-gea) carry no log_helper.php at all, so CI3's own _exception_handler() already sets 500 and exits there and neither ever had the defect; 4 (evaspharmacy, v4-demo, v4-heals, v4-wecare) are byte-identical to upstream pre-fix; the remaining 18 are byte-identical to each other, tracing not to 18 local patches but to one shared older upstream revision (bccd7cbad3, 2026-05-08). Zero forks carry a local patch to either handler, and zero declare _exception_handler() / _error_handler() anywhere else in application/ or ecommercen/. Consequence for a fork operator: all 22 affected forks take this fix on an ordinary upstream sync — nothing to hand-apply, nothing to reconcile; a fork on the older shared revision merges normally, a conflict at worst, never a silent miss. Per-fork detail is in #543's comment thread. The redeclare-ahead-of-the-guard seam described above remains real for any future fork, so it is worth re-checking if this note ever grows stale.
  • [4.123.0] Monitoring: this is the point of the fix, not a regression — uncaught exceptions and PHP fatals will start counting toward 5xx-rate alerting and answering response.ok === false on uptime checks, where before they counted as successful 200s. A team with 5xx alerting already configured may see a step change in alert volume right after this ships; that reflects pre-existing failures becoming visible for the first time, not a new outage.
  • [4.123.0] CLI-invoked fatals now exit with code 1 instead of PHP's default 255. Both are non-zero, so cron and Kubernetes failure detection (anything checking exit-code-zero-or-not) is unaffected; anything matching on the exact code 255 would see the change.
  • [4.123.0] On the PHP-fatal path only — _shutdown_handler() → _error_handler() → _log_helper_fatal_exit() — set_status_header(500) can run after a request has already flushed output, and PHP then raises its own "Cannot modify header information — headers already sent" as an E_WARNING. This is not a second defect and not a loop: that warning re-enters _error_handler(), maps to 'warning' in the severity match, gets logged, and returns, because E_WARNING isn't in the fatal mask (E_ERROR | E_PARSE | E_COMPILE_ERROR | E_CORE_ERROR) and so cannot re-trigger the halt. The operator-visible effect is a new class of warning-level log entries on requests that suffer a PHP fatal mid-render, after this ships — expected and harmless, but worth knowing the shape of before puzzling over it in a log dashboard. It's also inherited, not introduced: CI3's own system/core/Common.php:474-483 sets the same 500 from the same shutdown-reachable path with no headers_sent() guard either.
  • [4.123.0] UI Update: a new storefront layout key, checkoutEthnikiFail, is now rendered on a declined ethniki return. application/config/mainTemplate.json already carried the mapping (stubbed, with no view behind it); this ships the view for the main template and adds both the key and the view for the default template. Both views reuse the existing checkout.error.bank, checkout.alpha.fail.msg, checkout.error.retry and checkout.back strings, so no new translations are required — all four are present in all eight locales.
  • [4.123.0] Check for overrides:
    • A fork with its own template config must add the key. Template::layoutView() (src/Template/Template.php:17-24) reads ->layouts->{$key}->view unguarded, so a template JSON without checkoutEthnikiFail resolves to a null-property read and then a CI3 view-not-found fatal the first time an ethniki payment is declined. A fork carrying its own application/config/template.json / mainTemplate.json, or its own template folder under application/views/, needs both the key and a matching view file.
    • Adv_checkout::ethnikiResponse() changed behaviour without changing its signature, and Adv_checkout::ethnikiResponseFail() is a new protected method. A fork that copied ethnikiResponse() into application/modules/checkout/controllers/Checkout.php — the designated fork seam, normally an empty class Checkout extends Adv_checkout — silently shadows this fix: it will not merge-conflict, and it keeps marking declined cards as paid.
    • A REST-placed ethniki order that declines now renders this deployment's failure page rather than the client's. ethniki has exactly one callback URL (confirmUrl), which carries the client's success destination, so sending a declined customer there would be worse than the legacy page. Giving ethniki a real failure leg remains #464/#734 territory.
  • [4.123.0] Existing connector tokens do NOT gain the blog tools. The tools are registered under a new content scope (Scopes::CONTENT, appended last in Scopes::ALL), not under seo. Scopes::parse() drops unknown entries and treats a NULL/legacy row as [seo], so every already-issued token parses to a scope set without content and the four blog tools stay invisible to it — the same "legacy token keeps its exact pre-change surface" rule #403 established for the analytics scope. To use them, generate a new token — or rotate an existing one — with the content checkbox ticked, in Settings → MCP Connector. This is deliberate: widening seo would have granted content authoring and record creation to every token ever issued for SEO work, and create_article is the first tool on this surface that can mint a record (see #352, per-capability token scoping).
  • [4.123.0] UI Update: the MCP Connector settings screen (application/views/admin/settings/mcp_connector.php) now renders a third scope checkbox, "Blog Content (read + write + create)", in the Generate a new token form. No view or controller edit was needed — AdvMcpConnectorSettings::index() feeds the view Scopes::ALL and the template's checked attribute is hardcoded to $scope === 'seo', so the new scope renders unticked by construction.
  • [4.123.0] New language key settings.mcp_connector.scope_content: added to all eight shipped locales (chinese, english, french, german, greek, italian, russian, spanish → ecommercen/language/*/adv_advisable_lang.php). Greek carries its own translation; the other seven carry the English string, matching the existing scope_seo / scope_analytics rows. A fork carrying its own copy of adv_advisable_lang.php for any locale will not have the key, and t() on a missing key renders the key itself (application/core/MY_Lang.php:192-193) — not a fatal, but the admin sees the raw string settings.mcp_connector.scope_content as the checkbox label until the fork adds it.
  • [4.123.0] Check for overrides: Advisable\Mcp\Auth\Scopes::ALL grew a third member, and its order is the canonical output order of both parse() and format() — i.e. the CSV byte layout stored in mcp_connector_tokens.scopes. content was appended, never inserted, so format(['analytics','seo']) still returns 'seo,analytics' and no persisted token's CSV changes meaning. A fork that hardcodes its own scope list, or that re-orders ALL, will rewrite that stored layout for tokens that already exist.
  • [4.123.0] Check for overrides: Advisable\Mcp\Server\McpServerFactory gained a private registerBlogTools(), a private articleListSchema() and the constant CONTENT_INSTRUCTIONS, plus a third in_array(Scopes::CONTENT, ...) branch in create() and in buildInstructions(). One existing constant changed: SEO_INSTRUCTIONS now scopes its create sentence to "among THESE tools ... product tags only" instead of stating it connector-wide. That wording is load-bearing rather than cosmetic — buildInstructions() concatenates the blocks a multi-scope token holds, so an unscoped claim in one block contradicts another, and create_article makes the connector-wide form false. What every seo-only token is told is unchanged in substance: product tags remain the one thing its own tools can create. WRITE_NOTE needed no change here — #713 had already rephrased it from a blanket claim to a per-tool "THIS tool ... cannot create one", which is why no separate article variant exists. A fork carrying a whole-class copy of McpServerFactory silently ships without the blog tools even for a token granted content, and keeps the unscoped instruction text.
  • [4.123.0] Check for overrides: src/Mcp/container.php gained $services->set(Tools\BlogTools::class);. The public() default is load-bearing — the SDK resolves handler classes out of the container by class name at tool-call time, so a fork that copies this file and drops the entry gets a runtime -32603 on every blog tool call rather than a boot failure.
  • [4.123.0] Known functional limitation, not an omission: an article created through the connector has no categories and no tags, and no tool can assign them — Cms\Blog\Article\WriteService has no categories / tags branch at all (unlike the product WriteService). Storefront blog listings are reached through BlogCategory.articles, so a published article created this way may be unreachable on the storefront even though the connector reports success. CONTENT_INSTRUCTIONS states this outright and directs the operator to assign categories and tags in the admin blog screen. Teaching the domain to accept categories/tags is a separate change.
  • [4.123.0] banner_image is a filename the operator asserts already exists. MCP is JSON-RPC with no file transport, so there is no upload path; nothing — not the tool, not the domain, not Resource::formatFile() — verifies the file exists under files/blogs/. The schema description says so and directs the operator to copy an existing article's banner_image (returned by get_article) or to upload through the admin blog screen first.
  • [4.123.0] No migration and no build step: nothing in this change touches the schema, the REST contract, or any OA\* annotation, so the OpenAPI spec was deliberately not regenerated.
  • [4.123.0] Check for overrides:
    • A fork that overrode payByBankResponse() wholesale keeps its own ungated cancel arm and is STILL EXPOSED. application/modules/checkout/controllers/Checkout.php is the designated fork seam and is an empty upstream subclass, so such an override silently shadows this fix with no merge conflict — same shape as the #619 note on the PAID arm, now true of the cancel arm too.
    • payByBankReportsPaid() is byte-identical in name, signature and visibility — only its body changed, from doing the work itself to delegating to the new shared helper. A fork overriding it is unaffected. Calling this out because the method is one day old (it shipped with #619) and a fork operator reading a diff that touches it will want to know nothing about its contract moved.
    • The new payByBankReportsStatusIn() is protected, so a fork needing a third accepted-status set can reuse the shared guard rather than duplicating it. It was private in the approved design and was widened at review: every sibling gateway-validation predicate in this class is protected, and private closed no attack surface, since a fork can already override payByBankReportsPaid() or payByBankReportsCancelled() wholesale.
  • [4.123.0] afterOrderCancelHooks()'s trigger became trustworthy, not merely unchanged. It is an empty upstream client-override seam (plausibly an ERP cancel) that previously fired off a forged callback alone. It now fires only after the gateway has confirmed the cancellation. A fork hooking it gains this for free.
  • [4.123.0] UI Update: new endpoint GET /rest/promotion/gift/active and its (\w{2})/ locale-prefixed twin — a guest-readable, read-only projection of the gift rules a shopper can earn right now, so a headless storefront can render the "current gift offers" block the legacy homepage shows. Every other gift-rule endpoint is auth: backend, which is why that block could not be built at all. "Active" is the full legacy definition, which is three things and not one: inside the date window, active = 1, stock remaining, and at least one gift choice in stock — except rule type 13 (cheapest-free), whose pool is computed from the cart and is therefore exempt from the stock check. Every active rule type is returned; there is no type exclusion — types 8 and 9 (brands/products-and-minimum-value) are ordinary supported rules the legacy homepage already displays. The response carries exactly twenty fields per rule, always all twenty — everything the legacy slider renders, so the block can be built from this one call: id, ruleId, isPromo, image, giftImage, description, choices, amountFrom, amountTo, giftPerCount, url, promoImage, extraProductImage, extraVendorImage, requirementOptionType, requirementName, requirementImage, requirementSlug, giftName, giftSlug. Watch the image fields — the legacy views swap between them rather than falling back: image is the badge (gifts_mui.image, documented as such in the schema), giftImage is the gift picture (legacy's computed gift_image), and extraProductImage / extraVendorImage replace the gift and requirement pictures on the slide when set. requirementOptionType (1 = product, 2 = vendor) is load-bearing rather than metadata: it tells a client which asset path requirementImage belongs under, so the image is unusable without it. giftName is the alt text for giftImage and describes the same gift choice. Two optional filters: ?filter[product]=&lt;id> narrows to rules whose requirement pool includes that product, and ?filter[isPromo]=&lt;0|1> selects one side of the promo/normal split (the homepage slider renders 0). They compose; no other filter and no sort is accepted, and ?with=choices is still the only relation.
  • [4.123.0] Deliberate divergence from the legacy homepage, recorded so it is not read as a bug: a NULL date_start / date_end is treated as open-ended rather than as a non-match. Both columns are NULLable and legacy's bare comparison rejects NULL, so a rule saved with no window is invisible on every legacy surface. This endpoint therefore shows more than the homepage for that one row shape. A second, and the one most likely to be noticed: the four requirement fields come from the rule's lowest-id requirement, where legacy LEFT-joins gift_requirements and collapses under group_by gifts.id — so a rule with more than one requirement renders an arbitrary member on the legacy homepage, and can render a different one between two page loads. Here the same rule renders the same slide every time. Two smaller ones, same reasoning: a rule with no gifts_mui row for the request language is still returned (with all six caption fields null) where legacy's INNER JOIN drops it, and pagination.total counts the rules passing the date/flag window before the in-stock filter, so a page can carry fewer items than per_page. One more, on field shape rather than row selection: amountFrom, amountTo and giftPerCount are a deliberate widening beyond every legacy storefront surface — the legacy AdvCartResource::mapGiftRuleFieldsForJson() allow-list withholds all three and Adv_gifts_model::getActiveGiftRules() doesn't select them for the homepage either — because #646's acceptance criteria need the threshold on a PDP gift card (e.g. "spend over EUR 149"); this is rule configuration, not customer data, so it's a scope widening rather than a disclosure concern.
  • [4.123.0] Nothing existing changes. The admin GET/POST/DELETE /rest/promotion/gift endpoints, Promotion\Resources\Gift\Resource, and the Gift / GiftChoice / GiftRequirement policies all keep their backend auth and their ADMIN/MARKETING roles. No migration, no config key, no schema change.
  • [4.123.0] Check for overrides: no existing signature changed, but three things a fork should know about. A fork overriding Advisable\Rest\Promotion\Controllers\Gift or Advisable\Rest\Promotion\Resources\Gift\Resource is unaffected — the new surface is a separate ActiveGift controller, resource and domain namespace. A fork that maintains its own copy of application/config/rest_routes.php or rest_policies.php must merge the new route pair (rest/promotion/gift/active plus its locale twin) and the new ActiveGift::class policy entry, or the endpoint 404s / 401s in that fork: PolicyResolver falls back to the parent class's entry for an unlisted controller, which lands on the global auth: backend default. A fork wanting a different active-window rule, a different field set or a scoped gift pool overrides Advisable\Domains\Promotion\ActiveGift\Repository\Repository::rowInvariant() / ::now(), Advisable\Domains\Promotion\ActiveGift\Service::withInStockChoice() / ::decorate() / ::requestLanguage(). A fork that wants the requirement or gift display fields picked differently — legacy's arbitrary member, or a richer requirement over the lowest-id one — overrides Repository::giftImagesFor() / ::requirementDisplayFor(); the anchor rule itself is private pickOnePerGift() and is deliberately not a seam, because two different pick rules on one endpoint is what the determinism divergence exists to prevent. A fork can also re-point the Advisable\Domains\Promotion\ActiveGift\GiftChoiceStock DI alias — all protected/interface seams, registered in src/Domains/Promotion/container.php.
  • [4.123.0] Compiled DI container must be rebuilt (delete cache/container.php) — this PR adds six new registrations: the ActiveGift repository, MUI repository, repository configurator, LegacyGiftChoiceStock and the GiftChoiceStock alias, and the service, all in src/Domains/Promotion/container.php, plus the ActiveGift controller in src/Rest/Promotion/container.php (#646). A deploy that serves a stale compiled container has no ActiveGift::class registered; RouterDispatcher catches the resulting throw and returns 500 on every request to the new route. K8s rollouts get this for free (fresh pods); bare-metal deploys should clear the cache.
  • [4.123.0] Check for overrides: a client fork carrying its own dev.compose.yml, web-dev.compose.yml, or compose.sh for its dev/integration stack must apply the same three mount renames (php-fpm, php-cli, and the read-only nginx mount, all from ./.env.placeholder:/usr/local/var/www/html/.env to ./.env.container:/usr/local/var/www/html/.env) plus compose.sh's unconditional touch (it runs on every invocation), or it keeps the bug. It must also carry the .gitignore entry for .docker/integration/.env.container — renaming the mounts without it only converts the dirty tree from a modified tracked file into an untracked one, which is the same symptom reported in #692 wearing a different git status label. The bare .env rule matches that basename only, so the entry is required, and it is the step most likely to be dropped because .gitignore merges separately from the compose files. Nothing else in this repo mounts .env.placeholder, but a fork's copies of these files are invisible from here and won't be caught by this change.
  • [4.123.0] Existing checkouts get the new .docker/integration/.env.container file automatically on the next ./compose.sh up -d — the touch creates it before docker compose runs. Their prior container's mount of .env.placeholder is a stale inode until the containers are recreated.
  • [4.123.0] No REST contract, schema, or OpenAPI change. No composer install, no npm run all-production.
  • [4.123.0] Check for overrides:
    • Adv_base_controller::terminate() is a new protected method — additive, and per the #417 precedent an additive protected method cannot break a fork by itself. But a fork that copied the whole payByBankResponse() body into its own override keeps its own bare exit and simply does not gain this seam — it is not made safe by this change, only unaffected by it.
    • Response body on the unconfirmed/miss paths changed from EMPTY to OK. No PHP signature changed here, so this is a silent, non-fatal divergence, same shape as #760's paymerch/gen_tax_service note. Affected paths: a serial matching no order, a serial matching a non-paybybank order, an order that is no longer PENDING, and a PAID claim the gateway does not confirm — all four previously emitted nothing and now emit OK.
    • The one thing a fork operator most needs to check:application/modules/checkout/controllers/Checkout.php is the designated fork seam — an empty class Checkout extends Adv_checkout. A fork that instead overrode payByBankResponse() there silently shadows this fix: it will not merge-conflict, and it keeps the vulnerability this change closes.
    • A fork whose monitoring or gateway-side integration keys off the previously-empty response body on the four paths above will now see OK instead.
  • [4.123.0] REQUIRES php migrator.php migrate: two new migrations — 20260904120000_add_category_id_index_to_coupon_categories.php (adds the previously missing index on coupon_categories.category_id that the new delete now filters by; the guard keys on the column in leading position, not on the index name, so it is a no-op on a fork that already covers category_id under any index name — and still adds the index where only a non-leading composite covers it) and 20260904120100_clean_orphan_coupon_category_rows.php (runs CleanOrphanCouponCategoryRows, a one-time data sweep of the historical residue this defect left behind — expect it to delete rows on any environment with a nontrivial coupon history).
  • [4.123.0] Check for overrides: Product_category_model::delete_record() (application/modules/eshop/models/Product_category_model.php, extending Adv_product_category_model) — a fork that overrides delete_record() wholesale does not inherit this fix and keeps leaking coupon_categories rows on every category delete it makes, silently reproducing both the AND-path (coupon stops applying) and OR-path (coupon discounts the wrong category) failure modes above. The new protected string $couponCategories = 'coupon_categories' property on the parent also joins the fork override surface: a whole-class copy of Adv_product_category_model predating this change won't have it and needs to pick it up alongside the delete_record() body if the fork wants the fix.
  • [4.123.0] Check for overrides: Custom\Domains\Product\Category\Repository\WriteRepository::deleteDependentRows() — the modern seam, container-bound (src/Domains/Product/container.php:58,61). The table list is private const DEPENDENT_REFERENCES, read as self::, so a fork cannot extend it by redeclaring the constant the way InternalMoveFolder::GUARD_FILES is extended: it must either override deleteDependentRows() and call parent::deleteDependentRows($id) before deleting its own extra tables (safe — inherits this fix), or ship its own copy of the method or the class (does not inherit it, silently — the container still compiles and DELETE /rest/product/category/{id} still returns its usual response; only the orphan rows show it). A fork whose Custom\ WriteService::delete() overrides delete() without calling deleteDependentRows() at all is in the same position. A fork that has not touched src/Domains/Product/Category/ inherits the fix.
  • [4.123.0] Supersedes three [4.122.0] claims about the cleaned set; that file is shipped history and will not be edited to say so, so a fork operator following those notes forward must treat this fragment, not the released text, as current:
    • The legacy override note's count — "the transaction-wrapped seven-table delete (five dependents + _mui + the category row)" — is now an eight-table delete (six dependents + _mui + the category row).
    • The modern override note's "must keep the same five tables" is now six, the added one being coupon_categories.category_id.
    • The entry body's "coupon_categories.category_id is deliberately NOT cleaned", together with its instruction to "read the list above literally", is now false — that exclusion was #677's cross-module scope call and #694 is the follow-up it filed. This is the most misleading of the three for a fork deciding what its override must adopt, because it reads as a deliberate design decision rather than a count that drifted.
  • [4.123.0] UI Update: GET /rest/cms/blog/article?filter[title.{locale}]=&lt;term> — and its /item, /{id} and locale-prefixed twins — now returns partially-matched articles instead of HTTP 500. The filter key was whitelisted against blog_mui.name, a column blog_mui does not have (database/initial/initial.sql:207-226 — it is title), so every request carrying the key died with Error Number: 1054 Unknown column 'blog_mui.name' in 'WHERE'. A headless storefront reaching for blog search therefore had no working title filter at all, not a degraded one; the consumer that shipped against it is useBlogArticles({ title }) in @advisable-com/velora-storefront-core 1.22.2 (Advisable-com/velora#622). Nothing else on the endpoint moves: filter[slug.{locale}], filter[categorySlug.{locale}], filter[notEmptyContent.{locale}] and sort=title.{locale} were already declared against real columns — the sort half of the same file has always used blog_mui.title correctly, which is why a green suite said nothing about the filter half. The forced storefront published scope from #624 still applies, so an unpublished article whose title matches the term stays hidden.
  • [4.123.0] Check for overrides: Advisable\Domains\Cms\Blog\Article\ListRequest::setAllowedFilters() — no signature change, but a fork that overrode this protected method to add its own filter keys carries its own copy of the title.{locale} declaration and keeps returning 500 until it applies the same one-word change (blog_mui.name → blog_mui.title).
  • [4.123.0] Check for overrides: no PHP signature changed, but the value contract on two columns did. shop_order.paymerch is now written by the REST checkout as 'invoice'/'receipt' (previously always NULL), and shop_order.gen_tax_service is no longer written by it at all (previously the truncated in/re). Every consumer below tests paymerch for equality with 'invoice' and falls through on anything else, so a NULL previously read as receipt everywhere — a fork that overrode any of these on the assumption paymerch is always NULL for REST-placed orders needs to reconcile. This is not hypothetical: v4-wecare is a named instance — this issue's reporter, and the holder of a local patch for this same defect (docs/handoff/patches/202-invoice-flag-paymerch.patch, per the issue report) — so that patch needs reconciling against this upstream fix on the fork's next upstream sync, not assumed compatible. Any other fork carrying a similar override should check the same list:
    • Printed fiscal document — application/views/admin/orders/invoice_body.php:40, application/views/admin/orders/invoice.php:52
    • IQVIA upload — ecommercen/iqvia/libraries/AdvIqviaUpload.php:227
    • Europharmacy — ecommercen/job/libraries/AdvPostOrdersEuropharmacy.php:129
    • Farmakon — ecommercen/libraries/Adv_Farmakon.php:215
    • Admin order view — application/views/admin/orders/update.php:105
    • Admin filter — application/views/admin/orders/list.php:265-266
    • Customer order views — application/views/main/components/customer/order_item.php:290, application/views/main/layouts/customer/order_track.php:179
    • REST API — src/Rest/Order/Resources/Order/Resource.php:178
  • [4.123.0] UI Update: REST receipt orders stop persisting submitted afm/doy/company/profession/ company_address — those columns are now written as the empty string on the receipt branch, matching what the legacy checkout has always done (Adv_order_model::setUpAdminOrderDataReceiptInvoice()). This is data the platform currently keeps and will no longer keep: anyone reading a receipt order's tax fields — the admin invoice views, ERP exports, or the Order REST resource — will see the empty string instead of the customer's input from now on. This is intended legacy parity, not a regression, but it is the change most likely to generate a support question, so flagging it here rather than leaving it as a quiet internal note.
  • [4.123.0] Request contract: the place-order endpoint now additionally accepts company_address (and its companyAddress camelCase alias, which takes precedence when a request sends both). Additive — no existing request is invalidated. It is deliberately not declared in the endpoint's OpenAPI request schema, which already omits five sibling fields (wantsInvoice, afm, doy, company, profession) — a pre-existing documentation gap, not something this fix introduces or was meant to close.
  • [4.123.0] UI Update — BREAKING, unlike every note above. POST /rest/checkout/place-order now refuses an order where wantsInvoice is true and any of afm, doy, company, profession or companyAddress is empty after trimming — a whitespace-only value is refused exactly like an absent one, matching legacy's own rule (each field trim|required when the choice is invoice, ecommercen/eshop/controllers/Adv_order.php:543-549). The response is 422 with error.code = invoice_identity_incomplete and error.missingFields — an array naming the offending request keys — so a client can act on it without parsing the message text. Who has to do something: any client posting wantsInvoice: true without all five fields — which the NULL-paymerch bug this same fragment fixes made silently succeed — must start sending them, or start handling the 422. A client that only places receipt orders is unaffected. Note that missingFields names request-field keys (companyAddress), not column names (company_address) — a client maps them straight onto its own form — which means the error can name a key the endpoint's OpenAPI request schema does not declare either: the same pre-existing gap the Request contract note above already describes for company_address and its four siblings, seen from the error-response side rather than a second, separate gap.
  • [4.123.0] UI Update: REST response-contract change on POST rest/checkout/place-order. paymentRedirect.params.cancelUrl on alpha/eurobank stops echoing the client's submitted value and becomes a site_url(...) value, exactly as confirmUrl did in #728; CardLinkAdapter's SHA-256 digest changes with it, since cancelUrl is one of its hash inputs, so a client that recomputed or hard-coded the previous digest is affected. ethniki_ee's response shape does not change at all — its failure leg is held (see What still does NOT work), so no errorUrl field and no advOrderSerial parameter ship, and paymentRedirect.params remains exactly the MPGS Checkout.configure() payload it has always been.
  • [4.123.0] Check for overrides:
    • PaymentCallbackUrlBuilder — a fork subclassing the old signature gets a loud fatal on merge. The class is container-registered (src/Domains/Checkout/container.php:64) precisely as the client-override seam, so this is self-announcing rather than silent.
    • PaymentContext::$failureUrl — optional, and therefore NOT a fork fatal, unlike the two required params #728 added. The consequence a fork owner needs to know: a fork that overrides PlaceOrderService rather than the builder gets cancelUrl => '' on CardLink rather than a fatal, i.e. it silently keeps the old behaviour. There is deliberately no ?: $context->cancelUrl fallback, because that would re-open the #728 defect class.
    • EthnikiEEAdapter and PaymentRedirect — nothing to check. An earlier revision of this branch added an errorUrl field to PaymentRedirect and emitted it from this adapter; both were removed when the ethniki_ee failure leg was held, so neither class changes its constructor signature or its serialised shape. A fork overriding either is unaffected.
  • [4.123.0] Check for overrides: three protected seams in Adv_vendors_admin.php change shape but not signature (no LSP fatal for a fork that overrides them):
    • applyVendorMuiValidationRules() now registers 10 set_rules_mui rules per language instead of 7 when the metatags permission is granted. A fork overriding this method loses trim normalisation and set_value() repopulation of the three meta inputs on a validation bounce — but not persistence, which is a separate seam — so such a fork renders and saves meta but silently discards typed-but-unsaved meta on a bounce.
    • getAddVendorDataMuiPost() / getEditVendorDataMuiPost() — the per-language inner array goes from a fixed 11 keys to 11-or-14, conditional on the same permission gate, so it is no longer a fixed-arity record. A fork overriding either method loses meta persistence entirely while the form still renders the inputs — the highest-consequence silent loss in this change.
    • beforeAddEntityRecord() / beforeEditEntityRecord() receive the MUI data array as-is; a fork hook that rebuilds rather than mutates that array will drop the new keys too.
  • [4.123.0] UI Update: application/views/admin/vendors/create.php and update.php are whole-file merge surfaces — a fork carries its own copies and inherits nothing from this change, so a fork that takes the controller half but keeps its own views ends up gating and persisting meta with no UI to enter it. A fork hand-porting the panel must use vendor-seo-collapsible for the heading class and vendor-seo-content for the panel's inner id — not the product-category panel's .collapsible / ceo-content pair. application/views/admin/footer_js.php:4294-4308 binds a click handler to every .collapsible but toggles a single hardcoded getElementById('ceo-content'), and the vendor views already spend that id on their banners panel; copying the product-category panel verbatim reproduces that id collision and leaves the SEO panel permanently hidden with its inputs unreachable. The toggle is deliberately delegated from document and placed outside the #AiContentGeneration Vue root, since that bundle mounts at footer_js.php:2991 (after the view body) and would otherwise discard an in-body listener bound before it. One accepted cosmetic trade: because the heading can't carry .collapsible, it doesn't get that class's gradient or rotating-arrow indicator (assets/admin/scss/admin.scss:1376-1396) and instead renders as a solid btn btn-primary with no arrow — the platform-wide single-id limitation in footer_js.php is left in place and out of scope here.
  • [4.123.0] Fields are gated on the metatags permission (canAccess() → [AUTH_ROLE_ADVISABLE, AUTH_ROLE_ADMIN]) in both the views and the two POST-payload hooks, so a user without it neither sees the inputs nor can persist them via a crafted POST. Because updateOrInsertMui() performs a partial UPDATE over only the supplied keys, such a user's save leaves any REST/MCP-set meta values intact instead of blanking them.

No deployment action needed — no migration, no composer install, no npm build (these are server-rendered CI3 templates; nothing under public/ui/** changes).

  • [4.123.0] UI Update: on invalid UTF-8 input, xmlSanitise() now returns '' instead of fatalling — the affected field ships empty and the feed still generates, rather than the whole export 500ing. This was a deliberate choice over scrubbing with mb_convert_encoding($string, 'UTF-8', 'UTF-8'), which would have preserved the valid remainder of the string instead of emptying it; the minimal fallback was chosen to restore the feed without changing output for input that already worked. Net effect for a feed consumer: a product whose description contains invalid UTF-8 now ships with an empty g:description (or equivalent field) rather than the feed failing to generate at all — an empty field can get a Google Shopping item disapproved, where before this fix the whole feed was down for every item. See docs/decisions/607-xmlsanitise-null-fatal.md for the rejected alternative.
  • [4.123.0] xmlSanitise() (application/helpers/MY_text_helper.php:390) is wrapped in if (! function_exists('xmlSanitise')). The parameter widening itself is source-compatible — contravariant, so no existing caller needs a change, and the (string) casts already at AdvAgoraCatalog.php:159-161 stay valid. A scan across all 34 local fork checkouts found no fork declaring its own xmlSanitise() in a separate, earlier-loaded helper — that override seam exists but is currently unexercised. What the scan did find: 18 forks carry the inherited application/helpers/MY_text_helper.php. 17 of them are still on the original strict xmlSanitise(string $string): string and take this fix cleanly on their next upstream sync — no action needed. Megastore has diverged: its copy already reads xmlSanitise(string|null $string): string, with the preg_replace() return still unguarded — that local patch fixes only the NULL-argument outage, the invalid-UTF-8 fatal stays armed, and passing null straight into preg_replace() raises a PHP 8.1 deprecation on top. Megastore will hit a merge conflict on that line at its next upstream sync; the correct resolution is to take upstream, which is strictly more complete than its local patch. Megastore also declares xmlSanitise as a private function xmlSanitise(string $s): string on two feed controllers — Facebook_catalog.php:16 and Google_catalog.php:91 — each with the original strict signature and the same unguarded preg_replace() body, so both fatals stay fully intact there. That's a different mechanism from the helper-file divergence above: a private method never collides with the global helper, so this fix can't reach it and no redeclare error will ever surface it. Where the helper-file divergence announces itself as a merge conflict, this bypass announces nothing — those two feeds on that fork stay broken silently after this ships, and need a separate manual fix in that fork. The reported outage client (easypharmacy) also carries a client-local patch per the issue; it isn't among the local checkouts so its exact shape wasn't verified, but it should likewise drop its patch in favour of upstream on next sync.
  • [4.123.0] The defect. myTasks() and tasksTo() both called create_admin_pagination() with the literal '/auth/tasks' as the base URI (Adv_auth.php:252, :291), instead of '/auth/myTasks' / '/auth/tasksTo'. tasks() itself already passed the correct literal (:213) and is unchanged. Two independent failure modes followed, split by role: tasks() is supervisor-gated (allowRole([AUTH_ROLE_ADVISABLE, AUTH_ROLE_ADMIN], ...) → error_401(), Adv_auth.php:200-202), while myTasks/tasksTo carry no such gate and are open to every admin role by design ('roles' => [] at application/config/admin_menu.php:375 and :382). So for the 7 non-supervisor roles (CMS, PRODUCTS, ORDERS, REPORTING, MARKETING, MEDIA, DEVELOPER), clicking page 2 on "My tasks" or "Tasks I created" hit the gate and returned error_401() — those admins could not page their own list past page 1. For the 2 supervisor roles (ADVISABLE, ADMIN), the link resolved fine but landed on tasks()'s handler instead, which loads the unscoped all-tasks list rather than the assignee-/creator-scoped one — so a supervisor clicking page 2 got no error and silently paged through everyone's tasks instead of their own, with no visual cue that the list had changed shape.
  • [4.123.0] The fix. myTasks()'s create_admin_pagination() call now passes '/auth/myTasks'; tasksTo()'s now passes '/auth/tasksTo'. tasks() is untouched — its '/auth/tasks' literal was already correct. No other change: create_admin_pagination()'s signature, the allowRole() gate on tasks(), and the absence of a gate on myTasks/tasksTo are all unchanged and out of scope here. Rejected alternative (recorded in docs/decisions/751-74-task-list-pagination-and-countall.md): extracting the three route literals onto a shared constant or helper — that restructuring belongs to the pending #311 task-management refactor and would collide with it if done here.
  • [4.123.0] UI Update: on "My tasks" and "Tasks I created" with more than one page, page-number links now stay on the screen you are reading — auth/myTasks/&lt;offset> and auth/tasksTo/&lt;offset> respectively — instead of both pointing at auth/tasks/&lt;offset>. Concretely: a non-supervisor admin can now page past page 1 of their own lists instead of hitting a 401; a supervisor's page-2+ links on those two screens now show their own scoped tasks instead of silently switching to the unscoped all-tasks list.
  • [4.123.0] Tag DELETION is deliberately NOT implemented, and this is the recorded reason.DELETE /rest/product/tag/{id} already orphans pivot rows today: shop_product_product_tags has no foreign key and no cascade, and Tag\Tag\WriteService::delete() has no in-use guard and no pivot cleanup — where the legacy Adv_product_tags_model::delete() refuses the delete outright. A delete_product_tag tool would therefore inherit a live defect and hand an LLM a one-call way to silently orphan rows. Fixing the endpoint first (a Tag\LegacyDeletionRule + validateForDelete(), the shape Category\LegacyDeletionRule already established) turns a live 200 into a 422 on a routed, backend-writable endpoint — a breaking REST change that owes its own issue and its own triage, and does not belong inside a purely additive connector feature.
  • [4.123.0] Scope: the new tools take the existing seo scope — no Scopes.php change, no token rotation.seo already means "the catalog/SEO tool set (read + write)" and all existing write tools live in it; there is no per-tool scope annotation in this codebase, so a tool's scope is the register*Tools() method it sits in. Every existing token gains these tools on deploy. A new content-write scope was rejected for now because Scopes::parse() defaults every existing token to [SEO], so introducing one would require every shop to rotate its token to get tag tools, and it would auto-render an admin checkbox needing a new locale key in 8 language files. Cross-reference #352 (per-capability token scoping: read / content-write / pricing-write) — that is where the scope boundaries get redrawn once, for the whole surface, rather than #713 inventing one boundary unilaterally.
  • [4.123.0] Check for overrides: Advisable\Domains\Product\Product\WriteService::__construct() gains a required 9th constructor argument, TagPivotWriteRepository $tagPivotWriteRepository. A client fork that extends Product\WriteService and declares its own constructor — or that constructs the class directly rather than resolving it from the container — breaks on this: the parent constructor now needs an argument the override does not pass. Autowired resolutions are unaffected (the repository is registered in src/Domains/Product/container.php). This mirrors how CategoryPivotWriteRepository was added as the 8th argument; a nullable trailing parameter with a di() fallback was considered and rejected as a divergence from that precedent, so the breakage is surfaced here instead.
  • [4.123.0] Check for overrides: Advisable\Domains\Product\Product\Validator::__construct() gains a second argument, Tag\Tag\Repository\Repository $tagRepository (autowired, no container edit). Same caveat as above for a fork that subclasses or hand-constructs the validator.
  • [4.123.0] Check for overrides: Advisable\Mcp\Tools\ProductTools::updateProduct() gains a 9th parameter, ?array $tag_ids = null, appended last. Unlike the two constructor cases above, this one is a public method signature, so a client fork overriding it against the old 8-parameter signature does not wait for something to construct or call the class — it fatals at class load under PHP's LSP compatibility check. The fix is the same either way: add the trailing parameter to the override. There is no established override pattern for MCP tool classes yet, so whether any fork actually has one is unknown — this note exists because the surface is now breakable, not because a break is known to exist.
  • [4.123.0] Check for overrides: src/Domains/Product/container.php gains one line — $services->set(Product\Repository\TagPivotWriteRepository::class); (currently line 49). This is the case most likely to bite, because unlike the three above it needs no subclass and no hand-construction: every client fork carries its own copy of that container file and those copies have already diverged from upstream, so a merge can take WriteService.php and still drop or conflict away that single line. The result compiles fine under PHP — nothing references the class by name at parse time — and only fails when Symfony resolves the service, so the symptom is a container resolution failure (php cli.php job/check-container exits non-zero) plus a 500 on every REST and MCP product write, not a fatal at load. After merging, run check-container and confirm the line is present.
  • [4.123.0] The tags write key is a REST contract change too, not only an MCP one. Every caller of POST /rest/product/product and POST /rest/product/product/{id} gains a tags key (alias tag_ids) the moment Product\WriteService accepts one, with the same four states as above — absent (untouched) / [] (clears) / [ids] (replaces) / explicit null or any non-array value (also clears, because it is coalesced to an empty set), identical to categories. The write key is documented on Product\WriteData's #[OA\Schema], and the pre-existing but undocumented categories key was documented alongside it in the same change rather than shipping a second silent write key.
  • [4.123.0] list_product_tags has no tag_cat_id filter. Tag\Tag\ListRequest allows exactly id, priority, name.{locale} and slug.{locale}, and GenerateListRequest silently skips an unknown filter key — an unsupported filter would return 200 with unfiltered rows, which is a worse failure than an error. Adding one is simultaneously a REST contract change (GET /rest/product/tag?filter[tagCatId]=) and is out of scope here; the model narrows by tag category via list_product_tag_categories plus each row's embedded tag_category.
  • [4.123.0] No database migration and no schema change: the pivot table and both tag tables already exist.
  • [4.123.0] UI Update: REST response contract and status change on POST rest/checkout/place-order. A failed gateway initialisation changes from 400 → 502 on ten payways (apcopay, ethniki_ee, ethniki_nbgpay, iris, jcc, klarna_payments, paypal, paypaladvanced, vivawallet, xpay) and from a generic 500 → 502 on three more — piraeus and paybybank (the unhandled TypeError) and stripe (a vendor exception nothing converted). Thirteen payways in total, and any future one: the conversion catches whatever an adapter raises rather than an enumerated list of exception types. The 502 response carries the usual {success, message} pair plus a new error.code = payment_initialization_failed and error.payway. The previous 400 was wrong — this is a status correction, not new behaviour, and it does not mean the order was rolled back: the order survives PENDING with its basket, coupon and points already committed and the cart already cleared. A client branching on 400 for a gateway failure on any of the ten payways above must update that branch; a client that assumed a 500/400 meant nothing happened must not carry that assumption forward. A retry also needs a fresh coupon and does not get the spent points back. No new config key, no new route, no schema change.
  • [4.123.0] Check for overrides:
    • Loud case — a Custom\ adapter still declaring ?PaymentInitResult fatals at class load on merge. PHP return types are covariant, so a nullable implementation is invalid against the now non-nullable PaymentAdapterInterface::initializePayment(). Self-announcing: the shop fails to boot rather than mis-behaving quietly. The remedy is dropping the ? and replacing any return null with a PaymentInitializationFailedException throw.
    • Silent case — the dangerous one. PaymentInitializer is DI-registered by id (src/Domains/Checkout/container.php:65), so a fork that aliases it in its own container keeps the old, unconverted behaviour after merging this fix — a routine gateway failure still raises an uncaught TypeError for that fork's two null-returning adapters, or still answers a bare 400 for the other ten. There is no signature conflict and no merge conflict to surface this — a signature-only override note would miss it entirely. Such a fork must port the chokepoint conversion by hand into its own PaymentInitializer alias.
  • [4.123.0] UI Update: REST response contract change on POST rest/checkout/place-order. paymentRedirect.params.confirmUrl (alpha, eurobank) and params['data-redirect-url'] (ethniki) stop echoing the submitted returnUrl and become site_url(...) values on this deployment; klarna_payments' merchant_urls.confirmation and .notification likewise become site_url(...) values. alpha/eurobank additionally get a new digest, since CardLinkAdapter's hash is computed over confirmUrl among its inputs — a stored/precomputed digest from before this change no longer validates. A client integration relying on receiving the gateway's result callback on its own submitted returnUrl for any of these six payways must stop relying on that and instead expect the confirmation on this deployment.
  • [4.123.0] Check for overrides:
    • DI lane. PaymentContext::__construct() gained two required string params and PlaceOrderService::__construct() gained one (the new builder). A fork that constructs either directly gets a loud fatal on merge — self-announcing, nothing to check for. The dangerous case is silent: a fork that aliases PlaceOrderService in its own container keeps building one-URL PaymentContexts after merging this fix, with no signature conflict and no merge conflict — its gateways keep posting results to the fork's client page and it silently keeps the original defect. Advisable\Domains\Checkout\Payment\PaymentCallbackUrlBuilder is the intended override point instead — it's container-registered by id (src/Domains/Checkout/container.php) precisely so a fork can alias just that one class rather than the whole checkout orchestrator.
    • application/** lane. The new callbacks target checkout/get_response/{payway}/..., handled by Adv_checkout::get_response(). A fork whose shadowing application/** copy of Adv_checkout lacks an arm for one of the six payways answers error_401() instead of confirming the order. application/config/rest_api_versions.php is fork-shadowable too, so a fork tracking its own REST changelog there won't pick up the '1.X' entry this change adds automatically.
  • [4.123.0] UI Update: on a multi-language install, the gift-rule sliders (application/views/main/components/sliders/gift_rules_slider.php and gift_rules_promo_slider.php) can now render an empty requirement_name label on the vendor-requirement path (option_type = 2), and — on both that path and the product-requirement path (option_type = 1) — a requirement_slug that resolves to NULL and links to the vendor index instead of a specific vendor page: on the vendor path via the newly-scoped unaliased join, on the product path because the sop_vendor_mui alias feeds CONCAT(sop_vendor_mui.vendor_slug, '/', shop_product_mui.slug), which an untranslated vendor NULLs outright. This happens when a vendor has no shop_vendor_mui row in the active language — there is deliberately no fallback to a master-language row (recorded decision, docs/decisions/727-vendor-mui-lang-predicate.md). On the vendor-requirement path, that empty label also now trips a PHP deprecation on every render: strip_quotes() (system/helpers/string_helper.php:59-62) passes requirement_name straight into str_replace() with no null guard, logging Deprecated: str_replace(): Passing null to parameter #3 ($subject) at the six call sites in gift_rules_slider.php:21,29,37 and gift_rules_promo_slider.php:30,38,46. This is disclosure, not a fix — the view-layer guard is deliberately deferred (recorded decision D3, same doc). Previously the two shop_vendor_mui joins in this query carried no lang predicate at all, so requirement_name/requirement_slug resolved to an arbitrary language's row; the slug half of that was already a broken link in practice, since the vendor page resolves by slug and language (ecommercen/eshop/controllers/Adv_vendors.php:229,445,697) and a wrong-language slug already 404'd. What changes here is that a wrong-language name can no longer leak into the label, at the cost of an empty label where the vendor isn't translated into the active language. This is latent on the shipped single-language config (application/config/languages.php:8 ships only 'el' => 'greek') and only bites multi-language clients.
  • [4.123.0] Check for overrides: Gifts_model::getActiveGiftRules() — no signature change, but the query shape changed: both shop_vendor_mui joins now carry `lang` = '{$this->languageAbbr}', matching the joins in the same query already scoped to the active language — the unaliased join feeds both requirement_name/requirement_slug on the vendor-requirement path (option_type = 2); the sop_vendor_mui alias feeds only the vendor half of requirement_slug on the product-requirement path (option_type = 1). Both remain LEFT joins. Note the multi-requirement fan-out (the separate LEFT JOIN gift_requirements, one row per option_type_id) is untouched by this fix and still picks arbitrarily among a gift's requirements — the language nondeterminism is now resolved on both paths' affected columns (the vendor-requirement label and the vendor half of the product-requirement slug); the multi-requirement fan-out is the nondeterminism that remains.
  • [4.123.0] UI Update: GET/POST/DELETE /rest/product/product-code now requires AUTH_ROLE_ADMIN or AUTH_ROLE_PRODUCTS (matching its sibling ProductCodeAttribute), and GET/POST/DELETE /rest/seo/custom-meta-tag now requires AUTH_ROLE_ADMIN or AUTH_ROLE_CMS (matching its sibling DefaultMetaTag) — both on every routed verb (index/show/item plus the store/update/destroy writes), not only the reads. Previously neither controller had an entry in application/config/rest_policies.php, so PolicyResolver fell through to the platform-wide global default (auth => backend, roles => []) and any authenticated backend user of any role could read and write both. A backend caller authenticated with a role outside the pinned set now receives 403 Forbidden where it previously received 200. Guest/customer traffic is unaffected — both controllers were already backend-only in practice.
  • [4.123.0] Check for overrides: PolicyResolver::__construct() does a bare require APPPATH . 'config/rest_policies.php' — there is no merge layer over a client fork's own copy of that file. A fork that (a) calls either endpoint from a role outside [AUTH_ROLE_ADMIN, AUTH_ROLE_PRODUCTS] / [AUTH_ROLE_ADMIN, AUTH_ROLE_CMS] will start getting 403 on next merge, or (b) maintains its own edits to the ProductCode::class / CustomMetaTag::class region of rest_policies.php should expect a merge conflict there and reconcile against the roles above rather than silently taking either side.
  • [4.123.0] UI Update: admin Payment settings → Viva Wallet gains a new External Source Code field (application/views/admin/settings/payment_settings.php).
  • [4.123.0] Check for overrides: Adv_settings.php saves VIVAWALLET.EXTERNAL_SOURCE_CODEunconditionally from $this->input->post('viva_external_source_code'), like every neighbouring Viva field. A client fork that overrides payment_settings.php and does not add this input will null the key on every settings save — which defeats even a direct database edit, since the next admin save wipes it again. The fork must add the input to its overridden view. Same hazard #674 documented for PIRAEUSBANK_EXTERNAL.
  • [4.123.0] Deployment action — fail-closed, not backward compatible by default. A deployment that upgrades without configuring the new key registers no vivawallet adapter for the external channel: vivawallet silently disappears from GET /rest/checkout/payment-methods, and place-order refuses it with the existing 422 payway_not_available — nothing is logged, it is simply a missing payway. To restore it: (1) create a second source code in the Viva merchant portal with the external frontend's own success/failure URLs registered against it, and (2) save it in admin under Payment settings → Viva Wallet → External Source Code. There is no migration to run — the registry key is created on first save.
  • [4.123.0] UI Update: the admin order clone/edit screen now tells the admin when the source order it was supposed to cancel did not actually cancel. Adv_orders_admin::edit() (ecommercen/eshop/controllers/Adv_orders_admin.php:1187-1188) captures setBatchCanceled()'s per-serial result — previously discarded — and passes it to the new reportSourceOrderNotCanceledIfNeeded() (:2371), which flashes an error through the existing SESS_KEY_ESHOP_ERROR session channel (the same one reportRefusedLoyaltyRedemption() uses) when the source serial comes back in setBatchCanceled()'s error set — i.e. it was already CANCELED, a non-paybybank PENDING order, a PAID + paybybank order, or on one of eurobank, proxypay, paypal, alpha, ethniki, ethniki_ee, piraeus, apcopay. No view template changed — the flash renders through the existing admin error partial. Control flow is deliberately unchanged: the clone is still created either way; only the admin is now told.
  • [4.123.0] Check for overrides:
    • All three legacy signature changes below are FATAL at class-load time for an extends-based override that keeps the old signature — not silent. PHP enforces that a child cannot declare fewer required parameters, nor a narrower return type, than its parent. Measured on this codebase's PHP 8.1.13: Fatal error: Declaration of C::updateGifts(array $b): void must be compatible with P::updateGifts(array $b, int $id): void. A shop running such a fork fails to boot; it does not quietly mis-behave.
      • Adv_checkout::updateGifts() gained a required int $orderId parameter (ecommercen/checkout/controllers/Adv_checkout.php:2380, called from afterSuccess() at :2188).
      • Adv_gifts_model::updateCounter() changed return type void -> bool (ecommercen/eshop/models/Adv_gifts_model.php:1636). This is a well-established override point, so of the three this is the one most likely to actually hit a fork.
      • Adv_order_model::updateGiftCounters() gained a bool return type where it previously had none (ecommercen/eshop/models/Adv_order_model.php:1257). Same covariance mechanics as above; lower likelihood since it is a more obscure protected method, but structurally identical.
    • The genuinely silent case is different in kind: a whole-class copy, not an extends-based override. A fork carrying a whole-class copy of application/models/Adv_gifts_model.php that keeps the old void updateCounter() return type is picked up by CI3's classmap loader in place of the upstream class, which bypasses the language's compatibility check entirely — there is no fatal. Upstream Adv_checkout::updateGifts() then reads the null return as failure, no order is ever marked via markGiftsApplied(), and cancellation silently never restores anything for that shop — quietly reproducing the exact #688 bug with no error to surface it.
    • A whole-class replacement of Adv_order_basket_model (application/models/) makes the new getGiftQuantitiesByOrderSerial() fatal on every cancellation, not a silent no-op. The same is true of a replaced Adv_gifts_model and its new restoreCounter().
    • A Custom\ alias of either Order\WriteRepository or Gift\WriteRepository that does not extends the upstream class is fatal on the new methods it doesn't have.
    • A Custom\ subclass of DecrementGiftStockOnPaidListener with a 2-argument constructor fails DI — the upstream constructor gained a third dependency (Order\WriteRepository) to write the marker.
    • Gift\WriteRepository::decrementRemaining() changed its return type from void to bool. Any Custom\ subclass declaring the old : void signature is an LSP fatal.
    • Overrides of set_status(), of Adv_checkout::afterSuccess() / updateGifts(), or a forked copy of src/Domains/Order/container.php all degrade silently instead: the restore simply never fires and gifts leak exactly as before this fix — the safe direction, but worth checking for on the next merge.
    • A fifth break shape, different in kind from the four above — a missing language key, not a signature mismatch. The new key eshop.admin.order.error.source_not_canceled was added to all 8 shipped locales (english, greek, chinese, french, german, italian, russian, spanish). A fork carrying its own copy of adv_advisable_lang.php for any of those locales will not have the key, and t() on a missing key falls back to rendering the key itself, not an empty string (application/core/MY_Lang.php:192-193) — not a fatal either way. The admin sees the raw string eshop.admin.order.error.source_not_canceled in the error banner where the new "source order not cancelled" error text should be — a cosmetic break, but an obvious one the fork owner will notice the first time the branch fires.
  • [4.123.0] Historical orders are not backfilled. gifts_applied defaults to 0, so cancelling an order placed before this deploy restores nothing for it. This is deliberate, not an oversight — no ship-safe patcher can be written: the units are technically reconstructable from shop_order_basket.gift_id + qty, but nothing in the data distinguishes a decremented historical PENDING order (legacy PayByBank) from a never-decremented one (modern REST pre-confirm) on that status alone, and with no ceiling column on gifts an over-crediting backfill would be silent and permanent. Under-restoring is the accepted, safe direction. See the decision record for the rejected backfill design.
  • [4.123.0] Only CANCELED triggers the restore — RETURN does not. RETURN is not dormant — a live cron job writes it (AdvSyncOrdersStatusEuropharmacy::getOrderStatusID() maps vendor code 7 to it), but Adv_order_model::update_order() (ecommercen/eshop/models/Adv_order_model.php:951-953) only routes into set_status(..., 'CANCELED', ...) when the incoming status is CANCELED; a RETURN write just updates the status column and stops, so a returned order restores nothing today, product stock included. Making gift stock the sole exception would be inconsistent in the merchant's favour — that gap is real and knowingly out of scope here; see the decision record's "Correction — RETURN IS written" section.
  • [4.123.0] The pre-existing product-stock double-restore in set_status() is unaffected by this change. returnOrderStock()'s guard matches on order_serial alone with no idempotency check, so a repeat cancel still double-restores product_codes.stock today, exactly as before. Only the new gift-stock restore added by this fix is guarded and idempotent — do not read this change as having fixed the product-stock side too.
  • [4.123.0] A failed legacy restore is logged, not raised — no batch is aborted.Adv_order_model::restoreOrderGifts() (ecommercen/eshop/models/Adv_order_model.php:1025-1067) wraps the claim-then-restore sequence in trans_start()/trans_complete(); on a failed transaction it calls log_message('error', ...) and returns normally. The rollback has already re-armed gifts_applied by that point, so a later cancellation of the same order can still restore the units — the claim-then-restore sequence stays inside its transaction, only the failure handling differs from a throw. No exception propagates out of set_status(), so Adv_order_model::setBatchCanceled()'s batch loop (:3166-3220, the per-order set_status() call at :3214) is never aborted partway through, and none of set_status()'s other callers (AdvCancelIncompleteOrders, AdvApiKlarna, the PayByBank sweep, admin edit-order) sees an unhandled fatal from this path. This matches the modern path, where OrderEventDispatcher catches and swallows each listener's exceptions with a warning log — both paths now fail the same way: logged, recovered, and invisible to the caller.
  • [4.123.0] REQUIRES php migrator.php migrate:20260831120000_add_gifts_applied_to_shop_order.php — adds shop_order.gifts_applied (TINYINT(1) NOT NULL DEFAULT 0, AFTER points_added). Must land before deploying this fix; the restore code depends on the column existing.
  • [4.123.0] UI Update: the offers listing page changes behaviour in two ways. (1) An unresolvable category slug now returns 404 instead of a PHP fatal — and because the category lookup became language-aware, a slug whose offer_categories_mui row exists only in another language is now unresolvable where it previously matched. A shop whose offer categories are translated in only one language will see 404s on the others until the missing offer_categories_mui rows are added. (2) The already-published /ειδικεσ-προσφορεσ/ολεσ URL keeps listing all offers on every shop, whatever its configured language, via the new Adv_offers::LEGACY_ALL_OFFERS_SLUG alias. That alias is deliberately language-independent: application/config/routes.php registers offers with no (\w{2})/ language-prefixed variant, so that one Greek-spelt URL is the only "all offers" URL every storefront has. Do not remove it.
  • [4.123.0] New language key routes.offers_all: added to all eight shipped language files (ecommercen/language/*/adv_theme_lang.php) alongside the existing routes.offers. Greek keeps the value ολεσ. A client cannot shadow the platform adv_theme_lang.php itself — Adv_base_controller::loadFrontLangFiles() loads it with an explicit $alt_path, which takes application/core/MY_Lang.php's if ($alt_path !== '') branch and bypasses the package-path scan entirely — so the platform value is always present. A client that wants a different "all offers" segment redefines routes.offers_all (and, if needed, routes.offers) in its own {client_views}_theme / _user_theme language file, which is merged over the platform file.
  • [4.123.0] Check for overrides: Product_reviews_model::getProductReviews() — signature changed (($productId, $lang = 'el') → ($productId, ?string $lang = null)). The parameter is still not applied: the method returns reviews across every language exactly as before, so this is a signature clarification, not a behaviour fix — $lang = 'el' implied a Greek scoping the body never applied, which is what actually changed. A client override that still declares $lang = 'el' remains legal (PHP permits a child to redeclare a default) and keeps working.
  • [4.123.0] Check for overrides: Offers_model::getCategoryMuiGeneric() — signature changed (($where, $lang = 'el') → ($where, ?string $lang = null)) and behaviour changed: the $lang argument was previously declared and never applied, and the mui join carried no lang predicate at all. It now filters on the language, defaulting to the active one. A slug that only exists in another language stops resolving.
  • [4.123.0] Check for overrides: Product_category_model::categories_of_vendor() — signature changed (($vendor_id = 0, $lang = 'el') → ($vendor_id = 0, ?string $lang = null)).
  • [4.123.0] Check for overrides: Product_category_model::getVendorsCategoriesForSiteMap() — no signature change, but the query shape changed: both MUI joins are now bound parameters carrying the active language instead of the literal 'el'.
  • [4.123.0] Check for overrides: Video_model::getAllVideosAssignedToVendor() — signature changed (($vendorId, $lang = 'el') → ($vendorId, ?string $lang = null)).
  • [4.123.0] Check for overrides: Video_model::getAllVideosPaginated() — signature changed (($limit, $offset, $lang = 'el') → ($limit, $offset, ?string $lang = null)).
  • [4.123.0] Check for overrides: Waiting_list_model::getRecordsToSendEmails() — signature changed (($lang = 'el') → (?string $lang = null)). This method is one the flow docs actively instruct clients to override (docs/flows/system/SY-13-waiting-list-notifications.md, docs/flows/customer/CF-15-waiting-list.md).
  • [4.123.0] Check for overrides: Offers::index() — signature changed (($offerCategorySlug = 'ολεσ', $offset = 0) → ($offerCategorySlug = null, $offset = 0)). The sentinel comparison moved into a new protected isAllOffersSlug() (additive — a client override of index() that inlines the old != 'ολεσ' comparison keeps working, but will not pick up the per-language segment). The method also now calls show_404() when a category slug does not resolve, where it previously fatalled on a null dereference.
  • [4.123.0] Check for overrides: Cronjob::sendEmailForWaitingList() — signature changed (($lang = 'el') → (?string $lang = null)). CLI-only entry point; a client crontab invoking it with no argument now gets the configured language_abbr rather than Greek.
  • [4.123.0] Check for overrides — BREAKING for callers, unlike every note above.Advisable\Domains\Order\SalesAnalytics\Service::breakdown() and ::ordersList() lost their $lang default, which under PHP 8.1 promotes every preceding optional parameter to required: breakdown() goes from 3 required parameters to 7 ($dimension, $dateFrom, $dateTo, $sortBy, $limit, $compare, $lang) and ordersList() from 2 to 6 ($dateFrom, $dateTo, $status, $cursor, $limit, $lang). Every other note in this list preserves call compatibility; this one does not — a client caller relying on any of those defaults now gets an ArgumentCountError at runtime. Audit call sites, not just subclasses. Platform callers (src/Mcp/Tools/AnalyticsTools.php) already pass every argument.
  • [4.123.0] All four remaining $lang = 'el' defaults in the legacy layer resolve in-method via $lang ?? $this->languageAbbr ?? config_item('language_abbr'). Parameter order and arity are unchanged for categories_of_vendor, getAllVideosAssignedToVendor and getAllVideosPaginated, which are dispatched positionally by name through Pscache::model().
  • [4.123.0] Behaviour change: /robots.txt now returns 200 text/plain on image-deployed tenants, where it previously returned the CodeIgniter 404 page. Two defects stacked: the image docroot is populated by an allowlist matching only public/*.php (.docker/images/app.dockerfile:42), so the tracked, per-client public/robots.txt never reached the image; and even once present it would have returned 403, because root-level paths match neither the ^/(ui|files|favicon|media|sitemap)/ static location nor anything but the FastCGI location /, and PHP-FPM's security.limit_extensions default (.php .phar) rejects .txt — the same failure fixed for /sitemap/*.xml in 4.103. Fixing only the COPY would have converted the 404 into a 403. This activates the 4.104.0 faceted-navigation crawl directives (products-for, products-with, attributes, Disallow: /search), which had never taken effect on any Docker/K8s tenant. Plesk/Apache tenants are unaffected — public/.htaccess:30 already served the file through its RewriteCond %{REQUEST_FILENAME} !-f guard, which is why the bug was invisible there.
  • [4.123.0] Requires an image rebuild: inert until the tenant's app image is rebuilt. Tenants whose production nginx configuration is managed outside this repo also need the location = /robots.txt block mirrored there, or the file returns 403 once it is present.
  • [4.123.0] Check for overrides: forks carrying their own .docker/images/app.dockerfile should expect a conflict on the public/*.php COPY region and take both lines. Per #598 the statement sits at line 28 in v4-advisable-shop and line 26 in v4-dioptra. Forks with no .docker/ tree are Plesk-only and unaffected.
  • [4.123.0] Not fixed here: the same allowlist still drops every other tracked root-level static file (api-docs.html, api-versions.json, openapi*.json, favicon.ico, builder-assets/, captcha/, samples/, barcode/*.php). Scoped out deliberately; #598 records the wider set.
  • [4.123.0] The defect. Adv_tasks_model::countAll() (Adv_tasks_model.php:98-104) ran fixJoins() + fixWhere($where), then compiled SELECT * across tasks joined to users twice (creator and assignee), fetched every matching row, and returned $tasks->num_rows(). It is the only read path on this model that omits fixSelect(). getAll() at least applies LIMIT 20, so the count query backing pagination was heavier than the list it paginated — and every row it pulled, including both users.password columns, was discarded immediately after counting. This was never a disclosure issue: the rows never reach an output sink, only num_rows() is read.
  • [4.123.0] The fix. countAll() keeps both fixJoins() and fixWhere($where) — so the counted set stays structurally identical to the one getAll() pages over — and now returns $this->db->count_all_results($this->table) directly. count_all_results() honours the accumulated joins and where clauses (unlike count_all(), which ignores query-builder state entirely and would silently drop every $where filter) and compiles COUNT(*), so no columns are selected or materialised. The ($tasks && $tasks->num_rows()) ? ... : 0 guard is gone because count_all_results() already returns an int; the method's signature and return type are unchanged. Full rationale, including the rejected count_all() alternative, is recorded in docs/decisions/751-74-task-list-pagination-and-countall.md.
  • [4.123.0] The gain is removed row materialisation, not scan cost. tasks is indexed only on id, creator_id, assignee_id; the LIKE filters on title/description and the date-range filters stay unindexed either way, before and after this change. No index was added — that is a separate, unrequested change with its own migration and risk profile.
  • [4.123.0] Check for overrides: Adv_tasks_model::countAll() — no signature change (still countAll($where = []), still returns an int), but the emitted query changes: previously SELECT * against the joined tasks/users set followed by a PHP row count, now a single SELECT COUNT(*) against the same joins and where clause. A client override that calls parent::countAll() is unaffected; one that reimplements the method independently is untouched by this fix and keeps its own behaviour.