From ddded2775770798beae04761b34c4d1933904917 Mon Sep 17 00:00:00 2001 From: Andreas Nedbal Date: Mon, 21 Nov 2022 22:28:38 +0100 Subject: [PATCH 1/6] Remove anonymous block AJAX implementation --- .../ajax/anonymous_block_controller.rb | 39 --------------- .../features/inbox/entry/blockAnon.ts | 31 ------------ .../retrospring/features/inbox/entry/index.ts | 4 +- .../features/moderation/blockAnon.ts | 47 ------------------- .../retrospring/features/moderation/index.ts | 4 +- config/routes.rb | 2 - 6 files changed, 2 insertions(+), 125 deletions(-) delete mode 100644 app/controllers/ajax/anonymous_block_controller.rb delete mode 100644 app/javascript/retrospring/features/inbox/entry/blockAnon.ts delete mode 100644 app/javascript/retrospring/features/moderation/blockAnon.ts diff --git a/app/controllers/ajax/anonymous_block_controller.rb b/app/controllers/ajax/anonymous_block_controller.rb deleted file mode 100644 index 20503553..00000000 --- a/app/controllers/ajax/anonymous_block_controller.rb +++ /dev/null @@ -1,39 +0,0 @@ -# frozen_string_literal: true - -class Ajax::AnonymousBlockController < AjaxController - def create - params.require :question - - question = Question.find(params[:question]) - - raise Errors::Forbidden if params[:global] && !current_user.mod? - - AnonymousBlock.create!( - user: params[:global] ? nil : current_user, - identifier: question.author_identifier, - question: - ) - - question.inboxes.first&.destroy - - @response[:status] = :okay - @response[:message] = t(".success") - @response[:success] = true - end - - def destroy - params.require :id - - block = AnonymousBlock.find(params[:id]) - if current_user != block.user - @response[:status] = :nopriv - @response[:message] = t(".nopriv") - end - - block.destroy! - - @response[:status] = :okay - @response[:message] = t(".success") - @response[:success] = true - end -end diff --git a/app/javascript/retrospring/features/inbox/entry/blockAnon.ts b/app/javascript/retrospring/features/inbox/entry/blockAnon.ts deleted file mode 100644 index f0d184b7..00000000 --- a/app/javascript/retrospring/features/inbox/entry/blockAnon.ts +++ /dev/null @@ -1,31 +0,0 @@ -import { post } from '@rails/request.js'; - -import { showErrorNotification, showNotification } from "utilities/notifications"; -import I18n from "retrospring/i18n"; - -export function blockAnonEventHandler(event: Event): void { - const element: HTMLAnchorElement = event.target as HTMLAnchorElement; - - const data = { - question: element.getAttribute('data-q-id'), - }; - - post('/ajax/block_anon', { - body: data, - contentType: 'application/json' - }) - .then(async response => { - const data = await response.json; - - if (!data.success) return false; - const inboxEntry: Node = element.closest('.inbox-entry'); - - showNotification(data.message); - - (inboxEntry as HTMLElement).remove(); - }) - .catch(err => { - console.log(err); - showErrorNotification(I18n.translate('frontend.error.message')); - }); -} \ No newline at end of file diff --git a/app/javascript/retrospring/features/inbox/entry/index.ts b/app/javascript/retrospring/features/inbox/entry/index.ts index 27bbc5ac..54022129 100644 --- a/app/javascript/retrospring/features/inbox/entry/index.ts +++ b/app/javascript/retrospring/features/inbox/entry/index.ts @@ -3,15 +3,13 @@ import { answerEntryHandler, answerEntryInputHandler } from './answer'; import { deleteEntryHandler } from './delete'; import optionsEntryHandler from './options'; import { reportEventHandler } from './report'; -import { blockAnonEventHandler } from "retrospring/features/inbox/entry/blockAnon"; export default (): void => { registerEvents([ { type: 'click', target: 'button[name="ib-answer"]', handler: answerEntryHandler, global: true }, { type: 'click', target: '[name="ib-destroy"]', handler: deleteEntryHandler, global: true }, { type: 'click', target: '[name=ib-report]', handler: reportEventHandler, global: true }, - { type: 'click', target: '[name=ib-block-anon]', handler: blockAnonEventHandler, global: true }, { type: 'click', target: 'button[name=ib-options]', handler: optionsEntryHandler, global: true }, { type: 'keydown', target: 'textarea[name=ib-answer]', handler: answerEntryInputHandler, global: true } ]); -} \ No newline at end of file +} diff --git a/app/javascript/retrospring/features/moderation/blockAnon.ts b/app/javascript/retrospring/features/moderation/blockAnon.ts deleted file mode 100644 index 4f04004c..00000000 --- a/app/javascript/retrospring/features/moderation/blockAnon.ts +++ /dev/null @@ -1,47 +0,0 @@ -import { post } from '@rails/request.js'; -import swal from 'sweetalert'; - -import { showErrorNotification, showNotification } from "utilities/notifications"; -import I18n from "retrospring/i18n"; - -export function blockAnonEventHandler(event: Event): void { - event.preventDefault(); - - swal({ - title: I18n.translate('frontend.mod_mute.confirm.title'), - text: I18n.translate('frontend.mod_mute.confirm.text'), - type: 'warning', - showCancelButton: true, - confirmButtonColor: "#DD6B55", - confirmButtonText: I18n.translate('voc.y'), - cancelButtonText: I18n.translate('voc.n'), - closeOnConfirm: true, - }, (dialogResult) => { - if (!dialogResult) { - return; - } - - const sender: HTMLAnchorElement = event.target as HTMLAnchorElement; - - const data = { - question: sender.getAttribute('data-q-id'), - global: 'true' - }; - - post('/ajax/block_anon', { - body: data, - contentType: 'application/json' - }) - .then(async response => { - const data = await response.json; - - if (!data.success) return false; - - showNotification(data.message); - }) - .catch(err => { - console.log(err); - showErrorNotification(I18n.translate('frontend.error.message')); - }); - }); -} \ No newline at end of file diff --git a/app/javascript/retrospring/features/moderation/index.ts b/app/javascript/retrospring/features/moderation/index.ts index 7d0e639a..bafa6130 100644 --- a/app/javascript/retrospring/features/moderation/index.ts +++ b/app/javascript/retrospring/features/moderation/index.ts @@ -2,15 +2,13 @@ import registerEvents from 'utilities/registerEvents'; import { banCheckboxHandler, banFormHandler, permanentBanCheckboxHandler } from './ban'; import { destroyReportHandler } from './destroy'; import { privilegeCheckHandler } from './privilege'; -import { blockAnonEventHandler } from './blockAnon'; export default (): void => { registerEvents([ { type: 'click', target: '[type="checkbox"][name="check-your-privileges"]', handler: privilegeCheckHandler, global: true }, { type: 'click', target: '[name="mod-delete-report"]', handler: destroyReportHandler, global: true }, - { type: 'click', target: '[name="mod-block-anon"]', handler: blockAnonEventHandler, global: true }, { type: 'change', target: '[name="ban"][type="checkbox"]', handler: banCheckboxHandler, global: true }, { type: 'change', target: '[name="permaban"][type="checkbox"]', handler: permanentBanCheckboxHandler, global: true }, { type: 'submit', target: '#modal-ban form', handler: banFormHandler, global: true } ]); -} \ No newline at end of file +} diff --git a/config/routes.rb b/config/routes.rb index 62c550e8..bee8dc2a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -132,8 +132,6 @@ Rails.application.routes.draw do post "/list_membership", to: "list#membership", as: :list_membership post "/subscribe", to: "subscription#subscribe", as: :subscribe_answer post "/unsubscribe", to: "subscription#unsubscribe", as: :unsubscribe_answer - post "/block_anon", to: "anonymous_block#create", as: :block_anon - delete "/block_anon/:id", to: "anonymous_block#destroy", as: :unblock_anon end get "/discover", to: "discover#index", as: :discover From d000ddaae49ed37958e27fd51ae0e366d375b60d Mon Sep 17 00:00:00 2001 From: Andreas Nedbal Date: Mon, 21 Nov 2022 22:29:47 +0100 Subject: [PATCH 2/6] Add `AnonymousBlockController` --- app/controllers/anonymous_block_controller.rb | 43 +++++++++++++++++++ app/policies/anonymous_block_policy.rb | 18 ++++++++ config/routes.rb | 2 + 3 files changed, 63 insertions(+) create mode 100644 app/controllers/anonymous_block_controller.rb create mode 100644 app/policies/anonymous_block_policy.rb diff --git a/app/controllers/anonymous_block_controller.rb b/app/controllers/anonymous_block_controller.rb new file mode 100644 index 00000000..80c64799 --- /dev/null +++ b/app/controllers/anonymous_block_controller.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +class AnonymousBlockController < ApplicationController + before_action :authenticate_user! + + def create + params.require :question + + question = Question.find(params[:question]) + + authorize AnonymousBlock, :create_global? if params[:global] + + AnonymousBlock.create!( + user: params[:global] ? nil : current_user, + identifier: question.author_identifier, + question: + ) + + inbox_id = question.inboxes.first.id + question.inboxes.first&.destroy + + respond_to do |format| + format.turbo_stream do + render turbo_stream: turbo_stream.remove("inbox_#{inbox_id}") + end + end + end + + def destroy + params.require :id + + block = AnonymousBlock.find(params[:id]) + authorize block + + block.destroy! + + respond_to do |format| + format.turbo_stream do + render turbo_stream: turbo_stream.remove("block_#{params[:id]}") + end + end + end +end diff --git a/app/policies/anonymous_block_policy.rb b/app/policies/anonymous_block_policy.rb new file mode 100644 index 00000000..b16d04bf --- /dev/null +++ b/app/policies/anonymous_block_policy.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class AnonymousBlockPolicy + attr_reader :user, :anonymous_block + + def initialize(user, anonymous_block) + @user = user + @anonymous_block = anonymous_block + end + + def create_global? + user.mod? + end + + def destroy? + user == anonymous_block.user || user.mod? + end +end diff --git a/config/routes.rb b/config/routes.rb index bee8dc2a..8a753e5b 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -134,6 +134,8 @@ Rails.application.routes.draw do post "/unsubscribe", to: "subscription#unsubscribe", as: :unsubscribe_answer end + resource :anonymous_block, controller: :anonymous_block, only: %i[create destroy] + get "/discover", to: "discover#index", as: :discover match "/public", to: "timeline#public", via: [:get, :post], as: :public_timeline if APP_CONFIG.dig(:features, :public, :enabled) match "/list/:list_name", to: "timeline#list", via: [:get, :post], as: :list_timeline From b81fbb7fe6e6bf90e7c3df342a50abda131f256e Mon Sep 17 00:00:00 2001 From: Andreas Nedbal Date: Mon, 21 Nov 2022 22:30:15 +0100 Subject: [PATCH 3/6] Wire up Turbo Streams action calls in templates --- app/views/actions/_question.html.haml | 4 ++-- app/views/inbox/_entry.html.haml | 2 +- app/views/shared/_anonymous_block.html.haml | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/views/actions/_question.html.haml b/app/views/actions/_question.html.haml index f82a1613..8e960655 100644 --- a/app/views/actions/_question.html.haml +++ b/app/views/actions/_question.html.haml @@ -8,11 +8,11 @@ %i.fa.fa-fw.fa-exclamation-triangle = t("voc.report") - if question.anonymous? && !question.generated? - %a.dropdown-item{ name: "ib-block-anon", data: { q_id: question.id } } + = button_to anonymous_block_path, method: :post, params: { question: question.id }, class: "dropdown-item" do %i.fa.fa-fw.fa-minus-circle = t("voc.block") - if current_user.mod? - %a.dropdown-item{ href: "#", name: "mod-block-anon", data: { q_id: question.id } } + = button_to anonymous_block_path, method: :post, params: { question: question.id, global: true }, class: "dropdown-item" do %i.fa.fa-fw.fa-volume-off = t("voc.block_site_wide") %a.dropdown-item{ href: moderation_questions_path(author_identifier: question.author_identifier) } diff --git a/app/views/inbox/_entry.html.haml b/app/views/inbox/_entry.html.haml index 2a1b6e0b..25931da2 100644 --- a/app/views/inbox/_entry.html.haml +++ b/app/views/inbox/_entry.html.haml @@ -1,4 +1,4 @@ -.card.inbox-entry{ class: i.new? ? "inbox-entry--new" : "", data: { id: i.id } } +.card.inbox-entry{ id: "inbox_#{i.id}", class: i.new? ? "inbox-entry--new" : "", data: { id: i.id } } .card-header .media - unless i.question.author_is_anonymous diff --git a/app/views/shared/_anonymous_block.html.haml b/app/views/shared/_anonymous_block.html.haml index 39cb20a9..e3e21349 100644 --- a/app/views/shared/_anonymous_block.html.haml +++ b/app/views/shared/_anonymous_block.html.haml @@ -1,4 +1,4 @@ -%li.list-group-item +%li.list-group-item{ id: "block_#{block.id}" } .d-flex %div - if block.question.present? @@ -7,5 +7,5 @@ %p.mb-0.text-muted.font-italic= t(".deleted_question") %p.text-muted.mb-0= t(".blocked", time: time_ago_in_words(block.created_at)) .ml-auto.d-inline-flex - %button.btn.btn-default.align-self-center{ data: { action: "anon-unblock", target: block.id } } - %span.pe-none= t("voc.unblock") \ No newline at end of file + = button_to anonymous_block_path, method: :delete, params: { id: block.id }, class: "btn btn-default align-self-center" do + %span.pe-none= t("voc.unblock") From d9991f5fa7f68ee33bff3c3dec2c3e5ff513c27a Mon Sep 17 00:00:00 2001 From: Andreas Nedbal Date: Mon, 21 Nov 2022 23:05:04 +0100 Subject: [PATCH 4/6] Add tests --- app/controllers/anonymous_block_controller.rb | 4 ++ .../anonymous_block_controller_spec.rb | 63 ++----------------- 2 files changed, 10 insertions(+), 57 deletions(-) rename spec/controllers/{ajax => }/anonymous_block_controller_spec.rb (56%) diff --git a/app/controllers/anonymous_block_controller.rb b/app/controllers/anonymous_block_controller.rb index 80c64799..4fdda8cd 100644 --- a/app/controllers/anonymous_block_controller.rb +++ b/app/controllers/anonymous_block_controller.rb @@ -23,6 +23,8 @@ class AnonymousBlockController < ApplicationController format.turbo_stream do render turbo_stream: turbo_stream.remove("inbox_#{inbox_id}") end + + format.html { redirect_back(fallback_location: inbox_path) } end end @@ -38,6 +40,8 @@ class AnonymousBlockController < ApplicationController format.turbo_stream do render turbo_stream: turbo_stream.remove("block_#{params[:id]}") end + + format.html { redirect_back(fallback_location: settings_blocks_path) } end end end diff --git a/spec/controllers/ajax/anonymous_block_controller_spec.rb b/spec/controllers/anonymous_block_controller_spec.rb similarity index 56% rename from spec/controllers/ajax/anonymous_block_controller_spec.rb rename to spec/controllers/anonymous_block_controller_spec.rb index 689916d8..76b737cb 100644 --- a/spec/controllers/ajax/anonymous_block_controller_spec.rb +++ b/spec/controllers/anonymous_block_controller_spec.rb @@ -2,7 +2,7 @@ require "rails_helper" -describe Ajax::AnonymousBlockController, :ajax_controller, type: :controller do +describe AnonymousBlockController, type: :controller do describe "#create" do subject { post(:create, params: params) } @@ -20,19 +20,9 @@ describe Ajax::AnonymousBlockController, :ajax_controller, type: :controller do { question: question.id } end - let(:expected_response) do - { - "success" => true, - "status" => "okay", - "message" => anything - } - end - it "creates an anonymous block" do expect { subject }.to(change { AnonymousBlock.count }.by(1)) end - - include_examples "returns the expected response" end context "when blocking a user globally" do @@ -43,14 +33,6 @@ describe Ajax::AnonymousBlockController, :ajax_controller, type: :controller do end context "as a moderator" do - let(:expected_response) do - { - "success" => true, - "status" => "okay", - "message" => anything - } - end - before do user.add_role(:moderator) end @@ -59,43 +41,14 @@ describe Ajax::AnonymousBlockController, :ajax_controller, type: :controller do expect { subject }.to(change { AnonymousBlock.count }.by(1)) expect(AnonymousBlock.last.user_id).to be_nil end - - include_examples "returns the expected response" end context "as a regular user" do - let(:expected_response) do - { - "success" => false, - "status" => "forbidden", - "message" => anything - } - end - it "does not create an anonymous block" do - expect { subject }.not_to(change { AnonymousBlock.count }) + expect { subject }.to raise_error(Pundit::NotAuthorizedError) end - - include_examples "returns the expected response" end end - - context "when parameters are missing" do - let(:params) { {} } - let(:expected_response) do - { - "success" => false, - "status" => "parameter_error", - "message" => anything - } - end - - it "does not create an anonymous block" do - expect { subject }.not_to(change { AnonymousBlock.count }) - end - - include_examples "returns the expected response" - end end end @@ -116,15 +69,11 @@ describe Ajax::AnonymousBlockController, :ajax_controller, type: :controller do { id: block.id } end - let(:expected_response) do - { - "success" => true, - "status" => "okay", - "message" => anything - } - end + it "destroys the anonymous block" do + subject - include_examples "returns the expected response" + expect(AnonymousBlock.exists?(block.id)).to eq(false) + end end end end From 1c4993df3fade65435d8b4654bc98cd536b9610b Mon Sep 17 00:00:00 2001 From: Andreas Nedbal Date: Mon, 21 Nov 2022 23:07:18 +0100 Subject: [PATCH 5/6] Fix lints --- app/controllers/anonymous_block_controller.rb | 1 - spec/controllers/anonymous_block_controller_spec.rb | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/app/controllers/anonymous_block_controller.rb b/app/controllers/anonymous_block_controller.rb index 4fdda8cd..404d396e 100644 --- a/app/controllers/anonymous_block_controller.rb +++ b/app/controllers/anonymous_block_controller.rb @@ -7,7 +7,6 @@ class AnonymousBlockController < ApplicationController params.require :question question = Question.find(params[:question]) - authorize AnonymousBlock, :create_global? if params[:global] AnonymousBlock.create!( diff --git a/spec/controllers/anonymous_block_controller_spec.rb b/spec/controllers/anonymous_block_controller_spec.rb index 76b737cb..15dfef01 100644 --- a/spec/controllers/anonymous_block_controller_spec.rb +++ b/spec/controllers/anonymous_block_controller_spec.rb @@ -4,7 +4,7 @@ require "rails_helper" describe AnonymousBlockController, type: :controller do describe "#create" do - subject { post(:create, params: params) } + subject { post(:create, params:) } context "user signed in" do let(:user) { FactoryBot.create(:user) } @@ -53,7 +53,7 @@ describe AnonymousBlockController, type: :controller do end describe "#destroy" do - subject { delete(:destroy, params: params) } + subject { delete(:destroy, params:) } context "user signed in" do let(:user) { FactoryBot.create(:user) } @@ -64,7 +64,7 @@ describe AnonymousBlockController, type: :controller do context "when all parameters are given" do let(:question) { FactoryBot.create(:question, author_identifier: "someidentifier") } - let(:block) { AnonymousBlock.create(user: user, identifier: "someidentifier", question: question) } + let(:block) { AnonymousBlock.create(user:, identifier: "someidentifier", question:) } let(:params) do { id: block.id } end From 55f26cb7d445639ec3c8d415d5b09668e648ef48 Mon Sep 17 00:00:00 2001 From: Andreas Nedbal Date: Tue, 22 Nov 2022 08:38:33 +0100 Subject: [PATCH 6/6] Remove settings unblock TypeScript --- .../retrospring/features/settings/block.ts | 21 ------------------- .../retrospring/features/settings/index.ts | 4 +--- 2 files changed, 1 insertion(+), 24 deletions(-) delete mode 100644 app/javascript/retrospring/features/settings/block.ts diff --git a/app/javascript/retrospring/features/settings/block.ts b/app/javascript/retrospring/features/settings/block.ts deleted file mode 100644 index a84136d7..00000000 --- a/app/javascript/retrospring/features/settings/block.ts +++ /dev/null @@ -1,21 +0,0 @@ -import { destroy } from '@rails/request.js'; -import { showNotification, showErrorNotification } from 'utilities/notifications'; -import I18n from 'retrospring/i18n'; - -export function unblockAnonymousHandler(event: Event): void { - const button: HTMLButtonElement = event.currentTarget as HTMLButtonElement; - const targetId = button.dataset.target; - - destroy(`/ajax/block_anon/${targetId}`) - .then(async response => { - if (!response.ok) return; - - const data = await response.json; - showNotification(data.message, data.success); - button.closest('.list-group-item').remove(); - }) - .catch(err => { - console.log(err); - showErrorNotification(I18n.translate('frontend.error.message')); - }); -} \ No newline at end of file diff --git a/app/javascript/retrospring/features/settings/index.ts b/app/javascript/retrospring/features/settings/index.ts index 0b525372..629d5264 100644 --- a/app/javascript/retrospring/features/settings/index.ts +++ b/app/javascript/retrospring/features/settings/index.ts @@ -2,7 +2,6 @@ import registerEvents from "utilities/registerEvents"; import { profileHeaderChangeHandler, profilePictureChangeHandler } from "./crop"; import { themeDocumentHandler, themeSubmitHandler } from "./theme"; import { userSubmitHandler } from "./password"; -import { unblockAnonymousHandler } from "./block"; export default (): void => { themeDocumentHandler(); @@ -11,7 +10,6 @@ export default (): void => { { type: 'submit', target: document.querySelector('form.edit_theme, form.new_theme'), handler: themeSubmitHandler }, { type: 'submit', target: document.querySelector('#edit_user'), handler: userSubmitHandler }, { type: 'change', target: document.querySelector('#user_profile_picture[type=file]'), handler: profilePictureChangeHandler }, - { type: 'change', target: document.querySelector('#user_profile_header[type=file]'), handler: profileHeaderChangeHandler }, - { type: 'click', target: document.querySelectorAll('[data-action="anon-unblock"]'), handler: unblockAnonymousHandler } + { type: 'change', target: document.querySelector('#user_profile_header[type=file]'), handler: profileHeaderChangeHandler } ]); }