From 62ba205d3e2c2725e6ab1f6cd6a53424f368afa5 Mon Sep 17 00:00:00 2001 From: Andreas Nedbal Date: Sat, 27 Jan 2024 13:31:36 +0100 Subject: [PATCH] Fix rubocop nits --- app/controllers/ajax/inbox_controller.rb | 10 ++-- app/models/question.rb | 7 ++- ...7112216_rename_inboxes_to_inbox_entries.rb | 2 + .../controllers/ajax/inbox_controller_spec.rb | 52 +++++++++---------- 4 files changed, 39 insertions(+), 32 deletions(-) diff --git a/app/controllers/ajax/inbox_controller.rb b/app/controllers/ajax/inbox_controller.rb index 2a7c67a4..77042d62 100644 --- a/app/controllers/ajax/inbox_controller.rb +++ b/app/controllers/ajax/inbox_controller.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class Ajax::InboxController < AjaxController def remove params.require :id @@ -28,7 +30,7 @@ class Ajax::InboxController < AjaxController raise unless user_signed_in? begin - InboxEntry.where(user: current_user).each { |i| i.remove } + InboxEntry.where(user: current_user).find_each(&:remove) rescue => e Sentry.capture_exception(e) @response[:status] = :err @@ -43,10 +45,10 @@ class Ajax::InboxController < AjaxController def remove_all_author begin - @target_user = User.where('LOWER(screen_name) = ?', params[:author].downcase).first! + @target_user = User.where("LOWER(screen_name) = ?", params[:author].downcase).first! @inbox = current_user.inbox_entries.joins(:question) - .where(questions: { user_id: @target_user.id, author_is_anonymous: false }) - @inbox.each { |i| i.remove } + .where(questions: { user_id: @target_user.id, author_is_anonymous: false }) + @inbox.each(&:remove) rescue => e Sentry.capture_exception(e) @response[:status] = :err diff --git a/app/models/question.rb b/app/models/question.rb index 2c289a88..2e7549af 100644 --- a/app/models/question.rb +++ b/app/models/question.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class Question < ApplicationRecord include Question::AnswerMethods @@ -11,7 +13,7 @@ class Question < ApplicationRecord SHORT_QUESTION_MAX_LENGTH = 512 before_destroy do - rep = Report.where(target_id: self.id, type: 'Reports::Question') + rep = Report.where(target_id: self.id, type: "Reports::Question") rep.each do |r| unless r.nil? r.deleted = true @@ -25,8 +27,9 @@ class Question < ApplicationRecord end def can_be_removed? - return false if self.answers.count > 0 + return false if self.answers.count.positive? return false if InboxEntry.where(question: self).count > 1 + true end diff --git a/db/migrate/20240127112216_rename_inboxes_to_inbox_entries.rb b/db/migrate/20240127112216_rename_inboxes_to_inbox_entries.rb index 6c1aeff5..03a8f55b 100644 --- a/db/migrate/20240127112216_rename_inboxes_to_inbox_entries.rb +++ b/db/migrate/20240127112216_rename_inboxes_to_inbox_entries.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class RenameInboxesToInboxEntries < ActiveRecord::Migration[7.0] def change rename_table :inboxes, :inbox_entries diff --git a/spec/controllers/ajax/inbox_controller_spec.rb b/spec/controllers/ajax/inbox_controller_spec.rb index 074a92ae..6e48e53e 100644 --- a/spec/controllers/ajax/inbox_controller_spec.rb +++ b/spec/controllers/ajax/inbox_controller_spec.rb @@ -7,11 +7,11 @@ describe Ajax::InboxController, :ajax_controller, type: :controller do describe "#remove" do let(:params) do { - id: inbox_entry_id + id: inbox_entry_id, } end - subject { delete(:remove, params: params) } + subject { delete(:remove, params:) } context "when user is signed in" do before(:each) { sign_in(user) } @@ -28,8 +28,8 @@ describe Ajax::InboxController, :ajax_controller, type: :controller do let(:expected_response) do { "success" => true, - "status" => "okay", - "message" => anything + "status" => "okay", + "message" => anything, } end @@ -46,8 +46,8 @@ describe Ajax::InboxController, :ajax_controller, type: :controller do let(:expected_response) do { "success" => false, - "status" => "fail", - "message" => anything + "status" => "fail", + "message" => anything, } end @@ -65,8 +65,8 @@ describe Ajax::InboxController, :ajax_controller, type: :controller do let(:expected_response) do { "success" => false, - "status" => "not_found", - "message" => anything + "status" => "not_found", + "message" => anything, } end @@ -79,8 +79,8 @@ describe Ajax::InboxController, :ajax_controller, type: :controller do let(:expected_response) do { "success" => false, - "status" => "not_found", - "message" => anything + "status" => "not_found", + "message" => anything, } end @@ -97,8 +97,8 @@ describe Ajax::InboxController, :ajax_controller, type: :controller do let(:expected_response) do { "success" => true, - "status" => "okay", - "message" => anything + "status" => "okay", + "message" => anything, } end @@ -107,7 +107,7 @@ describe Ajax::InboxController, :ajax_controller, type: :controller do context "when user has some inbox entries" do let(:some_other_user) { FactoryBot.create(:user) } before do - 10.times { FactoryBot.create(:inbox_entry, user: user) } + 10.times { FactoryBot.create(:inbox_entry, user:) } 10.times { FactoryBot.create(:inbox_entry, user: some_other_user) } end @@ -123,8 +123,8 @@ describe Ajax::InboxController, :ajax_controller, type: :controller do let(:expected_response) do { "success" => false, - "status" => "err", - "message" => anything + "status" => "err", + "message" => anything, } end @@ -135,11 +135,11 @@ describe Ajax::InboxController, :ajax_controller, type: :controller do describe "#remove_all_author" do let(:params) do { - author: author + author:, } end - subject { delete(:remove_all_author, params: params) } + subject { delete(:remove_all_author, params:) } context "when user is signed in" do before(:each) { sign_in(user) } @@ -148,8 +148,8 @@ describe Ajax::InboxController, :ajax_controller, type: :controller do let(:expected_response) do { "success" => true, - "status" => "okay", - "message" => anything + "status" => "okay", + "message" => anything, } end @@ -162,9 +162,9 @@ describe Ajax::InboxController, :ajax_controller, type: :controller do normal_question = FactoryBot.create(:question, user: some_other_user, author_is_anonymous: false) anon_question = FactoryBot.create(:question, user: some_other_user, author_is_anonymous: true) - 10.times { FactoryBot.create(:inbox_entry, user: user) } - 3.times { FactoryBot.create(:inbox_entry, user: user, question: normal_question) } - 2.times { FactoryBot.create(:inbox_entry, user: user, question: anon_question) } + 10.times { FactoryBot.create(:inbox_entry, user:) } + 3.times { FactoryBot.create(:inbox_entry, user:, question: normal_question) } + 2.times { FactoryBot.create(:inbox_entry, user:, question: anon_question) } end it "deletes all the entries asked by some other user which are not anonymous from the user's inbox" do @@ -179,8 +179,8 @@ describe Ajax::InboxController, :ajax_controller, type: :controller do let(:expected_response) do { "success" => false, - "status" => "err", - "message" => anything + "status" => "err", + "message" => anything, } end @@ -193,8 +193,8 @@ describe Ajax::InboxController, :ajax_controller, type: :controller do let(:expected_response) do { "success" => false, - "status" => "err", - "message" => anything + "status" => "err", + "message" => anything, } end