From e25425bd77e83e351b4c0a8d5e846afb05e7b239 Mon Sep 17 00:00:00 2001 From: Stella Rouzi Date: Thu, 10 Sep 2015 12:39:52 +0300 Subject: [PATCH 1/2] Questions: fixes, redesign form, fix edit error --- app/controllers/admin/questions_controller.rb | 81 ++++++++++++------- app/models/ability.rb | 1 + app/models/answer.rb | 15 +++- app/models/qanswer.rb | 2 +- app/models/question.rb | 7 -- app/views/admin/questions/_form.html.haml | 47 ++++++++--- .../admin/questions/_questions.html.haml | 27 ------- app/views/admin/questions/destroy.js.erb | 1 - app/views/admin/questions/edit.html.haml | 10 +-- app/views/admin/questions/index.html.haml | 54 +++++++++---- app/views/admin/questions/show.html.haml | 3 +- .../_questions.html.haml | 4 +- config/routes.rb | 4 +- db/seeds.rb | 14 ++-- .../admin/questions_controller_spec.rb | 26 ++++++ spec/factories/answer.rb | 18 ++++- spec/factories/question.rb | 24 +++++- spec/factories/question_type.rb | 14 +++- .../admin/questions/index.html.haml_spec.rb | 30 +++++++ .../admin/questions/show.html.haml_spec.rb | 35 ++++++++ 20 files changed, 300 insertions(+), 117 deletions(-) delete mode 100644 app/views/admin/questions/_questions.html.haml delete mode 100644 app/views/admin/questions/destroy.js.erb create mode 100644 spec/controllers/admin/questions_controller_spec.rb create mode 100644 spec/views/admin/questions/index.html.haml_spec.rb create mode 100644 spec/views/admin/questions/show.html.haml_spec.rb diff --git a/app/controllers/admin/questions_controller.rb b/app/controllers/admin/questions_controller.rb index b55ff496..0211aca8 100644 --- a/app/controllers/admin/questions_controller.rb +++ b/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_conference = @conference.questions - @new_question = @conference.questions.new + @questions = Question.where(global: true).all | Question.where(conference_id: @conference.id) | @conference.questions + @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 - @question.answers = [Answer.find_by(title: 'Yes'), Answer.find_by(title: 'No')] + 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 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 - 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 + def edit; end # PUT questions/1 def update - if @question.update_attributes(params[:question]) - redirect_to(admin_conference_questions_path(conference_id: @conference.short_title), notice: "Question '#{@question.title}' for #{@conference.short_title} successfully updated.") + @question.assign_attributes(params[:question]) + + 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_conference = @conference.questions + @questions = Question.where(global: true).all | Question.where(conference_id: @conference.id) | @conference.questions + redirect_to admin_conference_questions_path(@conference.short_title) end end end diff --git a/app/models/ability.rb b/app/models/ability.rb index bd30a8fd..71caa466 100644 --- a/app/models/ability.rb +++ b/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) diff --git a/app/models/answer.rb b/app/models/answer.rb index 0a38d33c..7e9c9e05 100644 --- a/app/models/answer.rb +++ b/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 diff --git a/app/models/qanswer.rb b/app/models/qanswer.rb index 48448b04..14773da9 100644 --- a/app/models/qanswer.rb +++ b/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 diff --git a/app/models/question.rb b/app/models/question.rb index 28153dc6..ef0089c1 100644 --- a/app/models/question.rb +++ b/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 diff --git a/app/views/admin/questions/_form.html.haml b/app/views/admin/questions/_form.html.haml index abefd902..cd302dd7 100644 --- a/app/views/admin/questions/_form.html.haml +++ b/app/views/admin/questions/_form.html.haml @@ -1,22 +1,45 @@ .row - .col-md-6 - %legend Question - = f.input :title, label: 'Your Question' - = f.input :question_type - = f.input :global, label: 'Make Global', - hint: '(Global questions are available for selection to all conferences)' - .col-md-6.hidden{id: 'answers_col'} - = dynamic_association :answers, 'Answers', f, - hint: 'Insert your answers in the order you want them to appear' + .col-md-12 + - unless @question.new_record? + .page-header + %h2 + Edit Question + .text-muted + = @question.title +.row + = 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 + } }); diff --git a/app/views/admin/questions/_questions.html.haml b/app/views/admin/questions/_questions.html.haml deleted file mode 100644 index 8417a0ef..00000000 --- a/app/views/admin/questions/_questions.html.haml +++ /dev/null @@ -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) diff --git a/app/views/admin/questions/destroy.js.erb b/app/views/admin/questions/destroy.js.erb deleted file mode 100644 index dbae0ed8..00000000 --- a/app/views/admin/questions/destroy.js.erb +++ /dev/null @@ -1 +0,0 @@ -$('#myquestions').html("<%= escape_javascript(render :partial => 'questions') %>"); diff --git a/app/views/admin/questions/edit.html.haml b/app/views/admin/questions/edit.html.haml index 4b261f38..a4092f81 100644 --- a/app/views/admin/questions/edit.html.haml +++ b/app/views/admin/questions/edit.html.haml @@ -1,9 +1 @@ -%h2 - 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?' += render 'form' diff --git a/app/views/admin/questions/index.html.haml b/app/views/admin/questions/index.html.haml index f69d2846..698a8d43 100644 --- a/app/views/admin/questions/index.html.haml +++ b/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'} - = render partial: 'questions' - - if can? :update, Question.new(conference_id: @conference.id) - = f.submit "Save Questions", class: 'btn btn-primary pull-right', - confirm: 'Are you sure you want to make these changes?' + + .questions + %table.table.table-hover.datatable#questions + %thead + %th Enabled + %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', locals: {f: f} - %button{class: 'btn btn-primary'} - Save - .pull-right - %button{class: 'btn btn-danger', 'data-dismiss'=> 'modal', 'aria-hidden'=>'true'} - Cancel + = render partial: 'form' diff --git a/app/views/admin/questions/show.html.haml b/app/views/admin/questions/show.html.haml index 0f01d41d..6ddf5633 100644 --- a/app/views/admin/questions/show.html.haml +++ b/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 diff --git a/app/views/conference_registrations/_questions.html.haml b/app/views/conference_registrations/_questions.html.haml index 56ee7836..3460944a 100644 --- a/app/views/conference_registrations/_questions.html.haml +++ b/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} diff --git a/config/routes.rb b/config/routes.rb index 05766f76..955eded8 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -66,8 +66,8 @@ Osem::Application.routes.draw do end resources :questions do - collection do - patch :update_conference + member do + patch :toggle_question end end diff --git a/db/seeds.rb b/db/seeds.rb index bf3567e0..5efe7166 100644 --- a/db/seeds.rb +++ b/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', - username: 'deleted_user', is_disabled: true, - biography: 'Data is no longer available for deleted user.') +user = User.find_or_initialize_by(email: 'deleted@localhost.osem') +user.name = 'User deleted' +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| - q = Question.find_or_initialize_by(title: i, question_type_id: qtype_yesno.id, global: true) - q.answers = [answer_yes, answer_no] +questions_yes_no.each do |question_title| + q = Question.find_or_initialize_by(title: question_title, question_type_id: qtype_yesno.id, global: true) + q.answers = [ answer_yes, answer_no ] q.save! end diff --git a/spec/controllers/admin/questions_controller_spec.rb b/spec/controllers/admin/questions_controller_spec.rb new file mode 100644 index 00000000..9c168721 --- /dev/null +++ b/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 diff --git a/spec/factories/answer.rb b/spec/factories/answer.rb index 367ba3a9..b2a6ad97 100644 --- a/spec/factories/answer.rb +++ b/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 diff --git a/spec/factories/question.rb b/spec/factories/question.rb index 9bb933cb..fe017949 100644 --- a/spec/factories/question.rb +++ b/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) - question.conferences << build(:conference) + + factory :question_with_answers do + 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 diff --git a/spec/factories/question_type.rb b/spec/factories/question_type.rb index aeb77096..f1e64b8a 100644 --- a/spec/factories/question_type.rb +++ b/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 diff --git a/spec/views/admin/questions/index.html.haml_spec.rb b/spec/views/admin/questions/index.html.haml_spec.rb new file mode 100644 index 00000000..309fccef --- /dev/null +++ b/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 diff --git a/spec/views/admin/questions/show.html.haml_spec.rb b/spec/views/admin/questions/show.html.haml_spec.rb new file mode 100644 index 00000000..0c410cd8 --- /dev/null +++ b/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 From a04df00bdcd2a0e03ae0f2090566f89fd533bbe7 Mon Sep 17 00:00:00 2001 From: Stella Rouzi Date: Wed, 16 Sep 2015 12:20:52 +0300 Subject: [PATCH 2/2] Adds model and controller tests for Question. --- app/controllers/admin/questions_controller.rb | 28 ++- .../admin/questions_controller_spec.rb | 238 +++++++++++++++++- spec/factories/answer.rb | 4 +- spec/factories/question.rb | 4 +- spec/models/answer_spec.rb | 39 +++ spec/models/question_spec.rb | 22 ++ 6 files changed, 311 insertions(+), 24 deletions(-) create mode 100644 spec/models/answer_spec.rb create mode 100644 spec/models/question_spec.rb diff --git a/app/controllers/admin/questions_controller.rb b/app/controllers/admin/questions_controller.rb index 0211aca8..6aec757c 100644 --- a/app/controllers/admin/questions_controller.rb +++ b/app/controllers/admin/questions_controller.rb @@ -34,10 +34,10 @@ module Admin respond_to do |format| # 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.' } + format.html { redirect_to admin_conference_questions_path(@conference.short_title), notice: 'Question was successfully created.' } else flash[:error] = "Oops, couldn't save Question. #{@question.errors.full_messages.join('. ')}" - format.html { redirect_to admin_conference_questions_path } + format.html { redirect_to admin_conference_questions_path(@conference.short_title) } end end end @@ -59,10 +59,11 @@ module Admin @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.") + flash[:notice] = "Question '#{@question.title}' for #{@conference.short_title} updated successfully." + redirect_to admin_conference_questions_path(@conference.short_title) else 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) + redirect_to admin_conference_questions_path(@conference.short_title) end end @@ -77,16 +78,19 @@ module Admin 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." + if @conference.update(question_ids: ids) + flash[:notice] = "Question '#{@question.title}' #{params[:enable]=='true' ? 'enabled' : 'disabled'} for #{@conference.title}." else - flash[:error] = "Update of questions for #{@conference.short_title} failed." + flash[:error] = "Failed to #{params[:enable]=='true' ? 'enable' : 'disable'} question '#{@question.title}' for #{@conference.title}. Note: Only questions with answers can be enabled for a conference." end - if request.xhr? - render js: 'index' - else - redirect_to admin_conference_questions_path(conference_id: @conference.short_title) + respond_to do |format| + format.html do + redirect_to admin_conference_questions_path(@conference.short_title) + end + format.js do + render js: 'index' + end end end @@ -114,7 +118,7 @@ module Admin 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.' + flash[:error] = 'You cannot delete the question. Do you have the necessary permissions?' end @questions = Question.where(global: true).all | Question.where(conference_id: @conference.id) | @conference.questions diff --git a/spec/controllers/admin/questions_controller_spec.rb b/spec/controllers/admin/questions_controller_spec.rb index 9c168721..92be5984 100644 --- a/spec/controllers/admin/questions_controller_spec.rb +++ b/spec/controllers/admin/questions_controller_spec.rb @@ -1,16 +1,21 @@ 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!(:conference) { create(:conference, title: 'my conference') } + let!(:question_with_answers) { create(:question_with_answers, conference_id: conference.id) } + let!(:question_without_answers) { create(:question, conference_id: conference.id) } + let(:question_type_yes_no) { QuestionType.find_by(title: 'Yes/No') } + let(:question_type_single_choice) { create(:single_choice) } + let(:first_answer) { create(:first_answer) } + let(:second_answer) { create(:second_answer) } let(:organizer) { create(:organizer) } + let(:question) { create(:question, title: 'test title', question_type_id: create(:single_choice).id) } - describe 'PATCH #update_conference' do - before(:each) do - sign_in(organizer) - end + before(:each) do + sign_in(organizer) + end + describe 'PATCH #toggle_question' do 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' @@ -19,8 +24,225 @@ describe Admin::QuestionsController do 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(flash[:notice]).to eq("Question 'Which do you choose?' enabled for my conference.") + expect(response).to redirect_to admin_conference_questions_path(conference.short_title) + end + + it 'disables 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' + patch :toggle_question, conference_id: conference.short_title, id: question_with_answers, enable: 'false' + + conference.reload + + expect(conference.question_ids).to eq([]) + expect(conference.question_ids).to_not include([question_with_answers.id]) + + expect(flash[:notice]).to eq("Question 'Which do you choose?' disabled for my conference.") expect(response).to redirect_to admin_conference_questions_path(conference.short_title) end end + + describe 'GET #index' do + it 'renders the index template' do + + get :index, conference_id: conference.short_title + expect(response).to render_template :index + end + + it 'properly assigns questions, when conference does not have enabled questions' do + questions_global = Question.where(global: true) + + get :index, conference_id: conference.short_title + + questions_array = questions_global.to_a + questions_array << question_with_answers + questions_array << question_without_answers + expect(assigns(:questions)).to match_array(questions_array) + end + end + + describe 'GET #show' do + before :each do + @registration = create(:registration, conference: conference) + conference.questions << question_with_answers + @registration.qanswers << question_with_answers.qanswers.first + end + it 'properly assigns registrations' do + get :show, conference_id: conference.short_title, id: question_with_answers.id + expect(assigns(:registrations).to_a).to eq [@registration] + end + end + + describe 'POST #create' do + context 'creates a question' do + before :each do + @expected = Question.count + 1 + post :create, conference_id: conference.short_title, question: attributes_for(:question, title: 'test', + question_type_id: question_type_single_choice.id) + end + + it 'successfully' do + expect(Question.count).to eq @expected + end + + it 'assigns question object with correct attributes' do + expect(assigns(:question).title).to eq 'test' + expect(assigns(:question).conference_id).to eq conference.id + end + + it 'renders flash with success message' do + expect(flash[:notice]).to eq 'Question was successfully created.' + end + + it 'redirects to index' do + expect(response).to redirect_to admin_conference_questions_path(conference.short_title) + end + end + + context 'creates a question with Yes/No question type' do + before :each do + @expected = Question.count + 1 + post :create, conference_id: conference.short_title, question: attributes_for(:question, title: 'test', + question_type_id: QuestionType.find_by(title: 'Yes/No').id) + end + + it 'successfully' do + expect(Question.count).to eq @expected + end + + it 'assigns question object with correct attributes' do + expect(assigns(:question).title).to eq 'test' + expect(assigns(:question).conference_id).to eq conference.id + end + + it 'renders flash with success message' do + expect(flash[:notice]).to eq 'Question was successfully created.' + end + + it 'redirects to index' do + expect(response).to redirect_to admin_conference_questions_path(conference.short_title) + end + + it 'assigns answers automatically' do + expect(assigns(:question).answers.to_a).to eq [ Answer.find_by(title: 'Yes'), Answer.find_by(title: 'No') ] + end + end + + context 'with invalid attributes' do + before :each do + @expected = Question.count + post :create, conference_id: conference.short_title, question: attributes_for(:question, title: '', question_type_id: question_type_single_choice.id) + end + + it 'does not save the new question in database' do + expect(Question.count).to eq @expected + end + + it 'redirects to index page' do + expect(response).to redirect_to admin_conference_questions_path(conference.short_title) + end + + it 'shows flash error message' do + expect(flash[:error]).to eq "Oops, couldn't save Question. Title can't be blank" + end + end + end + + describe 'PATCH #update' do + before :each do + question.answers << first_answer + question.answers << second_answer + question.conferences << conference + end + + context 'with invalid attributes' do + before :each do + patch :update, conference_id: conference.short_title, id: question.id, question: attributes_for(:question, title: '') + end + + it 'does not save the question' do + expect(question.title).to eq 'test title' + end + + it 'renders failure flash error message' do + expect(flash[:error]).to eq "Update of questions for #{conference.short_title} failed. Title can't be blank" + end + + it 'redirects to index' do + expect(response).to redirect_to admin_conference_questions_path(conference.short_title) + end + end + + context 'with valid attibutes' do + before :each do + patch :update, conference_id: conference.short_title, id: question.id, question: attributes_for(:question, title: 'new title') + end + + it 'successfully' do + question.reload + expect(question.title).to eq 'new title' + end + + it 'renders success flash notice message' do + question.reload + expect(flash[:notice]).to eq "Question 'new title' for #{conference.short_title} updated successfully." + end + + it 'redirects to index' do + expect(response).to redirect_to admin_conference_questions_path(conference.short_title) + end + end + + it 'disables question for conference, if question does not have answers' do + question.answers = [] + patch :update, conference_id: conference.short_title, id: question.id, question: attributes_for(:question) + + question.reload + expect(question.conferences).to eq [] + end + end + + describe 'DELETE #destroy' do + context 'global questions' do + it 'deletes question not used in any conference' do + global_question = Question.find_by(global: true) + + expected = Question.count - 1 + delete :destroy, conference_id: conference.short_title, id: global_question.id + + expect(Question.count).to eq expected + end + + it 'does not delete question used in a conference' do + global_question = Question.find_by(global: true) + conference.questions << global_question + + expected = Question.count + delete :destroy, conference_id: conference.short_title, id: global_question.id + + expect(Question.count).to eq expected + end + end + + context 'not global questions' do + it 'deletes a question not used in the conference' do + expected = Question.count - 1 + expect(conference.questions).not_to include question_with_answers + + delete :destroy, conference_id: conference.short_title, id: question_with_answers.id + + expect(Question.count).to eq expected + end + + it 'deletes question used in a conference' do + conference.questions << question_with_answers + expect(question_with_answers.conferences).to include conference + + expected = Question.count - 1 + delete :destroy, conference_id: conference.short_title, id: question_with_answers.id + + expect(Question.count).to eq expected + end + end + end end diff --git a/spec/factories/answer.rb b/spec/factories/answer.rb index b2a6ad97..aadb98f7 100644 --- a/spec/factories/answer.rb +++ b/spec/factories/answer.rb @@ -4,11 +4,11 @@ FactoryGirl.define do factory :answer do title 'I do' - factory :answer1 do + factory :first_answer do title 'First Answer' end - factory :answer2 do + factory :second_answer do title 'Second Answer' end diff --git a/spec/factories/question.rb b/spec/factories/question.rb index fe017949..9fdacc92 100644 --- a/spec/factories/question.rb +++ b/spec/factories/question.rb @@ -9,8 +9,8 @@ FactoryGirl.define do title 'Which do you choose?' after(:build) do |question| - question.answers << build(:answer1) - question.answers << build(:answer2) + question.answers << build(:first_answer) + question.answers << build(:second_answer) end end diff --git a/spec/models/answer_spec.rb b/spec/models/answer_spec.rb new file mode 100644 index 00000000..e7ad5653 --- /dev/null +++ b/spec/models/answer_spec.rb @@ -0,0 +1,39 @@ +require 'spec_helper' + +describe Answer do + let(:conference) { create(:conference) } + let(:question) { create(:question) } + let(:second_answer) { create(:second_answer) } + let(:registration) { create(:registration, conference: conference) } + + describe 'validations' do + + it 'has a valid factory' do + expect(build(:answer)).to be_valid + end + + it 'is not valid without a title' do + should validate_presence_of(:title) + expect(build(:answer, title: nil)).not_to be_valid + end + + it 'cannot be modified if the question is being used' do + question.answers << second_answer + second_answer.title = 'new title' + + expect(second_answer.valid?).to be false + end + + end + + describe '#sum_replies' do + before :each do + conference.questions << question + registration.qanswers << create(:qanswer, question: question, answer: second_answer) + end + + it 'returns no of replies for the answer, given a question and a conference' do + expect(second_answer.sum_replies(question, conference)).to eq 1 + end + end +end diff --git a/spec/models/question_spec.rb b/spec/models/question_spec.rb new file mode 100644 index 00000000..e61efe22 --- /dev/null +++ b/spec/models/question_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' + +describe Question do + + describe 'validations' do + + it 'has a valid factory' do + expect(build(:question)).to be_valid + end + + it 'is not valid without a title' do + should validate_presence_of(:title) + expect(build(:question, title: nil)).not_to be_valid + end + + it 'is not valid without a question type' do + should validate_presence_of(:question_type_id) + expect(build(:question, question_type_id: nil)).not_to be_valid + end + + end +end