DEV: group_list site settings should store IDs instead of group names (#7860)

* DEV: group_list site settings should store IDs instead of group names

* Ship site setting to know when we should migrate group_list settings

* Migrate existing group_list site settings

* Bump migration timestamp and don't set null when migrating is not possible.

app/assets/javascripts/admin/components/site-settings/group-list.js.es6

@@ -3,6 +3,8 @@ import computed from "ember-addons/ember-computed-decorators";
 export default Ember.Component.extend({
   @computed()
   groupChoices() {
-    return this.site.get("groups").map(g => g.name);
+    return this.site.get("groups").map(g => {
+      return { name: g.name, id: g.id.toString() };
+    });
   }
 });

app/assets/javascripts/admin/templates/components/site-settings/group-list.hbs

@@ -1,3 +1,3 @@
-{{list-setting settingValue=value choices=groupChoices settingName=setting.setting}}
+{{list-setting settingValue=value choices=groupChoices settingName='name'}}
 {{setting-validation-message message=validationMessage}}
 <div class='desc'>{{{unbound setting.description}}}</div>

db/migrate/20190717133743_migrate_group_list_site_settings.rb

@@ -0,0 +1,27 @@
+# frozen_string_literal: true
+
+class MigrateGroupListSiteSettings < ActiveRecord::Migration[5.2]
+  def up
+    migrate_value(:name, :id)
+  end
+
+  def down
+    migrate_value(:id, :name)
+  end
+
+  def migrate_value(from, to)
+    cast_type = from == :id ? '::int[]' : ''
+    DB.exec <<~SQL
+      UPDATE site_settings
+      SET value = COALESCE(array_to_string(
+        (
+          SELECT array_agg(groups.#{to})
+          FROM groups
+          WHERE groups.#{from} = ANY (string_to_array(site_settings.value, '|', '')#{cast_type})
+        ),
+        '|', ''
+      ), site_settings.value)
+      WHERE data_type = #{SiteSettings::TypeSupervisor.types[:group_list]}
+    SQL
+  end
+end

lib/discourse.rb

@@ -743,5 +743,4 @@ module Discourse
   def self.skip_post_deployment_migrations?
     ['1', 'true'].include?(ENV["SKIP_POST_DEPLOYMENT_MIGRATIONS"]&.to_s)
   end
-
 end

test/javascripts/admin/components/group-list-setting-test.js.es6

@@ -32,7 +32,7 @@ componentTest("default", {
         setting: "foo_bar",
         type: "group_list",
         validValues: undefined,
-        value: "Donuts"
+        value: "1"
       })
     );
   },
@@ -42,16 +42,16 @@ componentTest("default", {
 
     assert.equal(
       subject.header().value(),
-      "Donuts",
+      "1",
       "it selects the setting's value"
     );
 
     await subject.expand();
-    await subject.selectRowByValue("Cheese cake");
+    await subject.selectRowByValue("2");
 
     assert.equal(
       subject.header().value(),
-      "Donuts,Cheese cake",
+      "1,2",
       "it allows to select a setting from the list of choices"
     );
   }