From fabaeb546164ebb87c573014d632fad298ce6635 Mon Sep 17 00:00:00 2001 From: Anna Buianova Date: Thu, 1 Aug 2019 16:29:13 +0300 Subject: [PATCH] Introduced published field to podcasts (#3593) * Published field for podcasts * Add podcasts migration * More specs for podcasts availability * Show published in podcasts internal * Update schema.rb with db:migrate --- app/controllers/admin/podcasts_controller.rb | 2 +- .../api/v0/podcast_episodes_controller.rb | 2 +- app/controllers/podcast_episodes_controller.rb | 4 ++-- app/controllers/stories_controller.rb | 6 +++--- app/dashboards/podcast_dashboard.rb | 4 ++++ app/models/podcast.rb | 2 ++ app/models/podcast_episode.rb | 2 ++ app/views/internal/podcasts/index.html.erb | 10 ++++++---- app/views/layouts/internal.html.erb | 4 ++-- .../20190801083510_add_published_to_podcasts.rb | 9 +++++++++ db/schema.rb | 3 ++- lib/tasks/fetch.rake | 2 +- spec/factories/podcasts.rb | 1 + spec/models/podcast_episode_spec.rb | 16 ++++++++++++++++ spec/models/podcast_spec.rb | 17 ++++++++++++----- spec/requests/admin/podcasts_spec.rb | 8 ++++++++ spec/requests/podcasts/podcast_show_spec.rb | 8 ++++++++ .../user_visits_podcasts_root_page_spec.rb | 10 +++++++++- 18 files changed, 89 insertions(+), 21 deletions(-) create mode 100644 db/migrate/20190801083510_add_published_to_podcasts.rb diff --git a/app/controllers/admin/podcasts_controller.rb b/app/controllers/admin/podcasts_controller.rb index 2f6769c33..178b33fc8 100644 --- a/app/controllers/admin/podcasts_controller.rb +++ b/app/controllers/admin/podcasts_controller.rb @@ -5,7 +5,7 @@ module Admin authorize_resource(resource) if resource.save - Podcasts::GetEpisodesJob.perform_later(podcast_id: resource.id) + Podcasts::GetEpisodesJob.perform_later(podcast_id: resource.id) if resource.published redirect_to( [namespace, resource], notice: translate_with_resource("create.success"), diff --git a/app/controllers/api/v0/podcast_episodes_controller.rb b/app/controllers/api/v0/podcast_episodes_controller.rb index 4cac03221..8eebb689d 100644 --- a/app/controllers/api/v0/podcast_episodes_controller.rb +++ b/app/controllers/api/v0/podcast_episodes_controller.rb @@ -13,7 +13,7 @@ module Api @page = params[:page] if params[:username] - @podcast = Podcast.reachable.find_by!(slug: params[:username]) + @podcast = Podcast.available.find_by!(slug: params[:username]) @podcast_episodes = @podcast. podcast_episodes.reachable.order("published_at desc"). page(@page). diff --git a/app/controllers/podcast_episodes_controller.rb b/app/controllers/podcast_episodes_controller.rb index 86c3fb1ca..91617dacc 100644 --- a/app/controllers/podcast_episodes_controller.rb +++ b/app/controllers/podcast_episodes_controller.rb @@ -4,8 +4,8 @@ class PodcastEpisodesController < ApplicationController def index @podcast_index = true - @podcasts = Podcast.reachable.order("title asc") - @podcast_episodes = PodcastEpisode.reachable.order("published_at desc").first(20) + @podcasts = Podcast.available.order("title asc") + @podcast_episodes = PodcastEpisode.available.order("published_at desc").first(20) if params[:q].blank? set_surrogate_key_header("podcast_episodes_all " + params[:q].to_s, @podcast_episodes.map { |e| e["record_key"] }) diff --git a/app/controllers/stories_controller.rb b/app/controllers/stories_controller.rb index fc31ae9fe..225dd16fe 100644 --- a/app/controllers/stories_controller.rb +++ b/app/controllers/stories_controller.rb @@ -23,8 +23,8 @@ class StoriesController < ApplicationController elsif (@article = Article.find_by(slug: params[:slug])&.decorate) handle_possible_redirect else - @podcast = Podcast.reachable.find_by!(slug: params[:username]) - @episode = PodcastEpisode.reachable.find_by!(slug: params[:slug]) + @podcast = Podcast.available.find_by!(slug: params[:username]) + @episode = PodcastEpisode.available.find_by!(slug: params[:slug]) handle_podcast_show end end @@ -64,7 +64,7 @@ class StoriesController < ApplicationController end def handle_user_or_organization_or_podcast_or_page_index - @podcast = Podcast.reachable.find_by(slug: params[:username].downcase) + @podcast = Podcast.available.find_by(slug: params[:username].downcase) @organization = Organization.find_by(slug: params[:username].downcase) @page = Page.find_by(slug: params[:username].downcase, is_top_level_path: true) if @podcast diff --git a/app/dashboards/podcast_dashboard.rb b/app/dashboards/podcast_dashboard.rb index 382b7de69..303d6127c 100644 --- a/app/dashboards/podcast_dashboard.rb +++ b/app/dashboards/podcast_dashboard.rb @@ -24,6 +24,7 @@ class PodcastDashboard < Administrate::BaseDashboard pattern_image: CarrierwaveField, slug: Field::String, reachable: Field::Boolean, + published: Field::Boolean, status_notice: Field::Text, created_at: Field::DateTime, updated_at: Field::DateTime @@ -38,6 +39,7 @@ class PodcastDashboard < Administrate::BaseDashboard podcast_episodes id reachable + published status_notice title description @@ -63,6 +65,7 @@ class PodcastDashboard < Administrate::BaseDashboard pattern_image slug reachable + published created_at updated_at ].freeze @@ -87,6 +90,7 @@ class PodcastDashboard < Administrate::BaseDashboard image slug reachable + published ].freeze # Overwrite this method to customize how podcasts are displayed diff --git a/app/models/podcast.rb b/app/models/podcast.rb index b040d9149..1b0784311 100644 --- a/app/models/podcast.rb +++ b/app/models/podcast.rb @@ -18,6 +18,8 @@ class Podcast < ApplicationRecord after_save :bust_cache scope :reachable, -> { where(id: PodcastEpisode.reachable.select(:podcast_id)) } + scope :published, -> { where(published: true) } + scope :available, -> { reachable.published } alias_attribute :path, :slug alias_attribute :profile_image_url, :image_url diff --git a/app/models/podcast_episode.rb b/app/models/podcast_episode.rb index a4159e065..cf8e5d673 100644 --- a/app/models/podcast_episode.rb +++ b/app/models/podcast_episode.rb @@ -26,6 +26,8 @@ class PodcastEpisode < ApplicationRecord before_validation :prefix_all_images scope :reachable, -> { where(reachable: true) } + scope :published, -> { joins(:podcast).where(podcasts: { published: true }) } + scope :available, -> { reachable.published } algoliasearch per_environment: true do attribute :id diff --git a/app/views/internal/podcasts/index.html.erb b/app/views/internal/podcasts/index.html.erb index 336e894ea..54f325a9f 100644 --- a/app/views/internal/podcasts/index.html.erb +++ b/app/views/internal/podcasts/index.html.erb @@ -14,23 +14,25 @@ <%= paginate @podcasts %>
-
+
ID
Title
Feed URL
-
Episodes
-
Reachable
+
Eps
+
Reach
+
Pub
Status Notice
Admins
<% @podcasts.each do |podcast| %> -
+
<%= podcast.id %>
<%= podcast.podcast_episodes.count %>
<%= podcast.reachable %>
+
<%= podcast.published %>
<%= podcast.status_notice %>
<% podcast.admins.pluck(:username).each do |username| %> diff --git a/app/views/layouts/internal.html.erb b/app/views/layouts/internal.html.erb index 3c196e8d7..7b8dc06d9 100644 --- a/app/views/layouts/internal.html.erb +++ b/app/views/layouts/internal.html.erb @@ -42,9 +42,9 @@ padding: 2px; } - .wrapper-7 { + .wrapper-8 { display: grid; - grid-template-columns: 5% 15% 25% 10% 10% 20% 15%; + grid-template-columns: 4% 15% 25% 7% 7% 7% 20% 15%; padding: 2px; } diff --git a/db/migrate/20190801083510_add_published_to_podcasts.rb b/db/migrate/20190801083510_add_published_to_podcasts.rb new file mode 100644 index 000000000..eb919ed46 --- /dev/null +++ b/db/migrate/20190801083510_add_published_to_podcasts.rb @@ -0,0 +1,9 @@ +class AddPublishedToPodcasts < ActiveRecord::Migration[5.2] + class Podcast < ApplicationRecord; end + + def change + add_column :podcasts, :published, :boolean, default: false + + Podcast.update_all(published: true) + end +end diff --git a/db/schema.rb b/db/schema.rb index 44cf75984..b4913a598 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -12,7 +12,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2019_07_23_094834) do +ActiveRecord::Schema.define(version: 2019_08_01_083510) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -737,6 +737,7 @@ ActiveRecord::Schema.define(version: 2019_07_23_094834) do t.string "main_color_hex", null: false t.string "overcast_url" t.string "pattern_image" + t.boolean "published", default: false t.boolean "reachable", default: true t.string "slug", null: false t.string "soundcloud_url" diff --git a/lib/tasks/fetch.rake b/lib/tasks/fetch.rake index 1b1f99ac3..28eba3f2d 100644 --- a/lib/tasks/fetch.rake +++ b/lib/tasks/fetch.rake @@ -1,7 +1,7 @@ desc "This task is called by the Heroku scheduler add-on" task get_podcast_episodes: :environment do - Podcast.select(:id).find_each do |podcast| + Podcast.published.select(:id).find_each do |podcast| Podcasts::GetEpisodesJob.perform_later(podcast_id: podcast.id, limit: 5) end end diff --git a/spec/factories/podcasts.rb b/spec/factories/podcasts.rb index 5a135b44d..3aabcb6ad 100644 --- a/spec/factories/podcasts.rb +++ b/spec/factories/podcasts.rb @@ -11,5 +11,6 @@ FactoryBot.define do slug { "slug-#{rand(10_000)}" } feed_url { Faker::Internet.url } main_color_hex { "ffffff" } + published { true } end end diff --git a/spec/models/podcast_episode_spec.rb b/spec/models/podcast_episode_spec.rb index 5c4120c3e..0ec78c52d 100644 --- a/spec/models/podcast_episode_spec.rb +++ b/spec/models/podcast_episode_spec.rb @@ -33,6 +33,22 @@ RSpec.describe PodcastEpisode, type: :model do end end + describe "#available" do + let(:podcast) { create(:podcast) } + let(:unpodcast) { create(:podcast, published: false) } + let!(:episode) { create(:podcast_episode, podcast: podcast) } + + before do + create(:podcast_episode, podcast: unpodcast) + create(:podcast_episode, podcast: podcast, reachable: false) + end + + it "is available when reachable and published" do + available_ids = described_class.available.pluck(:id) + expect(available_ids).to eq([episode.id]) + end + end + describe "#description" do it "strips tags from the body" do podcast_episode.body = "

