Questions can belong to many categories

While starting to input quiz questions for the arch tester quizzes we
found out that it would be best if questions could belong to many
categories. Now the relationship between questions and categories is
many to many. Bug #356179.

20 files changed, 129 insertions, 55 deletions

diff --git a/app/models/answer.rb b/app/models/answer.rb
index b214ebb..26a273f 100644
--- a/app/models/answer.rb
+++ b/app/models/answer.rb
@@ -45,7 +45,7 @@ class Answer < ActiveRecord::Base
     :joins => :owner, :conditions => { 'users.mentor_id', mentor } } }
 
   named_scope   :in_category, lambda { |category| {
-    :joins => :question, :conditions => { 'questions.category_id', category} } }
+    :joins => {:question => :question_categories}, :conditions => { 'question_categories.category_id', category} } }
 
   named_scope   :with_feedback, lambda { |opt| {
     :conditions => { :feedback => opt } } }
diff --git a/app/models/category.rb b/app/models/category.rb
index a52a50d..146921b 100644
--- a/app/models/category.rb
+++ b/app/models/category.rb
@@ -25,7 +25,8 @@ class Category < ActiveRecord::Base
 
   validates_presence_of :name
 
-  has_many :questions
+  has_many :question_categories
+  has_many :questions, :through => :question_categories
   has_many :user_categories
   has_many :users, :through => :user_categories, :accessible => true
 
diff --git a/app/models/question.rb b/app/models/question.rb
index a660de6..fc9b176 100644
--- a/app/models/question.rb
+++ b/app/models/question.rb
@@ -39,7 +39,8 @@ class Question < ActiveRecord::Base
   #maybe add a page for not complete questions
 
   belongs_to  :user, :creator => true
-  belongs_to  :category
+  has_many    :question_categories
+  has_many    :categories, :through => :question_categories
   belongs_to  :question_group
   has_many    :answers
   has_one     :reference_answer, :class_name => "Answer", :conditions => ["answers.reference = ?", true]
@@ -58,7 +59,7 @@ class Question < ActiveRecord::Base
       return true if new_record?
 
       # when it's not new record allow changing only some properties
-      return only_changed?(:title, :content, :documentation, :category)
+      return only_changed?(:title, :content, :documentation)
     end
 
     false
