Questions: fixes, redesign form, fix edit error

app/controllers/admin/questions_controller.rb

@@ -1,17 +1,16 @@
 module Admin
   class QuestionsController < Admin::BaseController
     load_and_authorize_resource :conference, find_by: :short_title
-    load_and_authorize_resource through: :conference, except: [:new, :create]
+    load_and_authorize_resource except: [:create]
 
     def index
       authorize! :index, Question.new(conference_id: @conference.id)
-      @questions = Question.where(global: true).all | Question.where(conference_id: @conference.id)
+      @questions = Question.where(global: true).all | Question.where(conference_id: @conference.id) | @conference.questions
-      @questions_conference = @conference.questions
+      @question = @conference.questions.new
-      @new_question = @conference.questions.new
     end
 
     def show
-      @registrations = @conference.registrations.joins(:qanswers).uniq
+      @registrations = @conference.registrations.joins(:qanswers).where(qanswers: { question: @question })
     end
 
     def new
@@ -22,14 +21,19 @@ module Admin
     def create
       @question = @conference.questions.new(params[:question])
       @question.conference_id = @conference.id
+
+      # We need to authorize the @question after a conference_id has been associated with the question,
+      # because authorization in ability.rb is based on existence of conference_id attribute
+      # (and the controller does not authorize through conference)
       authorize! :create, @question
 
-      if @question.question_type_id == QuestionType.find_by(title: 'Yes/No').id
+      if @question.question_type == QuestionType.find_by(title: 'Yes/No')
-        @question.answers = [Answer.find_by(title: 'Yes'), Answer.find_by(title: 'No')]
+        @question.answers = [ Answer.find_or_create_by(title: 'Yes'), Answer.find_or_create_by(title: 'No') ]
       end
 
       respond_to do |format|
-        if @conference.save
+        # Do not automatically associate newly created question with the conference. The new question shall be enabled for the conference manually.
+        if @question.save
           format.html { redirect_to admin_conference_questions_path, notice: 'Question was successfully created.' }
         else
           flash[:error] = "Oops, couldn't save Question. #{@question.errors.full_messages.join('. ')}"
@@ -39,28 +43,50 @@ module Admin
     end
 
     # GET questions/1/edit
-    def edit
+    def edit; end
-      if @question.global
-        redirect_to(admin_conference_questions_path(conference_id: @conference.short_title), alert: 'Sorry, you cannot edit global questions. Create a new one.')
-      end
-    end
 
     # PUT questions/1
     def update
-      if @question.update_attributes(params[:question])
+      @question.assign_attributes(params[:question])
-        redirect_to(admin_conference_questions_path(conference_id: @conference.short_title), notice: "Question '#{@question.title}' for #{@conference.short_title} successfully updated.")
+
+      if @question.question_type == QuestionType.find_by(title: 'Yes/No')
+        @question.answers = [ Answer.find_or_create_by(title: 'Yes'), Answer.find_or_create_by(title: 'No') ]
+      end
+
+      if @question.save
+        if @question.answers.blank?
+          # A question without answers cannot be enabled for a conference
+          @conference.questions.delete(@question)
+        end
+
+        redirect_to(admin_conference_questions_path(conference_id: @conference.short_title), notice: "Question '#{@question.title}' for #{@conference.short_title} updated successfully.")
       else
-        redirect_to(admin_conference_questions_path(conference_id: @conference.short_title), notice: "Update of questions for #{@conference.short_title} failed. #{@question.errors.full_messages.join('. ')}")
+        flash[:error] = "Update of questions for #{@conference.short_title} failed. #{@question.errors.full_messages.join('. ')}"
+        redirect_to admin_conference_questions_path(conference_id: @conference.short_title)
       end
     end
 
     # Update questions used for the conference
-    def update_conference
+    def toggle_question
       authorize! :update, Question.new(conference_id: @conference.id)