Body with HTML tags

" diff --git a/spec/models/podcast_spec.rb b/spec/models/podcast_spec.rb index c7d4d5708..c387996ae 100644 --- a/spec/models/podcast_spec.rb +++ b/spec/models/podcast_spec.rb @@ -65,16 +65,18 @@ RSpec.describe Podcast, type: :model do end end - describe "#reachable" do - let(:podcast) { create(:podcast, reachable: false) } - let!(:unpodcast) { create(:podcast, reachable: false) } - let!(:unpodcast2) { create(:podcast, reachable: false) } - let!(:cool_podcast) { create(:podcast, reachable: true) } + describe "#reachable and #available" do + let(:podcast) { create(:podcast, reachable: false, published: true) } + let!(:unpodcast) { create(:podcast, reachable: false, published: true) } + let!(:unpodcast2) { create(:podcast, reachable: false, published: true) } + let!(:cool_podcast) { create(:podcast, reachable: true, published: false) } + let!(:reachable_podcast) { create(:podcast, reachable: true, published: true) } before do create(:podcast_episode, reachable: true, podcast: podcast) create(:podcast_episode, reachable: false, podcast: unpodcast2) create(:podcast_episode, reachable: true, podcast: cool_podcast) + create(:podcast_episode, reachable: true, podcast: reachable_podcast) end it "is reachable when the feed is unreachable but the podcast has reachable podcasts" do @@ -84,6 +86,11 @@ RSpec.describe Podcast, type: :model do expect(reachable_ids).not_to include(unpodcast.id) expect(reachable_ids).not_to include(unpodcast2.id) end + + it "is available only when reachable and published" do + available_ids = described_class.available.pluck(:id) + expect(available_ids.sort).to eq([podcast.id, reachable_podcast.id].sort) + end end describe "#existing_episode" do diff --git a/spec/requests/admin/podcasts_spec.rb b/spec/requests/admin/podcasts_spec.rb index 62712c8f9..0f260e927 100644 --- a/spec/requests/admin/podcasts_spec.rb +++ b/spec/requests/admin/podcasts_spec.rb @@ -15,6 +15,7 @@ RSpec.describe "Admin::Podcasts", type: :request do description: "Super Podcast", feed_url: "http://feeds.feedburner.com/developertea", slug: "devtea", + published: true, main_color_hex: "333333", image: Rack::Test::UploadedFile.new(image_file, "image/jpeg") } @@ -31,5 +32,12 @@ RSpec.describe "Admin::Podcasts", type: :request do post "/admin/podcasts", params: { podcast: valid_attributes } end.to have_enqueued_job(Podcasts::GetEpisodesJob).exactly(:once) end + + it "doesn't enqueue a job when creating an unpublished podcast" do + valid_attributes[:published] = false + expect do + post "/admin/podcasts", params: { podcast: valid_attributes } + end.not_to have_enqueued_job(Podcasts::GetEpisodesJob) + end end end diff --git a/spec/requests/podcasts/podcast_show_spec.rb b/spec/requests/podcasts/podcast_show_spec.rb index 74cacb367..1fe5834db 100644 --- a/spec/requests/podcasts/podcast_show_spec.rb +++ b/spec/requests/podcasts/podcast_show_spec.rb @@ -24,5 +24,13 @@ RSpec.describe "PodcastShow", type: :request do expect(response.body).to include(CGI.escapeHTML("Bats' life")) expect(response.body).not_to include("Really old one") end + + it "renders 404 when podcast is unpublished" do + unpodcast = create(:podcast, reachable: true, published: false) + create(:podcast_episode, reachable: true) + expect do + get "/#{unpodcast.slug}" + end.to raise_error(ActiveRecord::RecordNotFound) + end end end diff --git a/spec/system/podcasts/user_visits_podcasts_root_page_spec.rb b/spec/system/podcasts/user_visits_podcasts_root_page_spec.rb index 7ba9009a7..23e81f3d0 100644 --- a/spec/system/podcasts/user_visits_podcasts_root_page_spec.rb +++ b/spec/system/podcasts/user_visits_podcasts_root_page_spec.rb @@ -3,8 +3,10 @@ require "rails_helper" RSpec.describe "User visits /pod page", type: :system do let!(:podcast_episode) { create(:podcast_episode) } let!(:podcast_episode2) { create(:podcast_episode) } - let(:podcast) { create(:podcast, reachable: false) } + let(:podcast) { create(:podcast, reachable: true, published: false) } + let(:unpublished_podcast) { create(:podcast, reachable: false) } let!(:un_podcast_episode) { create(:podcast_episode, podcast: podcast, reachable: false) } + let!(:unpublished_episode) { create(:podcast_episode, podcast: podcast) } before { visit "/pod" } @@ -20,4 +22,10 @@ RSpec.describe "User visits /pod page", type: :system do expect(page).not_to have_link(nil, href: un_podcast_episode.path) end end + + it "doesn't dsplay a podcast that is not published" do + within "#articles-list" do + expect(page).not_to have_link(nil, href: unpublished_episode.path) + end + end end