From 57eecbef4bed28ae8f43dcd653d81e8597e0d79b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9gis=20Hanol?= <regis@hanol.fr>
Date: Thu, 27 Jun 2024 19:10:59 +0200
Subject: [PATCH] FIX: invalid user locale when accepting group membership

If, for whatever reasons, the user's locale is "blank" and an admin is accepting their group membership request, there will be an error because we're generating posts with the locale of recipient.

In order to fix this, we now use the `user.effective_locale` which takes care of multiple things, including returning the default locale when the user's locale is blank.

Internal ref - t/132347
---
 app/controllers/groups_controller.rb    | 14 ++++++---
 app/models/discourse_connect.rb         |  2 +-
 app/services/badge_granter.rb           |  2 +-
 spec/requests/groups_controller_spec.rb | 39 ++++++++++++++++++++-----
 4 files changed, 43 insertions(+), 14 deletions(-)

diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb
index 72d33b7abe0..01b0f4a2cee 100644
--- a/app/controllers/groups_controller.rb
+++ b/app/controllers/groups_controller.rb
@@ -484,10 +484,12 @@ class GroupsController < ApplicationController
     request_topic =
       Topic.find_by(
         title:
-          (
-            I18n.t "groups.request_membership_pm.title", group_name: group.name, locale: user.locale
+          I18n.t(
+            "groups.request_membership_pm.title",
+            group_name: group.name,
+            locale: user.effective_locale,
           ),
-        archetype: "private_message",
+        archetype: Archetype.private_message,
         user_id: user.id,
       )
 
@@ -506,7 +508,11 @@ class GroupsController < ApplicationController
         post_type: Post.types[:regular],
         topic_id: request_topic.id,
         raw:
-          (I18n.t "groups.request_accepted_pm.body", group_name: group.name, locale: user.locale),
+          I18n.t(
+            "groups.request_accepted_pm.body",
+            group_name: group.name,
+            locale: user.effective_locale,
+          ),
         reply_to_post_number: 1,
         target_usernames: user.username,
         skip_validations: true,
diff --git a/app/models/discourse_connect.rb b/app/models/discourse_connect.rb
index c1e3f1c1a6b..36e7bf3606a 100644
--- a/app/models/discourse_connect.rb
+++ b/app/models/discourse_connect.rb
@@ -353,7 +353,7 @@ class DiscourseConnect < DiscourseConnectBase
       user.name = name || User.suggest_name(username.blank? ? email : username)
     end
 
-    if locale_force_update && SiteSetting.allow_user_locale && locale &&
+    if locale_force_update && SiteSetting.allow_user_locale && locale.present? &&
          LocaleSiteSetting.valid_value?(locale)
       user.locale = locale
     end
diff --git a/app/services/badge_granter.rb b/app/services/badge_granter.rb
index ab2c76b6edf..d8af5a607a7 100644
--- a/app/services/badge_granter.rb
+++ b/app/services/badge_granter.rb
@@ -102,7 +102,7 @@ class BadgeGranter
         user_id: user.id,
         badge_id: badge.id,
       )
-      notification = send_notification(user.id, user.username, user.locale, badge)
+      notification = send_notification(user.id, user.username, user.effective_locale, badge)
 
       DB.exec(<<~SQL, notification_id: notification.id, user_id: user.id, badge_id: badge.id)
         UPDATE user_badges
diff --git a/spec/requests/groups_controller_spec.rb b/spec/requests/groups_controller_spec.rb
index 3afc1cae506..e25aa3a96e5 100644
--- a/spec/requests/groups_controller_spec.rb
+++ b/spec/requests/groups_controller_spec.rb
@@ -2276,7 +2276,7 @@ RSpec.describe GroupsController do
         title: I18n.t("groups.request_membership_pm.title", group_name: group.name),
         raw: "*British accent* Please, sir, may I have some group?",
         archetype: Archetype.private_message,
-        target_usernames: "#{user.username}",
+        target_usernames: user.username,
         skip_validations: true,
       ).create!
 
@@ -2312,15 +2312,10 @@ RSpec.describe GroupsController do
       # send the initial request PM
       PostCreator.new(
         other_user,
-        title:
-          (
-            I18n.t "groups.request_membership_pm.title",
-                   group_name: group.name,
-                   locale: other_user.locale
-          ),
+        title: I18n.t("groups.request_membership_pm.title", group_name: group.name, locale: "fr"),
         raw: "*French accent* Please let me in!",
         archetype: Archetype.private_message,
-        target_usernames: "#{user.username}",
+        target_usernames: user.username,
         skip_validations: true,
       ).create!
 
@@ -2346,6 +2341,34 @@ RSpec.describe GroupsController do
         I18n.t("groups.request_accepted_pm.body", group_name: group.name, locale: "fr").strip,
       )
     end
+
+    it "works even though the user has no locale" do
+      other_user.update!(locale: "")
+
+      GroupRequest.create!(group: group, user: other_user)
+
+      # send the initial request PM
+      PostCreator.new(
+        other_user,
+        title: I18n.t("groups.request_membership_pm.title", group_name: group.name),
+        raw: "*Alien accent* Can I join?!",
+        archetype: Archetype.private_message,
+        target_usernames: user.username,
+        skip_validations: true,
+      ).create!
+
+      topic = Topic.last
+
+      expect {
+        put "/groups/#{group.id}/handle_membership_request.json",
+            params: {
+              user_id: other_user.id,
+              accept: true,
+            }
+      }.to_not change { Topic.count }
+
+      expect(topic.posts.count).to eq(2)
+    end
   end
 
   describe "#histories" do