Merge pull request #1417 from Retrospring/feature/turbo-subscriptions

Move subscription functionality to Turbo Streams

app/controllers/ajax/subscription_controller.rb

@@ -1,17 +0,0 @@
-class Ajax::SubscriptionController < AjaxController
-  before_action :authenticate_user!
-
-  def subscribe
-    params.require :answer
-    @response[:status] = :okay
-    result = Subscription.subscribe(current_user, Answer.find(params[:answer]))
-    @response[:success] = result.present?
-  end
-
-  def unsubscribe
-    params.require :answer
-    @response[:status] = :okay
-    result = Subscription.unsubscribe(current_user, Answer.find(params[:answer]))
-    @response[:success] = result&.destroyed? || false
-  end
-end

app/controllers/subscriptions_controller.rb

@@ -0,0 +1,41 @@
+# frozen_string_literal: true
+
+class SubscriptionsController < ApplicationController
+  include TurboStreamable
+
+  before_action :authenticate_user!
+
+  turbo_stream_actions :create, :destroy
+
+  def create
+    answer = Answer.find(params[:answer])
+    result = Subscription.subscribe(current_user, answer)
+
+    respond_to do |format|
+      format.turbo_stream do
+        render turbo_stream: [
+          turbo_stream.replace("subscription-#{answer.id}", partial: "subscriptions/destroy", locals: { answer: }),
+          render_toast(t(result.present? ? ".success" : ".error"), result.present?)
+        ]
+      end
+
+      format.html { redirect_to answer_path(username: answer.user.screen_name, id: answer.id) }
+    end
+  end
+
+  def destroy
+    answer = Answer.find(params[:answer])
+    result = Subscription.unsubscribe(current_user, answer)
+
+    respond_to do |format|
+      format.turbo_stream do
+        render turbo_stream: [
+          turbo_stream.replace("subscription-#{answer.id}", partial: "subscriptions/create", locals: { answer: }),
+          render_toast(t(result.present? ? ".success" : ".error"), result.present?)
+        ]
+      end
+
+      format.html { redirect_to answer_path(username: answer.user.screen_name, id: answer.id) }
+    end
+  end
+end

app/javascript/retrospring/features/answerbox/index.ts

@@ -3,11 +3,9 @@ import registerAnswerboxCommentEvents from './comment';
 import { answerboxDestroyHandler } from './destroy';
 import { answerboxReportHandler } from './report';
 import { answerboxSmileHandler } from './smile';
