Appearance
<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>
Version 4
version 4.123
- [4.123.0] fix(transporters/speedex): correct 'succcess' key in transformTracking (Advisable-com/ecommercen#770)
Speedex::transformTracking()readfinalVoucherStatusCodes['succcess'](threecs) whileSpeedexConfig::initialize()defines the key as'success', so the misspelled subscript yieldednullandin_array($needle, null)raised aTypeErroron PHP 8. Every Speedex voucher carrying at least one checkpoint fataled — in-transit ones included — andAdv_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/<event>→webhooks/viva/handleWebhook) and applied no authenticity check of any kind. Its only gate was that the notification's ownEventData.OrderCodematched ashop_order.tran_ticketwithpayway = 'vivawallet'— a gateway-issued bearer identifier used as an authenticator, not a secret. An unauthenticated caller could therefore flip anyPENDINGvivawallet order toPAID(goods shipped) or toCANCELED, and on the gift-card arm triggeracceptGiftCard(), 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
transactionPaymentCreatedandtransactionFailed, 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_vaton the order arm,gift_card_orders.amounton 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 orderPENDING. 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 bareexits route through the inheritedAdv_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-
PENDINGstatus, and the gift-card arm still leaves the Pending-only claim to the model (#583) rather than deciding for it. src/VivaWallet/Base.phpgained a dead-catchfix and a return-type widening.catch (GuzzleException | Exception $e)referenced an unqualifiedExceptioninside namespaceAdvisable\VivaWallet, where no such class exists — so onlyGuzzleExceptionwas ever caught and any other throwable escapeddoRequest()as an uncontrolled 500. It is now\Exception. In the same method,doRequest()andhandleRequest()widened fromarrayto?array:nullnow 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.Baseis 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.
- The defect.
- [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:155mappedblog/blog_comments_admin/(.+)ontoblog/blog_comments_admin/index/$1, so every sub-path of the controller was rewritten into an argument forindex()rather than reaching its own method..../setStatus/1/1arrived asindex('setStatus', '1', '1'), andindex(int $offset = 0)raised an uncaughtTypeError—Adv_blog_comments_admin::index(): Argument #1 ($offset) must be of type int, string given— which surfaces as a bare 500.resetIndexandbatchActionwere 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 andindex()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 —
1approve,0pending,2reject (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.activeis atinyint(1)documented0 = inactive-pending, 1 = active, 2 = rejected. Butblog_comments.statusisenum('approved','pending','rejected'), andgetStatus()matched only the string literals, so every per-row click fell todefaultand 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 noblogCommentPendingtemplate and no outcome to announce for a neutral action. - A third divergence, found while writing the test.
getStatus()used aswitch, 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_charsallows 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_adminandseo/custom_metatagsin the same file:(:num)continues toindex/$offsetfor 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_mailergains aprotected blogCommentEmailOutcome()seam returning the same literal the database receives, ornullto 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 originalindex($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).
- The outage: approving a comment returned HTTP 500.
- [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.oktests 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_errormask minusE_USER_ERROR: that one type is reachable while a request is still alive (guzzlehttp/psr7raises it from a__toString()guard), so including it would abort mid-request and strand thepost_systemdeferred-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, sincepublic/index.phpsetserror_reporting(0)in bothtestingandproduction) andshow_exception()(a no-op in production, sincedisplay_errorsisOffthere, and a liability in any environment where it isn't). - Full design — the
E_USER_ERRORexclusion, the rejected alternatives, and the in-process test seam for the now-fatalexit(1)— is recorded indocs/decisions/543-exception-handler-status-500.md.
- The defect.
- [4.123.0] fix(checkout): stop marking an order
PAIDwhen NBG Simplify (ethniki) declined the card (#741)- The defect.
Adv_checkout::ethnikiResponse()verified the NBG Simplify MD5 signature and nothing else.NBGHelper::ethnikiValidateResponse()requirespaymentStatusto be present and folds it into the signature pre-image, but never compares it to anything — so a genuinely signedDECLINEDreturn 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.ethnikirecorded 0 hits in 7 days of legacyget_responsetraffic, so this was latent in practice and unconditional in code. - The fix.
ethnikiResponse()now checks the outcome after thenbg_simplify_loggingwrite and inside the existingPENDINGguard: anything other thanpaymentStatus === 'APPROVED'routes to a newethnikiResponseFail()instead ofethnikiResponseSuccess(). The predicate is an allowlist, not=== 'DECLINED'— NBG documents onlyAPPROVEDandDECLINEDand 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, '+')thenafterOrderCancelHooks()— thefalse, '+'stock arguments 17 of the 18 pre-existingcancelOrder()sites in this controller already pass, with theget_response:<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-minuteAdvCancelIncompleteOrderssweep. The customer sees a newcheckoutEthnikiFailview rather thaninactive_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 ownresultIndicatorvalidator and reaches the sharedethnikiResponseSuccess()by delegation, which is why the check sits inethnikiResponse()and not in the shared success method or inNBGHelper. Thenbg_simplify_loggingwrite still happens on both outcomes: it is the only placepaymentStatusis persisted, and a validator-level check would have exited above it. The non-PENDINGarm still renderspaymentsInactive. - 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 markedPAIDon aDECLINEDreturn are identifiable by joiningnbg_simplify_logging.referenceagainstshop_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.
- The defect.
- [4.123.0] feat(mcp): blog article tools — list, read, create and update articles (#714)
- Four MCP connector tools over the existing
Cms\Blog\Articledomain, in a newsrc/Mcp/Tools/BlogTools.php:list_articles(paginated id / title / slug /is_published/blog_dateplus a body-length indicator),get_article(one article in one language, including its full untruncated body),create_articleandupdate_article. All four go throughCms\Blog\Article\Service/WriteService— no hand-built queries — and every write is audited throughMcpAuditLogger.src/Domains/Cms/Blog/Article/**is unchanged. - A created article lands as a DRAFT unless
is_published: trueis passed.blog.is_publishedistinyint(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 andArticleVisibilityScopeare a REST controller / relation concern and are not on this path. update_articlenever hands one language's row to the domain.MuiWriteRepository::replaceForEntity()isDELETE FROM blog_mui WHERE blog_id = ?with nolangpredicate followed by a re-insert of only what was passed, so every write routes throughTranslationMerger::mergedPayload(), which rebuilds every language's row. That merge is also what carries the existingblog_mui.slug(NOT NULL, no default) through, sinceWriteService::update()never regenerates it — henceslugis not an exposed field.create_articlere-derives, explicitly, the three enforcements the update path inherits.HtmlSanitizer::sanitize()ondescription/small_description(AC 5),ToolResult::enforceLengths()on the meta columns, and non-empty checks ontitleandbanner_image— all three live insideMergesTranslations::diffFields(), which no create path in the repo reaches. It also refuses atitlewith no alphanumeric character (it would slugify to''and produce an unreachable URL) and ablog_datethat is not a realYYYY-MM-DD, and maps anullreturn fromWriteService::create()— a transaction failure, not "not found" — to a tool error.list_articlesexposes a partialtitlefilter, which depends on#579. That fix (already ondevelop) corrected the domain'sfilter[title.{locale}]mapping from the nonexistent columnblog_mui.nametoblog_mui.title; before it, any title filter was an unrecoverable SQL 1054, so this tool originally shipped without one.ListRequest.phpis 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 theUnitsuite rather than 500ing at call time. Articles remain findable byslug,category_slug,author_id,tag_idor a publication-date range, andsort=title.{locale}is exposed as before.- Tests live in
tests/Unit/Mcp/Tools/BlogToolsTest.php(theUnittestsuite), following the mocked-collaborator precedent of{Category,Vendor}ToolsTest.tests/Integration/Mcp/is in no testsuite (#526), soServerFactoryTestgains aBLOG_TOOLSpeer 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.
- Four MCP connector tools over the existing
- [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()'sCANCELLED/READY_TO_CANCELbranch (ecommercen/checkout/controllers/Adv_checkout.php) is #619's other half: #619 gated thePAIDarm against gateway confirmation and left this one untouched. (1) It applied no gateway confirmation at all — an unauthenticated caller could flip anyPENDINGpaybybank order toCANCELEDon 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 floorlessis_used - 1that 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 honouredREADY_TO_CANCEL— a reversible gateway state the gateway may still flip back toPAID— 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 orderCANCELLEDorCANCELLED_BY_MERCHANTbefore 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'sPAID-arm gate.READY_TO_CANCELis deliberately excluded from the accepted set, so a gateway-confirmedREADY_TO_CANCELcallback now leaves the orderPENDINGinstead of cancelling it. The refusal path reuses the existingacknowledgePayByBankCallback()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 newpayByBankReportsCancelled()are both thinprotectedpredicates now delegating to oneprotected payByBankReportsStatusIn(string $serial, array $accepted): boolholding 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 twinmarkCouponUnusedByCode()),returnOrderStock()'s missing idempotency latch (acknowledged in its own docblock), and the pre-validationinsertPBBLog()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 inlinePENDINGguard earlier in the handler already refuses it once the order isCANCELED. - Full design, the rejected alternatives, and the acceptance criteria are recorded in
docs/decisions/780-paybybank-cancel-arm-gate.md.
- The defect, two parts, same arm.
- [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_PREFIXinto a tracked file (Advisable-com/ecommercen#692)- The bug.
php migrator.php migrateagainst the dev/integration stack appended a generatedFILES_S3_PREFIXinto.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.phpwrote to the bare relative path'.env'(resolved against the process cwd);dev.compose.ymlandweb-dev.compose.ymlbind-mounted that tracked placeholder at/usr/local/var/www/html/.envread-write; and the guardisset($_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.placeholderfile") 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.ymlandweb-dev.compose.yml(php-fpm, php-cli, and the read-only nginx mount) now points at a new untracked.docker/integration/.env.container, created bycompose.shon every invocation via an unconditionaltouch— nevercp, becausetouchnever truncates, so aFILES_S3_PREFIXalready written survives every subsequent stack restart — and gitignored by its own.gitignoreentry — the existing bare.envrule matches only that exact basename, not.env.*siblings..env.placeholderstays tracked, stays empty, and is mounted by nothing; it is now only the documented reference for what the container's.envshould look like. The mounts stay read-write on purpose — a:romount would stop the dirty-tree problem but also stop the prefix ever being generated, which a real S3-backed install needs:FILES_S3_PREFIXis read atapplication/config/storage.phpfor 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.
- The bug.
- [4.123.0] fix(checkout): stop treating an unauthenticated PayByBank callback as proof of payment (#619)
- The defect.
Adv_checkout::payByBankResponse()is publicly reachable throughget_response()and applied no authenticity check of any kind. Its only gate was amerchantOrderIdmatched againstshop_order.order_serial— enumerable, not secret — so an unauthenticated caller could flip anyPENDINGorder toPAID(goods shipped) or toCANCELED(stock restored, coupon released, loyalty points returned — money out). Production ingress showed 152 hits to/checkout/get_response/paybybankover 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) thePAIDarm no longer trusts the callback body as evidence — it asks PayByBank itself viagetOrdersBySerial()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 longerPENDING, and aPAIDclaim 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, wrapsexit— 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
PAIDarm only; theCANCELLED/READY_TO_CANCELarm was left ungated, so a forged cancel of a genuinePENDINGpaybybank order still credited stock and returned the customer's loyalty points. That half is closed by #780, which ships in this same release — seedocs/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.
- The defect.
- [4.123.0] fix(coupons): delete
coupon_categoriesrows when their product category is deleted (Advisable-com/ecommercen#694)- The defect.
coupon_categories.category_idreferencesshop_product_category.id, and this schema has no foreign keys anywhere, so cleanup on category delete is application-level only. Nothing removedcoupon_categoriesrows when a product category was deleted — the last of eight references toshop_product_category.idleft 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_typeis0 => OR, 1 => AND. On the AND path a dead category id can never accumulate a count, soisValidCoupon()returnsfalseand 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, onceAUTO_INCREMENTreissues the id, the coupon silently discounts an unrelated category. - The fix, all three sites. Legacy:
Adv_product_category_model::delete_record()now deletescoupon_categoriesinside its existing transaction (six dependent tables, up from five). Modern:Product\Category\Repository\WriteRepository::DEPENDENT_REFERENCESgained the matching entry, soDELETE /rest/product/category/{id}cleans it too viaWriteService::delete()'stransactional()wrapper — no endpoint, response shape, or API version changed. Historical residue:patches/CleanOrphanCouponCategoryRows.phpsweeps 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.
- The defect.
- [4.123.0] fix(domains/cms): map the blog-article
title.{locale}filter toblog_mui.title, the column that exists (#579) - [4.123.0] fix(checkout): write the invoice/receipt choice to
shop_order.paymerchinstead of truncating it into the courier'sgen_tax_servicefield (#760) - [4.123.0] fix(checkout): clear the invoice identity fields on receipt orders and accept
company_addressat order creation, matching legacy parity (#760) - [4.123.0] feat(rest/checkout)!: refuse an invoice order (
wantsInvoice: true) that is missingafm/doy/company/profession/companyAddressafter 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 submittedreturnUrl. - The defect, failure leg (the mirror, and #728's amendment A3).
alphaandeurobankstill handed the gateway the client's owncancelUrlas 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 stayedPENDING, the coupon stayed consumed and the customer's redeemed loyalty points stayed debited until the 180-minuteAdvCancelIncompleteOrderssweep.ethniki_eewas 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'sreturnUrland appends it via a newcallbackUrl()helper; a newfailureUrl()method builds.../get_response/{payway}/failand appends the client'scancelUrlthe same way. The parameter name is published asCLIENT_RETURN_URL_PARAMso producer and consumer can't drift.Adv_checkoutgainsredirectToClientReturnUrl(), called last inalphaResponseSuccess(),eurobankResponseSuccess()and the sharedethnikiResponseSuccess()(success leg), and inalphaResponseFail()andeurobankResponseFail()(failure leg) — five methods, not eight, sinceethnikiandethniki_eealready funnel into one shared success method andethniki_ee's failure leg is held (see What still does NOT work). - The failure leg covers
alphaandeurobank; the success leg covers all four.ethniki_eeis success-leg only, likeethniki. - The write-then-redirect ordering is preserved, not replaced. The redirect is appended after each arm's existing terminal write —
PAIDon the success arms,CANCELEDplus the coupon release and points return (cancelOrder()) on the failure arms — never in place of any part of it. Each arm sets a303Locationheader instead of callingredirect(), whoseexitwould skippost_system, this app's onlyDeferredTaskRunner::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 markedPAID, four of themPRIORITY_CRITICAL, with noregister_shutdown_functionfallback anywhere in the app. - REST-only by construction, not by a per-order lookup.
PaymentCallbackUrlBuilder's only consumer isPlaceOrderService; 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.
ethnikigets no failure leg: that integration exposes no failure destination anywhere, andNBGHelper::ethnikiValidateResponse()checks only the signature, so a signature-valid declined return is markedPAIDtoday on the rendered storefront too — a separate money-correctness defect, tracked as #741, not attempted here.ethniki_eegets 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 behindgetExternalPayWays()(#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 failedethniki_eepayment still reaches no terminal status, leaving the orderPENDINGwith 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'scancelUrlis 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) andklarna_payments(#737) get neither leg; neither arm reaches a terminal order status yet.returnUrl/cancelUrlremain 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
PaymentRedirectvalue object, plus 30 legacy tests covering all three arms on both branches — including one that drivesplaceOrder()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 (persistedshop_ordercolumns,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 indocs/decisions/734-payway-onward-redirect.md.
- The defect, success leg. #728 correctly made the field a gateway posts its payment result to server-owned, but on four payways —
- [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 declaredxmlSanitise(string $string): string, but every feed controller feeds it a nullable DB column —shop_product_mui.descriptionismediumtext DEFAULT NULL,blog_mui.descriptionislongtext DEFAULT NULL— so a single product or blog post with aNULLdescription raised an uncaughtTypeErrorand 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/umodifier returnsnullwhen its subject is not valid UTF-8 — exactly the input this helper exists to clean — which against the declared: stringreturn type raised its own uncaughtTypeError. - The fix.
xmlSanitise()widens its parameter to?string, short-circuitsnulland''to'', and adds a?? ''fallback on thepreg_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.
- The defect — two independent fatals in one function.
- [4.123.0] fix(auth): point
myTasks()andtasksTo()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'stag_category{id, name}),list_product_tag_categories,create_product_tagandupdate_product_tag.list_product_tag_categoriesexists becausecreate_product_tagrequires atag_cat_idand 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_producttool, which gains atag_idsfull replacement set alongsidecategory_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 explicitnull(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": nullfor a field the caller never populated. This is not a divergence to fix here:Product\WriteService::parseTagIds()is a byte-identical mirror of the liveparseCategoryIds(), 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_getnow also return the product'stagsso the model can read the set before replacing it. The MCP layer never writesshop_product_product_tagsitself — it hands atagskey toProduct\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_tagsdeclaresproduct_tag_idas a plain compositeKEY, notUNIQUE, 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_tagis the firstcreate_*verb on the connector, so the server's own instructions were corrected in the same change.SEO_INSTRUCTIONSno 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 sharedWRITE_NOTEappended 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
namebygenerateMuiSlugs()→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 throughMuiWriteRepository::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_muihas exactlyid, tag_id, name, slug, content, lang— nometa_*columns at all — sohas_meta,meta_*_lengthandhas_complete_descriptionwould have to be invented over a facet label rather than read. Writes are audited throughMcpAuditLoggerlike 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 returnednullon failure.PaymentInitializer::initialize()was declared: PaymentInitResultwith no null guard, sonullraised aTypeError— whichTypeError extends \Errorput outside every armCheckout::placeOrder()catches.SafeActionDispatchcaught it as\Throwableand 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 bareRuntimeExceptionon failure, which answered400 Bad Request— wrongly blaming the client for an upstream failure. And a thirteenth payway failed a third way, found in review:StripeAdaptercalls\Stripe\Checkout\Session::create()with notry/catch, and\Stripe\Exception\ApiErrorExceptionextends\Exception— notRuntimeException— so a Stripe outage matched neither idiom and fell through toplaceOrder()'s generic\Exceptionarm as its own bare 500. - The fix.
PaymentAdapterInterface::initializePayment()is narrowed toPaymentInitResult(no more?). A newAdvisable\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\Exceptionan adapter throws — preserving the original as$previousso the log keeps the real gateway stack. The catch is\Exceptionand notRuntimeExceptionprecisely so thatstripeis covered, and it stops there on purpose:\Errorand its subclasses (aTypeError, a call on null — a bug of ours inside an adapter) are not converted and still surface as a 500, so a502always 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 threereturn nullsites 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 acatch (PaymentInitializationFailedException $e)arm returning the same{success, message}envelope aspayway_not_availableandloyalty_redemption_exceeds_order_total, pluserror.codeanderror.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 leftPENDINGin 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;422instead of502), and the GATE-1 decision on the HTTP status are recorded indocs/decisions/731-payment-init-null-contract.md.
- The defect.
- [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-suppliedPaymentContext::returnUrlto 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 stayedPENDINGindefinitely. - The fix. A new
Advisable\Domains\Checkout\Payment\PaymentCallbackUrlBuilder(src/Domains/Checkout/Payment/) maps payway + order serial to a server-owned confirmation URL and, forklarna_payments, a separate notification URL — both built withsite_url()behind the samefunction_exists('site_url')guard thexpay/apcopayfactory precedent already uses, so all 14 unit-testPaymentContextconstruction sites keep working with no CI3 boot.PlaceOrderServicenow depends on the builder (autowired, nocontainer.phparg needed) and passes its output into two new requiredPaymentContextparams,$confirmationUrland$notificationUrl, positioned right aftercancelUrl. Each of the five other adapters (CardLinkAdapter,EthnikiAdapter,EthnikiEEAdapter,KlarnaAdapter) stops assigningreturnUrlto the gateway-result field;irisinstead stops overridingsite_url('checkout/get_response/iris') . '?initiatingPartyRefId=...', whichIris.phpalready built and which carries the ref id the builder cannot reconstruct.CardLinkAdapter's SHA-256digestfollows automatically, since it hashes whateverconfirmUrlnow holds. - Fixes four of the six payways end-to-end;
irisandklarna_paymentsstill don't reachPAID. Each of those two carries a second, independent defect out of scope here: REST-placedirisorders get noiris_ordersrow at all, so both the cron recovery and the legacy return handler miss even with a correct callback; the Klarna adapter returnsPENDINGwhere legacy writesPAIDsynchronously, and the webhook designated as the real confirmation point writes no status. This change fully fixesalpha,eurobank,ethnikiandethniki_ee; foririsandklarna_paymentsit 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 legacycheckout/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 theirreturnUrl, 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/eurobankpayment, the server still learns nothing.cancelUrldeliberately still carries the client's URL — only the success-result field moved. Legacy sends.../alpha/failthere instead. This is milder than the success-path defect: the order sitsPENDINGand the existing 180-minuteAdvCancelIncompleteOrderssweep 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
matchinPlaceOrderService, a closure builder), and the two GATE-1 decisions (both new params required, the builder as a separate class) are recorded indocs/decisions/728-split-payment-confirmation-callback.md.
- The defect. Six REST payways —
- [4.123.0] fix(eshop): apply the active-language predicate to both
shop_vendor_muijoins ingetActiveGiftRules()(#727) - [4.123.0] fix(rest): pin
ProductCodeandCustomMetaTagto explicitrest_policies.phprole 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 withvivawalletfrom 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_CODEregistry key, read by a newgetVivaWalletExternalSettings()(ecommercen/helpers/registry_helper.php), now feedsPaymentInitializerFactory::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_CODEviaAdv_checkout. Gift-card purchases are also unaffected (still on the storefrontgift_card_source_code); an external gift-card source code is explicitly out of scope.
- 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
- [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, atOrderPaidon 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 ofremaining. Because the discovery predicate requiresremaining > 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 inAdv_order_model::set_status(), beside the existingreturnOrderStock()call — no call-site changes, since all legacy cancellation entry points already converge onset_status(). Modern: a newRestoreGiftStockOnCanceledListeneronOrderCanceled, registered alongside the existing coupon/points cancel listeners insrc/Domains/Order/container.php. - Gated on a new marker, not on order status. Neither
statusnoris_paidreliably distinguishes "this order's gift stock is currently consumed" — three separate writers decrement gifts under different status/payment combinations, and the two decremented-at-PENDINGlegacy PayByBank cohorts are swept by cron with no origin filter, which rules out any old-status inference inset_status(). A newshop_order.gifts_applied(TINYINT(1) NOT NULL DEFAULT 0, addedAFTER 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 onaffected_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()guardremaining IS NOT NULLin one atomicUPDATE ... SET remaining = remaining + ?, skipping unlimited gifts. Both cancellation paths claim the marker before reading the basket inside their own transaction: the modernRestoreGiftStockOnCanceledListenerand legacy'sAdv_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 aRuntimeException, which is safe becauseOrderEventDispatchercatches and swallows every listener's exceptions with a warning log. The legacy method no longer throws (#260 review): it callslog_message('error', ...)and returns normally, becauseset_status()has no dispatcher of its own around it — a throw there would abortAdv_order_model::setBatchCanceled()'s batch loop partway through and surface as an unhandled fatal to the other callers ofset_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 typevoid->bool:truemeans this gift's stock is correctly accounted for — the guardedUPDATEactually decremented it, or it's unlimited and had nothing to decrement —falsemeans exhausted, missing, or a non-positive quantity.DecrementGiftStockOnPaidListenerfilters basket lines withqty <= 0out of its per-gift aggregation before any of them reachdecrementRemaining(), so such a line cannot veto the marker for gifts on the same order that genuinely were decremented; it setsgifts_appliedonly when every decrement that was attempted comes backtrue(an order whose only gift line is qty-0 is still left unmarked, since nothing was attempted for it). The legacyupdateGiftCounters()mirrors this — it now skips entries whose count is<= 0before 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 —giftshas 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.
- The defect. Gift stock (
- [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_muionvendor_idrather than its unrelatedidPK, so a gift choice'sgift_slugcarries 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
$langon the sales-analytics breakdown and orders-list services instead of defaulting to Greek (#700) - [4.123.0] fix(docker): ship
public/robots.txtin 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 callingnum_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: arraystill declares, still instantiates and still runs — PHP raises nothing. What that fork loses is the distinction itself: its override can never returnnull, sounreachableis 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?arrayand decide what its owncatchreturns. There are no such overrides upstream.Advisable\VivaWallet\VivaWallet::verifyOrderIsPaid()and::vivaWalletValidateOrderIsPaid()return additional keys —found,unreachable,amountandorderCode, on the confirmed shape and on every unconfirmed one. Purely additive; all four upstream callers read by named key. A fork reading the result witharray_keys(),count()or a whole-array comparison would see the change.Advisable\VivaWallet\Base::httpClient()is a newprotectedmethod returning the Guzzle clientdoRequest()uses — additive, and per the #417 precedent an additiveprotectedmethod cannot break a fork by itself.AdvVivagained aprotectedvivaWallet()factory and threeprotectedpredicates (gatewayConfirms(),transactionBelongsToOrderCode(),amountMatches()). Additive. But a fork that overrodehandleWebhook()wholesale inapplication/controllers/webhooks/Viva.php— the designated fork seam, upstream an emptyclass 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.phpis unchanged, deliberately. The method discrimination lives inAdvViva.phpprecisely 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
blogCommentPendingtemplate 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 newblogCommentEmailOutcome()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.011and still runs the pre-refactor controller —index($filterType = 0, $offset = 0)callinggetFilterData(), with a numericswitch, its owngetAdminRecordsAndCount()model method, and alist.phpexpecting the oldrecords/records_total/filterSelectedLabelrender 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,approvedandrejected(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.phpentry. This is legacy admin only. The REST surface (/rest/cms/blog/comment) never had either defect:rest_routes.phpbinds each URI and HTTP verb to an explicit controller method, so nothing can be swallowed intoindex(), and the write path has always used theblog_comments.statusliterals. Itsstore/update/destroyactions 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 ofsendUserReviewStatusUpdateEmail(), so the gap is systemic rather than specific to comments);Domains\Cms\Blog\Comment\Validatoris empty, so withstricton => falseindatabase.phpa backend caller writing an invalidstatusgets''stored silently rather than a 422 — recoverable through the admin "All" filter, though it then displays as pending; andWriteData's OpenAPIstatusproperty documents "(approved/pending)", omittingrejected. Also unchanged:batchAction()sends no notification for any of its three values, androutes.php:263(\w{2})/video_showcase_admin/(.+)has the same catch-all shape as the route fixed here, thoughAdvVideoShowcaseAdmin::index($offset = 0)is untyped so it degrades silently instead of returning 500. - [4.123.0] Check for overrides:
application/helpers/log_helper.phpis 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 ownfunction_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 nolog_helper.phpat 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 inapplication/orecommercen/. 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 === falseon 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 anE_WARNING. This is not a second defect and not a loop: that warning re-enters_error_handler(), maps to'warning'in the severitymatch, gets logged, and returns, becauseE_WARNINGisn'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 ofwarning-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 ownsystem/core/Common.php:474-483sets the same 500 from the same shutdown-reachable path with noheaders_sent()guard either. - [4.123.0] UI Update: a new storefront layout key,
checkoutEthnikiFail, is now rendered on a declinedethnikireturn.application/config/mainTemplate.jsonalready carried the mapping (stubbed, with no view behind it); this ships the view for themaintemplate and adds both the key and the view for thedefaulttemplate. Both views reuse the existingcheckout.error.bank,checkout.alpha.fail.msg,checkout.error.retryandcheckout.backstrings, 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}->viewunguarded, so a template JSON withoutcheckoutEthnikiFailresolves to a null-property read and then a CI3 view-not-found fatal the first time anethnikipayment is declined. A fork carrying its ownapplication/config/template.json/mainTemplate.json, or its own template folder underapplication/views/, needs both the key and a matching view file. Adv_checkout::ethnikiResponse()changed behaviour without changing its signature, andAdv_checkout::ethnikiResponseFail()is a newprotectedmethod. A fork that copiedethnikiResponse()intoapplication/modules/checkout/controllers/Checkout.php— the designated fork seam, normally an emptyclass Checkout extends Adv_checkout— silently shadows this fix: it will not merge-conflict, and it keeps marking declined cards as paid.- A REST-placed
ethnikiorder that declines now renders this deployment's failure page rather than the client's.ethnikihas 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. Givingethnikia real failure leg remains #464/#734 territory.
- A fork with its own template config must add the key.
- [4.123.0] Existing connector tokens do NOT gain the blog tools. The tools are registered under a new
contentscope (Scopes::CONTENT, appended last inScopes::ALL), not underseo.Scopes::parse()drops unknown entries and treats a NULL/legacy row as[seo], so every already-issued token parses to a scope set withoutcontentand the four blog tools stay invisible to it — the same "legacy token keeps its exact pre-change surface" rule#403established for theanalyticsscope. To use them, generate a new token — or rotate an existing one — with thecontentcheckbox ticked, in Settings → MCP Connector. This is deliberate: wideningseowould have granted content authoring and record creation to every token ever issued for SEO work, andcreate_articleis 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 viewScopes::ALLand the template'scheckedattribute 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 existingscope_seo/scope_analyticsrows. A fork carrying its own copy ofadv_advisable_lang.phpfor any locale will not have the key, andt()on a missing key renders the key itself (application/core/MY_Lang.php:192-193) — not a fatal, but the admin sees the raw stringsettings.mcp_connector.scope_contentas the checkbox label until the fork adds it. - [4.123.0] Check for overrides:
Advisable\Mcp\Auth\Scopes::ALLgrew a third member, and its order is the canonical output order of bothparse()andformat()— i.e. the CSV byte layout stored inmcp_connector_tokens.scopes.contentwas appended, never inserted, soformat(['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-ordersALL, will rewrite that stored layout for tokens that already exist. - [4.123.0] Check for overrides:
Advisable\Mcp\Server\McpServerFactorygained a privateregisterBlogTools(), a privatearticleListSchema()and the constantCONTENT_INSTRUCTIONS, plus a thirdin_array(Scopes::CONTENT, ...)branch increate()and inbuildInstructions(). One existing constant changed:SEO_INSTRUCTIONSnow 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, andcreate_articlemakes the connector-wide form false. What everyseo-only token is told is unchanged in substance: product tags remain the one thing its own tools can create.WRITE_NOTEneeded no change here —#713had 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 ofMcpServerFactorysilently ships without the blog tools even for a token grantedcontent, and keeps the unscoped instruction text. - [4.123.0] Check for overrides:
src/Mcp/container.phpgained$services->set(Tools\BlogTools::class);. Thepublic()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-32603on 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\WriteServicehas nocategories/tagsbranch at all (unlike the product WriteService). Storefront blog listings are reached throughBlogCategory.articles, so a published article created this way may be unreachable on the storefront even though the connector reports success.CONTENT_INSTRUCTIONSstates this outright and directs the operator to assign categories and tags in the admin blog screen. Teaching the domain to acceptcategories/tagsis a separate change. - [4.123.0]
banner_imageis 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, notResource::formatFile()— verifies the file exists underfiles/blogs/. The schema description says so and directs the operator to copy an existing article'sbanner_image(returned byget_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.phpis 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 thePAIDarm, 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()isprotected, so a fork needing a third accepted-status set can reuse the shared guard rather than duplicating it. It wasprivatein the approved design and was widened at review: every sibling gateway-validation predicate in this class isprotected, andprivateclosed no attack surface, since a fork can already overridepayByBankReportsPaid()orpayByBankReportsCancelled()wholesale.
- A fork that overrode
- [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/activeand 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 isauth: 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:imageis the badge (gifts_mui.image, documented as such in the schema),giftImageis the gift picture (legacy's computedgift_image), andextraProductImage/extraVendorImagereplace 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 pathrequirementImagebelongs under, so the image is unusable without it.giftNameis the alt text forgiftImageand describes the same gift choice. Two optional filters:?filter[product]=<id>narrows to rules whose requirement pool includes that product, and?filter[isPromo]=<0|1>selects one side of the promo/normal split (the homepage slider renders0). They compose; no other filter and no sort is accepted, and?with=choicesis 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_endis 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-joinsgift_requirementsand collapses undergroup_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 nogifts_muirow for the request language is still returned (with all six caption fields null) where legacy's INNER JOIN drops it, andpagination.totalcounts the rules passing the date/flag window before the in-stock filter, so a page can carry fewer items thanper_page. One more, on field shape rather than row selection:amountFrom,amountToandgiftPerCountare a deliberate widening beyond every legacy storefront surface — the legacyAdvCartResource::mapGiftRuleFieldsForJson()allow-list withholds all three andAdv_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/giftendpoints,Promotion\Resources\Gift\Resource, and theGift/GiftChoice/GiftRequirementpolicies all keep their backend auth and theirADMIN/MARKETINGroles. 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\GiftorAdvisable\Rest\Promotion\Resources\Gift\Resourceis unaffected — the new surface is a separateActiveGiftcontroller, resource and domain namespace. A fork that maintains its own copy ofapplication/config/rest_routes.phporrest_policies.phpmust merge the new route pair (rest/promotion/gift/activeplus its locale twin) and the newActiveGift::classpolicy entry, or the endpoint 404s / 401s in that fork:PolicyResolverfalls back to the parent class's entry for an unlisted controller, which lands on the globalauth: backenddefault. A fork wanting a different active-window rule, a different field set or a scoped gift pool overridesAdvisable\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 — overridesRepository::giftImagesFor()/::requirementDisplayFor(); the anchor rule itself isprivate 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 theAdvisable\Domains\Promotion\ActiveGift\GiftChoiceStockDI alias — allprotected/interface seams, registered insrc/Domains/Promotion/container.php. - [4.123.0] Compiled DI container must be rebuilt (delete
cache/container.php) — this PR adds six new registrations: theActiveGiftrepository, MUI repository, repository configurator,LegacyGiftChoiceStockand theGiftChoiceStockalias, and the service, all insrc/Domains/Promotion/container.php, plus theActiveGiftcontroller insrc/Rest/Promotion/container.php(#646). A deploy that serves a stale compiled container has noActiveGift::classregistered;RouterDispatchercatches 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, orcompose.shfor 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/.envto./.env.container:/usr/local/var/www/html/.env) pluscompose.sh's unconditionaltouch(it runs on every invocation), or it keeps the bug. It must also carry the.gitignoreentry 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 differentgit statuslabel. The bare.envrule matches that basename only, so the entry is required, and it is the step most likely to be dropped because.gitignoremerges 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.containerfile automatically on the next./compose.sh up -d— thetouchcreates it beforedocker composeruns. Their prior container's mount of.env.placeholderis a stale inode until the containers are recreated. - [4.123.0] No REST contract, schema, or OpenAPI change. No
composer install, nonpm run all-production. - [4.123.0] Check for overrides:
Adv_base_controller::terminate()is a newprotectedmethod — additive, and per the #417 precedent an additiveprotectedmethod cannot break a fork by itself. But a fork that copied the wholepayByBankResponse()body into its own override keeps its own bareexitand 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'spaymerch/gen_tax_servicenote. Affected paths: a serial matching no order, a serial matching a non-paybybankorder, an order that is no longerPENDING, and aPAIDclaim the gateway does not confirm — all four previously emitted nothing and now emitOK. - The one thing a fork operator most needs to check:
application/modules/checkout/controllers/Checkout.phpis the designated fork seam — an emptyclass Checkout extends Adv_checkout. A fork that instead overrodepayByBankResponse()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
OKinstead.
- [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 oncoupon_categories.category_idthat 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 coverscategory_idunder any index name — and still adds the index where only a non-leading composite covers it) and20260904120100_clean_orphan_coupon_category_rows.php(runsCleanOrphanCouponCategoryRows, 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, extendingAdv_product_category_model) — a fork that overridesdelete_record()wholesale does not inherit this fix and keeps leakingcoupon_categoriesrows 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 newprotected string $couponCategories = 'coupon_categories'property on the parent also joins the fork override surface: a whole-class copy ofAdv_product_category_modelpredating this change won't have it and needs to pick it up alongside thedelete_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 isprivate const DEPENDENT_REFERENCES, read asself::, so a fork cannot extend it by redeclaring the constant the wayInternalMoveFolder::GUARD_FILESis extended: it must either overridedeleteDependentRows()and callparent::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 andDELETE /rest/product/category/{id}still returns its usual response; only the orphan rows show it). A fork whoseCustom\WriteService::delete()overridesdelete()without callingdeleteDependentRows()at all is in the same position. A fork that has not touchedsrc/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_idis 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.
- The legacy override note's count — "the transaction-wrapped seven-table delete (five dependents +
- [4.123.0] UI Update:
GET /rest/cms/blog/article?filter[title.{locale}]=<term>— and its/item,/{id}and locale-prefixed twins — now returns partially-matched articles instead of HTTP 500. The filter key was whitelisted againstblog_mui.name, a columnblog_muidoes not have (database/initial/initial.sql:207-226— it istitle), so every request carrying the key died withError 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 isuseBlogArticles({ title })in@advisable-com/velora-storefront-core1.22.2 (Advisable-com/velora#622). Nothing else on the endpoint moves:filter[slug.{locale}],filter[categorySlug.{locale}],filter[notEmptyContent.{locale}]andsort=title.{locale}were already declared against real columns — the sort half of the same file has always usedblog_mui.titlecorrectly, which is why a green suite said nothing about the filter half. The forced storefrontpublishedscope 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 thisprotectedmethod to add its own filter keys carries its own copy of thetitle.{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.paymerchis now written by the REST checkout as'invoice'/'receipt'(previously always NULL), andshop_order.gen_tax_serviceis no longer written by it at all (previously the truncatedin/re). Every consumer below testspaymerchfor 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 assumptionpaymerchis always NULL for REST-placed orders needs to reconcile. This is not hypothetical:v4-wecareis 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
- Printed fiscal document —
- [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 itscompanyAddresscamelCase 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-ordernow refuses an order wherewantsInvoiceistrueand any ofafm,doy,company,professionorcompanyAddressis empty after trimming — a whitespace-only value is refused exactly like an absent one, matching legacy's own rule (each fieldtrim|requiredwhen the choice is invoice,ecommercen/eshop/controllers/Adv_order.php:543-549). The response is 422 witherror.code = invoice_identity_incompleteanderror.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 postingwantsInvoice: truewithout all five fields — which the NULL-paymerchbug 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 thatmissingFieldsnames 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 forcompany_addressand 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.cancelUrlonalpha/eurobankstops echoing the client's submitted value and becomes asite_url(...)value, exactly asconfirmUrldid in #728;CardLinkAdapter's SHA-256digestchanges with it, sincecancelUrlis 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 noerrorUrlfield and noadvOrderSerialparameter ship, andpaymentRedirect.paramsremains exactly the MPGSCheckout.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 overridesPlaceOrderServicerather than the builder getscancelUrl => ''on CardLink rather than a fatal, i.e. it silently keeps the old behaviour. There is deliberately no?: $context->cancelUrlfallback, because that would re-open the #728 defect class.EthnikiEEAdapterandPaymentRedirect— nothing to check. An earlier revision of this branch added anerrorUrlfield toPaymentRedirectand emitted it from this adapter; both were removed when theethniki_eefailure 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
protectedseams inAdv_vendors_admin.phpchange shape but not signature (no LSP fatal for a fork that overrides them):applyVendorMuiValidationRules()now registers 10set_rules_muirules per language instead of 7 when themetatagspermission is granted. A fork overriding this method losestrimnormalisation andset_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.phpandupdate.phpare 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 usevendor-seo-collapsiblefor the heading class andvendor-seo-contentfor the panel's inner id — not the product-category panel's.collapsible/ceo-contentpair.application/views/admin/footer_js.php:4294-4308binds a click handler to every.collapsiblebut toggles a single hardcodedgetElementById('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 fromdocumentand placed outside the#AiContentGenerationVue root, since that bundle mounts atfooter_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 solidbtn btn-primarywith no arrow — the platform-wide single-id limitation infooter_js.phpis left in place and out of scope here. - [4.123.0] Fields are gated on the
metatagspermission (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. BecauseupdateOrInsertMui()performs a partialUPDATEover 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 withmb_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 emptyg: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. Seedocs/decisions/607-xmlsanitise-null-fatal.mdfor the rejected alternative. - [4.123.0]
xmlSanitise()(application/helpers/MY_text_helper.php:390) is wrapped inif (! function_exists('xmlSanitise')). The parameter widening itself is source-compatible — contravariant, so no existing caller needs a change, and the(string)casts already atAdvAgoraCatalog.php:159-161stay valid. A scan across all 34 local fork checkouts found no fork declaring its ownxmlSanitise()in a separate, earlier-loaded helper — that override seam exists but is currently unexercised. What the scan did find: 18 forks carry the inheritedapplication/helpers/MY_text_helper.php. 17 of them are still on the original strictxmlSanitise(string $string): stringand take this fix cleanly on their next upstream sync — no action needed. Megastore has diverged: its copy already readsxmlSanitise(string|null $string): string, with thepreg_replace()return still unguarded — that local patch fixes only the NULL-argument outage, the invalid-UTF-8 fatal stays armed, and passingnullstraight intopreg_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 declaresxmlSanitiseas aprivate function xmlSanitise(string $s): stringon two feed controllers —Facebook_catalog.php:16andGoogle_catalog.php:91— each with the original strict signature and the same unguardedpreg_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()andtasksTo()both calledcreate_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), whilemyTasks/tasksTocarry no such gate and are open to every admin role by design ('roles' => []atapplication/config/admin_menu.php:375and: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 returnederror_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 ontasks()'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()'screate_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, theallowRole()gate ontasks(), and the absence of a gate onmyTasks/tasksToare all unchanged and out of scope here. Rejected alternative (recorded indocs/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/<offset>andauth/tasksTo/<offset>respectively — instead of both pointing atauth/tasks/<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_tagshas no foreign key and no cascade, andTag\Tag\WriteService::delete()has no in-use guard and no pivot cleanup — where the legacyAdv_product_tags_model::delete()refuses the delete outright. Adelete_product_tagtool would therefore inherit a live defect and hand an LLM a one-call way to silently orphan rows. Fixing the endpoint first (aTag\LegacyDeletionRule+validateForDelete(), the shapeCategory\LegacyDeletionRulealready established) turns a live200into a422on 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
seoscope — noScopes.phpchange, no token rotation.seoalready 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 theregister*Tools()method it sits in. Every existing token gains these tools on deploy. A newcontent-writescope was rejected for now becauseScopes::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 extendsProduct\WriteServiceand 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 insrc/Domains/Product/container.php). This mirrors howCategoryPivotWriteRepositorywas added as the 8th argument; a nullable trailing parameter with adi()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.phpgains 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 takeWriteService.phpand 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-containerexits non-zero) plus a 500 on every REST and MCP product write, not a fatal at load. After merging, runcheck-containerand confirm the line is present. - [4.123.0] The
tagswrite key is a REST contract change too, not only an MCP one. Every caller ofPOST /rest/product/productandPOST /rest/product/product/{id}gains atagskey (aliastag_ids) the momentProduct\WriteServiceaccepts one, with the same four states as above — absent (untouched) /[](clears) /[ids](replaces) / explicitnullor any non-array value (also clears, because it is coalesced to an empty set), identical tocategories. The write key is documented onProduct\WriteData's#[OA\Schema], and the pre-existing but undocumentedcategorieskey was documented alongside it in the same change rather than shipping a second silent write key. - [4.123.0]
list_product_tagshas notag_cat_idfilter.Tag\Tag\ListRequestallows exactlyid,priority,name.{locale}andslug.{locale}, andGenerateListRequestsilently skips an unknown filter key — an unsupported filter would return200with 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 vialist_product_tag_categoriesplus each row's embeddedtag_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 from400→502on ten payways (apcopay,ethniki_ee,ethniki_nbgpay,iris,jcc,klarna_payments,paypal,paypaladvanced,vivawallet,xpay) and from a generic500→502on three more —piraeusandpaybybank(the unhandledTypeError) andstripe(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. The502response carries the usual{success, message}pair plus a newerror.code = payment_initialization_failedanderror.payway. The previous400was wrong — this is a status correction, not new behaviour, and it does not mean the order was rolled back: the order survivesPENDINGwith its basket, coupon and points already committed and the cart already cleared. A client branching on400for a gateway failure on any of the ten payways above must update that branch; a client that assumed a500/400meant 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?PaymentInitResultfatals at class load on merge. PHP return types are covariant, so a nullable implementation is invalid against the now non-nullablePaymentAdapterInterface::initializePayment(). Self-announcing: the shop fails to boot rather than mis-behaving quietly. The remedy is dropping the?and replacing anyreturn nullwith aPaymentInitializationFailedExceptionthrow. - Silent case — the dangerous one.
PaymentInitializeris 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 uncaughtTypeErrorfor that fork's two null-returning adapters, or still answers a bare400for 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 ownPaymentInitializeralias.
- Loud case — a
- [4.123.0] UI Update: REST response contract change on
POST rest/checkout/place-order.paymentRedirect.params.confirmUrl(alpha,eurobank) andparams['data-redirect-url'](ethniki) stop echoing the submittedreturnUrland becomesite_url(...)values on this deployment;klarna_payments'merchant_urls.confirmationand.notificationlikewise becomesite_url(...)values.alpha/eurobankadditionally get a newdigest, sinceCardLinkAdapter's hash is computed overconfirmUrlamong 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 submittedreturnUrlfor 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 requiredstringparams andPlaceOrderService::__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 aliasesPlaceOrderServicein its own container keeps building one-URLPaymentContexts 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\PaymentCallbackUrlBuilderis 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 targetcheckout/get_response/{payway}/..., handled byAdv_checkout::get_response(). A fork whose shadowingapplication/**copy ofAdv_checkoutlacks an arm for one of the six payways answerserror_401()instead of confirming the order.application/config/rest_api_versions.phpis fork-shadowable too, so a fork tracking its own REST changelog there won't pick up the'1.X'entry this change adds automatically.
- DI lane.
- [4.123.0] UI Update: on a multi-language install, the gift-rule sliders (
application/views/main/components/sliders/gift_rules_slider.phpandgift_rules_promo_slider.php) can now render an emptyrequirement_namelabel on the vendor-requirement path (option_type = 2), and — on both that path and the product-requirement path (option_type = 1) — arequirement_slugthat 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 thesop_vendor_muialias feedsCONCAT(sop_vendor_mui.vendor_slug, '/', shop_product_mui.slug), which an untranslated vendor NULLs outright. This happens when a vendor has noshop_vendor_muirow 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) passesrequirement_namestraight intostr_replace()with no null guard, loggingDeprecated: str_replace(): Passing null to parameter #3 ($subject)at the six call sites ingift_rules_slider.php:21,29,37andgift_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 twoshop_vendor_muijoins in this query carried nolangpredicate at all, sorequirement_name/requirement_slugresolved 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:8ships 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: bothshop_vendor_muijoins now carry`lang` = '{$this->languageAbbr}', matching the joins in the same query already scoped to the active language — the unaliased join feeds bothrequirement_name/requirement_slugon the vendor-requirement path (option_type = 2); thesop_vendor_muialias feeds only the vendor half ofrequirement_slugon the product-requirement path (option_type = 1). Both remainLEFTjoins. Note the multi-requirement fan-out (the separateLEFT JOIN gift_requirements, one row peroption_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-codenow requiresAUTH_ROLE_ADMINorAUTH_ROLE_PRODUCTS(matching its siblingProductCodeAttribute), andGET/POST/DELETE /rest/seo/custom-meta-tagnow requiresAUTH_ROLE_ADMINorAUTH_ROLE_CMS(matching its siblingDefaultMetaTag) — both on every routed verb (index/show/item plus the store/update/destroy writes), not only the reads. Previously neither controller had an entry inapplication/config/rest_policies.php, soPolicyResolverfell 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 barerequire 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 theProductCode::class/CustomMetaTag::classregion ofrest_policies.phpshould 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.phpsavesVIVAWALLET.EXTERNAL_SOURCE_CODEunconditionally from$this->input->post('viva_external_source_code'), like every neighbouring Viva field. A client fork that overridespayment_settings.phpand 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 forPIRAEUSBANK_EXTERNAL. - [4.123.0] Deployment action — fail-closed, not backward compatible by default. A deployment that upgrades without configuring the new key registers no
vivawalletadapter for the external channel:vivawalletsilently disappears fromGET /rest/checkout/payment-methods, andplace-orderrefuses it with the existing422 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) capturessetBatchCanceled()'s per-serial result — previously discarded — and passes it to the newreportSourceOrderNotCanceledIfNeeded()(:2371), which flashes an error through the existingSESS_KEY_ESHOP_ERRORsession channel (the same onereportRefusedLoyaltyRedemption()uses) when the source serial comes back insetBatchCanceled()'serrorset — i.e. it was alreadyCANCELED, a non-paybybankPENDINGorder, aPAID+paybybankorder, or on one ofeurobank, 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 requiredint $orderIdparameter (ecommercen/checkout/controllers/Adv_checkout.php:2380, called fromafterSuccess()at:2188).Adv_gifts_model::updateCounter()changed return typevoid->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 aboolreturn 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 obscureprotectedmethod, 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 ofapplication/models/Adv_gifts_model.phpthat keeps the oldvoidupdateCounter()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. UpstreamAdv_checkout::updateGifts()then reads thenullreturn as failure, no order is ever marked viamarkGiftsApplied(), 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 newgetGiftQuantitiesByOrderSerial()fatal on every cancellation, not a silent no-op. The same is true of a replacedAdv_gifts_modeland its newrestoreCounter(). - A
Custom\alias of eitherOrder\WriteRepositoryorGift\WriteRepositorythat does notextendsthe upstream class is fatal on the new methods it doesn't have. - A
Custom\subclass ofDecrementGiftStockOnPaidListenerwith 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 fromvoidtobool. AnyCustom\subclass declaring the old: voidsignature is an LSP fatal.- Overrides of
set_status(), ofAdv_checkout::afterSuccess()/updateGifts(), or a forked copy ofsrc/Domains/Order/container.phpall 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_canceledwas added to all 8 shipped locales (english, greek, chinese, french, german, italian, russian, spanish). A fork carrying its own copy ofadv_advisable_lang.phpfor any of those locales will not have the key, andt()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 stringeshop.admin.order.error.source_not_canceledin 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.
- All three legacy signature changes below are FATAL at class-load time for an
- [4.123.0] Historical orders are not backfilled.
gifts_applieddefaults to0, 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 fromshop_order_basket.gift_id+qty, but nothing in the data distinguishes a decremented historicalPENDINGorder (legacy PayByBank) from a never-decremented one (modern REST pre-confirm) on that status alone, and with no ceiling column ongiftsan 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
CANCELEDtriggers the restore —RETURNdoes not.RETURNis not dormant — a live cron job writes it (AdvSyncOrdersStatusEuropharmacy::getOrderStatusID()maps vendor code7to it), butAdv_order_model::update_order()(ecommercen/eshop/models/Adv_order_model.php:951-953) only routes intoset_status(..., 'CANCELED', ...)when the incoming status isCANCELED; aRETURNwrite 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 —RETURNIS written" section. - [4.123.0] The pre-existing product-stock double-restore in
set_status()is unaffected by this change.returnOrderStock()'s guard matches onorder_serialalone with no idempotency check, so a repeat cancel still double-restoresproduct_codes.stocktoday, 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 intrans_start()/trans_complete(); on a failed transaction it callslog_message('error', ...)and returns normally. The rollback has already re-armedgifts_appliedby 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 ofset_status(), soAdv_order_model::setBatchCanceled()'s batch loop (:3166-3220, the per-orderset_status()call at:3214) is never aborted partway through, and none ofset_status()'s other callers (AdvCancelIncompleteOrders,AdvApiKlarna, the PayByBank sweep, admin edit-order) sees an unhandled fatal from this path. This matches the modern path, whereOrderEventDispatchercatches 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— addsshop_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_muirow 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 missingoffer_categories_muirows are added. (2) The already-published/ειδικεσ-προσφορεσ/ολεσURL keeps listing all offers on every shop, whatever its configured language, via the newAdv_offers::LEGACY_ALL_OFFERS_SLUGalias. That alias is deliberately language-independent:application/config/routes.phpregisters 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 existingroutes.offers. Greek keeps the valueολεσ. A client cannot shadow the platformadv_theme_lang.phpitself —Adv_base_controller::loadFrontLangFiles()loads it with an explicit$alt_path, which takesapplication/core/MY_Lang.php'sif ($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 redefinesroutes.offers_all(and, if needed,routes.offers) in its own{client_views}_theme/_user_themelanguage 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$langargument was previously declared and never applied, and the mui join carried nolangpredicate 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 newprotected isAllOffersSlug()(additive — a client override ofindex()that inlines the old!= 'ολεσ'comparison keeps working, but will not pick up the per-language segment). The method also now callsshow_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 configuredlanguage_abbrrather 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$langdefault, 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) andordersList()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 anArgumentCountErrorat 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 forcategories_of_vendor,getAllVideosAssignedToVendorandgetAllVideosPaginated, which are dispatched positionally by name throughPscache::model(). - [4.123.0] Behaviour change:
/robots.txtnow returns200 text/plainon image-deployed tenants, where it previously returned the CodeIgniter 404 page. Two defects stacked: the image docroot is populated by an allowlist matching onlypublic/*.php(.docker/images/app.dockerfile:42), so the tracked, per-clientpublic/robots.txtnever 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 FastCGIlocation /, and PHP-FPM'ssecurity.limit_extensionsdefault (.php .phar) rejects.txt— the same failure fixed for/sitemap/*.xmlin 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:30already served the file through itsRewriteCond %{REQUEST_FILENAME} !-fguard, 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.txtblock mirrored there, or the file returns 403 once it is present. - [4.123.0] Check for overrides: forks carrying their own
.docker/images/app.dockerfileshould expect a conflict on thepublic/*.phpCOPY 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) ranfixJoins()+fixWhere($where), then compiledSELECT *acrosstasksjoined touserstwice (creator and assignee), fetched every matching row, and returned$tasks->num_rows(). It is the only read path on this model that omitsfixSelect().getAll()at least appliesLIMIT 20, so the count query backing pagination was heavier than the list it paginated — and every row it pulled, including bothusers.passwordcolumns, was discarded immediately after counting. This was never a disclosure issue: the rows never reach an output sink, onlynum_rows()is read. - [4.123.0] The fix.
countAll()keeps bothfixJoins()andfixWhere($where)— so the counted set stays structurally identical to the onegetAll()pages over — and now returns$this->db->count_all_results($this->table)directly.count_all_results()honours the accumulated joins and where clauses (unlikecount_all(), which ignores query-builder state entirely and would silently drop every$wherefilter) and compilesCOUNT(*), so no columns are selected or materialised. The($tasks && $tasks->num_rows()) ? ... : 0guard is gone becausecount_all_results()already returns an int; the method's signature and return type are unchanged. Full rationale, including the rejectedcount_all()alternative, is recorded indocs/decisions/751-74-task-list-pagination-and-countall.md. - [4.123.0] The gain is removed row materialisation, not scan cost.
tasksis indexed only onid,creator_id,assignee_id; theLIKEfilters ontitle/descriptionand 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 (stillcountAll($where = []), still returns anint), but the emitted query changes: previouslySELECT *against the joinedtasks/usersset followed by a PHP row count, now a singleSELECT COUNT(*)against the same joins and where clause. A client override that callsparent::countAll()is unaffected; one that reimplements the method independently is untouched by this fix and keeps its own behaviour.