FIX: Store the site contact group by id - #42771
Open
ZogStriP wants to merge 1 commit into
Open
Conversation
ZogStriP
force-pushed
the
site-contact-group-by-id
branch
from
August 20, 2026 09:56
74ae060 to
43b8c55
Compare
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
force-pushed
the
site-contact-group-by-id
branch
from
August 20, 2026 14:53
43b8c55 to
c3760f7
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previously,
site_contact_group_namewas written as a group name by theAbout config page but as a group id by the all-settings page, which grew
a group picker in #40981 — and
GroupSettingValidatorwas relaxed in thesame commit to accept either, so the mismatched value saved cleanly and
the row still displayed the right group.
SystemMessagelooks the valueup 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.
TypeSupervisorconverts 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
SystemMessageresolves it through a newDiscourse.site_contact_group, which still accepts a name for sites thatconfigure this through a
DISCOURSE_SITE_CONTACT_GROUP_NAMEglobaloverride, 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")intoWHERE id = 0— theeveryone 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
Stafffor a group namedstaff— whichGroup.exists?(name:)never matched — starts invitingthat group again.
site_settings.errors.invalid_groupnow says "ID" rather than "name". Itis shared with
AtLeastOneGroupValidator, which has only ever validatedids, 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.