-import { answerboxSubscribeHandler } from './subscribe';
 
 export default (): void => {
   registerEvents([
-    { type: 'click', target: '[data-action=ab-submarine]', handler: answerboxSubscribeHandler, global: true },
     { type: 'click', target: '[data-action=ab-report]', handler: answerboxReportHandler, global: true },
     { type: 'click', target: '[data-action=ab-destroy]', handler: answerboxDestroyHandler, global: true },
     { type: 'click', target: '[name=ab-smile]', handler: answerboxSmileHandler, global: true }

app/javascript/retrospring/features/answerbox/subscribe.ts

@@ -1,44 +0,0 @@
-import { post } from '@rails/request.js';
-
-import I18n from 'retrospring/i18n';
-import { showNotification, showErrorNotification } from 'utilities/notifications';
-
-export function answerboxSubscribeHandler(event: Event): void {
-  const button = event.target as HTMLButtonElement;
-  const id = button.dataset.aId;
-  let torpedo = 0;
-  let targetUrl;
-  event.preventDefault();
-
-  if (button.dataset.torpedo === 'yes') {
-    torpedo = 1;
-  }
-
-  if (torpedo) {
-    targetUrl = '/ajax/subscribe';
-  } else {
-    targetUrl = '/ajax/unsubscribe';
-  }
-
-  post(targetUrl, {
-    body: {
-      answer: id
-    },
-    contentType: 'application/json'
-  })
-    .then(async response => {
-      const data = await response.json;
-
-      if (data.success) {
-        button.dataset.torpedo = ["yes", "no"][torpedo];
-        button.children[0].nextSibling.textContent = ' ' + (torpedo ? I18n.translate('voc.unsubscribe') : I18n.translate('voc.subscribe'));
-        showNotification(I18n.translate(`frontend.subscription.${torpedo ? 'subscribe' : 'unsubscribe'}`));
-      } else {
-        showErrorNotification(I18n.translate(`frontend.subscription.fail.${torpedo ? 'subscribe' : 'unsubscribe'}`));
-      }
-    })
-    .catch(err => {
-      console.log(err);
-      showErrorNotification(I18n.translate('frontend.error.message'));
-    });
-}

app/views/actions/_answer.html.haml

@@ -1,13 +1,8 @@
 .dropdown-menu.dropdown-menu-end{ role: :menu }
   - if subscribed_answer_ids&.include?(answer.id)
-    -#                                            fun joke    should subscribe?
+    = render "subscriptions/destroy", answer: answer
-    %a.dropdown-item{ href: "#", data: { a_id: answer.id, action: "ab-submarine", torpedo: "no" } }
-      %i.fa.fa-fw.fa-anchor
-      = t("voc.unsubscribe")
   - else
-    %a.dropdown-item{ href: "#", data: { a_id: answer.id, action: "ab-submarine", torpedo: "yes" } }
+    = render "subscriptions/create", answer: answer
-      %i.fa.fa-fw.fa-anchor
-      = t("voc.subscribe")
   - if privileged? answer.user
     %a.dropdown-item.text-danger{ href: "#", data: { a_id: answer.id, action: "ab-destroy" } }
       %i.fa.fa-fw.fa-trash-o

app/views/subscriptions/_create.html.haml

@@ -0,0 +1,3 @@
+= button_to subscriptions_path(answer: answer.id), class: "dropdown-item", form: { id: "subscription-#{answer.id}" } do
+  %i.fa.fa-fw.fa-bell
+  = t("voc.subscribe")

app/views/subscriptions/_destroy.html.haml

@@ -0,0 +1,3 @@
+= button_to subscriptions_path(answer: answer.id), method: :delete, class: "dropdown-item", form: { id: "subscription-#{answer.id}" } do
+  %i.fa.fa-fw.fa-bell-slash
+  = t("voc.unsubscribe")

config/locales/controllers.en.yml

@@ -196,6 +196,13 @@ en:
           error: :errors.invalid_otp
         destroy:
           success: "Two factor authentication has been disabled for your account."
+  subscriptions:
+    create:
+      success: "Successfully subscribed."
+      error: "Failed to subscribe to answer."
+    destroy:
+      success: "Successfully unsubscribed."
+      error: "Failed to unsubscribe from answer."
   user:
     sessions:
       create:

config/locales/frontend.en.yml

@@ -6,12 +6,6 @@ en:
     error:
       title: "Uh-oh…"
       message: "An error occurred, a developer should check the console for details"
-    subscription:
-      subscribe: "Successfully subscribed."
-      unsubscribe: "Successfully unsubscribed."
-      fail:
-        subscribe: "Failed to subscribe to answer."
-        unsubscribe: "Failed to unsubscribe from answer."
     list:
       confirm:
         title: "Are you sure?"

config/routes.rb

@@ -128,8 +128,6 @@ Rails.application.routes.draw do
     post "/create_list", to: "list#create", as: :create_list
     post "/destroy_list", to: "list#destroy", as: :destroy_list
     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
     get "/webpush/key", to: "web_push#key", as: :webpush_key
     post "/webpush/check", to: "web_push#check", as: :webpush_check
     post "/webpush", to: "web_push#subscribe", as: :webpush_subscribe
@@ -148,6 +146,8 @@ Rails.application.routes.draw do
   post "/inbox/create", to: "inbox#create", as: :inbox_create
   get "/inbox", to: "inbox#show", as: :inbox
 
+  resource :subscriptions, controller: :subscriptions, only: %i[create destroy]
+
   get "/user/:username", to: "user#show"
   get "/@:username", to: "user#show", as: :user
   get "/@:username/a/:id", to: "answer#show", as: :answer

spec/controllers/subscriptions_controller_spec.rb

@@ -2,41 +2,33 @@
 
 require "rails_helper"
 
-describe Ajax::SubscriptionController, :ajax_controller, type: :controller do
+describe SubscriptionsController, type: :controller do
   # need to use a different user here, as after a create the user owning the
   # answer is automatically subscribed to it
   let(:answer_user) { FactoryBot.create(:user) }
   let(:answer) { FactoryBot.create(:answer, user: answer_user) }
+  let(:user) { FactoryBot.create(:user) }
 
-  describe "#subscribe" do
+  describe "#create" do
     let(:params) do
       {
-        answer: answer_id
+        answer: answer_id,
       }
     end
 
-    subject { post(:subscribe, params: params) }
+    subject { post(:create, params:, format: :turbo_stream) }
 
     context "when user is signed in" do
       before(:each) { sign_in(user) }
 
       context "when answer exists" do
         let(:answer_id) { answer.id }
-        let(:expected_response) do
-          {
-            "success" => true,
-            "status" =>  "okay",
-            "message" => anything
-          }
-        end
 
         context "when subscription does not exist" do
           it "creates a subscription on the answer" do
             expect { subject }.to(change { answer.subscriptions.count }.by(1))
             expect(answer.subscriptions.map { |s| s.user.id }.sort).to eq([answer_user.id, user.id].sort)
           end
-
-          include_examples "returns the expected response"
         end
 
         context "when subscription already exists" do
@@ -46,26 +38,15 @@ describe Ajax::SubscriptionController, :ajax_controller, type: :controller do
             expect { subject }.to(change { answer.subscriptions.count }.by(0))
             expect(answer.subscriptions.map { |s| s.user.id }.sort).to eq([answer_user.id, user.id].sort)
           end
-
-          include_examples "returns the expected response"
         end
       end
 
       context "when answer does not exist" do
         let(:answer_id) { "Bielefeld" }
-        let(:expected_response) do
-          {
-            "success" => false,
-            "status" => "not_found",
-            "message" => anything
-          }
-        end
 
         it "does not create a new subscription" do
           expect { subject }.not_to(change { Subscription.count })
         end
-
-        include_examples "returns the expected response"
       end
     end
 
@@ -79,27 +60,20 @@ describe Ajax::SubscriptionController, :ajax_controller, type: :controller do
     end
   end
 
-  describe "#unsubscribe" do
+  describe "#destroy" do
     let(:params) do
       {
-        answer: answer_id
+        answer: answer_id,
       }
     end
 
-    subject { post(:unsubscribe, params: params) }
+    subject { delete(:destroy, params:, format: :turbo_stream) }
 
     context "when user is signed in" do
       before(:each) { sign_in(user) }
 
       context "when answer exists" do
         let(:answer_id) { answer.id }
-        let(:expected_response) do
-          {
-            "success" => true,
-            "status" =>  "okay",
-            "message" => anything
-          }
-        end
 
         context "when subscription exists" do
           before(:each) { Subscription.subscribe(user, answer) }
@@ -108,43 +82,22 @@ describe Ajax::SubscriptionController, :ajax_controller, type: :controller do
             expect { subject }.to(change { answer.subscriptions.count }.by(-1))
             expect(answer.subscriptions.map { |s| s.user.id }.sort).to eq([answer_user.id].sort)
           end
-
-          include_examples "returns the expected response"
         end
 
         context "when subscription does not exist" do
-          let(:expected_response) do
-            {
-              "success" => false,
-              "status" =>  "okay",
-              "message" => anything
-            }
-          end
-
           it "does not modify the answer's subscriptions" do
             expect { subject }.to(change { answer.subscriptions.count }.by(0))
             expect(answer.subscriptions.map { |s| s.user.id }.sort).to eq([answer_user.id].sort)
           end
-
-          include_examples "returns the expected response"
         end
       end
 
       context "when answer does not exist" do
         let(:answer_id) { "Bielefeld" }
-        let(:expected_response) do
-          {
-            "success" => false,
-            "status" => "not_found",
-            "message" => anything
-          }
-        end
 
         it "does not create a new subscription" do
           expect { subject }.not_to(change { Subscription.count })
         end
-
-        include_examples "returns the expected response"
       end
     end