From c2baa86c09a56a267b166a004a6dca7d332a99d9 Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Sun, 29 Jan 2023 00:56:47 +0100 Subject: [PATCH 01/23] Add `pinned_at` to answers --- app/models/answer.rb | 4 ++++ db/migrate/20230128233136_add_pinned_at_to_answers.rb | 8 ++++++++ db/schema.rb | 2 ++ 3 files changed, 14 insertions(+) create mode 100644 db/migrate/20230128233136_add_pinned_at_to_answers.rb diff --git a/app/models/answer.rb b/app/models/answer.rb index 91a6c956..d1e3e645 100644 --- a/app/models/answer.rb +++ b/app/models/answer.rb @@ -14,6 +14,8 @@ class Answer < ApplicationRecord validates :question_id, uniqueness: { scope: :user_id } # rubocop:enable Rails/UniqueValidationWithoutIndex + scope :pinned, -> { where.not(pinned_at: nil) } + SHORT_ANSWER_MAX_LENGTH = 640 # rubocop:disable Rails/SkipsModelValidations @@ -56,4 +58,6 @@ class Answer < ApplicationRecord end def long? = content.length > SHORT_ANSWER_MAX_LENGTH + + def pinned? = pinned_at.present? end diff --git a/db/migrate/20230128233136_add_pinned_at_to_answers.rb b/db/migrate/20230128233136_add_pinned_at_to_answers.rb new file mode 100644 index 00000000..8501ba1e --- /dev/null +++ b/db/migrate/20230128233136_add_pinned_at_to_answers.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +class AddPinnedAtToAnswers < ActiveRecord::Migration[6.1] + def change + add_column :answers, :pinned_at, :timestamp + add_index :answers, %i[user_id pinned_at] + end +end diff --git a/db/schema.rb b/db/schema.rb index 0d70a9eb..33418a33 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -48,8 +48,10 @@ ActiveRecord::Schema.define(version: 2023_02_12_181044) do t.datetime "created_at" t.datetime "updated_at" t.integer "smile_count", default: 0, null: false + t.datetime "pinned_at" t.index ["question_id"], name: "index_answers_on_question_id" t.index ["user_id", "created_at"], name: "index_answers_on_user_id_and_created_at" + t.index ["user_id", "pinned_at"], name: "index_answers_on_user_id_and_pinned_at" end create_table "appendables", force: :cascade do |t| From ed4ec98455e024a3d11d09934770a20c6ebfb59e Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Sun, 29 Jan 2023 21:00:47 +0100 Subject: [PATCH 02/23] Add use case for pinning answers --- lib/use_case/answer/pin.rb | 28 ++++++++++++++++++++++++ spec/lib/use_case/answer/pin_spec.rb | 32 ++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+) create mode 100644 lib/use_case/answer/pin.rb create mode 100644 spec/lib/use_case/answer/pin_spec.rb diff --git a/lib/use_case/answer/pin.rb b/lib/use_case/answer/pin.rb new file mode 100644 index 00000000..dd574e71 --- /dev/null +++ b/lib/use_case/answer/pin.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +module UseCase + module Answer + class Pin < UseCase::Base + option :user, type: Types.Instance(::User) + option :answer, type: Types.Instance(::Answer) + + def call + check_ownership! + + answer.pinned_at = Time.now.utc + answer.save! + + { + status: 200, + resource: answer + } + end + + private + + def check_ownership! + raise ::Errors::NotAuthorized unless answer.user == user + end + end + end +end diff --git a/spec/lib/use_case/answer/pin_spec.rb b/spec/lib/use_case/answer/pin_spec.rb new file mode 100644 index 00000000..920d8f02 --- /dev/null +++ b/spec/lib/use_case/answer/pin_spec.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +require "rails_helper" + +describe UseCase::Answer::Pin do + include ActiveSupport::Testing::TimeHelpers + + subject { UseCase::Answer::Pin.call(user:, answer:) } + + context "answer exists" do + let(:answer) { FactoryBot.create(:answer, user: FactoryBot.create(:user)) } + + context "as answer owner" do + let(:user) { answer.user } + + it "pins the answer" do + travel_to(Time.at(1603290950).utc) do + expect { subject }.to change { answer.pinned_at }.from(nil).to(Time.at(1603290950).utc) + end + end + end + + context "as other user" do + let(:user) { FactoryBot.create(:user) } + + it "does not pin the answer" do + expect { subject }.to raise_error(Errors::NotAuthorized) + expect(answer.reload.pinned_at).to eq(nil) + end + end + end +end From 3451ae1fb04c2502bead1e4a6025a146adc1a01e Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Sun, 29 Jan 2023 21:01:03 +0100 Subject: [PATCH 03/23] Display pinned answers on profiles --- app/views/actions/_answer.html.haml | 9 +++++++++ app/views/user/show.html.haml | 6 +++++- config/locales/controllers.en.yml | 2 ++ config/locales/views.en.yml | 2 ++ 4 files changed, 18 insertions(+), 1 deletion(-) diff --git a/app/views/actions/_answer.html.haml b/app/views/actions/_answer.html.haml index 1fcfdca8..9f778bbe 100644 --- a/app/views/actions/_answer.html.haml +++ b/app/views/actions/_answer.html.haml @@ -16,6 +16,15 @@ %a.dropdown-item{ href: "#", data: { a_id: answer.id, action: "ab-report" } } %i.fa.fa-fw.fa-exclamation-triangle = t("voc.report") + - else + - if answer.pinned? + %a.dropdown-item{ href: "#", data: { a_id: answer.id, action: "ab-unpin" } } + %i.fa.fa-fw.fa-thumbtack + = t(".unpin") + - else + %a.dropdown-item{ href: "#", data: { a_id: answer.id, action: "ab-pin" } } + %i.fa.fa-fw.fa-thumbtack + = t(".pin") - if current_user.admin? %a.dropdown-item{ href: rails_admin_path_for_resource(answer), target: "_blank" } %i.fa.fa-fw.fa-gears diff --git a/app/views/user/show.html.haml b/app/views/user/show.html.haml index ac6452a0..6796f5c0 100644 --- a/app/views/user/show.html.haml +++ b/app/views/user/show.html.haml @@ -1,7 +1,11 @@ - unless @user.banned? + #pinned_answers + - @user.answers.pinned.each do |a| + = render 'answerbox', a: + #answers - @answers.each do |a| - = render 'answerbox', a: a + = render 'answerbox', a: - if @more_data_available .d-flex.justify-content-center.justify-content-sm-start#paginator diff --git a/config/locales/controllers.en.yml b/config/locales/controllers.en.yml index d94570f8..ebd15114 100644 --- a/config/locales/controllers.en.yml +++ b/config/locales/controllers.en.yml @@ -26,6 +26,8 @@ en: destroy: nopriv: "You cannot delete other people's answers." success: "Successfully deleted answer." + pin: + success: "Successfully pinned answer." comment: create: invalid: "Your comment is too long." diff --git a/config/locales/views.en.yml b/config/locales/views.en.yml index c944b72c..4b06bb01 100644 --- a/config/locales/views.en.yml +++ b/config/locales/views.en.yml @@ -73,6 +73,8 @@ en: actions: answer: return: "Return to Inbox" + pin: "Pin to Profile" + unpin: "Unpin from Profile" comment: view_smiles: "View comment smiles" share: From 5f50a08f03fe0160f01f60a7d33dbadc26110ae8 Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Mon, 30 Jan 2023 17:39:34 +0100 Subject: [PATCH 04/23] Adjust answer export test to include pinned_at field --- spec/lib/use_case/data_export/answers_spec.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/lib/use_case/data_export/answers_spec.rb b/spec/lib/use_case/data_export/answers_spec.rb index 3fa24f8e..8298fbf1 100644 --- a/spec/lib/use_case/data_export/answers_spec.rb +++ b/spec/lib/use_case/data_export/answers_spec.rb @@ -32,7 +32,8 @@ describe UseCase::DataExport::Answers, :data_export do user_id: user.id, created_at: "2022-12-10T13:37:42.000Z", updated_at: "2022-12-10T13:37:42.000Z", - smile_count: 0 + smile_count: 0, + pinned_at: nil } ] } From 5b1340b793968f4650a0e7f395b4b3c91ecb5d92 Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Mon, 30 Jan 2023 17:54:10 +0100 Subject: [PATCH 05/23] Appease the dog overlords --- app/views/user/show.html.haml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/views/user/show.html.haml b/app/views/user/show.html.haml index 6796f5c0..4456aa90 100644 --- a/app/views/user/show.html.haml +++ b/app/views/user/show.html.haml @@ -1,11 +1,11 @@ - unless @user.banned? - #pinned_answers + #pinned-answers - @user.answers.pinned.each do |a| - = render 'answerbox', a: + = render "answerbox", a: #answers - @answers.each do |a| - = render 'answerbox', a: + = render "answerbox", a: - if @more_data_available .d-flex.justify-content-center.justify-content-sm-start#paginator From b196909b796794c9a20792dbddae89d6e0e45eb1 Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Tue, 7 Feb 2023 08:32:25 +0100 Subject: [PATCH 06/23] Create frontend endpoint for pinning answers --- app/controllers/answer_controller.rb | 12 ++++++++++++ app/views/actions/_answer.html.haml | 9 +-------- app/views/actions/_pin.html.haml | 14 ++++++++++++++ app/views/answer/pin.turbo_stream.haml | 2 ++ config/locales/views.en.yml | 5 +++-- config/routes.rb | 1 + 6 files changed, 33 insertions(+), 10 deletions(-) create mode 100644 app/views/actions/_pin.html.haml create mode 100644 app/views/answer/pin.turbo_stream.haml diff --git a/app/controllers/answer_controller.rb b/app/controllers/answer_controller.rb index b63be49e..33057ceb 100644 --- a/app/controllers/answer_controller.rb +++ b/app/controllers/answer_controller.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class AnswerController < ApplicationController + before_action :authenticate_user!, only: :pin + def show @answer = Answer.includes(comments: %i[user smiles], question: [:user], smiles: [:user]).find(params[:id]) @display_all = true @@ -16,4 +18,14 @@ class AnswerController < ApplicationController notif.update_all(new: false) unless notif.empty? end end + + def pin + answer = Answer.includes(:user).find(params[:id]) + UseCase::Answer::Pin.call(user: current_user, answer:) + + respond_to do |format| + format.html { redirect_to(user_path(username: current_user.screen_name)) } + format.turbo_stream { render "pin", locals: { answer: } } + end + end end diff --git a/app/views/actions/_answer.html.haml b/app/views/actions/_answer.html.haml index 9f778bbe..de81c031 100644 --- a/app/views/actions/_answer.html.haml +++ b/app/views/actions/_answer.html.haml @@ -17,14 +17,7 @@ %i.fa.fa-fw.fa-exclamation-triangle = t("voc.report") - else - - if answer.pinned? - %a.dropdown-item{ href: "#", data: { a_id: answer.id, action: "ab-unpin" } } - %i.fa.fa-fw.fa-thumbtack - = t(".unpin") - - else - %a.dropdown-item{ href: "#", data: { a_id: answer.id, action: "ab-pin" } } - %i.fa.fa-fw.fa-thumbtack - = t(".pin") + = render "actions/pin", answer: - if current_user.admin? %a.dropdown-item{ href: rails_admin_path_for_resource(answer), target: "_blank" } %i.fa.fa-fw.fa-gears diff --git a/app/views/actions/_pin.html.haml b/app/views/actions/_pin.html.haml new file mode 100644 index 00000000..aae54317 --- /dev/null +++ b/app/views/actions/_pin.html.haml @@ -0,0 +1,14 @@ +- if answer.pinned? + = button_to pin_answer_path(id: answer.id), + class: "dropdown-item", + method: :delete, + form: { id: "ab-pin-#{answer.id}", data: { turbo_stream: true } } do + %i.fa.fa-fw.fa-thumbtack + = t(".unpin") +- else + = button_to pin_answer_path(id: answer.id), + class: "dropdown-item", + method: :post, + form: { id: "ab-pin-#{answer.id}", data: { turbo_stream: true } } do + %i.fa.fa-fw.fa-thumbtack + = t(".pin") diff --git a/app/views/answer/pin.turbo_stream.haml b/app/views/answer/pin.turbo_stream.haml new file mode 100644 index 00000000..bc2c107a --- /dev/null +++ b/app/views/answer/pin.turbo_stream.haml @@ -0,0 +1,2 @@ += turbo_stream.update("ab-pin-#{answer.id}") do + = render "actions/pin", answer: diff --git a/config/locales/views.en.yml b/config/locales/views.en.yml index 4b06bb01..41652ab5 100644 --- a/config/locales/views.en.yml +++ b/config/locales/views.en.yml @@ -73,10 +73,11 @@ en: actions: answer: return: "Return to Inbox" - pin: "Pin to Profile" - unpin: "Unpin from Profile" comment: view_smiles: "View comment smiles" + pin: + pin: "Pin to Profile" + unpin: "Unpin from Profile" share: twitter: "Share on Twitter" tumblr: "Share on Tumblr" diff --git a/config/routes.rb b/config/routes.rb index 74ebc5b0..c8a12365 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -146,6 +146,7 @@ Rails.application.routes.draw do get "/user/:username", to: "user#show" get "/@:username", to: "user#show", as: :user get "/@:username/a/:id", to: "answer#show", as: :answer + post "/@:username/a/:id/pin", to: "answer#pin", as: :pin_answer get "/@:username/q/:id", to: "question#show", as: :question get "/@:username/followers", to: "user#followers", as: :show_user_followers get "/@:username/followings", to: "user#followings", as: :show_user_followings From 410d9b5d8ee5e2587d62ada37533479a0569bfe9 Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Tue, 7 Feb 2023 08:36:29 +0100 Subject: [PATCH 07/23] Implement unpinning answers --- app/controllers/answer_controller.rb | 10 +++++++++ app/views/actions/_pin.html.haml | 2 +- config/routes.rb | 1 + lib/use_case/answer/unpin.rb | 33 ++++++++++++++++++++++++++++ 4 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 lib/use_case/answer/unpin.rb diff --git a/app/controllers/answer_controller.rb b/app/controllers/answer_controller.rb index 33057ceb..aaa5f07b 100644 --- a/app/controllers/answer_controller.rb +++ b/app/controllers/answer_controller.rb @@ -28,4 +28,14 @@ class AnswerController < ApplicationController format.turbo_stream { render "pin", locals: { answer: } } end end + + def unpin + answer = Answer.includes(:user).find(params[:id]) + UseCase::Answer::Unpin.call(user: current_user, answer:) + + respond_to do |format| + format.html { redirect_to(user_path(username: current_user.screen_name)) } + format.turbo_stream { render "pin", locals: { answer: } } + end + end end diff --git a/app/views/actions/_pin.html.haml b/app/views/actions/_pin.html.haml index aae54317..c6d6c4bc 100644 --- a/app/views/actions/_pin.html.haml +++ b/app/views/actions/_pin.html.haml @@ -1,5 +1,5 @@ - if answer.pinned? - = button_to pin_answer_path(id: answer.id), + = button_to unpin_answer_path(id: answer.id), class: "dropdown-item", method: :delete, form: { id: "ab-pin-#{answer.id}", data: { turbo_stream: true } } do diff --git a/config/routes.rb b/config/routes.rb index c8a12365..744a8d83 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -147,6 +147,7 @@ Rails.application.routes.draw do get "/@:username", to: "user#show", as: :user get "/@:username/a/:id", to: "answer#show", as: :answer post "/@:username/a/:id/pin", to: "answer#pin", as: :pin_answer + delete "/@:username/a/:id/pin", to: "answer#unpin", as: :unpin_answer get "/@:username/q/:id", to: "question#show", as: :question get "/@:username/followers", to: "user#followers", as: :show_user_followers get "/@:username/followings", to: "user#followings", as: :show_user_followings diff --git a/lib/use_case/answer/unpin.rb b/lib/use_case/answer/unpin.rb new file mode 100644 index 00000000..ac945e63 --- /dev/null +++ b/lib/use_case/answer/unpin.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +module UseCase + module Answer + class Unpin < UseCase::Base + option :user, type: Types.Instance(::User) + option :answer, type: Types.Instance(::Answer) + + def call + check_ownership! + check_pinned! + + answer.pinned_at = nil + answer.save! + + { + status: 200, + resource: nil + } + end + + private + + def check_ownership! + raise ::Errors::NotAuthorized unless answer.user == user + end + + def check_pinned! + raise ::Errors::BadRequest if answer.pinned_at.nil? + end + end + end +end From 438884e13ad2781d5c0ea65874649588ee0e64fb Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Tue, 7 Feb 2023 22:06:35 +0100 Subject: [PATCH 08/23] Add trailing commas (lint) --- lib/use_case/answer/pin.rb | 2 +- lib/use_case/answer/unpin.rb | 2 +- spec/lib/use_case/data_export/answers_spec.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/use_case/answer/pin.rb b/lib/use_case/answer/pin.rb index dd574e71..a14e8674 100644 --- a/lib/use_case/answer/pin.rb +++ b/lib/use_case/answer/pin.rb @@ -14,7 +14,7 @@ module UseCase { status: 200, - resource: answer + resource: answer, } end diff --git a/lib/use_case/answer/unpin.rb b/lib/use_case/answer/unpin.rb index ac945e63..f9553b8d 100644 --- a/lib/use_case/answer/unpin.rb +++ b/lib/use_case/answer/unpin.rb @@ -15,7 +15,7 @@ module UseCase { status: 200, - resource: nil + resource: nil, } end diff --git a/spec/lib/use_case/data_export/answers_spec.rb b/spec/lib/use_case/data_export/answers_spec.rb index 8298fbf1..99335a91 100644 --- a/spec/lib/use_case/data_export/answers_spec.rb +++ b/spec/lib/use_case/data_export/answers_spec.rb @@ -33,7 +33,7 @@ describe UseCase::DataExport::Answers, :data_export do created_at: "2022-12-10T13:37:42.000Z", updated_at: "2022-12-10T13:37:42.000Z", smile_count: 0, - pinned_at: nil + pinned_at: nil, } ] } From 664bf5eab295e9f60308b784919904fe2b9e811a Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Tue, 7 Feb 2023 22:11:36 +0100 Subject: [PATCH 09/23] Add test for unpin use case --- spec/lib/use_case/answer/unpin_spec.rb | 31 ++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 spec/lib/use_case/answer/unpin_spec.rb diff --git a/spec/lib/use_case/answer/unpin_spec.rb b/spec/lib/use_case/answer/unpin_spec.rb new file mode 100644 index 00000000..b6f5ea9c --- /dev/null +++ b/spec/lib/use_case/answer/unpin_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require "rails_helper" + +describe UseCase::Answer::Unpin do + include ActiveSupport::Testing::TimeHelpers + + subject { UseCase::Answer::Unpin.call(user:, answer:) } + + context "answer exists" do + let(:pinned_at) { Time.at(1603290950).utc } + let(:answer) { FactoryBot.create(:answer, user: FactoryBot.create(:user), pinned_at:) } + + context "as answer owner" do + let(:user) { answer.user } + + it "unpins the answer" do + expect { subject }.to change { answer.pinned_at }.from(pinned_at).to(nil) + end + end + + context "as other user" do + let(:user) { FactoryBot.create(:user) } + + it "does not unpin the answer" do + expect { subject }.to raise_error(Errors::NotAuthorized) + expect(answer.reload.pinned_at).to eq(pinned_at) + end + end + end +end From 04303c667e98da34c18ad90bc166befd9226ab53 Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Tue, 7 Feb 2023 22:52:36 +0100 Subject: [PATCH 10/23] Add tests for pin/unpin endpoints --- spec/controllers/answer_controller_spec.rb | 62 +++++++++++++++++++++- 1 file changed, 61 insertions(+), 1 deletion(-) diff --git a/spec/controllers/answer_controller_spec.rb b/spec/controllers/answer_controller_spec.rb index b2c39fd5..86514985 100644 --- a/spec/controllers/answer_controller_spec.rb +++ b/spec/controllers/answer_controller_spec.rb @@ -2,7 +2,9 @@ require "rails_helper" -describe AnswerController do +describe AnswerController, type: :controller do + include ActiveSupport::Testing::TimeHelpers + let(:user) do FactoryBot.create :user, otp_module: :disabled, @@ -39,4 +41,62 @@ describe AnswerController do end end end + + describe "#pin" do + subject { post :pin, params: { username: user.screen_name, id: answer.id } } + + context "user signed in" do + before(:each) { sign_in user } + + it "pins the answer" do + travel_to(Time.at(1603290950).utc) do + expect { subject }.to change { answer.reload.pinned_at }.from(nil).to(Time.at(1603290950).utc) + expect(response).to redirect_to(user_path(user)) + end + end + end + + context "other user signed in" do + let(:other_user) { FactoryBot.create(:user) } + + before(:each) { sign_in other_user } + + it "does not pin the answer do" do + travel_to(Time.at(1603290950).utc) do + expect { subject }.to raise_error(Errors::NotAuthorized) + expect(answer.reload.pinned_at).to eq(nil) + end + end + end + end + + describe "#unpin" do + subject { delete :unpin, params: { username: user.screen_name, id: answer.id } } + + context "user signed in" do + before(:each) do + sign_in user + answer.update!(pinned_at: Time.at(1603290950).utc) + end + + it "pins the answer" do + expect { subject }.to change { answer.reload.pinned_at }.from(Time.at(1603290950).utc).to(nil) + expect(response).to redirect_to(user_path(user)) + end + end + + context "other user signed in" do + let(:other_user) { FactoryBot.create(:user) } + + before(:each) do + sign_in other_user + answer.update!(pinned_at: Time.at(1603290950).utc) + end + + it "does not pin the answer do" do + expect { subject }.to raise_error(Errors::NotAuthorized) + expect(answer.reload.pinned_at).to eq(Time.at(1603290950).utc) + end + end + end end From baea94297514c2691f90238fce7f8e85de42ce37 Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Tue, 7 Feb 2023 22:55:39 +0100 Subject: [PATCH 11/23] Add check for pinning when the answer is already pinned --- lib/use_case/answer/pin.rb | 5 +++++ spec/lib/use_case/answer/pin_spec.rb | 11 +++++++++++ 2 files changed, 16 insertions(+) diff --git a/lib/use_case/answer/pin.rb b/lib/use_case/answer/pin.rb index a14e8674..0e0d76d4 100644 --- a/lib/use_case/answer/pin.rb +++ b/lib/use_case/answer/pin.rb @@ -8,6 +8,7 @@ module UseCase def call check_ownership! + check_unpinned! answer.pinned_at = Time.now.utc answer.save! @@ -23,6 +24,10 @@ module UseCase def check_ownership! raise ::Errors::NotAuthorized unless answer.user == user end + + def check_unpinned! + raise ::Errors::BadRequest if answer.pinned_at.present? + end end end end diff --git a/spec/lib/use_case/answer/pin_spec.rb b/spec/lib/use_case/answer/pin_spec.rb index 920d8f02..26fa29d4 100644 --- a/spec/lib/use_case/answer/pin_spec.rb +++ b/spec/lib/use_case/answer/pin_spec.rb @@ -18,6 +18,17 @@ describe UseCase::Answer::Pin do expect { subject }.to change { answer.pinned_at }.from(nil).to(Time.at(1603290950).utc) end end + + context "answer is already pinned" do + before do + answer.update!(pinned_at: Time.at(1603290950).utc) + end + + it "raises an error" do + expect { subject }.to raise_error(Errors::BadRequest) + expect(answer.reload.pinned_at).to eq(Time.at(1603290950).utc) + end + end end context "as other user" do From dd8f51160fd191bcca171f98de22e349bbc8c8b8 Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Tue, 7 Feb 2023 22:56:41 +0100 Subject: [PATCH 12/23] Add test for unpinning when the answer is not pinned --- spec/lib/use_case/answer/unpin_spec.rb | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/spec/lib/use_case/answer/unpin_spec.rb b/spec/lib/use_case/answer/unpin_spec.rb index b6f5ea9c..f4d7a3cb 100644 --- a/spec/lib/use_case/answer/unpin_spec.rb +++ b/spec/lib/use_case/answer/unpin_spec.rb @@ -17,6 +17,15 @@ describe UseCase::Answer::Unpin do it "unpins the answer" do expect { subject }.to change { answer.pinned_at }.from(pinned_at).to(nil) end + + context "answer is already unpinned" do + let(:pinned_at) { nil } + + it "raises an error" do + expect { subject }.to raise_error(Errors::BadRequest) + expect(answer.reload.pinned_at).to eq(nil) + end + end end context "as other user" do From 6cbce2c1579f83710ba3b65194a8cdfdd08b695c Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Tue, 7 Feb 2023 22:59:48 +0100 Subject: [PATCH 13/23] Require authentication on unpin endpoint --- app/controllers/answer_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/answer_controller.rb b/app/controllers/answer_controller.rb index aaa5f07b..6b045f7a 100644 --- a/app/controllers/answer_controller.rb +++ b/app/controllers/answer_controller.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class AnswerController < ApplicationController - before_action :authenticate_user!, only: :pin + before_action :authenticate_user!, only: %i[pin unpin] def show @answer = Answer.includes(comments: %i[user smiles], question: [:user], smiles: [:user]).find(params[:id]) From de73532befdf22ab56ef114befc6f419a72760a9 Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Tue, 7 Feb 2023 23:12:13 +0100 Subject: [PATCH 14/23] Show indicator for pinned answers --- app/assets/stylesheets/components/_answerbox.scss | 12 ++++++++++++ app/views/application/_answerbox.html.haml | 5 +++++ config/locales/views.en.yml | 1 + 3 files changed, 18 insertions(+) diff --git a/app/assets/stylesheets/components/_answerbox.scss b/app/assets/stylesheets/components/_answerbox.scss index 2eae714c..15909d38 100644 --- a/app/assets/stylesheets/components/_answerbox.scss +++ b/app/assets/stylesheets/components/_answerbox.scss @@ -82,11 +82,23 @@ } } + &__pinned { + display: none; + } + .card-body { padding-bottom: .6rem; } } +#pinned-answers { + .answerbox { + &__pinned { + display: inline; + } + } +} + body:not(.cap-web-share) { [name="ab-share"] { display: none; diff --git a/app/views/application/_answerbox.html.haml b/app/views/application/_answerbox.html.haml index fe27f953..e6b9504b 100644 --- a/app/views/application/_answerbox.html.haml +++ b/app/views/application/_answerbox.html.haml @@ -27,6 +27,11 @@ .col-md-6.text-start.text-muted %i.fa.fa-clock-o = link_to(raw(t("time.distance_ago", time: time_tooltip(a))), answer_path(a.user.screen_name, a.id), class: "answerbox__permalink") + - if instance_variable_defined?(:@user) && @user == a.user && a.pinned_at.present? + %span.answerbox__pinned + · + %i.fa.fa-thumbtack + = t(".pinned") .col-md-6.d-md-flex.answerbox__actions = render "answerbox/actions", a: a, display_all: display_all .card-footer{ id: "ab-comments-section-#{a.id}", class: display_all.nil? ? "d-none" : nil } diff --git a/config/locales/views.en.yml b/config/locales/views.en.yml index 41652ab5..351e2c03 100644 --- a/config/locales/views.en.yml +++ b/config/locales/views.en.yml @@ -126,6 +126,7 @@ en: read: "Read the entire answer" answered: "%{hide} %{user}" # resolves into "Answered by %{user}" hide: "Answered by" + pinned: "Pinned" questionbox: title: "Ask something!" placeholder: "Type your question here…" From fa68ab27d764a36106d6eae3b5c9d4dbd1c58d60 Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Tue, 7 Feb 2023 23:14:33 +0100 Subject: [PATCH 15/23] Limit to 10 pinned answers --- app/controllers/user_controller.rb | 1 + app/views/user/show.html.haml | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb index 17926429..58060d22 100644 --- a/app/controllers/user_controller.rb +++ b/app/controllers/user_controller.rb @@ -7,6 +7,7 @@ class UserController < ApplicationController def show @answers = @user.cursored_answers(last_id: params[:last_id]) + @pinned_answers = @user.answers.pinned.limit(10) @answers_last_id = @answers.map(&:id).min @more_data_available = !@user.cursored_answers(last_id: @answers_last_id, size: 1).count.zero? diff --git a/app/views/user/show.html.haml b/app/views/user/show.html.haml index 4456aa90..25240a33 100644 --- a/app/views/user/show.html.haml +++ b/app/views/user/show.html.haml @@ -1,6 +1,6 @@ - unless @user.banned? #pinned-answers - - @user.answers.pinned.each do |a| + - @pinned_answers.each do |a| = render "answerbox", a: #answers From 2ee25d264f327a5ae5473d0edf9da98a08b95b57 Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Tue, 7 Feb 2023 23:20:47 +0100 Subject: [PATCH 16/23] Simplify pinned check in answerbox This is hidden by CSS in the prior case anyway --- app/views/application/_answerbox.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/application/_answerbox.html.haml b/app/views/application/_answerbox.html.haml index e6b9504b..20b88963 100644 --- a/app/views/application/_answerbox.html.haml +++ b/app/views/application/_answerbox.html.haml @@ -27,7 +27,7 @@ .col-md-6.text-start.text-muted %i.fa.fa-clock-o = link_to(raw(t("time.distance_ago", time: time_tooltip(a))), answer_path(a.user.screen_name, a.id), class: "answerbox__permalink") - - if instance_variable_defined?(:@user) && @user == a.user && a.pinned_at.present? + - if a.pinned_at.present? %span.answerbox__pinned · %i.fa.fa-thumbtack From 2d6ff76461f0bfdcaffb89ca0e32baf670d6fbdb Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Tue, 7 Feb 2023 23:23:49 +0100 Subject: [PATCH 17/23] Appease the dog overlords --- app/assets/stylesheets/components/_answerbox.scss | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/app/assets/stylesheets/components/_answerbox.scss b/app/assets/stylesheets/components/_answerbox.scss index 15909d38..adb1b732 100644 --- a/app/assets/stylesheets/components/_answerbox.scss +++ b/app/assets/stylesheets/components/_answerbox.scss @@ -92,10 +92,9 @@ } #pinned-answers { - .answerbox { - &__pinned { - display: inline; - } + + .answerbox__pinned { + display: inline; } } From 736ca4d6b08e2c6de64596770a7b83c4f332f45f Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Fri, 10 Feb 2023 11:34:55 +0100 Subject: [PATCH 18/23] Use a policy for pinning/unpinning --- app/policies/answer_policy.rb | 14 ++++++++++++++ lib/use_case/answer/pin.rb | 6 +----- lib/use_case/answer/unpin.rb | 6 +----- lib/use_case/base.rb | 6 ++++++ 4 files changed, 22 insertions(+), 10 deletions(-) create mode 100644 app/policies/answer_policy.rb diff --git a/app/policies/answer_policy.rb b/app/policies/answer_policy.rb new file mode 100644 index 00000000..c012c920 --- /dev/null +++ b/app/policies/answer_policy.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class AnswerPolicy + attr_reader :user, :answer + + def initialize(user, answer) + @user = user + @answer = answer + end + + def pin? = answer.user == user + + def unpin? = answer.user == user +end diff --git a/lib/use_case/answer/pin.rb b/lib/use_case/answer/pin.rb index 0e0d76d4..c790663c 100644 --- a/lib/use_case/answer/pin.rb +++ b/lib/use_case/answer/pin.rb @@ -7,7 +7,7 @@ module UseCase option :answer, type: Types.Instance(::Answer) def call - check_ownership! + authorize!(:pin, user, answer) check_unpinned! answer.pinned_at = Time.now.utc @@ -21,10 +21,6 @@ module UseCase private - def check_ownership! - raise ::Errors::NotAuthorized unless answer.user == user - end - def check_unpinned! raise ::Errors::BadRequest if answer.pinned_at.present? end diff --git a/lib/use_case/answer/unpin.rb b/lib/use_case/answer/unpin.rb index f9553b8d..8f12742e 100644 --- a/lib/use_case/answer/unpin.rb +++ b/lib/use_case/answer/unpin.rb @@ -7,7 +7,7 @@ module UseCase option :answer, type: Types.Instance(::Answer) def call - check_ownership! + authorize!(:unpin, user, answer) check_pinned! answer.pinned_at = nil @@ -21,10 +21,6 @@ module UseCase private - def check_ownership! - raise ::Errors::NotAuthorized unless answer.user == user - end - def check_pinned! raise ::Errors::BadRequest if answer.pinned_at.nil? end diff --git a/lib/use_case/base.rb b/lib/use_case/base.rb index e35585c2..7ff23bbf 100644 --- a/lib/use_case/base.rb +++ b/lib/use_case/base.rb @@ -9,5 +9,11 @@ module UseCase def self.call(...) = new(...).call def call = raise NotImplementedError + + private + + def authorize!(verb, user, record, error_class: Errors::NotAuthorized) + raise error_class unless Pundit.policy!(user, record).public_send("#{verb}?") + end end end From 854cf2662edeabb4f4ddb8ee2822b674d215f2c6 Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Fri, 10 Feb 2023 22:18:49 +0100 Subject: [PATCH 19/23] Specify username param for pin/unpin path --- app/views/actions/_pin.html.haml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/actions/_pin.html.haml b/app/views/actions/_pin.html.haml index c6d6c4bc..41a1fa59 100644 --- a/app/views/actions/_pin.html.haml +++ b/app/views/actions/_pin.html.haml @@ -1,12 +1,12 @@ - if answer.pinned? - = button_to unpin_answer_path(id: answer.id), + = button_to unpin_answer_path(username: current_user.screen_name, id: answer.id), class: "dropdown-item", method: :delete, form: { id: "ab-pin-#{answer.id}", data: { turbo_stream: true } } do %i.fa.fa-fw.fa-thumbtack = t(".unpin") - else - = button_to pin_answer_path(id: answer.id), + = button_to pin_answer_path(username: current_user.screen_name, id: answer.id), class: "dropdown-item", method: :post, form: { id: "ab-pin-#{answer.id}", data: { turbo_stream: true } } do From 6724aef105baff5f2e824b7c0eac458b8f907753 Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Fri, 10 Feb 2023 22:23:34 +0100 Subject: [PATCH 20/23] Order pinned answers by when they were pinned --- app/controllers/user_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb index 58060d22..1963b496 100644 --- a/app/controllers/user_controller.rb +++ b/app/controllers/user_controller.rb @@ -7,7 +7,7 @@ class UserController < ApplicationController def show @answers = @user.cursored_answers(last_id: params[:last_id]) - @pinned_answers = @user.answers.pinned.limit(10) + @pinned_answers = @user.answers.pinned.order(pinned_at: :desc).limit(10) @answers_last_id = @answers.map(&:id).min @more_data_available = !@user.cursored_answers(last_id: @answers_last_id, size: 1).count.zero? From dcad9073a8be220c0b3d9deae56a020ef23c0282 Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Fri, 10 Feb 2023 22:33:17 +0100 Subject: [PATCH 21/23] Fix typos in pinning tests --- spec/controllers/answer_controller_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/controllers/answer_controller_spec.rb b/spec/controllers/answer_controller_spec.rb index 86514985..2c967c40 100644 --- a/spec/controllers/answer_controller_spec.rb +++ b/spec/controllers/answer_controller_spec.rb @@ -61,7 +61,7 @@ describe AnswerController, type: :controller do before(:each) { sign_in other_user } - it "does not pin the answer do" do + it "does not pin the answer" do travel_to(Time.at(1603290950).utc) do expect { subject }.to raise_error(Errors::NotAuthorized) expect(answer.reload.pinned_at).to eq(nil) @@ -79,7 +79,7 @@ describe AnswerController, type: :controller do answer.update!(pinned_at: Time.at(1603290950).utc) end - it "pins the answer" do + it "unpins the answer" do expect { subject }.to change { answer.reload.pinned_at }.from(Time.at(1603290950).utc).to(nil) expect(response).to redirect_to(user_path(user)) end @@ -93,7 +93,7 @@ describe AnswerController, type: :controller do answer.update!(pinned_at: Time.at(1603290950).utc) end - it "does not pin the answer do" do + it "does not unpin the answer" do expect { subject }.to raise_error(Errors::NotAuthorized) expect(answer.reload.pinned_at).to eq(Time.at(1603290950).utc) end From 520f7eb9ef7501b906a1175b252a46da8f8213f6 Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Sun, 12 Feb 2023 20:29:36 +0100 Subject: [PATCH 22/23] Show toasts on pin/unpin --- app/controllers/answer_controller.rb | 18 ++++++++++++++++-- config/locales/controllers.en.yml | 5 +++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/app/controllers/answer_controller.rb b/app/controllers/answer_controller.rb index 6b045f7a..d5e70881 100644 --- a/app/controllers/answer_controller.rb +++ b/app/controllers/answer_controller.rb @@ -3,6 +3,10 @@ class AnswerController < ApplicationController before_action :authenticate_user!, only: %i[pin unpin] + include TurboStreamable + + turbo_stream_actions :pin, :unpin + def show @answer = Answer.includes(comments: %i[user smiles], question: [:user], smiles: [:user]).find(params[:id]) @display_all = true @@ -25,7 +29,12 @@ class AnswerController < ApplicationController respond_to do |format| format.html { redirect_to(user_path(username: current_user.screen_name)) } - format.turbo_stream { render "pin", locals: { answer: } } + format.turbo_stream do + render turbo_stream: [ + turbo_stream.update("ab-pin-#{answer.id}", partial: "actions/pin", locals: { answer: }), + render_toast(t(".success")) + ] + end end end @@ -35,7 +44,12 @@ class AnswerController < ApplicationController respond_to do |format| format.html { redirect_to(user_path(username: current_user.screen_name)) } - format.turbo_stream { render "pin", locals: { answer: } } + format.turbo_stream do + render turbo_stream: [ + turbo_stream.update("ab-pin-#{answer.id}", partial: "actions/pin", locals: { answer: }), + render_toast(t(".success")) + ] + end end end end diff --git a/config/locales/controllers.en.yml b/config/locales/controllers.en.yml index ebd15114..ff41c0c6 100644 --- a/config/locales/controllers.en.yml +++ b/config/locales/controllers.en.yml @@ -212,3 +212,8 @@ en: timeline: public: title: "Public Timeline" + answer: + pin: + success: "This answer will now appear at the top of your profile." + unpin: + success: "This answer will no longer be pinned to the top of your profile." From 793fec7da173a33554b5883f643c5ad74a518b9b Mon Sep 17 00:00:00 2001 From: Karina Kwiatek Date: Sun, 12 Feb 2023 21:04:35 +0100 Subject: [PATCH 23/23] Update pinning tests to match new Turbo Stream behaviour --- spec/controllers/answer_controller_spec.rb | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/spec/controllers/answer_controller_spec.rb b/spec/controllers/answer_controller_spec.rb index 2c967c40..108b3969 100644 --- a/spec/controllers/answer_controller_spec.rb +++ b/spec/controllers/answer_controller_spec.rb @@ -43,7 +43,7 @@ describe AnswerController, type: :controller do end describe "#pin" do - subject { post :pin, params: { username: user.screen_name, id: answer.id } } + subject { post :pin, params: { username: user.screen_name, id: answer.id }, format: :turbo_stream } context "user signed in" do before(:each) { sign_in user } @@ -51,7 +51,7 @@ describe AnswerController, type: :controller do it "pins the answer" do travel_to(Time.at(1603290950).utc) do expect { subject }.to change { answer.reload.pinned_at }.from(nil).to(Time.at(1603290950).utc) - expect(response).to redirect_to(user_path(user)) + expect(response.body).to include("turbo-stream action=\"update\" target=\"ab-pin-#{answer.id}\"") end end end @@ -63,15 +63,14 @@ describe AnswerController, type: :controller do it "does not pin the answer" do travel_to(Time.at(1603290950).utc) do - expect { subject }.to raise_error(Errors::NotAuthorized) - expect(answer.reload.pinned_at).to eq(nil) + expect { subject }.not_to(change { answer.reload.pinned_at }) end end end end describe "#unpin" do - subject { delete :unpin, params: { username: user.screen_name, id: answer.id } } + subject { delete :unpin, params: { username: user.screen_name, id: answer.id }, format: :turbo_stream } context "user signed in" do before(:each) do @@ -81,7 +80,7 @@ describe AnswerController, type: :controller do it "unpins the answer" do expect { subject }.to change { answer.reload.pinned_at }.from(Time.at(1603290950).utc).to(nil) - expect(response).to redirect_to(user_path(user)) + expect(response.body).to include("turbo-stream action=\"update\" target=\"ab-pin-#{answer.id}\"") end end @@ -94,8 +93,7 @@ describe AnswerController, type: :controller do end it "does not unpin the answer" do - expect { subject }.to raise_error(Errors::NotAuthorized) - expect(answer.reload.pinned_at).to eq(Time.at(1603290950).utc) + expect { subject }.not_to(change { answer.reload.pinned_at }) end end end