-      if @conference.update_attributes(params[:conference])
+
-        redirect_to(admin_conference_questions_path(conference_id: @conference.short_title), notice: "Questions for #{@conference.short_title} successfully updated.")
+      ids = @conference.question_ids
+      if params[:enable] == 'true'
+        ids = ids.push(@question.id)
+      elsif params[:enable] == 'false'
+        ids.delete(@question.id)
+      end
+
+      if @conference.update_attributes(question_ids: ids)
+        flash[:notice] = "Questions for #{@conference.short_title} successfully updated. Note: Only questions with answers can be enabled for a conference."
       else
-        redirect_to(admin_conference_questions_path(conference_id: @conference.short_title), notice: "Update of questions for #{@conference.short_title} failed.")
+        flash[:error] = "Update of questions for #{@conference.short_title} failed."
+      end
+
+      if request.xhr?
+            render js: 'index'
+        else
+          redirect_to admin_conference_questions_path(conference_id: @conference.short_title)
       end
     end
 
@@ -68,30 +94,31 @@ module Admin
     def destroy
       if can? :destroy, @question
         # Do not delete global questions
-        if !@question.global
+        if !@question.global || @question.conferences.blank?
 
           # Delete question and its answers
           begin
             Question.transaction do
 
-              @question.destroy
               @question.answers.each do |a|
-                a.destroy
+                a.destroy unless a.questions.any?
               end
-              flash[:notice] = "Deleted question: #{@question.title} and its answers: #{@question.answers.map {|a| a.title}.join ','}"
+
+              @question.destroy
+              flash[:notice] = "Deleted question: #{@question.title}"
             end
           rescue ActiveRecord::RecordInvalid
             flash[:error] = 'Could not delete question.'
           end
         else
-          flash[:error] = 'You cannot delete global questions.'
+          flash[:error] = 'You cannot delete global questions that are currently being used for a conference.'
         end
       else
         flash[:error] = 'You must be an admin to delete a question.'
       end
 
-      @questions = Question.where(global: true).all | Question.where(conference_id: @conference.id)
+      @questions = Question.where(global: true).all | Question.where(conference_id: @conference.id) | @conference.questions
-      @questions_conference = @conference.questions
+      redirect_to admin_conference_questions_path(@conference.short_title)
     end
   end
 end

app/models/ability.rb

@@ -173,6 +173,7 @@ class Ability
     can :manage, Question do |question|
       !(question.conferences.pluck(:id) & conf_ids_for_info_desk).empty?
     end
+    can [ :show, :toggle_question ], Question, global: true
   end
 
   def signed_in_with_volunteers_coordinator_role(user)

app/models/answer.rb

@@ -1,8 +1,21 @@
 class Answer < ActiveRecord::Base
   attr_accessible :title
 
-  has_many :qanswers
+  has_many :qanswers, dependent: :destroy
   has_many :questions, through: :qanswers
 
   validates :title, presence: true
+  validate :no_modification_if_used
+
+  def no_modification_if_used
+    errors.add('', 'cannot be altered or deleted, if they are being used.') if self.title_changed? && self.questions.any?
+  end
+
+  # Gets answer, question, conference
+  ## Returns
+  # the amount of replies for a given answer
+  # + integer +
+  def sum_replies question, conference
+    self.qanswers.find_by(question: question).registrations.where(conference: conference).count
+  end
 end

app/models/qanswer.rb

@@ -2,7 +2,7 @@ class Qanswer < ActiveRecord::Base
   attr_accessible :question_id, :answer_id
 
   belongs_to :question
-  belongs_to :answer, dependent: :delete
+  belongs_to :answer
 
   has_and_belongs_to_many :registrations
 

app/models/question.rb

@@ -8,12 +8,5 @@ class Question < ActiveRecord::Base
   has_many :answers, through: :qanswers, dependent: :delete_all
 
   validates :title, :question_type_id, presence: true
-  validate :existing_answers
   accepts_nested_attributes_for :answers, allow_destroy: true
-
-  private
-
-  def existing_answers
-    errors.add(:base, 'Must have answers') if self.answers.blank?
-  end
 end

