From 98f25e2b9da2f2b0365e29c338dc1cab8be65021 Mon Sep 17 00:00:00 2001 From: Molly Struve Date: Tue, 18 Feb 2020 10:06:30 -0500 Subject: [PATCH] Remove UserHistory Feature and PageViews from Algolia (#6127) [deploy] --- app/controllers/history_controller.rb | 17 --- .../__snapshots__/history.test.jsx.snap | 53 --------- .../history/__tests__/history.test.jsx | 10 -- app/javascript/history/history.jsx | 112 ------------------ app/javascript/packs/history.jsx | 22 ---- app/models/page_view.rb | 54 --------- app/services/pro_memberships/creator.rb | 2 - app/views/articles/_sidebar_nav.html.erb | 3 - app/views/history/index.html.erb | 26 ---- .../populate_history_worker.rb | 14 --- config/routes.rb | 1 - spec/models/page_view_spec.rb | 14 --- spec/requests/history_spec.rb | 28 ----- spec/requests/pro_memberships_spec.rb | 10 -- .../populate_history_worker_spec.rb | 19 --- 15 files changed, 385 deletions(-) delete mode 100644 app/controllers/history_controller.rb delete mode 100644 app/javascript/history/__tests__/__snapshots__/history.test.jsx.snap delete mode 100644 app/javascript/history/__tests__/history.test.jsx delete mode 100644 app/javascript/history/history.jsx delete mode 100644 app/javascript/packs/history.jsx delete mode 100644 app/views/history/index.html.erb delete mode 100644 app/workers/pro_memberships/populate_history_worker.rb delete mode 100644 spec/requests/history_spec.rb delete mode 100644 spec/workers/pro_memberships/populate_history_worker_spec.rb diff --git a/app/controllers/history_controller.rb b/app/controllers/history_controller.rb deleted file mode 100644 index 8de2cf3d8..000000000 --- a/app/controllers/history_controller.rb +++ /dev/null @@ -1,17 +0,0 @@ -class HistoryController < ApplicationController - before_action :authenticate_user! - before_action :generate_algolia_search_key - - def index - authorize current_user, :pro_user? - @history_index = true # used exclusively by the ERb templates - end - - private - - def generate_algolia_search_key - params = { filters: "viewable_by:#{current_user.id}" } - key = ApplicationConfig["ALGOLIASEARCH_SEARCH_ONLY_KEY"] - @secured_algolia_key = Algolia.generate_secured_api_key(key, params) - end -end diff --git a/app/javascript/history/__tests__/__snapshots__/history.test.jsx.snap b/app/javascript/history/__tests__/__snapshots__/history.test.jsx.snap deleted file mode 100644 index b365d6db1..000000000 --- a/app/javascript/history/__tests__/__snapshots__/history.test.jsx.snap +++ /dev/null @@ -1,53 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[` renders properly 1`] = ` -
-
-
- -
- - #discuss - -
-
-
-
-
-
- History (empty) -
-
-
-

- Your History is Lonely -

-
-
-
-
-
-`; diff --git a/app/javascript/history/__tests__/history.test.jsx b/app/javascript/history/__tests__/history.test.jsx deleted file mode 100644 index eda668f35..000000000 --- a/app/javascript/history/__tests__/history.test.jsx +++ /dev/null @@ -1,10 +0,0 @@ -import { h } from 'preact'; -import render from 'preact-render-to-json'; -import { History } from '../history'; - -describe('', () => { - it('renders properly', () => { - const tree = render(); - expect(tree).toMatchSnapshot(); - }); -}); diff --git a/app/javascript/history/history.jsx b/app/javascript/history/history.jsx deleted file mode 100644 index c0cb7da81..000000000 --- a/app/javascript/history/history.jsx +++ /dev/null @@ -1,112 +0,0 @@ -import { h, Component } from 'preact'; -import { PropTypes } from 'preact-compat'; -import debounce from 'lodash.debounce'; - -import { - defaultState, - loadNextPage, - onSearchBoxType, - performInitialSearch, - search, - toggleTag, -} from '../searchableItemList/searchableItemList'; -import { ItemListLoadMoreButton } from '../src/components/ItemList/ItemListLoadMoreButton'; -import { ItemListTags } from '../src/components/ItemList/ItemListTags'; -import { ItemListItem } from '../src/components/ItemList/ItemListItem'; - -export class History extends Component { - constructor(props) { - super(props); - - const { availableTags } = this.props; - this.state = defaultState({ availableTags }); - - // bind and initialize all shared functions - this.onSearchBoxType = debounce(onSearchBoxType.bind(this), 300, { - leading: true, - }); - this.loadNextPage = loadNextPage.bind(this); - this.performInitialSearch = performInitialSearch.bind(this); - this.search = search.bind(this); - this.toggleTag = toggleTag.bind(this); - } - - componentDidMount() { - const { hitsPerPage } = this.state; - - this.performInitialSearch({ - containerId: 'history', - indexName: 'UserHistory', - searchOptions: { - hitsPerPage, - }, - }); - } - - renderEmptyItems() { - const { selectedTags, query } = this.state; - - return ( -
-
-

- {selectedTags.length === 0 && query.length === 0 - ? 'Your History is Lonely' - : 'Nothing with this filter 🤔'} -

-
-
- ); - } - - render() { - const { - items, - itemsLoaded, - totalCount, - availableTags, - selectedTags, - showLoadMoreButton, - } = this.state; - - const itemsToRender = items.map(item => ); - - return ( -
-
-
- - - -
-
- -
-
-
- History - {` (${totalCount > 0 ? totalCount : 'empty'})`} -
- {items.length > 0 ? itemsToRender : this.renderEmptyItems()} -
- - -
-
- ); - } -} - -History.propTypes = { - availableTags: PropTypes.arrayOf(PropTypes.string).isRequired, -}; diff --git a/app/javascript/packs/history.jsx b/app/javascript/packs/history.jsx deleted file mode 100644 index 310600ee6..000000000 --- a/app/javascript/packs/history.jsx +++ /dev/null @@ -1,22 +0,0 @@ -import { h, render } from 'preact'; -import { getUserDataAndCsrfToken } from '../chat/util'; -import { History } from '../history/history'; - -function loadComponent() { - getUserDataAndCsrfToken().then(({ currentUser }) => { - const root = document.getElementById('history'); - if (root) { - render( - , - root, - root.firstElementChild, - ); - } - }); -} - -window.InstantClick.on('change', () => { - loadComponent(); -}); - -loadComponent(); diff --git a/app/models/page_view.rb b/app/models/page_view.rb index 21530ee0a..78611d5f7 100644 --- a/app/models/page_view.rb +++ b/app/models/page_view.rb @@ -1,59 +1,9 @@ class PageView < ApplicationRecord - include AlgoliaSearch - belongs_to :user, optional: true belongs_to :article before_create :extract_domain_and_path - algoliasearch index_name: "UserHistory", per_environment: true, if: :belongs_to_pro_user?, enqueue: :trigger_index_sync do - attributes :referrer, :user_agent, :article_tags - - attribute(:article_title) { article.title } - attribute(:article_path) { article.path } - attribute(:article_reading_time) { article.reading_time } - attribute(:viewable_by) { user_id } - attribute(:visited_at_timestamp) { created_at.to_i } - - attribute :article_user do - user = article.user - { - username: user.username, - name: user.name, - profile_image_90: user.profile_image_90 - } - end - - attribute :readable_visited_at do - if created_at.year == Time.current.year - created_at.strftime("%b %e") - else - created_at.strftime("%b %e '%y") - end - end - - searchableAttributes( - %i[referrer user_agent article_title article_searchable_tags article_searchable_text], - ) - - tags { article_tags } - - attributesForFaceting ["filterOnly(viewable_by)"] - - attributeForDistinct :article_path - distinct true - - customRanking ["desc(visited_at_timestamp)"] - end - - def self.trigger_index_sync(record, remove) - if remove - Search::RemoveFromIndexWorker.perform_async(algolia_index_name, record.id) - else - Search::IndexWorker.perform_async("PageView", record.id) - end - end - private def extract_domain_and_path @@ -64,10 +14,6 @@ class PageView < ApplicationRecord self.path = parsed_url.path end - def belongs_to_pro_user? - user&.pro? - end - def article_searchable_tags article.cached_tag_list end diff --git a/app/services/pro_memberships/creator.rb b/app/services/pro_memberships/creator.rb index 94ab66e1c..2195342fc 100644 --- a/app/services/pro_memberships/creator.rb +++ b/app/services/pro_memberships/creator.rb @@ -10,8 +10,6 @@ module ProMemberships def call if purchase_pro_membership - ProMemberships::PopulateHistoryWorker.perform_async(user.id) - channel = ChatChannel.find_by(slug: "pro-members") channel&.add_users(user) diff --git a/app/views/articles/_sidebar_nav.html.erb b/app/views/articles/_sidebar_nav.html.erb index c3711ea9a..2b2565dc5 100644 --- a/app/views/articles/_sidebar_nav.html.erb +++ b/app/views/articles/_sidebar_nav.html.erb @@ -26,9 +26,6 @@ " alt="moderation icon" /> Moderation - - " alt="history icon" /> History - " alt="listings icon" /> Listings diff --git a/app/views/history/index.html.erb b/app/views/history/index.html.erb deleted file mode 100644 index 87d8ed2df..000000000 --- a/app/views/history/index.html.erb +++ /dev/null @@ -1,26 +0,0 @@ -<% title "History" %> - -<%= content_for :page_meta do %> - <% page_url = "#{ApplicationConfig['APP_PROTOCOL']}#{ApplicationConfig['APP_DOMAIN']}#{history_path}" %> - - - - - - - - - - - - - - -<% end %> - -
-
-
-
- -<%= javascript_pack_tag "history", defer: true %> diff --git a/app/workers/pro_memberships/populate_history_worker.rb b/app/workers/pro_memberships/populate_history_worker.rb deleted file mode 100644 index 1daf9f2f9..000000000 --- a/app/workers/pro_memberships/populate_history_worker.rb +++ /dev/null @@ -1,14 +0,0 @@ -module ProMemberships - class PopulateHistoryWorker - include Sidekiq::Worker - - sidekiq_options queue: :medium_priority, retry: 10 - - def perform(user_id) - user = User.find_by(id: user_id) - return unless user&.pro? - - user.page_views.reindex! - end - end -end diff --git a/config/routes.rb b/config/routes.rb index f22113b8e..57aba8470 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -361,7 +361,6 @@ Rails.application.routes.draw do get "/podcasts", to: redirect("pod") get "/readinglist" => "reading_list_items#index" get "/readinglist/:view" => "reading_list_items#index", :constraints => { view: /archive/ } - get "/history", to: "history#index", as: :history get "/feed" => "articles#feed", :as => "feed", :defaults => { format: "rss" } get "/feed/tag/:tag" => "articles#feed", diff --git a/spec/models/page_view_spec.rb b/spec/models/page_view_spec.rb index 2141b4d69..28cebadc9 100644 --- a/spec/models/page_view_spec.rb +++ b/spec/models/page_view_spec.rb @@ -19,18 +19,4 @@ RSpec.describe PageView, type: :model do end end end - - describe "indexing" do - it "indexes updated records" do - sidekiq_assert_enqueued_with(job: Search::IndexWorker, args: ["PageView", page_view.id]) do - page_view.update(path: "/") - end - end - - it "removes deleted records" do - sidekiq_assert_enqueued_with(job: Search::RemoveFromIndexWorker, args: [described_class.algolia_index_name, page_view.id]) do - page_view.destroy - end - end - end end diff --git a/spec/requests/history_spec.rb b/spec/requests/history_spec.rb deleted file mode 100644 index f848b061c..000000000 --- a/spec/requests/history_spec.rb +++ /dev/null @@ -1,28 +0,0 @@ -require "rails_helper" - -RSpec.describe "History", type: :request do - let(:user) { create(:user) } - let(:pro_user) { create(:user, :pro) } - let(:pro_membership_user) { create(:user, :with_pro_membership) } - - describe "GET /history" do - it "does not allow access to a regular user" do - sign_in user - expect { get history_path }.to raise_error(Pundit::NotAuthorizedError) - end - - it "allows access to a pro user" do - sign_in pro_user - get history_path - expect(response).to have_http_status(:ok) - expect(response.body).to include("History") - end - - it "allows access to a user with a pro membership" do - sign_in pro_membership_user - get history_path - expect(response).to have_http_status(:ok) - expect(response.body).to include("History") - end - end -end diff --git a/spec/requests/pro_memberships_spec.rb b/spec/requests/pro_memberships_spec.rb index 96b53fea8..34b62ec38 100644 --- a/spec/requests/pro_memberships_spec.rb +++ b/spec/requests/pro_memberships_spec.rb @@ -68,16 +68,6 @@ RSpec.describe "Pro Memberships", type: :request do end.to change(user.credits.spent, :count).by(ProMembership::MONTHLY_COST) end - it "enqueues a job to populate the history" do - sidekiq_assert_enqueued_with( - job: ProMemberships::PopulateHistoryWorker, - args: [user.id], - queue: "medium_priority", - ) do - post pro_membership_path - end - end - it "enqueues a job to bust the user's cache" do ActiveJob::Base.queue_adapter.enqueued_jobs.clear # make sure it hasn't been previously queued sidekiq_assert_enqueued_with(job: Users::BustCacheWorker, args: [user.id]) do diff --git a/spec/workers/pro_memberships/populate_history_worker_spec.rb b/spec/workers/pro_memberships/populate_history_worker_spec.rb deleted file mode 100644 index 04a8ff073..000000000 --- a/spec/workers/pro_memberships/populate_history_worker_spec.rb +++ /dev/null @@ -1,19 +0,0 @@ -require "rails_helper" - -RSpec.describe ProMemberships::PopulateHistoryWorker, type: :worker do - include_examples "#enqueues_on_correct_queue", "medium_priority", 1 - - describe "#perform" do - let(:user) { create(:user, :pro) } - - before do - allow(User).to receive(:find_by).and_return(user) - allow(user.page_views).to receive(:reindex!) - end - - it "indexes user page views" do - described_class.new.perform(user.id) - expect(user.page_views).to have_received(:reindex!).once - end - end -end