diff --git a/app/controllers/notes_controller.rb b/app/controllers/notes_controller.rb index 0b1f1c9..1ebe316 100644 --- a/app/controllers/notes_controller.rb +++ b/app/controllers/notes_controller.rb @@ -12,7 +12,7 @@ @answer = nil # if defined within the transaction block, was not accessable afterward # ensure user has access to plan BEFORE creating/finding answer rails Pundit::NotAuthorizedError unless Plan.find(plan_id).readable_by?(@note.user_id) - Answer.transaction do + Answer.transaction do if params[:note][:answer_id].present? @answer = Answer.find(params[:note][:answer_id]) end diff --git a/test/functional/answers_controller_test.rb b/test/functional/answers_controller_test.rb index 6a05efc..0ac190f 100644 --- a/test/functional/answers_controller_test.rb +++ b/test/functional/answers_controller_test.rb @@ -1,12 +1,12 @@ require 'test_helper' class AnswersControllerTest < ActionDispatch::IntegrationTest - + include Devise::Test::IntegrationHelpers - + setup do @user = User.last - + scaffold_plan end @@ -14,28 +14,28 @@ # ---------------------------------------------------------- test "should be able to update an answer" do sign_in @user - + # Test an answer for each Querstion Format QuestionFormat.all.each do |format| question = Question.find_by(question_format: format) template = question.section.phase.template - - plan = Plan.create(title: "Testing Answer For #{format.title}", + + plan = Plan.create(title: "Testing Answer For #{format.title}", template: template, visibility: :is_test) - + Role.create!(user_id: @user.id, plan_id: plan.id, access: 4) plan.reload - + referrer = "/#{FastGettext.locale}/plans/#{plan.id}/phases/#{question.section.phase.id}/edit" answer = Answer.find_by(plan: plan, question: question) assert_not answer.id.nil?, "expected the answer to have been created and for an id to be present after creating a #{format.title} question!" - + # Try editing it form_attributes = { answer: {id: answer.id, - user_id: @user.id, - plan_id: answer.plan.id, + user_id: @user.id, + plan_id: answer.plan.id, question_id: answer.question.id, text: "Tested", lock_version: answer.lock_version} @@ -47,8 +47,8 @@ assert_equal "Tested", answer.text, "expected the text to have been updated for a #{format.title} question!" end end - - + + private def put_answer(answer, attributes, referrer) put answer_path(FastGettext.locale, answer, format: "json"), attributes, {'HTTP_REFERER': referrer} diff --git a/test/functional/notes_controller_test.rb b/test/functional/notes_controller_test.rb index 32848f4..9d0b59e 100644 --- a/test/functional/notes_controller_test.rb +++ b/test/functional/notes_controller_test.rb @@ -1,20 +1,20 @@ require 'test_helper' class NotesControllerTest < ActionDispatch::IntegrationTest - + include Devise::Test::IntegrationHelpers - + setup do @user = User.last - + scaffold_plan - - @question = Question.create(text: 'Answer Testing', number: 9, + + @question = Question.create(text: 'Answer Testing', number: 9, section: @plan.template.phases.first.sections.first, question_format: QuestionFormat.find_by(option_based: false)) - + @answer = Answer.create(user: @user, plan: @plan, question: @question, text: 'Testing') - + @note = Note.create(user: @user, plan: @plan, answer: @answer, question: @question, archived: false, text: 'Test Note') end @@ -36,20 +36,20 @@ # notes POST /notes notes#create # note PATCH /notes/:id notes#update # PUT /notes/:id notes#update - - - + + + # POST /notes (notes_path) # ---------------------------------------------------------- test "create a new note" do params = {user_id: @user.id, answer_id: @answer.id, plan_id: @plan.id, question_id: @question.id, text: 'Test Note'} - + # Should redirect user to the root path if they are not logged in! post notes_path, {note: params} assert_unauthorized_redirect_to_root_path - + sign_in @user - + post notes_path, {note: params}, {'ACCEPT': 'application/json'} assert_response :success assert assigns(:note) @@ -59,15 +59,15 @@ assert assigns(:notice) #assert_select '.welcome-message h2', _('Comment was successfully created.') assert_equal 'Test Note', Note.last.text, 'Expected the note to have been created' - + # No Answer post notes_path, {note: {user_id: @user.id, plan_id: @plan.id, question_id: @question.id}}, {'ACCEPT': 'application/json'} assert_response :bad_request - # TODO: expected the new note to have been added :/ + # TODO: expected the new note to have been added :/ #assert_equal 'Test Note no Answer', Note.last.text, 'Expected the note to have been created even if there was no answer' - + # Invalid object - post notes_path, {note: {user_id: @user.id, answer_id: @answer.id, plan_id: @plan.id, + post notes_path, {note: {user_id: @user.id, answer_id: @answer.id, plan_id: @plan.id, question_id: @question.id}}, {'ACCEPT': 'application/json'} assert_response :bad_request assert assigns(:note) @@ -75,15 +75,15 @@ assert assigns(:answer) assert assigns(:question) assert assigns(:notice) - end - + end + # PUT /notes/:id (note_path) # ---------------------------------------------------------- test "update the note" do # Should redirect user to the root path if they are not logged in! put note_path(@note), { note: { text: 'Test Note' }, id: @note.id }, {'ACCEPT': 'application/json'} assert_unauthorized_redirect_to_root_path - + sign_in @user # Valid save @@ -96,23 +96,23 @@ assert assigns(:notice) @note.reload assert_equal 'Test Note', @note.text, "expected the note's text to be 'Test Note'" - + # Invalid save put note_path(@note), { note: { text: nil }, id: @note.id }, {'ACCEPT': 'application/json'} assert_response :bad_request assert assigns(:notice) assert_equal 'Test Note', @note.text, "expected the note's text to Still be 'Test Note'" end - + # PATCH /notes/:id/archive (archive_note_path) # ---------------------------------------------------------- test "delete the note" do # Should redirect user to the root path if they are not logged in! patch archive_note_path(@note), { note: { archived_by: @user.id }, id: @note.id }, {'ACCEPT': 'application/json'} assert_unauthorized_redirect_to_root_path - + sign_in @user - + patch archive_note_path(@note), { note: { archived_by: @user.id }, id: @note.id }, {'ACCEPT': 'application/json'} assert_response :success assert assigns(:note) @@ -120,9 +120,9 @@ assert assigns(:answer) assert assigns(:question) assert assigns(:notice) - + @note.reload assert @note.archived, 'expected the archived flag to be true' assert_equal @user.id, @note.archived_by, 'expected the archived_by to be set to @user' end -end \ No newline at end of file +end diff --git a/test/integration/answer_locking_test.rb b/test/integration/answer_locking_test.rb index 00af733..729b351 100644 --- a/test/integration/answer_locking_test.rb +++ b/test/integration/answer_locking_test.rb @@ -6,63 +6,63 @@ setup do scaffold_template scaffold_plan - @question = Question.create(text: 'Test question', section: @plan.template.phases.first.sections.first, + @question = Question.create(text: 'Test question', section: @plan.template.phases.first.sections.first, question_format: QuestionFormat.where(option_based: false).first, number: 99) - + @collaborator = (User.first == @plan.owner ? User.last : User.first) - + # Make the 2nd user an editor of the plan Role.create!(user_id: @collaborator.id, plan_id: @plan.id, access: 4) @plan.reload end - + # ---------------------------------------------------------- test 'user receives not found when trying to save a non-existent answer' do - userA = Answer.new(user: @plan.owner, plan: @plan, question: @question, + userA = Answer.new(user: @plan.owner, plan: @plan, question: @question, text: "Initial answer - by UserA") - + userB = Answer.new(user: @collaborator, plan: @plan, question: @question, text: "Version conflict at onset - by UserB") - + # Signin as UserA and insert the new answer sign_in @plan.owner put answer_path(FastGettext.locale, userA, format: "json"), obj_to_params(userA.attributes) assert_response :not_found assert_equal "application/json", @response.content_type - + # Signin as UserB and try to insert the new answer but fail sign_in @collaborator put answer_path(FastGettext.locale, userB, format: "json"), obj_to_params(userB.attributes) assert_response :not_found assert_equal "application/json", @response.content_type end - + # ---------------------------------------------------------- test 'user receives a lock notification if the answer was UPDATED while they were working' do - userA = Answer.create!(user: @plan.owner, plan: @plan, question: @question, + userA = Answer.create!(user: @plan.owner, plan: @plan, question: @question, text: "Initial answer - by UserA").attributes userB = userA.clone # Signin as UserA and insert the new answer sign_in @plan.owner userA['text'] += " - Updated by userA" - + put answer_path(FastGettext.locale, userA['id'], format: "json"), obj_to_params(userA) assert_response :success assert_equal "application/json", @response.content_type updated = Answer.find_by(plan: @plan, question: @question) assert_equal "Initial answer - by UserA - Updated by userA", updated.text assert_equal @plan.owner.id, updated.user_id - + # Make sure the answers/locking partial is NOT displayed assert_not @response.body.include?(_('The following answer cannot be saved')), "expected there to be no lock error messaging" assert @response.body.include?(_('Answered')) assert @response.body.include?("#{_(' by')} #{@plan.owner.name}"), "expected the messaging to say the plan was updated by the plan owner" - + # Signin as UserB and try to insert the new answer but fail sign_in @collaborator userB['text'] += " - Updated by userB" - + put answer_path(FastGettext.locale, userB['id'], format: "json"), obj_to_params(userB) assert_response :success assert_equal "application/json", @response.content_type @@ -76,13 +76,13 @@ assert @response.body.include?(_('Answered')), "expected the messaging to include the status" end -# ---------------------------------------------------------- +# ---------------------------------------------------------- private def obj_to_params(attributes) - { + { answer: {id: attributes['id'], - user_id: attributes['user_id'], - plan_id: attributes['plan_id'], + user_id: attributes['user_id'], + plan_id: attributes['plan_id'], question_id: attributes['question_id'], text: attributes['text'], lock_version: attributes['lock_version']}