From f90155b52d3a6c5cbea4068c0a0b300adda0fc43 Mon Sep 17 00:00:00 2001 From: Matijs van Zuijlen Date: Sun, 13 Oct 2024 12:08:44 +0200 Subject: [PATCH 1/2] Use default describe blocks for Admin::NotesController specs --- .../admin/notes_controller_spec.rb | 153 +++++++++--------- 1 file changed, 74 insertions(+), 79 deletions(-) diff --git a/spec/controllers/admin/notes_controller_spec.rb b/spec/controllers/admin/notes_controller_spec.rb index c8af997e..49e9d9db 100644 --- a/spec/controllers/admin/notes_controller_spec.rb +++ b/spec/controllers/admin/notes_controller_spec.rb @@ -7,112 +7,107 @@ let(:admin) { create(:user, :as_admin, twitter: "@getpublify") } let!(:blog) { create(:blog, limit_article_display: 10) } + let(:note) { create(:note, user_id: admin) } before do sign_in admin end - context "with a blog" do - describe "index" do - let!(:notes) { create_list(:note, 2) } + describe "#index" do + let!(:notes) { create_list(:note, 2) } - it "shows the index template" do - get :index - expect(response).to render_template("index") - end + it "shows the index template" do + get :index + expect(response).to render_template("index") + end - it "lists existing notes" do - get :index - expect(assigns(:notes)).to match_array notes - end + it "lists existing notes" do + get :index + expect(assigns(:notes)).to match_array notes + end - it "assigns a new note for the note form" do - get :index + it "assigns a new note for the note form" do + get :index - aggregate_failures do - expect(assigns(:note)).to be_a(Note) - expect(assigns(:note).author).to eq(admin.login) - expect(assigns(:note).user).to eq(admin) - end + aggregate_failures do + expect(assigns(:note)).to be_a(Note) + expect(assigns(:note).author).to eq(admin.login) + expect(assigns(:note).user).to eq(admin) end + end - it "lists notes without publication date" do - create(:note, published_at: nil) + it "lists notes without publication date" do + create(:note, published_at: nil) - get :index - expect(response).to be_successful - end + get :index + expect(response).to be_successful end + end - describe "create" do - context "a simple note" do - before { post :create, params: { note: { body: "Emphasis _mine_" } } } + describe "#create" do + context "a simple note" do + before { post :create, params: { note: { body: "Emphasis _mine_" } } } - it { expect(response).to redirect_to(admin_notes_path) } - it { expect(flash[:notice]).to eq(I18n.t("notice.note_successfully_created")) } - end + it { expect(response).to redirect_to(admin_notes_path) } + it { expect(flash[:notice]).to eq(I18n.t("notice.note_successfully_created")) } + end + + it "creates a note" do + expect do + post :create, params: { note: { body: "Emphasis _mine_" } } + end.to change(Note, :count).from(0).to(1) + end - it "creates a note" do - expect do - post :create, params: { note: { body: "Emphasis _mine_" } } - end.to change(Note, :count).from(0).to(1) + context "with twitter access configured" do + before do + blog.twitter_consumer_key = "consumer_key" + blog.twitter_consumer_secret = "consumer_secret" + blog.save + + admin.twitter_oauth_token = "oauth_token" + admin.twitter_oauth_token_secret = "oauth_token" + admin.save end - context "with twitter access configured" do - before do - blog.twitter_consumer_key = "consumer_key" - blog.twitter_consumer_secret = "consumer_secret" - blog.save - - admin.twitter_oauth_token = "oauth_token" - admin.twitter_oauth_token_secret = "oauth_token" - admin.save - end - - it "sends the note to twitter" do - expect(Note.count).to eq(0) - twitter_cli = double(:twitter_cli) - expect(Twitter::Client).to receive(:new).and_return(twitter_cli) - tweet = Struct.new(:attrs).new({ id_str: "2344" }) - expect(twitter_cli).to receive(:update).and_return(tweet) - post :create, params: { note: { body: "Emphasis _mine_, arguments *strong*" }, - push_to_twitter: "true" } - expect(Note.first.twitter_id).to eq("2344") - end + it "sends the note to twitter" do + expect(Note.count).to eq(0) + twitter_cli = double(:twitter_cli) + expect(Twitter::Client).to receive(:new).and_return(twitter_cli) + tweet = Struct.new(:attrs).new({ id_str: "2344" }) + expect(twitter_cli).to receive(:update).and_return(tweet) + post :create, params: { note: { body: "Emphasis _mine_, arguments *strong*" }, + push_to_twitter: "true" } + expect(Note.first.twitter_id).to eq("2344") end end + end - context "with an existing note from current user" do - let(:note) { create(:note, user_id: admin) } - - describe "edit" do - before { get :edit, params: { id: note.id } } + describe "#edit" do + before { get :edit, params: { id: note.id } } - it { expect(response).to be_successful } - it { expect(response).to render_template("edit") } - it { expect(assigns(:note)).to eq(note) } - it { expect(assigns(:notes)).to eq([note]) } - end + it { expect(response).to be_successful } + it { expect(response).to render_template("edit") } + it { expect(assigns(:note)).to eq(note) } + it { expect(assigns(:notes)).to eq([note]) } + end - describe "update" do - before { post :update, params: { id: note.id, note: { body: "new body" } } } + describe "#update" do + before { post :update, params: { id: note.id, note: { body: "new body" } } } - it { expect(response).to redirect_to(action: :index) } - it { expect(note.reload.body).to eq("new body") } - end + it { expect(response).to redirect_to(action: :index) } + it { expect(note.reload.body).to eq("new body") } + end - describe "show" do - before { get :show, params: { id: note.id } } + describe "#show" do + before { get :show, params: { id: note.id } } - it { expect(response).to render_template("show") } - end + it { expect(response).to render_template("show") } + end - describe "Destroying a note" do - before { post :destroy, params: { id: note.id } } + describe "#destroy" do + before { post :destroy, params: { id: note.id } } - it { expect(response).to redirect_to(admin_notes_path) } - it { expect(Note.count).to eq(0) } - end - end + it { expect(response).to redirect_to(admin_notes_path) } + it { expect(Note.count).to eq(0) } end end From 83484aba6b4c71ce843e01df7174be052377b85f Mon Sep 17 00:00:00 2001 From: Matijs van Zuijlen Date: Sun, 13 Oct 2024 12:15:54 +0200 Subject: [PATCH 2/2] Limit assigned attributes when creating and updating Notes Using #permit! is unsafe and not necessary, since we have a fixed set of attributes used in the notes form. Use #permit with a list of attribute names instead. --- app/controllers/admin/notes_controller.rb | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/app/controllers/admin/notes_controller.rb b/app/controllers/admin/notes_controller.rb index c3ab98af..1104ee54 100644 --- a/app/controllers/admin/notes_controller.rb +++ b/app/controllers/admin/notes_controller.rb @@ -23,7 +23,7 @@ def create note = new_note note.state = "published" - note.attributes = params[:note].permit! + note.assign_attributes(note_params) note.text_filter ||= default_text_filter note.published_at ||= Time.zone.now if note.save @@ -41,7 +41,7 @@ def create end def update - @note.attributes = params[:note].permit! + @note.assign_attributes(note_params) @note.save redirect_to admin_notes_url end @@ -54,6 +54,15 @@ def destroy private + def note_params + params.require(:note).permit(:text_filter_name, + :body, + :push_to_twitter, + :in_reply_to_status_id, + :permalink, + :published_at) + end + def load_existing_notes @notes = Note.page(params[:page]).per(this_blog.limit_article_display) end