Skip to content

Send the merchant notification by default on a fresh install - #104

Draft
boo-code wants to merge 1 commit into
PrestaShop:devfrom
boo-code:fix/send-merchant-notification-by-default-16689
Draft

Send the merchant notification by default on a fresh install#104
boo-code wants to merge 1 commit into
PrestaShop:devfrom
boo-code:fix/send-merchant-notification-by-default-16689

Conversation

@boo-code

@boo-code boo-code commented Sep 9, 2026

Copy link
Copy Markdown
Questions Answers
Description? install() registered the module's two hooks and seeded no configuration, so CONTACTFORM_SEND_NOTIFICATION_EMAIL and CONTACTFORM_SEND_CONFIRMATION_EMAIL did not exist on a new shop. Both are read with Configuration::get() and the send block is guarded by their disjunction, so a freshly installed contact form accepted a message and mailed nobody: the merchant was never told it had arrived, and nothing on the page said why. This seeds the two settings with different defaults. The notification goes to the shop's own contact address and is the reason the form exists, so it is on. The confirmation goes to whatever address the visitor typed, which is what makes the form usable to mail a third party, so it stays an explicit opt-in and is off. Uninstalling does not remove a module's configuration, so install() runs again over existing values on a reset or a reinstall; seedDefault() writes only when no value exists yet and tests that with Configuration::get(), which cascades shop -> shop group -> global the way the module's own reads do. Configuration::hasKey() inspects one level and would report "absent" under multistore for a value set globally or on the shop group.
Type? bug fix
BC breaks? no
Deprecations? no
Fixed ticket? Fixes PrestaShop/PrestaShop#16689.
How to test? On a shop where the module is not installed, confirm SELECT name, value FROM ps_configuration WHERE name LIKE 'CONTACTFORM%' returns nothing, then install the module. Before this change the query still returns nothing and submitting the front-office contact form sends no mail at all; after it, the two keys exist as 1 and 0 and the merchant receives the notification while the visitor gets no confirmation. Then set the two settings by hand to the opposite values and reset the module: the values must survive, which is what separates this from a plain updateValue() in install().

Measured on dev (4.4.3 shop, module reinstalled through bin/console prestashop:module)

Only install() was varied; the module version was left alone so it is not a second variable.

                                       base (dev)        this branch
  fresh install, keys deleted first
    install result                     succeeded         succeeded
    CONTACTFORM_ rows after            0                 2
    SEND_NOTIFICATION_EMAIL            absent            1
    SEND_CONFIRMATION_EMAIL            absent            0
    hooks registered                   2                 2
    send block ($conf || $notif)       skipped           runs

  reset over a merchant's own choice
    merchant set notif=0, conf=1
    after `module reset contactform`   n/a               notif=0, conf=1  (kept)

The naive form of this fix - two bare Configuration::updateValue() calls - was measured
first and rejected: a reset flipped the merchant's notif=0, conf=1 back to 1, 0,
silently overriding both choices.

Two other things the measurement settled:

  • Configuration::updateValue($key, false) stores SQL NULL, not '0', because
    updateValue() inserts with $null_values = true and pSQL(false) === ''
    (classes/db/Db.php:444). The seeds use 1/0, matching what the settings form
    itself writes through Tools::getValue().
  • The two mails are independent sibling branches inside the disjunction
    (if ($sendNotificationEmail) -> Mail::Send to $contact->email, then
    if ($sendConfirmationEmail) -> Mail::Send to the visitor), so 1/0 really does
    send the merchant's mail and not the visitor's.

Shape follows ps_contactinfo::install(), which already chains
Configuration::updateValue('PS_CONTACT_INFO_DISPLAY_EMAIL', 1) into its && return.

Design decision

The 2019 thread did not converge: the request was to turn both mails on, and the
objection was that mailing the visitor's typed address makes the shop a relay. Splitting
them resolves it on merits rather than picking a side - the notification carries the
abuse objection not at all, and the confirmation carries all of it.

Issue is labelled Feature / Needs Specs, but a form that accepts input and notifies
nobody is a defect on its own terms; that is what was measured, and the fix is scoped to
it rather than to the broader settings discussion.

Gate

  • php -l 7.2 / 7.4 / 8.1 / 8.4 / 8.5: clean (CI lints 7.2 -> 8.5)
  • php-cs-fixer, module's own .php_cs.dist: exit 0; base file exit 0 as control
  • PHPStan level 5, PS bootstrap via .github/workflows/phpstan/autoload.php: 2 errors,
    base also 2, 0 new
  • Dup gate: no open PR in the repo touches install(). My own open Tell the customer when a duplicate message was not sent #103 has the exact
    same one-file file set but sits at sendMessage() (~line 666); no hunk overlap.

install() registered its hooks but seeded no configuration, so
CONTACTFORM_SEND_NOTIFICATION_EMAIL and CONTACTFORM_SEND_CONFIRMATION_EMAIL
did not exist on a new shop. Both are read with Configuration::get() before
anything is sent, and the send block is guarded by their disjunction, so a
freshly installed contact form accepted messages and mailed nobody - the
merchant was never told a message had arrived.

Seed the two settings, with different defaults. The notification goes to the
shop's own contact address and is the reason the form exists, so it is on.
The confirmation goes to whatever address the visitor typed, which is what
makes the form usable to mail a third party, so it stays an explicit opt-in
and is off.

Uninstalling a module does not remove its configuration, so install() runs
again over existing values on a reset or a reinstall. seedDefault() therefore
writes only when no value exists yet, and tests that through
Configuration::get(), which cascades shop -> shop group -> global the way the
module's own reads do. Configuration::hasKey() inspects a single level and
would report "absent" under multistore for a value set globally or on the
shop group, overriding a choice the merchant had already made.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

contact form module - set send mails ON by default

1 participant