UX: drop the `automatic_membership_retroactive` column from groups model. (#9430)

UX: drop the automatic_membership_retroactive column from groups model. (#9430)

diff --git a/app/assets/javascripts/discourse/components/group-manage-save-button.js b/app/assets/javascripts/discourse/components/group-manage-save-button.js
index 9adcf9e..31217a1 100644
--- a/app/assets/javascripts/discourse/components/group-manage-save-button.js
+++ b/app/assets/javascripts/discourse/components/group-manage-save-button.js
@@ -1,6 +1,7 @@
 import discourseComputed from "discourse-common/utils/decorators";
 import Component from "@ember/component";
 import { popupAjaxError } from "discourse/lib/ajax-error";
+import { popupAutomaticMembershipAlert } from "discourse/controllers/groups-new";
 
 export default Component.extend({
   saving: null,
@@ -14,8 +15,14 @@ export default Component.extend({
   actions: {
     save() {
       this.set("saving", true);
+      const group = this.model;
 
-      return this.model
+      popupAutomaticMembershipAlert(
+        group.id,
+        group.automatic_membership_email_domains
+      );
+
+      return group
         .save()
         .then(() => {
           this.set("saved", true);
diff --git a/app/assets/javascripts/discourse/controllers/groups-new.js b/app/assets/javascripts/discourse/controllers/groups-new.js
index 05013cb..25cefd4 100644
--- a/app/assets/javascripts/discourse/controllers/groups-new.js
+++ b/app/assets/javascripts/discourse/controllers/groups-new.js
@@ -1,18 +1,54 @@
 import { action } from "@ember/object";
 import Controller from "@ember/controller";
+import { ajax } from "discourse/lib/ajax";
 import { popupAjaxError } from "discourse/lib/ajax-error";
 
+export function popupAutomaticMembershipAlert(group_id, email_domains) {
+  if (!email_domains) {
+    return;
+  }
+
+  const data = {};
+  data.automatic_membership_email_domains = email_domains;
+
+  if (group_id) {
+    data.id = group_id;
+  }
+
+  ajax(`/admin/groups/automatic_membership_count.json`, {
+    type: "PUT",
+    data
+  }).then(result => {
+    const count = result.user_count;
+
+    if (count > 0) {
+      bootbox.alert(
+        I18n.t(
+          "admin.groups.manage.membership.automatic_membership_user_count",
+          { count }
+        )
+      );
+    }
+  });
+}
+
 export default Controller.extend({
   saving: null,
 
   @action
   save() {
     this.set("saving", true);
+    const group = this.model;
+
+    popupAutomaticMembershipAlert(
+      group.id,
+      group.automatic_membership_email_domains
+    );
 
-    this.model
+    group
       .create()
       .then(() => {
-        this.transitionToRoute("group.members", this.model.name);
+        this.transitionToRoute("group.members", group.name);
       })
       .catch(popupAjaxError)
       .finally(() => this.set("saving", false));
diff --git a/app/assets/javascripts/discourse/models/group.js b/app/assets/javascripts/discourse/models/group.js
index 43e1b52..ea4d03d 100644
--- a/app/assets/javascripts/discourse/models/group.js
+++ b/app/assets/javascripts/discourse/models/group.js
@@ -184,7 +184,6 @@ const Group = RestModel.extend({
       visibility_level: this.visibility_level,
       members_visibility_level: this.members_visibility_level,
       automatic_membership_email_domains: this.emailDomains,
-      automatic_membership_retroactive: !!this.automatic_membership_retroactive,
       title: this.title,
       primary_group: !!this.primary_group,
       grant_trust_level: this.grant_trust_level,
diff --git a/app/assets/javascripts/discourse/templates/components/groups-form-membership-fields.hbs b/app/assets/javascripts/discourse/templates/components/groups-form-membership-fields.hbs
index b92c678..2b4fdfa 100644
--- a/app/assets/javascripts/discourse/templates/components/groups-form-membership-fields.hbs
+++ b/app/assets/javascripts/discourse/templates/components/groups-form-membership-fields.hbs
@@ -17,14 +17,6 @@
       onChange=(action "onChangeEmailDomainsSetting")
       options=(hash allowAny=true)
     }}
-
-    <label>
-      {{input type="checkbox"
-          checked=model.automatic_membership_retroactive
-          class="groups-form-automatic-membership-retroactive"}}
-
-      {{i18n "admin.groups.manage.membership.automatic_membership_retroactive"}}
-    </label>
   </div>
 
   {{plugin-outlet name="groups-form-membership-below-automatic"
diff --git a/app/controllers/admin/groups_controller.rb b/app/controllers/admin/groups_controller.rb
index 0583187..a8413eb 100644
--- a/app/controllers/admin/groups_controller.rb
+++ b/app/controllers/admin/groups_controller.rb
@@ -121,6 +121,28 @@ class Admin::GroupsController < Admin::AdminController
     render json: success_json
   end
 
+  def automatic_membership_count
+    domains = Group.get_valid_email_domains(params.require(:automatic_membership_email_domains))
+    group_id = params[:id]
+    user_count = 0
+
+    if domains.present?
+      if group_id.present?
+        group = Group.find_by(id: group_id)
+        raise Discourse::NotFound unless group
+
+        return can_not_modify_automatic if group.automatic
+
+        existing_domains = group.automatic_membership_email_domains.split("|")
+        domains -= existing_domains
+      end
+
+      user_count = Group.automatic_membership_users(domains.join("|")).count
+    end
+
+    render json: { user_count: user_count }
+  end
+
   protected
 
   def can_not_modify_automatic
@@ -137,7 +159,6 @@ class Admin::GroupsController < Admin::AdminController
       :visibility_level,
       :members_visibility_level,
       :automatic_membership_email_domains,
-      :automatic_membership_retroactive,
       :title,
       :primary_group,
       :grant_trust_level,
diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb
index 0df1f49..c9a623d 100644
--- a/app/controllers/groups_controller.rb
+++ b/app/controllers/groups_controller.rb
@@ -537,7 +537,6 @@ class GroupsController < ApplicationController
             :name,
             :grant_trust_level,
             :automatic_membership_email_domains,
-            :automatic_membership_retroactive,
             :publish_read_state
           ])
 
diff --git a/app/jobs/regular/automatic_group_membership.rb b/app/jobs/regular/automatic_group_membership.rb
index e10bc7a..bf9f6b2 100644
--- a/app/jobs/regular/automatic_group_membership.rb
+++ b/app/jobs/regular/automatic_group_membership.rb
@@ -11,16 +11,10 @@ module Jobs
       group = Group.find_by(id: group_id)
       raise Discourse::InvalidParameters.new(:group_id) if group.nil?
 
-      return unless group.automatic_membership_retroactive
+      domains = group.automatic_membership_email_domains
+      return if domains.blank?
 
-      domains = group.automatic_membership_email_domains.gsub('.', '\.')
-
-      User.joins(:user_emails)
-        .where("user_emails.email ~* '@(#{domains})$'")
-        .where("users.id NOT IN (SELECT user_id FROM group_users WHERE group_users.group_id = ?)", group_id)
-        .activated
-        .where(staged: false)
-        .find_each do |user|
+      Group.automatic_membership_users(domains).find_each do |user|
         next unless user.email_confirmed?
         group.add(user, automatic: true)
         GroupActionLogger.new(Discourse.system_user, group).log_add_user_to_group(user)
diff --git a/app/models/group.rb b/app/models/group.rb
index 0f8f3d8..3e9db51 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -770,12 +770,10 @@ class Group < ActiveRecord::Base
   def automatic_membership_email_domains_format_validator
     return if self.automatic_membership_email_domains.blank?
 
-    domains = self.automatic_membership_email_domains.split("|")
-    domains.each do |domain|
-      domain.sub!(/^https?:\/\//, '')
-      domain.sub!(/\/.*$/, '')
+    domains = Group.get_valid_email_domains(self.automatic_membership_email_domains) do |domain|
       self.errors.add :base, (I18n.t('groups.errors.invalid_domain', domain: domain)) unless domain =~ /\A[a-z0-9]+([\-\.]{1}[a-z0-9]+)*\.[a-z]{2,24}(:[0-9]{1,5})?(\/.*)?\Z/i
     end
+

[... diff too long, it was truncated ...]

GitHub sha: df0c386f

This commit appears in #9430 which was approved by eviltrout. It was merged by vinothkannans.