@@ -89,7 +90,7 @@ class Question < ActiveRecord::Base
   end
 
   named_scope :unanswered_ungrouped, lambda { |uid|{
-      :joins => {:category => :user_categories},
+      :joins => {:question_categories => {:category => :user_categories}},
       :conditions => [ 'user_categories.user_id = ? AND questions.question_group_id IS NULL ' +
       'AND NOT EXISTS (' +
       'SELECT * FROM answers WHERE answers.owner_id = ? AND answers.question_id = questions.id)',
@@ -109,7 +110,7 @@ class Question < ActiveRecord::Base
       :conditions => ['questions.id != ?', id]}}
 
   named_scope :ungrouped_questions_of_user, lambda { |user_id|{
-    :joins => {:category => :user_categories},
+    :joins => {:question_categories => {:category => :user_categories}},
     :conditions => ['user_categories.user_id = ? AND questions.question_group_id IS NULL', user_id]}}
 
   named_scope :grouped_questions_of_user, lambda { |user_id|{
@@ -120,7 +121,7 @@ class Question < ActiveRecord::Base
     :conditions => { :user_id => user_id, :approved => false }}}
 
   named_scope :unanswered, lambda { |uid|{
-      :joins => {:category => {:user_categories => :user}},
+      :joins => {:question_categories => {:category => {:user_categories => :user}}},
       :conditions => [ 'users.id = ? AND NOT EXISTS ( ' +
       'SELECT * FROM answers WHERE answers.owner_id = ? AND answers.question_id = questions.id)', uid, uid]}}
 
@@ -183,9 +184,8 @@ class Question < ActiveRecord::Base
   protected
     # Sends notification about new question (TODO: check for group).
     def notify_new_question
-      # If category isn't assigned don't try to access it
-      if category && approved
-        for user in category.users
+      if approved
+        for user in categories.*.users.flatten
           UserMailer.delay.deliver_new_question(user, self)
         end
       end
@@ -193,7 +193,7 @@ class Question < ActiveRecord::Base
 
     # Sends notification about new question (TODO: check for group).
     def notify_approved_question
-      if category && !approved_was && approved
+      if !approved_was && approved
         notify_new_question
       end
     end
diff --git a/app/models/question_category.rb b/app/models/question_category.rb
new file mode 100644
index 0000000..c67ce9b
--- /dev/null
+++ b/app/models/question_category.rb
@@ -0,0 +1,27 @@
+#   Gentoo Recruiters Web App - to help Gentoo recruiters do their job better
+#   Copyright (C) 2011 Petteri Räty
+#
+#   This program is free software: you can redistribute it and/or modify
+#   it under the terms of the GNU Affero General Public License as
+#   published by the Free Software Foundation, version 3 of the License
+#
+#   This program is distributed in the hope that it will be useful,
+#   but WITHOUT ANY WARRANTY; without even the implied warranty of
+#   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+#   GNU Affero General Public License for more details.
+#
+#   You should have received a copy of the GNU Affero General Public License
+#   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Associates questions to categories
+class QuestionCategory < ActiveRecord::Base
+
+  hobo_model # Don't put anything above this
+
+  fields do
+  end
+
+  belongs_to :question, :null => false, :index => false
+  belongs_to :category, :null => false, :index => false
+  index [:question_id, :category_id], :unique => true
+end
diff --git a/app/models/question_group.rb b/app/models/question_group.rb
index 4ecd037..fa06cc9 100644
--- a/app/models/question_group.rb
+++ b/app/models/question_group.rb
@@ -32,12 +32,12 @@ class QuestionGroup < ActiveRecord::Base
   include Permissions::AnyoneCanViewAdminCanChange
 
   named_scope :in_category, lambda { |cid| {
-    :joins => :questions, :conditions => ['questions.category_id = ?', cid],
+    :joins => {:questions => :question_categories}, :conditions => ['question_categories.category_id = ?', cid],
     :group => 'question_groups.id, question_groups.name, question_groups.description,
       question_groups.created_at, question_groups.updated_at'}}
 
   named_scope :unassociated_in_category, lambda { |uid, cid| {
-    :joins => :questions, :conditions => ['questions.category_id = ? AND NOT EXISTS
+    :joins => {:questions => :question_categories}, :conditions => ['question_categories.category_id = ? AND NOT EXISTS
       (SELECT user_question_groups.* FROM user_question_groups INNER JOIN questions ON
       questions.id = user_question_groups.question_id WHERE (user_question_groups.user_id = ?
       AND questions.question_group_id = question_groups.id))', cid, uid],
diff --git a/app/models/user.rb b/app/models/user.rb
index ca7af6c..dff9295 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -49,9 +49,10 @@ class User < ActiveRecord::Base
   named_scope :mentorless_recruits, :conditions => { :role => 'recruit', :mentor_id => nil}
   named_scope :recruits_answered_all, :conditions => "role = 'recruit' AND NOT EXISTS
     (SELECT questions.id FROM questions
-    INNER JOIN categories cat ON questions.category_id = cat.id INNER JOIN
-    user_categories ON user_categories.category_id = cat.id  WHERE
-    user_categories.user_id = users.id AND questions.question_group_id IS NULL AND NOT EXISTS (
+    INNER JOIN question_categories ON question_categories.question_id = questions.id
+    INNER JOIN categories cat ON question_categories.category_id = cat.id
+    INNER JOIN user_categories ON user_categories.category_id = cat.id
+    WHERE user_categories.user_id = users.id AND questions.question_group_id IS NULL AND NOT EXISTS (
     SELECT answers.id FROM answers WHERE answers.question_id = questions.id AND answers.owner_id = users.id))
     AND NOT EXISTS
     (SELECT questions.id FROM questions INNER JOIN user_question_groups ON questions.id = user_question_groups.question_id
diff --git a/app/models/user_mailer.rb b/app/models/user_mailer.rb
index 4acaff2..71c03e2 100644
--- a/app/models/user_mailer.rb
+++ b/app/models/user_mailer.rb
@@ -32,8 +32,7 @@ class UserMailer < ActionMailer::Base
 
   def new_question(user, question)
     common(user, "New question")
-    @body = { :title=> question.title, :category => question.category,
-      :id => question.id}
+    @body = { :title=> question.title, :id => question.id}
   end
 
   def new_answer(user, answer)
diff --git a/app/views/user_mailer/new_question.erb b/app/views/user_mailer/new_question.erb
index 4005ec7..d97737a 100644
--- a/app/views/user_mailer/new_question.erb
+++ b/app/views/user_mailer/new_question.erb
@@ -1,3 +1,3 @@
-There is a new question "<%=@title%>" in category "<%=@category%>" you are assigned to.
+There is a new question "<%=@title%>" in one of the categories you are assigned to.
 
 <%= question_url(@id) %>
diff --git a/db/migrate/20110312181715_add_question_category_pivot.rb b/db/migrate/20110312181715_add_question_category_pivot.rb
new file mode 100644
index 0000000..ef1a810
--- /dev/null
+++ b/db/migrate/20110312181715_add_question_category_pivot.rb
@@ -0,0 +1,21 @@
+class AddQuestionCategoryPivot < ActiveRecord::Migration
+  def self.up
+    create_table :question_categories do |t|
+      t.integer :question_id, :null => false
+      t.integer :category_id, :null => false
+    end
+    add_index :question_categories, [:question_id, :category_id], :unique => true
+
+    remove_column :questions, :category_id
+
+    remove_index :questions, :name => :index_questions_on_category_id rescue ActiveRecord::StatementInvalid
+  end
+
+  def self.down
+    add_column :questions, :category_id, :integer
+
+    drop_table :question_categories
+
+    add_index :questions, [:category_id]
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index cbdde12..140e762 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -9,7 +9,7 @@
 #
 # It's strongly recommended to check this file into your version control system.
 
-ActiveRecord::Schema.define(:version => 20110312173240) do
+ActiveRecord::Schema.define(:version => 20110312181715) do
 
   create_table "answers", :force => true do |t|
     t.text     "content",     :default => "",    :null => false
@@ -78,6 +78,13 @@ ActiveRecord::Schema.define(:version => 20110312173240) do
 
   add_index "project_acceptances", ["user_id"], :name => "index_project_acceptances_on_user_id"
 
+  create_table "question_categories", :force => true do |t|
+    t.integer "question_id", :null => false
+    t.integer "category_id", :null => false
+  end
+
+  add_index "question_categories", ["question_id", "category_id"], :name => "index_question_categories_on_question_id_and_category_id", :unique => true
+
   create_table "question_content_emails", :force => true do |t|
     t.text     "requirements", :default => "", :null => false
     t.text     "description"
@@ -120,11 +127,9 @@ ActiveRecord::Schema.define(:version => 20110312173240) do
     t.datetime "created_at"
     t.datetime "updated_at"
     t.integer  "user_id"
-    t.integer  "category_id"
     t.integer  "question_group_id"
   end
 
-  add_index "questions", ["category_id"], :name => "index_questions_on_category_id"
   add_index "questions", ["question_group_id"], :name => "index_questions_on_question_group_id"
   add_index "questions", ["user_id"], :name => "index_questions_on_user_id"
 
diff --git a/features/clean_ui.feature b/features/clean_ui.feature
index 237136e..f475abf 100644
--- a/features/clean_ui.feature
+++ b/features/clean_ui.feature
@@ -63,7 +63,7 @@ Feature: Clean UI
 
     Given following questions:
       |question 1|category|
-      |question 3|category|
+      |question 2|category|
       |question 3|category|
     Given email question content for "question 1"
     Given text content "something" for question "question 2"
diff --git a/features/step_definitions/questions_steps.rb b/features/step_definitions/questions_steps.rb
index b0cd6fd..7f96e30 100644
--- a/features/step_definitions/questions_steps.rb
+++ b/features/step_definitions/questions_steps.rb
@@ -12,8 +12,7 @@ end
 Given /^a question "([^\"]*)" in category "([^\"]*)"$/ do |title, category|
   Given "a question \"#{title}\""
   Given "a category \"#{category}\""
-  @question.category = @category
-  @question.save!
+  QuestionCategory.create!(:category => @category, :question => @question)
 end
 
 Given /^a question "([^\"]*)" in group "([^\"]*)"$/ do |title, group|
diff --git a/spec/factories.rb b/spec/factories.rb
index 2c663af..3ead1e1 100644
--- a/spec/factories.rb
+++ b/spec/factories.rb
@@ -51,6 +51,11 @@
     q.name  { Factory.next(:category) }
   end
 
+  Factory.define :question_category do |qc|
+    qc.association :question
+    qc.association :category
+  end
+
   Factory.sequence :question do |n|
     "question-#{n}"
   end
@@ -58,7 +63,9 @@
   # it'll belong to new category by default
   Factory.define :question do |q|
     q.title             { Factory.next(:question) }
-    q.category { Factory(:category)}
+    q.after_build { |q|
+      q.categories = [Factory.build :category]
+    }
   end
 
   Factory.sequence :answer do |n|
diff --git a/spec/models/answer_spec.rb b/spec/models/answer_spec.rb
index e300537..61e23a8 100644
--- a/spec/models/answer_spec.rb
+++ b/spec/models/answer_spec.rb
@@ -250,7 +250,6 @@ describe Answer do
         (!produced_ans.attributes[i[0]]).should be_true # it can be nil or false
       end
     end
-
   end
 
   it "should produce proper updated answer from params" do
@@ -270,10 +269,10 @@ describe Answer do
   it "should properly return wrong answers of recruit" do
     recruit = Factory(:recruit)
     cat     = Factory(:category)
-    q1      = Factory(:question, :category => cat)
-    q2      = Factory(:question, :category => cat)
-    q3      = Factory(:question, :category => cat)
-    q4      = Factory(:question, :category => cat)
+    q1      = Factory(:question_category, :category => cat).question
+    q2      = Factory(:question_category, :category => cat).question
+    q3      = Factory(:question_category, :category => cat).question
+    q4      = Factory(:question_category, :category => cat).question
 
               Factory(:question_content_text, :question => q4)
 
diff --git a/spec/models/question_group_spec.rb b/spec/models/question_group_spec.rb
index b490f89..dc62f83 100644
--- a/spec/models/question_group_spec.rb
+++ b/spec/models/question_group_spec.rb
@@ -23,7 +23,9 @@ describe QuestionGroup do
     for n in 1..5
       groups_in_cat.push Factory(:question_group)
       for i in 1..n
-        Factory(:question, :category => category, :question_group => groups_in_cat.last)
+        Factory(:question_category,
+                :category => category,
+                :question => Factory(:question, :question_group => groups_in_cat.last))
       end
     end
 
@@ -44,7 +46,9 @@ describe QuestionGroup do
     for n in 1..5
       groups_in_cat.push Factory(:question_group)
       for i in 1..n
-        Factory(:question, :category => category, :question_group => groups_in_cat.last)
+        Factory(:question_category,
+                :category => category,
+                :question => Factory(:question, :question_group => groups_in_cat.last))
       end
     end
 
diff --git a/spec/models/question_spec.rb b/spec/models/question_spec.rb
index 5dddd70..99a286f 100644
--- a/spec/models/question_spec.rb
+++ b/spec/models/question_spec.rb
@@ -68,8 +68,8 @@ describe Question do
   it "should send email notifications to watching recruits when created by recruiter" do
     category  = Factory(:category)
     recruit   = Factory(:recruit, :categories => [category])
-    question  = Question.new(:title => "new question",
-      :category => category)
+    question  = Factory.build(:question)
+    question.categories << category
 
     UserMailer.should_receive_delayed(:deliver_new_question, recruit, question)
 
@@ -79,19 +79,19 @@ describe Question do
   it "should send email notifications to watching recruits when approved" do
     category  = Factory(:category)
     recruit   = Factory(:recruit, :categories => [category])
-    question  = Factory(:question, :title => "new question",
-      :category => category, :user => Factory(:recruit))
+    question  = Factory(:question, :user => Factory(:recruit))
+    question.categories << category
 
     UserMailer.should_receive_delayed(:deliver_new_question, recruit, question)
     question.approved = true
     question.save!
   end
 
-  it "should not send email notifications to watching recruits when approved is changed" do
+  it "should not send email notifications to watching recruits when approved is not changed" do
     category  = Factory(:category)
     recruit   = Factory(:recruit, :categories => [category])
-    question  = Factory(:question, :title => "new question",
-      :category => category, :user => Factory(:recruit), :approved => true)
+    question  = Factory(:question, :user => Factory(:recruit), :approved => true)
+    question.categories << category
 
     UserMailer.should_not_receive(:deliver_new_question).with(recruit, question)
 
@@ -146,7 +146,6 @@ describe Question do
     question.should     be_editable_by(recruit)
     question.should     be_editable_by(recruit, :title)
     question.should     be_editable_by(recruit, :documentation)
-    question.should     be_editable_by(recruit, :category)
 
     question.should_not be_editable_by(recruit, :user)
     question.should_not be_editable_by(recruit, :approved)
@@ -159,7 +158,6 @@ describe Question do
     question.should     be_editable_by(recruit)
     question.should     be_editable_by(recruit, :title)
     question.should     be_editable_by(recruit, :documentation)
-    question.should     be_editable_by(recruit, :category)
 
     question.should_not be_editable_by(recruit, :user)
     question.should_not be_editable_by(recruit, :approved)
@@ -172,7 +170,6 @@ describe Question do
     question.should be_editable_by(admin)
     question.should be_editable_by(admin, :title)
     question.should be_editable_by(admin, :documentation)
-    question.should be_editable_by(admin, :category)
     question.should be_editable_by(admin, :approved)
 
     question.should_not be_editable_by(admin, :user)
diff --git a/spec/models/user_category_spec.rb b/spec/models/user_category_spec.rb
index 99efdea..130a359 100644
--- a/spec/models/user_category_spec.rb
+++ b/spec/models/user_category_spec.rb
@@ -70,7 +70,9 @@ describe UserCategory do
     for n in 1..5
       groups_in_cat.push Factory(:question_group)
       for i in 1..n
-        Factory(:question, :category => category, :question_group => groups_in_cat.last)
+        Factory(:question_category,
+                :category => category,
+                :question => Factory(:question, :question_group => groups_in_cat.last))
       end
     end
 
diff --git a/spec/models/user_mailer_spec.rb b/spec/models/user_mailer_spec.rb
index 39a45e2..d681e9e 100644
--- a/spec/models/user_mailer_spec.rb
+++ b/spec/models/user_mailer_spec.rb
@@ -10,7 +10,7 @@ describe UserMailer do
     notification.should deliver_to(recruit.email_address)
     notification.should deliver_from("no-reply@localhost")
     notification.should have_text(/There is a new question "#{question.title}"/)
-    notification.should have_text(/in category "#{question.category.name}" you are assigned to./)
+    notification.should have_text(/one of the categories you are assigned to./)
     notification.should have_text(/http:\/\/localhost:3000\/questions\/#{question.id}/)
     notification.should have_subject('New question')
   end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index ee256c4..b4a3dbf 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -310,7 +310,7 @@ describe User do
     q1      = Factory(:question)
               Factory(:question_content_multiple_choice, :question => q1)
               Factory(:user_category,
-                      :category => q1.category,
+                      :category => q1.categories.first,
                       :user => recruit)
 
     recruit.answered_all_multi_choice_questions?.should be_false
@@ -337,7 +337,7 @@ describe User do
 
     q1 = Factory(:question)
     Factory(:user_category, :user => recruit,
-      :category => q1.category)
+      :category => q1.categories.first)
     recruit.progress.should == "Answered 0 of 1 questions."
 
     Factory(:answer, :owner => recruit, :question => q1)
diff --git a/spec/support/factory_orders.rb b/spec/support/factory_orders.rb
index d02249a..9548330 100644
--- a/spec/support/factory_orders.rb
+++ b/spec/support/factory_orders.rb
@@ -12,19 +12,27 @@ def recruit_with_answered_and_unanswered_questions(n=5)
     for i in 1..n
       # answered and unanswered ungrouped questions
       category =        Factory(:category)
-      r.answered.push   Factory(:question, :category => category)
-      r.unanswered.push Factory(:question, :category => category)
+      r.answered.push   Factory(:question_category, :category => category).question
+      r.unanswered.push Factory(:question_category, :category => category).question
 
       Factory(:answer, :owner => r.recruit, :question => r.answered.last)
 
       # and answered and unanswered question in one group
       group =           Factory(:question_group)
-      r.answered.push   Factory(:question, :category => category, :question_group => group)
+      r.answered.push   Factory(:question_category,
+                                :category => category,
+                                :question => Factory(:question, :question_group => group)
+                               ).question
                         Factory(:user_question_group, :user => r.recruit, :question => r.answered.last)
       # This question isn't unanswered! This is question user can't answer
-                        Factory(:question, :category => category, :question_group => group)
+                        Factory(:question_category,
+                                :category => category,
+                                :question => Factory(:question, :question_group => group))
       # add a unanswered grouped question
-      r.unanswered.push Factory(:question, :category => category, :question_group => Factory(:question_group))
+      r.unanswered.push Factory(:question_category,
+                                :category => category,
+                                :question => Factory(:question, :question_group => Factory(:question_group))
+                               ).question
                         Factory(:user_question_group, :user => r.recruit, :question => r.unanswered.last)
 
       Factory(:answer, :owner => r.recruit, :question => r.answered.last)
@@ -51,14 +59,18 @@ def recruit_with_answers_in_categories(mentor = nil, n_categories = 5, n_ans_in_
     r.categories.push     Factory(:category)
     r.answers_in_cat.push []
     for i in 1..n_ans_in_cat
-      question                    = Factory(:question, :category => r.categories.last)
+      question                    = Factory(:question_category, :category => r.categories.last).question
       r.all_answers.push          Factory(:answer, :owner => r.recruit, :question => question)
       r.answers_in_cat.last.push  r.all_answers.last
 
       # group of two questions, answered
       group                       = Factory(:question_group)
-      question                    = Factory(:question, :category => r.categories.last, :question_group => group)
-                                    Factory(:question, :category => r.categories.last, :question_group => group)
+      question                    = Factory(:question_category,
+                                            :category => r.categories.last,
+                                            :question => Factory(:question, :question_group => group)).question
+                                    Factory(:question_category,
+                                            :category => r.categories.last,
+                                            :question => Factory(:question, :question_group => group))
                                     Factory(:user_question_group, :user => r.recruit, :question => question)
       r.all_answers.push          Factory(:answer, :owner => r.recruit, :question => question)
       r.answers_in_cat.last.push  r.all_answers.last