mirror of
https://github.com/discourse/discourse.git
synced 2026-09-05 04:40:41 -05:00
Régis Hanol
6c7ef4dfec
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 post-deploy migration converts the values sites have already stored, post-deploy because the old code can only resolve a name. 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 conversions go through `Group.find_by_id_or_name`. The 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. 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 "There's no such group", since it fires for an unknown name as well as an unknown id and is shared with `AtLeastOneGroupValidator`, which has only ever validated ids. Finally, the setting keeps its name. Renaming it would change a key that self-hosters set through the environment and that third parties read, and the deprecated-settings alias does not cover global overrides, so that belongs in its own change rather than in a fix.