FIX: do not assign to same when already assigned

FIX: do not assign to same when already assigned
From a2e7fc4cace440114e73ad4ab85f650a9ab30dea Mon Sep 17 00:00:00 2001
From: Saurabh Patel <saurabh.finch@gmail.com>
Date: Thu, 6 Dec 2018 12:46:08 +0700
Subject: [PATCH] FIX: do not assign to same when already assigned


diff --git a/app/controllers/discourse_assign/assign_controller.rb b/app/controllers/discourse_assign/assign_controller.rb
index 2753480..32250bd 100644
--- a/app/controllers/discourse_assign/assign_controller.rb
+++ b/app/controllers/discourse_assign/assign_controller.rb
@@ -62,6 +62,10 @@ module DiscourseAssign
 
       raise Discourse::NotFound unless assign_to
 
+      if topic.custom_fields && topic.custom_fields['assigned_to_id'] == assign_to.id.to_s
+        return render json: { failed: 'Already assigned to the user' }, status: 400
+      end
+
       assigner = TopicAssigner.new(topic, current_user)
 
       # perhaps?
diff --git a/spec/requests/assign_controller_spec.rb b/spec/requests/assign_controller_spec.rb
new file mode 100644
index 0000000..1b63310
--- /dev/null
+++ b/spec/requests/assign_controller_spec.rb
@@ -0,0 +1,39 @@
+require 'rails_helper'
+
+RSpec.describe DiscourseAssign::AssignController do
+
+  let(:user) { Fabricate(:admin) }
+  let(:post) { Fabricate(:post) }
+  let(:user2) { Fabricate(:active_user) }
+
+  context 'assign' do
+
+    it 'assigns topic to a user' do
+      sign_in(user)
+
+      put '/assign/assign', params: {
+        topic_id: post.topic_id, username: user2.username
+      }
+
+      expect(response.status).to eq(200)
+    end
+
+    it 'fails to assign topic to the user if its already assigned to the same user' do
+      sign_in(user)
+
+      put '/assign/assign.json', params: {
+        topic_id: post.topic_id, username: user2.username
+      }
+
+      expect(response.status).to eq(200)
+
+      put '/assign/assign.json', params: {
+        topic_id: post.topic_id, username: user2.username
+      }
+
+      expect(response.status).to eq(400)
+    end
+
+  end
+
+end

GitHub

Follow up here is:

  1. We should localize 'Already assigned to the user' by adding the string into server.en and then using I18n.t to translate it.

  2. When you get the 400 you that the message you get back is correct by comparing

  3. When you get the 200 confirm the topic is properly assigned

Ok, noted.

I will make changes you requested in 1, 2 and 3.

Just one more question: how do you run unit-tests for discourse-assign engine? I just want to know if there is a simpler way.

Thanks

What I do is:

bin/rake autospec

Then I head to the test I want to run and hit save!

A trick I often use is insert “bang” or some other garbage string into my spec so it consistently fails while I am working on it, that way autospec stays focused on it.

But there is no file for rspec setup in engine? Do you run it inside discourse main repository while working on discourse-assign engine?

Yes I do, I check the plugin out to plugins/plugin-name and then run autospec in the root of discourse.

Also, related to this. When i run my test-cases, it fails because it doesn’t have poll table. Poll table is not created inside test environment. Do you run migrations for poll engine manually to get corresponding table for poll?

In dev environment, it is already created. So i guess its some setup issue. I can run it manually for time being though.

Correct me if i m wrong here about migrations.

Thanks

Yeah this is a terrible usability issue we have on dev @ZogStriP the simple workaround is to run:

LOAD_PLUGINS=1 RAILS_ENV=test bin/rake db:migrate

Having a look to see if I can force db migrate always to run with plugins in test

2 Likes

This issue should be a lot less painful going forward with:

2 Likes

Followed up in:

:blush: thanks @mrfinch

1 Like