Skip to content

FIX: Store the site contact group by id - #42771

Open
ZogStriP wants to merge 1 commit into
mainfrom
site-contact-group-by-id
Open

FIX: Store the site contact group by id#42771
ZogStriP wants to merge 1 commit into
mainfrom
site-contact-group-by-id

Conversation

@ZogStriP

Copy link
Copy Markdown
Member

Previously, site_contact_group_name was written as a group name by the
About config page but as a group id by the all-settings page, which grew
a group picker in #40981 — and GroupSettingValidator was relaxed in the
same commit to accept either, so the mismatched value saved cleanly and
the row still displayed the right group. SystemMessage looks the value
up by name, so an admin who set the contact group from All site settings
silently stopped it being invited to automated personal messages, and the
About page then showed no group selected at all.

This change makes the id the stored format everywhere, because a group
name can be renamed or localized while an id cannot. TypeSupervisor
converts a name to its id on write, so console, API and plugin callers
that pass a name keep working; the validator now accepts ids only; and a
migration converts the values sites have already stored. The About page
writes and reads the id, and SystemMessage resolves it through a new
Discourse.site_contact_group, which still accepts a name for sites that
configure this through a DISCOURSE_SITE_CONTACT_GROUP_NAME global
override, since no migration can reach those.

Both id lookups guard on a digits-only match rather than casting, because
Rails turns Group.find_by(id: "0support") into WHERE id = 0 — the
everyone group — and Integer("010") into 8.

Two behaviour changes are worth calling out. Renaming the contact group
no longer breaks the setting; a spec asserted the opposite, which was the
bug written down as intent. And the migration matches names
case-insensitively, so a site that stored Staff for a group named
staff — which Group.exists?(name:) never matched — starts inviting
that group again.

site_settings.errors.invalid_group now says "ID" rather than "name". It
is shared with AtLeastOneGroupValidator, which has only ever validated
ids, so the message was already wrong for the settings using it.

Finally, the setting keeps its name. Renaming it would change a key that
self-hosters set through the environment and that third parties read, so
that belongs in its own change rather than in a fix.

@github-actions github-actions Bot added i18n PRs which update English locale files or i18n related code discourse-narrative-bot labels Aug 20, 2026
@ZogStriP
ZogStriP force-pushed the site-contact-group-by-id branch from 74ae060 to 43b8c55 Compare August 20, 2026 09:56
Previously, `site_contact_group_name` was written as a group name by the
About config page but as a group id by the all-settings page, which grew
a group picker in #40981 — and `GroupSettingValidator` was relaxed in the
same commit to accept either, so the mismatched value saved cleanly and
the row still displayed the right group. `SystemMessage` looks the value
up by name, so an admin who set the contact group from All site settings
silently stopped it being invited to automated personal messages, and the
About page then showed no group selected at all.

This change makes the id the stored format everywhere, because a group
name can be renamed or localized while an id cannot. `TypeSupervisor`
converts a name to its id on write, so console, API and plugin callers
that pass a name keep working; the validator now accepts ids only; and a
migration converts the values sites have already stored. The About page
writes and reads the id, and `SystemMessage` resolves it through a new
`Discourse.site_contact_group`, which still accepts a name for sites that
configure this through a `DISCOURSE_SITE_CONTACT_GROUP_NAME` global
override, since no migration can reach those.

Both id lookups guard on a digits-only match rather than casting, because
Rails turns `Group.find_by(id: "0support")` into `WHERE id = 0` — the
everyone group — and `Integer("010")` into 8.

Two behaviour changes are worth calling out. Renaming the contact group
no longer breaks the setting; a spec asserted the opposite, which was the
bug written down as intent. And the migration matches names
case-insensitively, so a site that stored `Staff` for a group named
`staff` — which `Group.exists?(name:)` never matched — starts inviting
that group again.

`site_settings.errors.invalid_group` now says "ID" rather than "name". It
is shared with `AtLeastOneGroupValidator`, which has only ever validated
ids, so the message was already wrong for the settings using it.

Finally, the setting keeps its name. Renaming it would change a key that
self-hosters set through the environment and that third parties read, so
that belongs in its own change rather than in a fix.
@ZogStriP
ZogStriP force-pushed the site-contact-group-by-id branch from 43b8c55 to c3760f7 Compare August 20, 2026 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

discourse-narrative-bot i18n PRs which update English locale files or i18n related code

Development

Successfully merging this pull request may close these issues.

1 participant