app/views/admin/questions/_form.html.haml

@@ -1,22 +1,45 @@
 .row
-  .col-md-6
+  .col-md-12
-    %legend Question
+    - unless @question.new_record?
-    = f.input :title, label: 'Your Question'
+      .page-header
-    = f.input :question_type
+        %h2
-    = f.input :global, label: 'Make Global',
+          Edit Question
-              hint: '(Global questions are available for selection to all conferences)'
+        .text-muted
-  .col-md-6.hidden{id: 'answers_col'}
+          = @question.title
-    = dynamic_association :answers, 'Answers', f,
+.row
-                          hint: 'Insert your answers in the order you want them to appear'
+  = semantic_form_for(@question, url: @question.new_record? ? admin_conference_questions_path(@conference.short_title) : admin_conference_question_path(@conference.short_title, @question.id)) do |f|
+    .col-md-6
+      %br
+      %legend Question
+      = f.input :title, label: 'Your Question', input_html: { autofocus: true }
+      = f.input :question_type
+      = f.input :global, label: 'Make Global',
+                hint: '(Global questions are available for selection to all conferences)'
+      = f.submit 'Save', class: 'btn btn-primary'
+    .col-md-6.hidden{id: 'answers_col'}
+      = dynamic_association :answers, 'Answers', f,
+                              hint: 'Insert your answers in the order you want them to appear'
 
 :javascript
+  $(document).ready(function(){
+    var selected_type_original = $("#question_question_type_id").find('option:selected').text();
+
+    if ( selected_type_original == 'Yes/No' ) {
+      $('#answers_col').addClass('hidden');
+    }
+    else {
+      $('#answers_col').removeClass('hidden');
+    }
+  });
+
   $("#question_question_type_id").change(function () {
 
     var selected_type = $(this).find('option:selected').text();
 
-    if (selected_type == 'Yes/No')
+    if ( selected_type == 'Yes/No' ) {
       $('#answers_col').addClass('hidden');
-    else
+    }
+    else {
       $('#answers_col').removeClass('hidden');
-    end
+    }
   });

app/views/admin/questions/_questions.html.haml

@@ -1,27 +0,0 @@
-%table.table.table-hover#questions
-  %th Enabled
-  %th Question
-  %th Type
-  %th Answers
-  %th Actions
-  - @questions.each do |q|
-    %tr
-      %td
-        = hidden_field_tag "conference[question_ids][]", nil
-        = check_box_tag "conference[question_ids][]", q.id,
-                        @conference.question_ids.include?(q.id), id: dom_id(q)
-      %td
-        = q.title
-      %td
-        = q.question_type.title
-      %td
-        = q.answers.map {|a| a.title}.join(', ')
-
-      %td
-        .btn-group
-          = link_to 'Show', admin_conference_question_path(@conference.short_title, q), class: 'btn btn-success'
-          = link_to 'Edit', edit_admin_conference_question_path(@conference.short_title, q),
-                    class: 'btn btn-primary', disabled: !(can? :update, q)
-          = link_to 'Delete', admin_conference_question_path(@conference.short_title, q),
-                    method: :delete, remote: true, class: 'btn btn-danger',
-                    confirm: "Delete question '#{q.title}'?", disabled: !(can? :destroy, q)

app/views/admin/questions/destroy.js.erb

@@ -1 +0,0 @@
-$('#myquestions').html("<%= escape_javascript(render :partial => 'questions') %>");

app/views/admin/questions/edit.html.haml

@@ -1,9 +1 @@
-%h2
+=  render 'form'
-  Edit Question
-%h3
-  "#{@question.title}"
-= semantic_form_for(@question, url: admin_conference_question_path(@conference.short_title, @question.id)) do |f|
-
-  = render partial: 'form', locals: {f: f}
-
-  = f.submit 'Save', class: 'btn btn-primary', confirm: 'Are you sure you want to make these changes?'

