From 9f9236164cad2d1a8cc30784ab91896d39c87ad4 Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 8 Jul 2020 08:31:03 -0400 Subject: [PATCH] [deploy] Add subscriptions to the dashboard (#9161) * Add /dashboard/subscriptions route * Dashboard controller updates * Update ArticlePolicy with subscriptions? * Update article row view in dashboard * Add subscriptions view * Add specs * Add comment for pagination limit * Use cached user_subscriptions count * Add spec for cache counter * Add path helper --- app/controllers/dashboards_controller.rb | 15 ++++ app/models/user_subscription.rb | 1 + app/policies/article_policy.rb | 4 + .../_dashboard_article_row.html.erb | 15 ++-- app/views/dashboards/subscriptions.html.erb | 39 +++++++++ config/routes.rb | 3 +- ...dd_user_subscriptions_count_to_articles.rb | 9 ++ db/schema.rb | 3 +- .../user_subscription_sourceable_spec.rb | 19 ++++ spec/requests/dashboard_spec.rb | 86 +++++++++++++++++++ 10 files changed, 186 insertions(+), 8 deletions(-) create mode 100644 app/views/dashboards/subscriptions.html.erb create mode 100644 db/migrate/20200706184804_add_user_subscriptions_count_to_articles.rb diff --git a/app/controllers/dashboards_controller.rb b/app/controllers/dashboards_controller.rb index 5937e9844..1ba4e9540 100644 --- a/app/controllers/dashboards_controller.rb +++ b/app/controllers/dashboards_controller.rb @@ -2,6 +2,7 @@ class DashboardsController < ApplicationController before_action :set_no_cache_header before_action :authenticate_user! before_action :fetch_and_authorize_user, except: :pro + before_action :set_source, only: %i[subscriptions] before_action -> { limit_per_page(default: 80, max: 1000) }, except: %i[show pro] after_action :verify_authorized @@ -70,8 +71,22 @@ class DashboardsController < ApplicationController @organizations = current_user.member_organizations end + def subscriptions + authorize @source + @subscriptions = @source.user_subscriptions. + includes(:subscriber).order(created_at: :desc).page(params[:page]).per(100) + end + private + def set_source + source_type = params[:source_type] + not_found unless UserSubscription::ALLOWED_TYPES.include? source_type + + source = source_type.constantize.find_by(id: params[:source_id]) + @source = source || not_found + end + def fetch_and_authorize_user @user = if params[:username] && current_user.any_admin? User.find_by(username: params[:username]) diff --git a/app/models/user_subscription.rb b/app/models/user_subscription.rb index 8992c97ec..2a4c76c8c 100644 --- a/app/models/user_subscription.rb +++ b/app/models/user_subscription.rb @@ -5,6 +5,7 @@ class UserSubscription < ApplicationRecord ALLOWED_TYPES = %w[Article].freeze counter_culture :subscriber, column_name: "subscribed_to_user_subscriptions_count" + counter_culture :user_subscription_sourceable, column_name: "user_subscriptions_count" belongs_to :author, class_name: "User", inverse_of: :source_authored_user_subscriptions belongs_to :subscriber, class_name: "User", inverse_of: :subscribed_to_user_subscriptions diff --git a/app/policies/article_policy.rb b/app/policies/article_policy.rb index db01ff3b8..c5da8c04f 100644 --- a/app/policies/article_policy.rb +++ b/app/policies/article_policy.rb @@ -34,6 +34,10 @@ class ArticlePolicy < ApplicationPolicy archived] end + def subscriptions? + user_is_author? || user_admin? + end + private def user_is_author? diff --git a/app/views/dashboards/_dashboard_article_row.html.erb b/app/views/dashboards/_dashboard_article_row.html.erb index 7be512322..178cb2e72 100644 --- a/app/views/dashboards/_dashboard_article_row.html.erb +++ b/app/views/dashboards/_dashboard_article_row.html.erb @@ -58,11 +58,11 @@ <% end %> - <% if false %> - + <% if article.user_subscriptions_count > 0 %> + " class="flex items-center crayons-btn crayons-btn--ghost crayons-btn--s fw-normal color-base-70" title="Subscriptions"> <%= inline_svg_tag("small-data.svg", aria: true, class: "crayons-icon mr-1", title: "Data") %> - 4 - + <%= article.user_subscriptions_count %> + <% end %> <% end %> @@ -94,9 +94,12 @@ <%= inline_svg_tag("overflow-horizontal.svg", aria: true, class: "crayons-icon", title: "More...") %> -
+
+ <% if article.user_subscriptions_count > 0 %> + " class="crayons-link crayons-link--block w-100"><%= "Subscriptions (#{article.user_subscriptions_count})" %> + <% end %> <% if @current_user_pro %> - Stats + Stats <% end %> <%= form_for(article, method: :patch, remote: true, authenticity_token: true, html: { "data-type": "json", class: "js-archive-toggle" }) do |f| %> <%= f.hidden_field :archived, value: !article.archived, id: "article_archived_#{article.id}" %> diff --git a/app/views/dashboards/subscriptions.html.erb b/app/views/dashboards/subscriptions.html.erb new file mode 100644 index 000000000..b63bc2ab9 --- /dev/null +++ b/app/views/dashboards/subscriptions.html.erb @@ -0,0 +1,39 @@ +<% title "Subscriptions #{@source.title}" %> + +
+
+
+

Subscriptions

+

+ for + ><%= @source.title %> +

+
+ <%#TODO: [@thepracticaldev/delightful]: uncomment this when ready to implement CSV exports%> + <%#%> +
+ +
+ <% if @subscriptions.any? %> + + + <% @subscriptions.each do |subscription| %> + <% subscriber = subscription.subscriber %> + + + + + + <% end %> + +
<%= "#{subscriber.name} DEV profile" %>><%= subscriber.name %>"><%= subscription.subscriber_email %><%= "#{time_ago_in_words(subscription.created_at)} ago" %>
+
+ <%= paginate @subscriptions, params: { i: nil } %> +
+ <% else %> +
+ You don't have any subscribers for this <%= @source.class.name.downcase %> yet... +
+ <% end %> +
+
diff --git a/config/routes.rb b/config/routes.rb index c07b11440..5efa0f703 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -392,6 +392,7 @@ Rails.application.routes.draw do get "dashboard/following_users" => "dashboards#following_users" get "dashboard/following_organizations" => "dashboards#following_organizations" get "dashboard/following_podcasts" => "dashboards#following_podcasts" + get "/dashboard/subscriptions" => "dashboards#subscriptions" get "/dashboard/:which" => "dashboards#followers", :constraints => { which: /user_followers/ } get "/dashboard/:which/:org_id" => "dashboards#show", :constraints => { @@ -469,7 +470,7 @@ Rails.application.routes.draw do get "/:username/:slug" => "stories#show" get "/:sitemap" => "sitemaps#show", :constraints => { format: /xml/, sitemap: /sitemap-.+/ } - get "/:username" => "stories#index" + get "/:username" => "stories#index", :as => "user_profile" root "stories#index" end diff --git a/db/migrate/20200706184804_add_user_subscriptions_count_to_articles.rb b/db/migrate/20200706184804_add_user_subscriptions_count_to_articles.rb new file mode 100644 index 000000000..e850e5682 --- /dev/null +++ b/db/migrate/20200706184804_add_user_subscriptions_count_to_articles.rb @@ -0,0 +1,9 @@ +class AddUserSubscriptionsCountToArticles < ActiveRecord::Migration[6.0] + def self.up + add_column :articles, :user_subscriptions_count, :integer, null: false, default: 0 + end + + def self.down + remove_column :articles, :user_subscriptions_count + end +end diff --git a/db/schema.rb b/db/schema.rb index dbe66b4ca..3aec8f8a7 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2020_07_02_143618) do +ActiveRecord::Schema.define(version: 2020_07_06_184804) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -136,6 +136,7 @@ ActiveRecord::Schema.define(version: 2020_07_02_143618) do t.string "title" t.datetime "updated_at", null: false t.integer "user_id" + t.integer "user_subscriptions_count", default: 0, null: false t.string "video" t.string "video_closed_caption_track_url" t.string "video_code" diff --git a/spec/models/shared_examples/user_subscription_sourceable_spec.rb b/spec/models/shared_examples/user_subscription_sourceable_spec.rb index 47ef5c950..6942f1fd4 100644 --- a/spec/models/shared_examples/user_subscription_sourceable_spec.rb +++ b/spec/models/shared_examples/user_subscription_sourceable_spec.rb @@ -39,4 +39,23 @@ RSpec.shared_examples "UserSubscriptionSourceable" do end end end + + describe "counter_culture" do + context "when a UserSubscription is created" do + it "increments user_subscriptions_count" do + expect do + create(:user_subscription, user_subscription_sourceable: source) + end.to change { source.reload.user_subscriptions_count }.by(1) + end + end + + context "when a UserSubscription is destroyed" do + it "decrements user_subscriptions_count" do + user_subscription = create(:user_subscription, user_subscription_sourceable: source) + expect do + user_subscription.destroy + end.to change { source.reload.user_subscriptions_count }.by(-1) + end + end + end end diff --git a/spec/requests/dashboard_spec.rb b/spec/requests/dashboard_spec.rb index 540cfab02..6663c43ef 100644 --- a/spec/requests/dashboard_spec.rb +++ b/spec/requests/dashboard_spec.rb @@ -39,6 +39,19 @@ RSpec.describe "Dashboards", type: :request do expect(response.body).to include "Delete" end + it "renders subscriptions for articles with subscriptions" do + user.add_role(:admin) # TODO: (Alex Smith) - update roles before release + article_with_user_subscription_tag = create(:article, user: user, with_user_subscription_tag: true) + create(:user_subscription, + subscriber_id: second_user.id, + subscriber_email: second_user.email, + author_id: article_with_user_subscription_tag.user_id, + user_subscription_sourceable: article_with_user_subscription_tag) + + get "/dashboard" + expect(response.body).to include "Subscriptions" + end + it "renders pagination if minimum amount of posts" do create_list(:article, 52, user: user) get "/dashboard" @@ -300,4 +313,77 @@ RSpec.describe "Dashboards", type: :request do end end end + + # TODO: (Alex Smith) - update roles before release + describe "GET /dashboard/subscriptions" do + before do + sign_in user + end + + it "renders subscriptions" do + user.add_role(:admin) + article_with_user_subscription_tag = create(:article, user: user, with_user_subscription_tag: true) + user_subscription = create(:user_subscription, + subscriber_id: second_user.id, + subscriber_email: second_user.email, + author_id: article_with_user_subscription_tag.user_id, + user_subscription_sourceable: article_with_user_subscription_tag) + + get "/dashboard/subscriptions", params: { source_type: article_with_user_subscription_tag.class.name, source_id: article_with_user_subscription_tag.id } + expect(response.body).to include(user_subscription.subscriber_email) + end + + it "displays a message if no subscriptions are found" do + get "/dashboard/subscriptions", params: { source_type: article.class.name, source_id: article.id } + expect(response.body).to include("You don't have any subscribers for this") + end + + it "raises unauthorized when trying to access a source the user doesn't own" do + user.add_role(:admin) + article_with_user_subscription_tag = create(:article, :with_user_subscription_tag_role_user, with_user_subscription_tag: true) + create(:user_subscription, + subscriber_id: second_user.id, + subscriber_email: second_user.email, + author_id: article_with_user_subscription_tag.user_id, + user_subscription_sourceable: article_with_user_subscription_tag) + + expect do + get "/dashboard/subscriptions", params: { source_type: article_with_user_subscription_tag.class.name, source_id: article_with_user_subscription_tag.id } + end.to raise_error(Pundit::NotAuthorizedError) + end + + it "raises an error for disallowed source_types" do + expect do + get "/dashboard/subscriptions", params: { source_type: "Comment", source_id: 1 } + end.to raise_error(ActiveRecord::RecordNotFound) + end + + it "raises an error when the source can't be found" do + expect do + get "/dashboard/subscriptions", params: { source_type: article.class.name, source_id: article.id + 999 } + end.to raise_error(ActiveRecord::RecordNotFound) + end + + it "renders pagination if minimum amount of subscriptions" do + user.add_role(:admin) + article_with_user_subscription_tag = create(:article, user: user, with_user_subscription_tag: true) + create_list(:user_subscription, + 102, # Current pagination limit is 100 + author: user, + user_subscription_sourceable: article_with_user_subscription_tag) + get "/dashboard/subscriptions", params: { source_type: article_with_user_subscription_tag.class.name, source_id: article_with_user_subscription_tag.id } + expect(response.body).to include "pagination" + end + + it "does not render pagination if less than one full page" do + user.add_role(:admin) + article_with_user_subscription_tag = create(:article, user: user, with_user_subscription_tag: true) + create_list(:user_subscription, + 5, + author: user, + user_subscription_sourceable: article_with_user_subscription_tag) + get "/dashboard/subscriptions", params: { source_type: article_with_user_subscription_tag.class.name, source_id: article_with_user_subscription_tag.id } + expect(response.body).not_to include "pagination" + end + end end