Merge remote-tracking branch 'discourse/master' into emoji-5.0

app/models/group.rb

@@ -36,6 +36,7 @@ class Group < ActiveRecord::Base
   validates_uniqueness_of :name, case_sensitive: false
   validate :automatic_membership_email_domains_format_validator
   validate :incoming_email_validator
+  validate :can_allow_membership_requests
   validates :flair_url, url: true, if: Proc.new { |g| g.flair_url && g.flair_url[0,3] != 'fa-' }
 
   AUTO_GROUPS = {
@@ -511,6 +512,12 @@ SQL
 
   private
 
+    def can_allow_membership_requests
+      if self.allow_membership_requests && !self.group_users.where(owner: true).exists?
+        self.errors.add(:base, I18n.t('groups.errors.cant_allow_membership_requests'))
+      end
+    end
+
     def enqueue_update_mentions_job
       Jobs.enqueue(:update_group_mentions,
         previous_name: self.name_was,

config/locales/server.en.yml

@@ -264,6 +264,7 @@ en:
       invalid_incoming_email: "'%{email}' is not a valid email address."
       email_already_used_in_group: "'%{email}' is already used by the group '%{group_name}'."
       email_already_used_in_category: "'%{email}' is already used by the category '%{category_name}'."
+      cant_allow_membership_requests: "You cannot allow membership requests for a group without any owners."
     default_names:
       everyone: "everyone"
       admins: "admins"

db/migrate/20170524182846_add_unread_tracking_columns.rb

@@ -4,6 +4,6 @@ class AddUnreadTrackingColumns < ActiveRecord::Migration
   end
 
   def down
-    raise "Can not be reverted"
+    raise ActiveRecord::IrreversibleMigration
   end
 end

db/migrate/20170526125321_drop_unread_tracking_columns.rb

@@ -3,6 +3,6 @@ class DropUnreadTrackingColumns < ActiveRecord::Migration
   end
 
   def down
-    raise "Can not be reverted"
+    raise ActiveRecord::IrreversibleMigration
   end
 end

db/migrate/20170602132735_fix_group_allow_membership_requests.rb

@@ -0,0 +1,13 @@
+class FixGroupAllowMembershipRequests < ActiveRecord::Migration
+  def up
+    execute <<~SQL
+    UPDATE groups g
+    SET allow_membership_requests = 'f'
+    WHERE NOT EXISTS (SELECT 1 FROM group_users gu WHERE gu.owner = 't' AND gu.group_id = g.id LIMIT 1)
+    SQL
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end

plugins/discourse-narrative-bot/lib/discourse_narrative_bot/actions.rb

@@ -77,5 +77,20 @@ module DiscourseNarrativeBot
       topic.pm_with_non_human_user? &&
         topic.topic_allowed_users.where(user_id: -2).exists?
     end
+
+    def cancel_timeout_job(user)
+      Jobs.cancel_scheduled_job(:narrative_timeout, user_id: user.id, klass: self.class.to_s)
+    end
+
+    def enqueue_timeout_job(user)
+      return if Rails.env.test?
+
+      cancel_timeout_job(user)
+
+      Jobs.enqueue_in(TIMEOUT_DURATION, :narrative_timeout,
+        user_id: user.id,
+        klass: self.class.to_s
+      )
+    end
   end
 end

plugins/discourse-narrative-bot/lib/discourse_narrative_bot/base.rb

@@ -163,21 +163,6 @@ module DiscourseNarrativeBot
       topic_id == @data[:topic_id]
     end
 
-    def cancel_timeout_job(user)
-      Jobs.cancel_scheduled_job(:narrative_timeout, user_id: user.id, klass: self.class.to_s)
-    end
-
-    def enqueue_timeout_job(user)
-      return if Rails.env.test?
-
-      cancel_timeout_job(user)
-
-      Jobs.enqueue_in(TIMEOUT_DURATION, :narrative_timeout,
-        user_id: user.id,
-        klass: self.class.to_s
-      )
-    end
-
     def not_implemented
       raise 'Not implemented.'
     end

plugins/discourse-narrative-bot/lib/discourse_narrative_bot/track_selector.rb

@@ -28,7 +28,7 @@ module DiscourseNarrativeBot
     def select
       data = Store.get(@user.id)
 
-      if @post && !is_topic_action?
+      if @post && @post.post_type == Post.types[:regular] && !is_topic_action?
         is_reply = @input == :reply
         return if is_reply && reset_track
 
@@ -39,7 +39,7 @@ module DiscourseNarrativeBot
           klass = (data[:track] || NewUserNarrative.to_s).constantize
 
           if is_reply && like_user_post
-            Store.set(@user.id, data.merge!(state: nil, topic_id: nil))
+            terminate_track(data)
           elsif state&.to_sym == :end && is_reply
             bot_commands(bot_mentioned?) || generic_replies(klass.reset_trigger)
           elsif is_reply
@@ -253,5 +253,10 @@ module DiscourseNarrativeBot
       !SiteSetting.discourse_narrative_bot_disable_public_replies &&
         (bot_mentioned? || reply_to_bot_post?(@post))
     end
+
+    def terminate_track(data)
+      Store.set(@user.id, data.merge!(state: nil, topic_id: nil))
+      cancel_timeout_job(@user)
+    end
   end
 end

plugins/discourse-narrative-bot/spec/discourse_narrative_bot/track_selector_spec.rb

@@ -104,6 +104,18 @@ describe DiscourseNarrativeBot::TrackSelector do
           end
         end
 
+        context 'when a non regular post is created' do
+          it 'should not do anything' do
+            moderator_post = Fabricate(:moderator_post, user: user, topic: topic)
+
+            expect do
+              described_class.new(
+                :reply, user, post_id: moderator_post.id
+              ).select
+            end.to_not change { Post.count }
+          end
+        end
+
         describe 'when user thank the bot' do
           it 'should like the post' do
             post.update!(raw: 'thanks!')
@@ -115,9 +127,6 @@ describe DiscourseNarrativeBot::TrackSelector do
 
             expect(post_action.post).to eq(post)
             expect(post_action.post_action_type_id).to eq(PostActionType.types[:like])
-
-            post = Post.last
-
             expect(Post.last).to eq(post)
 
             expect(DiscourseNarrativeBot::NewUserNarrative.new.get_data(user)['state'])

spec/controllers/admin/groups_controller_spec.rb

@@ -1,6 +1,8 @@
 require 'rails_helper'
 
 describe Admin::GroupsController do
+  let(:user) { Fabricate(:user) }
+  let(:group) { Fabricate(:group) }
 
   before do
     @admin = log_in(:admin)
@@ -77,7 +79,7 @@ describe Admin::GroupsController do
     end
   end
 
-  context ".create" do
+  context "#create" do
 
     it "strip spaces on the group name" do
       xhr :post, :create, { group: { name: " bob " } }
@@ -92,23 +94,33 @@ describe Admin::GroupsController do
 
   end
 
-  context ".update" do
+  context "#update" do
+    it 'should update a group' do
+      group.add_owner(user)
+
+      expect do
+        xhr :put, :update, { id: group.id, group: {
+          visible: "false",
+          allow_membership_requests: "true"
+        } }
+      end.to change { GroupHistory.count }.by(2)
+
+      expect(response).to be_success
+
+      group.reload
+      expect(group.visible).to eq(false)
+      expect(group.allow_membership_requests).to eq(true)
+    end
 
     it "ignore name change on automatic group" do
       expect do
-        xhr :put, :update, { id: 1, group: {
-          name: "WAT",
-          visible: "true",
-          allow_membership_requests: "true"
-        } }
+        xhr :put, :update, { id: 1, group: { name: "WAT" } }
       end.to change { GroupHistory.count }.by(1)
 
       expect(response).to be_success
 
       group = Group.find(1)
       expect(group.name).not_to eq("WAT")
-      expect(group.visible).to eq(true)
-      expect(group.allow_membership_requests).to eq(true)
     end
 
     it "doesn't launch the 'automatic group membership' job when it's not retroactive" do

spec/models/group_spec.rb

@@ -77,6 +77,26 @@ describe Group do
       group.incoming_email = "foo@bar.org"
       expect(group.valid?).to eq(true)
     end
+
+    context 'when a group has no owners' do
+      it 'should not allow membership requests' do
+        group.allow_membership_requests = true
+
+        expect(group.valid?).to eq(false)
+
+        expect(group.errors.full_messages).to include(I18n.t(
+          "groups.errors.cant_allow_membership_requests"
+        ))
+
+        group.allow_membership_requests = false
+        group.save!
+
+        group.add_owner(user)
+        group.allow_membership_requests = true
+
+        expect(group.valid?).to eq(true)
+      end
+    end
   end
 
   def real_admins