app/views/admin/questions/index.html.haml

@@ -11,24 +11,50 @@
 .row
   .col-md-12
     - if @questions.count > 0
-      = semantic_form_for(@conference, url: update_conference_admin_conference_questions_path(@conference.short_title)) do |f|
+
-        .questions{id: 'myquestions'}
+      .questions
-          = render partial: 'questions'
+        %table.table.table-hover.datatable#questions
-        - if can? :update, Question.new(conference_id: @conference.id)
+          %thead
-          = f.submit "Save Questions", class: 'btn btn-primary pull-right',
+            %th Enabled
-                      confirm: 'Are you sure you want to make these changes?'
+            %th Title
+            %th Type
+            %th Answers
+            %th Actions
+          %tbody
+            - @questions.each do |question|
+              %tr
+                %td
+                  = check_box_tag @conference.short_title, question.id, (@conference.questions.include? question),
+                    method: :patch, url: "/admin/conference/#{@conference.short_title}/questions/#{question.id}/toggle_question?enable=",
+                    disabled: question.answers.blank? || !(can? :update, Question.new(conference_id: @conference.id) ), class: 'switch-checkbox', data: { size: 'small',
+                                                      off_color: 'warning',
+                                                      on_text: 'Yes',
+                                                      off_text: 'No' }
+
+
+                %td
+                  = question.title
+                %td
+                  = question.question_type.title
+                %td
+                  = question.answers.map {|answer| "#{answer.title} (#{answer.sum_replies question, @conference})"}.join(', ')
+
+                %td
+                  .btn-group
+                    = link_to 'Show', admin_conference_question_path(@conference.short_title, question), class: 'btn btn-success', disabled: !(can? :show, question)
+                    = link_to 'Edit', edit_admin_conference_question_path(@conference.short_title, question),
+                              class: 'btn btn-primary', disabled: !(can? :update, question)
+                    = link_to 'Delete', admin_conference_question_path(@conference.short_title, question),
+                              method: :delete, class: 'btn btn-danger',
+                              data: { confirm: "Delete question '#{question.title}'?" }, disabled: !(can? :destroy, question)
+
+
 
     .modal.fade{id: 'new-question', 'role' => 'dialog', 'aria-hidden' => 'true'}
       .modal-dialog
         .modal-content
           .modal-header
             %h3{id: 'new-question-header'}
-              Add new question
+              Create Question
           .modal-body
-            = semantic_form_for(@new_question, url: admin_conference_questions_path(@conference.short_title), method: :post) do |f|
+            = render partial: 'form'
-              = render partial: 'form', locals: {f: f}
-              %button{class: 'btn btn-primary'}
-                Save
-              .pull-right
-                %button{class: 'btn btn-danger', 'data-dismiss'=> 'modal', 'aria-hidden'=>'true'}
-                  Cancel

app/views/admin/questions/show.html.haml

@@ -5,8 +5,7 @@
         %h1= @question.title
         - @question.answers.each do |answer|
           = answer.title
