diff --git a/app/controllers/notes_controller.rb b/app/controllers/notes_controller.rb index cbe959a42..ec4246acb 100644 --- a/app/controllers/notes_controller.rb +++ b/app/controllers/notes_controller.rb @@ -1,26 +1,27 @@ class NotesController < ApplicationController respond_to :html, :xml, :json, :js - before_action :member_only, :except => [:index, :show, :search] def search end def index - @notes = Note.paginated_search(params) + @notes = authorize Note.paginated_search(params) @notes = @notes.includes(:post) if request.format.html? respond_with(@notes) end def show - @note = Note.find(params[:id]) + @note = authorize Note.find(params[:id]) respond_with(@note) do |format| format.html { redirect_to(post_path(@note.post, anchor: "note-#{@note.id}")) } end end def create - @note = Note.create(note_params(:create)) + @note = authorize Note.new(permitted_attributes(Note)) + @note.save + respond_with(@note) do |fmt| fmt.json do if @note.errors.any? @@ -33,8 +34,8 @@ class NotesController < ApplicationController end def update - @note = Note.find(params[:id]) - @note.update(note_params(:update)) + @note = authorize Note.find(params[:id]) + @note.update(permitted_attributes(@note)) respond_with(@note) do |format| format.json do if @note.errors.any? @@ -47,24 +48,15 @@ class NotesController < ApplicationController end def destroy - @note = Note.find(params[:id]) + @note = authorize Note.find(params[:id]) @note.update(is_active: false) respond_with(@note) end def revert - @note = Note.find(params[:id]) + @note = authorize Note.find(params[:id]) @version = @note.versions.find(params[:version_id]) @note.revert_to!(@version) respond_with(@note) end - - private - - def note_params(context) - permitted_params = %i[x y width height body] - permitted_params += %i[post_id html_id] if context == :create - - params.require(:note).permit(permitted_params) - end end diff --git a/app/policies/note_policy.rb b/app/policies/note_policy.rb new file mode 100644 index 000000000..24921c54e --- /dev/null +++ b/app/policies/note_policy.rb @@ -0,0 +1,13 @@ +class NotePolicy < ApplicationPolicy + def revert? + update? + end + + def permitted_attributes_for_create + [:x, :y, :width, :height, :body, :post_id, :html_id] + end + + def permitted_attributes_for_update + [:x, :y, :width, :height, :body] + end +end diff --git a/test/functional/notes_controller_test.rb b/test/functional/notes_controller_test.rb index b65bf812e..e6dae1114 100644 --- a/test/functional/notes_controller_test.rb +++ b/test/functional/notes_controller_test.rb @@ -4,9 +4,7 @@ class NotesControllerTest < ActionDispatch::IntegrationTest context "The notes controller" do setup do @user = create(:user) - as_user do - @note = create(:note, body: "000") - end + @note = as(@user) { create(:note, body: "000") } end context "index action" do @@ -22,9 +20,7 @@ class NotesControllerTest < ActionDispatch::IntegrationTest body_matches: "000", is_active: true, post_id: @note.post_id, - post_tags_match: @note.post.tag_array.first, - creator_name: @note.creator.name, - creator_id: @note.creator_id + post_tags_match: @note.post.tag_array.first } } @@ -43,10 +39,9 @@ class NotesControllerTest < ActionDispatch::IntegrationTest context "create action" do should "create a note" do assert_difference("Note.count", 1) do - as_user do - @post = create(:post) - end + @post = create(:post) post_auth notes_path, @user, params: {:note => {:x => 0, :y => 0, :width => 10, :height => 10, :body => "abc", :post_id => @post.id}, :format => :json} + assert_response :success end end end @@ -54,14 +49,14 @@ class NotesControllerTest < ActionDispatch::IntegrationTest context "update action" do should "update a note" do put_auth note_path(@note), @user, params: {:note => {:body => "xyz"}} + assert_redirected_to @note assert_equal("xyz", @note.reload.body) end should "not allow changing the post id to another post" do - as(@admin) do - @other = create(:post) - end + @other = create(:post) put_auth note_path(@note), @user, params: {:format => "json", :id => @note.id, :note => {:post_id => @other.id}} + assert_response 403 assert_not_equal(@other.id, @note.reload.post_id) end end @@ -69,6 +64,7 @@ class NotesControllerTest < ActionDispatch::IntegrationTest context "destroy action" do should "destroy a note" do delete_auth note_path(@note), @user + assert_redirected_to @note assert_equal(false, @note.reload.is_active?) end end @@ -87,6 +83,7 @@ class NotesControllerTest < ActionDispatch::IntegrationTest should "revert to a previous version" do put_auth revert_note_path(@note), @user, params: {:version_id => @note.versions.first.id} + assert_redirected_to @note assert_equal("000", @note.reload.body) end