Merge pull request #1276 from camallen/queue_improvements

Queue improvements

app/models/subject_queue.rb

@@ -98,9 +98,13 @@ def self.dequeue_update(query, sms_ids)
   # SQL optimisations welcome here...no postgres ops preseved order
   def self.enqueue_update(query, sms_ids)
     query.find_each do |sq|
-      sms_ids = Array.wrap(sms_ids)
-      enqueue_set = NonDuplicateSmsIds.new(sq, sms_ids).enqueue_sms_ids_set
-      sq.update_column(:set_member_subject_ids, enqueue_set)
+      if sq.below_minimum?
+        sms_ids = Array.wrap(sms_ids)
+        enqueue_set = NonDuplicateSmsIds.new(sq, sms_ids).enqueue_sms_ids_set
+        if !enqueue_set.empty? && enqueue_set != sq.set_member_subject_ids
+          sq.update_column(:set_member_subject_ids, enqueue_set)
+        end
+      end
     end
   end
 

lib/subject_selector.rb

@@ -77,9 +77,9 @@ def retrieve_subject_queue
 
     case
     when queue.nil?
-      queue = SubjectQueue.create_for_user(workflow, user.user, set: params[:subject_set_id])
+      queue = SubjectQueue.create_for_user(workflow, queue_user, set: params[:subject_set_id])
     when queue.below_minimum?
-      EnqueueSubjectQueueWorker.perform_async(workflow.id, user.id)
+      EnqueueSubjectQueueWorker.perform_async(workflow.id, queue_user.try(:id))
     end
     [queue, context]
   end

spec/lib/subject_selector_spec.rb

@@ -62,7 +62,6 @@
                subject_set: nil,
                set_member_subjects: [])
       end
-
       let!(:subjects) { create_list(:set_member_subject, 10, subject_set: workflow.subject_sets.first) }
 
       it 'should return 5 subjects' do
@@ -105,6 +104,56 @@
           subject.queued_subjects
         end
       end
+
+      describe "user has or workflow is finished" do
+        let(:queue_owner) { nil }
+        before(:each) do
+          subject_queue
+        end
+
+        shared_examples "enqueues for the logged out user" do
+          it 'should enqueue for logged out user' do
+            expect(EnqueueSubjectQueueWorker).to receive(:perform_async).with(workflow.id, nil)
+            subject.queued_subjects
+          end
+        end
+
+        shared_examples "creates for the logged out user" do
+          it 'should create for logged out user' do
+            expect(SubjectQueue).to receive(:create_for_user).with(workflow, nil, set: nil)
+            #non-logged in queue won't exist
+            expect { subject.queued_subjects }.to raise_error(SubjectSelector::MissingSubjectQueue)
+          end
+        end
+
+        context "when the workflow is finished" do
+          before(:each) do
+            allow_any_instance_of(Workflow).to receive(:finished?).and_return(true)
+          end
+
+          it_behaves_like "enqueues for the logged out user"
+
+          context "when the logged_out queue doesn't exist" do
+            let(:queue_owner) { user.user }
+
+            it_behaves_like "creates for the logged out user"
+          end
+        end
+
+        context "when the user has finished the workflow" do
+          before(:each) do
+            allow_any_instance_of(User).to receive(:has_finished?).and_return(true)
+          end
+
+          it_behaves_like "enqueues for the logged out user"
+
+          context "when the logged_out queue doesn't exist" do
+            let(:queue_owner) { user.user }
+
+            it_behaves_like "creates for the logged out user"
+          end
+        end
+      end
     end
   end
 end

spec/models/subject_queue_spec.rb

@@ -223,8 +223,8 @@
             }.to_not raise_error
           end
 
-          it 'not attempt to find or create a queue' do
-            expect(SubjectQueue).to_not receive(:find_or_create_by!)
+          it 'not attempt to find or create a queue'  do
+            expect(SubjectQueue).to_not receive(:where)
             SubjectQueue.enqueue(workflow, [], user: user)
           end
 
@@ -259,7 +259,7 @@
         end
       end
 
-      context "list exists for user" do
+      context "subject queue exists for user" do
         let!(:smses) { create_list(:set_member_subject, 3, subject_set: sms.subject_set) }
         let!(:ues) do
           create(:subject_queue,
@@ -268,9 +268,25 @@
                  workflow: workflow)
         end
 
-        it 'should call add_subject_id on the existing subject queue' do
-          SubjectQueue.enqueue(workflow, sms.id, user: user)
-          expect(ues.reload.set_member_subject_ids).to include(sms.id)
+        context "when the queue is above the enqueue threshold" do
+
+          it 'should add the sms id to the existing subject queue' do
+            allow_any_instance_of(SubjectQueue).to receive(:below_minimum?).and_return(true)
+            SubjectQueue.enqueue(workflow, sms.id, user: user)
+            expect(ues.reload.set_member_subject_ids).to include(sms.id)
+          end
+        end
+
+        context "when the queue is above the enqueue threshold" do
+
+          it 'should not add the sms id to the existing subject queue' do
+            allow_any_instance_of(SubjectQueue).to receive(:below_minimum?).and_return(false)
+            SubjectQueue.enqueue(workflow, sms.id, user: user)
+            aggregate_failures "don't enqueue" do
+              expect_any_instance_of(SubjectQueue).not_to receive(:enqueue_update)
+              expect(ues.reload.set_member_subject_ids).not_to include(sms.id)
+            end
+          end
         end
 
         it 'should not have a duplicate in the set' do
@@ -306,6 +322,26 @@
           end
           let(:enq_ids_with_dups) { [ sms.id ] | q_dups }
 
+          context "when the append queue is empty" do
+
+            it "should not attempt an enqueue" do
+              allow_any_instance_of(NonDuplicateSmsIds).to receive(:enqueue_sms_ids_set)
+                .and_return([])
+              expect_any_instance_of(SubjectQueue).not_to receive(:update_column)
+              SubjectQueue.enqueue_update(query, [])
+            end
+          end
+
+          context "when the append queue has not changed" do
+
+            it "should not attempt an enqueue" do
+              allow_any_instance_of(NonDuplicateSmsIds).to receive(:enqueue_sms_ids_set)
+                .and_return(ues.set_member_subject_ids)
+              expect_any_instance_of(SubjectQueue).not_to receive(:update_column)
+              SubjectQueue.enqueue_update(query, [])
+            end
+          end
+
           context "when the append queue has dups" do
 
             it "should notify HB with a custom error" do