-          (#{answer.qanswers.find_by(question: @question).registrations.where(conference: @conference).count})
+          (#{answer.sum_replies @question, @conference})
-
 
   .row
     .col-md-12

app/views/conference_registrations/_questions.html.haml

@@ -1,8 +1,8 @@
 = f.inputs 'Additional Info' do
   - @conference.questions.each do |q|
-    - if q.question_type.id == 1  || q.question_type.id == 2 # yes/no or single choice
+    - if q.question_type.title == 'Yes/No'  || q.question_type.title == 'Single Choice' # yes/no or single choice
       = f.input :qanswers, :collection => q.qanswers, :as => :select, :input_html => { :multiple => false }, label: q.title, :include_blank => "Please make your choice",
                 :member_label => Proc.new {|a| a.answer.title}
-    - if q.question_type.id == 3  # multiple choice
+    - if q.question_type.title == 'Multiple Choice'  # multiple choice
       = f.input :qanswers, :collection => q.qanswers, :as => :check_boxes, label: q.title,
                 :member_label => Proc.new {|a| a.answer.title}

config/routes.rb

@@ -66,8 +66,8 @@ Osem::Application.routes.draw do
       end
 
       resources :questions do
-        collection do
+        member do
-          patch :update_conference
+          patch :toggle_question
         end
       end
 

db/seeds.rb

@@ -7,9 +7,11 @@
 #   Mayor.create(name: 'Emanuel', city: cities.first)
 
 # Create sample user
-user = User.find_or_initialize_by(email: 'deleted@localhost.osem', name: 'User deleted',
+user = User.find_or_initialize_by(email: 'deleted@localhost.osem')
-                                  username: 'deleted_user', is_disabled: true,
+user.name = 'User deleted'
-                                  biography: 'Data is no longer available for deleted user.')
+user.username = 'deleted_user'
+user.is_disabled = true
+user.biography = 'Data is no longer available for deleted user.'
 user.password = Devise.friendly_token[0, 20]
 user.skip_confirmation!
 user.save!
@@ -27,8 +29,8 @@ questions_yes_no = ['Do you need handicapped access?',
                     'Will you attend the social event(s)?',
                     'Will you stay at one of the suggested hotels?']
 
-questions_yes_no.each do |i|
+questions_yes_no.each do |question_title|
-  q = Question.find_or_initialize_by(title: i, question_type_id: qtype_yesno.id, global: true)
+  q = Question.find_or_initialize_by(title: question_title, question_type_id: qtype_yesno.id, global: true)
-  q.answers = [answer_yes, answer_no]
+  q.answers = [ answer_yes, answer_no ]
   q.save!
 end

spec/controllers/admin/questions_controller_spec.rb

@@ -0,0 +1,26 @@
+require 'spec_helper'
+
+describe Admin::QuestionsController do
+  let!(:conference) { create(:conference) }
+  let!(:question_with_answers) { create(:question_with_answers) }
+  let!(:question_without_answers) { create(:question) }
+  let(:organizer) { create(:organizer) }
+
+  describe 'PATCH #update_conference' do
+    before(:each) do
+      sign_in(organizer)
+    end
+
+    it 'enables a question for a conference if the question has answers' do
+      patch :toggle_question, conference_id: conference.short_title, id: question_with_answers, enable: 'true'
+
+      conference.reload
+
+      expect(conference.question_ids).to eq([question_with_answers.id])
+      expect(conference.question_ids).to_not include([question_without_answers.id])
+
+      expect(flash[:notice]).to eq("Questions for #{conference.short_title} successfully updated. Note: Only questions with answers can be enabled for a conference.")
+      expect(response).to redirect_to admin_conference_questions_path(conference.short_title)
+    end
+  end
+end

spec/factories/answer.rb

@@ -2,6 +2,22 @@
 
 FactoryGirl.define do
   factory :answer do
-    title 'Do you?'
+    title 'I do'
+
+    factory :answer1 do
+      title 'First Answer'
+    end
+
+    factory :answer2 do
+      title 'Second Answer'
+    end
+
+    factory :answer_yes do
+      title 'Yes'
+    end
+
+    factory :answer_no do
+      title 'No'
+    end
   end
 end

spec/factories/question.rb

@@ -2,11 +2,27 @@
 
 FactoryGirl.define do
   factory :question do
-    title 'blah'
+    title 'Do you?'
     question_type
-    after(:build) do |question|
+
-      question.answers << build(:answer)
+    factory :question_with_answers do
-      question.conferences << build(:conference)
+      title 'Which do you choose?'
+
+      after(:build) do |question|
+        question.answers << build(:answer1)
+        question.answers << build(:answer2)
+      end
+    end
+
+    factory :attending_with_partner do
+      title 'Will you attend with a partner?'
+      association :question_type, factory: :yes_no
+      global true
+
+      after(:build) do |question|
+        question.answers << build(:answer_yes)
+        question.answers << build(:answer_no)
+      end
     end
   end
 end

spec/factories/question_type.rb

@@ -2,6 +2,18 @@
 
 FactoryGirl.define do
   factory :question_type do
-    title 'Multiple Choice'
+    title 'A type for question'
+
+    factory :yes_no do
+      title 'Yes/No'
+    end
+
+    factory :single_choice do
+      title 'Single Choice'
+    end
+
+    factory :multiple_choice do
+      title 'Multiple Choice'
+    end
   end
 end

spec/views/admin/questions/index.html.haml_spec.rb

@@ -0,0 +1,30 @@
+require 'spec_helper'
+
+describe 'admin/questions/index' do
+  let!(:conference) { create(:conference) }
+  let!(:question_type) { create(:question_type) }
+
+  before(:each) do
+    assign(:conference, conference)
+    assign(:question, build(:question))
+    assign(:questions, [ create(:attending_with_partner), create(:question_with_answers, conference_id: conference.id, title: 'Test question for this conf', question_type_id: question_type.id)])
+
+    render
+  end
+
+  it 'renders all available questions' do
+    expect(rendered).to have_selector('table th:nth-of-type(1)', text: 'Enabled')
+    expect(rendered).to have_selector('table th:nth-of-type(2)', text: 'Title')
+    expect(rendered).to have_selector('table th:nth-of-type(3)', text: 'Type')
+    expect(rendered).to have_selector('table th:nth-of-type(4)', text: 'Answers')
+    expect(rendered).to have_selector('table th:nth-of-type(5)', text: 'Actions')
+
+    expect(rendered).to have_selector('table tr:nth-of-type(1) td:nth-of-type(2)', text: 'Will you attend with a partner?')
+    expect(rendered).to have_selector('table tr:nth-of-type(1) td:nth-of-type(3)', text: 'Yes/No')
+    expect(rendered).to have_selector('table tr:nth-of-type(1) td:nth-of-type(4)', text: 'Yes (0), No (0)')
+
+    expect(rendered).to have_selector('table tr:nth-of-type(2) td:nth-of-type(2)', text: 'Test question for this conf')
+    expect(rendered).to have_selector('table tr:nth-of-type(2) td:nth-of-type(3)', text: 'A type for question')
+    expect(rendered).to have_selector('table tr:nth-of-type(2) td:nth-of-type(4)', text: 'First Answer (0), Second Answer (0)')
+  end
+end

spec/views/admin/questions/show.html.haml_spec.rb

@@ -0,0 +1,35 @@
+require 'spec_helper'
+
+describe 'admin/questions/show' do
+  let!(:conference) { create(:conference) }
+  let!(:question_type) { create(:question_type) }
+  let!(:question) { create(:question) }
+  let!(:attending_with_partner) { create(:attending_with_partner) }
+  let!(:user1) { create(:user, name: 'User 1') }
+  let!(:user2) { create(:user, name: 'User 2') }
+  let!(:user3) { create(:user, name: 'User 3') }
+
+  let!(:registration1) { create(:registration, conference: conference, user: user1) }
+  let!(:qanswer1) { create(:qanswer, question: attending_with_partner, answer: attending_with_partner.answers.first) }
+  let!(:registration2) { create(:registration, conference: conference, user: user2) }
+  let!(:qanswer2) { create(:qanswer, question: attending_with_partner, answer: attending_with_partner.answers.second) }
+  let!(:registration3) { create(:registration, conference: conference, user: user3) }
+
+  before(:each) do
+    assign :conference, conference
+    assign :question, attending_with_partner
+    assign :registrations, [registration1, registration2]
+    registration1.qanswers = [qanswer1]
+    registration2.qanswers = [qanswer2]
+    render
+  end
+
+  it 'renders all users that answered the question' do
+    expect(rendered).to include('User 1')
+    expect(rendered).to include('User 2')
+  end
+
+  it 'does not render users that have not answered the question' do
+    expect(rendered).to_not include('User 3')
+  end
+end