Podcast episodes statuses #2952 (#3386)

app/controllers/api/v0/podcast_episodes_controller.rb

@@ -13,13 +13,14 @@ module Api
         @page = params[:page]
 
         if params[:username]
-          @podcast = Podcast.find_by(slug: params[:username]) || not_found
+          @podcast = Podcast.reachable.find_by!(slug: params[:username])
           @podcast_episodes = @podcast.
-            podcast_episodes.order("published_at desc").
+            podcast_episodes.reachable.order("published_at desc").
             page(@page).
             per(30)
         else
           @podcast_episodes = PodcastEpisode.
+            reachable.
             includes(:podcast).
             order("published_at desc").page(@page).per(30)
         end

app/controllers/podcast_episodes_controller.rb

@@ -5,7 +5,7 @@ class PodcastEpisodesController < ApplicationController
   def index
     @podcast_index = true
     @podcasts = Podcast.reachable.order("title asc")
-    @podcast_episodes = PodcastEpisode.includes(:podcast).where(podcasts: { reachable: true }).order("published_at desc").first(20)
+    @podcast_episodes = PodcastEpisode.reachable.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"] })

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]) || not_found
-      @episode = PodcastEpisode.find_by(slug: params[:slug]) || not_found
+      @podcast = Podcast.reachable.find_by!(slug: params[:username])
+      @episode = PodcastEpisode.reachable.find_by!(slug: params[:slug])
       handle_podcast_show
     end
   end
@@ -141,7 +141,7 @@ class StoriesController < ApplicationController
     @podcast_index = true
     @article_index = true
     @list_of = "podcast-episodes"
-    @podcast_episodes = @podcast.podcast_episodes.order("published_at DESC").limit(30)
+    @podcast_episodes = @podcast.podcast_episodes.reachable.order("published_at DESC").limit(30)
     set_surrogate_key_header "podcast_episodes"
     render template: "podcast_episodes/index"
   end

app/dashboards/podcast_episode_dashboard.rb

@@ -29,6 +29,8 @@ class PodcastEpisodeDashboard < Administrate::BaseDashboard
     published_at: Field::DateTime,
     slug: Field::String,
     guid: Field::String,
+    reachable: Field::Boolean,
+    https: Field::Boolean,
     created_at: Field::DateTime,
     updated_at: Field::DateTime,
     social_image: CarrierwaveField
@@ -42,6 +44,9 @@ class PodcastEpisodeDashboard < Administrate::BaseDashboard
   COLLECTION_ATTRIBUTES = %i[
     id
     title
+    media_url
+    reachable
+    https
   ].freeze
 
   # SHOW_PAGE_ATTRIBUTES
@@ -61,6 +66,8 @@ class PodcastEpisodeDashboard < Administrate::BaseDashboard
     duration_in_seconds
     published_at
     slug
+    reachable
+    https
     created_at
     updated_at
   ].freeze

app/models/podcast_episode.rb

@@ -25,6 +25,8 @@ class PodcastEpisode < ApplicationRecord
 
   before_validation :prefix_all_images
 
+  scope :reachable, -> { joins(:podcast).where(reachable: true, podcasts: { reachable: true }) }
+
   algoliasearch per_environment: true do
     attribute :id
     add_index "searchables",

app/services/podcasts/create_episode.rb

@@ -21,7 +21,7 @@ module Podcasts
       get_media_url(ep)
       begin
         ep.published_at = item.pubDate.to_date
-      rescue StandardError => e
+      rescue ArgumentError, NoMethodError => e
         Rails.logger.error("not a valid date: #{e}")
       end
       ep.body = item.body
@@ -33,17 +33,11 @@ module Podcasts
 
     attr_reader :podcast_id, :item
 
-    # checking url when it is https is useless, the url is set to the enclosure url anyway
     def get_media_url(episode)
-      episode.media_url = if HTTParty.head(item.enclosure_url.gsub(/http:/, "https:")).code == 200
-                            item.enclosure_url.gsub(/http:/, "https:")
-                          else
-                            item.enclosure_url
-                          end
-    rescue StandardError
-      # podcast episode must have a media_url
-      episode.media_url = item.enclosure_url
-      episode.podcast.update(status_notice: I18n.t(:unplayable, scope: "podcasts.statuses")) if episode.podcast.status_notice.empty?
+      result = GetMediaUrl.call(item.enclosure_url)
+      episode.reachable = result.reachable
+      episode.media_url = result.url
+      episode.https = result.https
     end
   end
 end

app/services/podcasts/get_media_url.rb

@@ -0,0 +1,41 @@
+module Podcasts
+  class GetMediaUrl
+    def initialize(enclosure_url)
+      @enclosure_url = enclosure_url.to_s
+    end
+
+    def self.call(*args)
+      new(*args).call
+    end
+
+    def call
+      was_http = !enclosure_url.starts_with?(/https/i)
+      https_url = enclosure_url.sub(/http:/i, "https:")
+
+      # check https url first
+      if url_reachable?(https_url)
+        reachable = true
+        url = https_url
+      # if https is unreachable, check http url (if it was provided)
+      else
+        reachable = was_http ? url_reachable?(enclosure_url) : false
+        url = enclosure_url
+      end
+      result_struct.new(https: url.starts_with?(/https/i), reachable: reachable, url: url)
+    end
+
+    private
+
+    attr_reader :enclosure_url
+
+    def result_struct
+      Struct.new(:https, :reachable, :url, keyword_init: true)
+    end
+
+    def url_reachable?(url)
+      HTTParty.head(url).code == 200
+    rescue Net::OpenTimeout, Errno::ECONNREFUSED
+      false
+    end
+  end
+end

app/views/podcast_episodes/show.html.erb

@@ -72,11 +72,11 @@
       </div>
     </div>
   </div>
-  <%# temp, until podcast_episode statuses are introduced %>
-  <% if @episode.podcast.status_notice.to_s.include?("may not be playable") %>
+  <%# checking both podcast_episode and podcast status for now  %>
+  <% if !@episode.https? || @episode.podcast.status_notice.to_s.include?("may not be playable") %>
     <center>
       <h1 style="color: #e05252;">
-        <%= @episode.podcast.status_notice %>
+        <%= I18n.t "podcasts.statuses.unplayable" %>
       </h1>
       <p>
         <a href="<%= @episode.media_url %>">Click here to download/listen</a>

db/migrate/20190705111810_add_reachable_to_podcast_episodes.rb

@@ -0,0 +1,6 @@
+class AddReachableToPodcastEpisodes < ActiveRecord::Migration[5.2]
+  def change
+    add_column :podcast_episodes, :reachable, :boolean, default: true
+    add_column :podcast_episodes, :status_notice, :string
+  end
+end

db/migrate/20190705114625_add_https_status_to_podcast_episodes.rb

@@ -0,0 +1,9 @@
+class AddHttpsStatusToPodcastEpisodes < ActiveRecord::Migration[5.2]
+  class PodcastEpisode < ApplicationRecord; end
+
+  def change
+    add_column :podcast_episodes, :https, :boolean, default: true
+
+    PodcastEpisode.where("media_url ILIKE ?", "http:/%").update_all(https: false)
+  end
+end

db/schema.rb

@@ -646,6 +646,7 @@ ActiveRecord::Schema.define(version: 2019_07_11_070019) do
     t.boolean "featured", default: true
     t.integer "featured_number"
     t.string "guid", null: false
+    t.boolean "https", default: true
     t.string "image"
     t.string "itunes_url"
     t.string "media_url", null: false
@@ -654,9 +655,11 @@ ActiveRecord::Schema.define(version: 2019_07_11_070019) do
     t.text "processed_html"
     t.datetime "published_at"
     t.text "quote"
+    t.boolean "reachable", default: true
     t.integer "reactions_count", default: 0, null: false
     t.string "slug", null: false
     t.string "social_image"
+    t.string "status_notice"
     t.string "subtitle"
     t.text "summary"
     t.string "title", null: false

spec/requests/podcast_episodes_api_spec.rb

@@ -9,6 +9,9 @@ RSpec.describe "ArticlesApi", type: :request, vcr: vcr_option do
   let(:podcast) { create(:podcast, feed_url: "http://softwareengineeringdaily.com/feed/podcast/") }
 
   before do
+    stub_request(:head, "https://traffic.libsyn.com/sedaily/AnalyseAsia.mp3").to_return(status: 200)
+    stub_request(:head, "https://traffic.libsyn.com/sedaily/IFTTT.mp3").to_return(status: 200)
+
     perform_enqueued_jobs do
       Podcasts::Feed.new.get_episodes(podcast, 2)
     end

spec/requests/podcasts/podcast_episodes_index_spec.rb

@@ -6,5 +6,13 @@ RSpec.describe "PodcastEpisodesSpec", type: :request do
       get "/pod"
       expect(response.body).to include("If you know of a great dev podcast")
     end
+
+    it "shows reachable podcasts" do
+      create(:podcast_episode, title: "SuperMario")
+      create(:podcast_episode, reachable: false, title: "I'm unreachable")
+      get "/pod"
+      expect(response.body).to include("SuperMario")
+      expect(response.body).not_to include("unreachable")
+    end
   end
 end

spec/requests/podcasts/podcast_show_spec.rb

@@ -15,5 +15,14 @@ RSpec.describe "PodcastShow", type: :request do
       get "/#{podcast.slug}"
       expect(response).to have_http_status(:ok)
     end
+
+    it "shows reachable podcasts" do
+      podcast = create(:podcast)
+      create(:podcast_episode, title: "Bats' life", podcast: podcast)
+      create(:podcast_episode, reachable: false, title: "Really old one", podcast: podcast)
+      get "/#{podcast.slug}"
+      expect(response.body).to include(CGI.escapeHTML("Bats' life"))
+      expect(response.body).not_to include("Really old one")
+    end
   end
 end

spec/services/podcasts/create_episode_spec.rb

@@ -27,12 +27,25 @@ RSpec.describe Podcasts::CreateEpisode, type: :service do
       expect(episode.guid).to include("53b17a1e-271b-40e3-a084-a67b4fcba562")
     end
 
+    it "sets correct availability statuses" do
+      episode = described_class.call(podcast.id, item)
+      expect(episode.https?).to be true
+      expect(episode.reachable).to be true
+    end
+
     it "rescues an exception when pubDate is invalid" do
       allow(item).to receive(:pubDate).and_return("not a date, haha")
       episode = described_class.call(podcast.id, item)
       expect(episode).to be_persisted
       expect(episode.published_at).to eq(nil)
     end
+
+    it "rescues an exception when pubDate is nil" do
+      allow(item).to receive(:pubDate).and_return(nil)
+      episode = described_class.call(podcast.id, item)
+      expect(episode).to be_persisted
+      expect(episode.published_at).to eq(nil)
+    end
   end
 
   context "when item has an http media url" do
@@ -44,20 +57,17 @@ RSpec.describe Podcasts::CreateEpisode, type: :service do
       stub_request(:head, https_url).to_return(status: 200)
       episode = described_class.call(podcast.id, item)
       expect(episode.media_url).to eq(https_url)
+      expect(episode.https?).to be true
+      expect(episode.reachable).to be true
     end
 
     it "keeps an http media url when https version is not available" do
       stub_request(:head, https_url).to_return(status: 404)
+      stub_request(:head, item.enclosure_url).to_return(status: 200)
       episode = described_class.call(podcast.id, item)
       expect(episode.media_url).to eq(item.enclosure_url)
-    end
-
-    # enable when the logic will not rely solely on exception
-    xit "sets status notice when https version is not available" do
-      stub_request(:head, https_url).to_return(status: 404)
-      described_class.call(podcast.id, item)
-      podcast.reload
-      expect(podcast.status_notice).to include("may not be playable")
+      expect(episode.https?).to be false
+      expect(episode.reachable).to be true
     end
   end
 end

spec/services/podcasts/get_media_url_spec.rb

@@ -0,0 +1,50 @@
+require "rails_helper"
+
+RSpec.describe Podcasts::GetMediaUrl do
+  let(:https_url) { "https://hello.example.com" }
+  let(:http_url) { "http://hello.example.com" }
+
+  it "https, reachable" do
+    stub_request(:head, https_url).to_return(status: 200)
+    result = described_class.call(https_url)
+    expect(result.https).to be true
+    expect(result.reachable).to be true
+    expect(result.url).to eq(https_url)
+  end
+
+  it "https, unrechable" do
+    stub_request(:head, https_url).to_return(status: 404)
+    result = described_class.call(https_url)
+    expect(result.https).to be true
+    expect(result.reachable).to be false
+    expect(result.url).to eq(https_url)
+  end
+
+  it "http, https reachable" do
+    stub_request(:head, https_url).to_return(status: 200)
+    result = described_class.call(http_url)
+    expect(result.https).to be true
+    expect(result.reachable).to be true
+    expect(result.url).to eq(https_url)
+  end
+
+  it "http, https unreachable, http reachable" do
+    httparty_result = double
+    allow(httparty_result).to receive(:code).and_return(200)
+    allow(HTTParty).to receive(:head).with(http_url).and_return(httparty_result)
+    allow(HTTParty).to receive(:head).with(https_url).and_raise(Errno::ECONNREFUSED)
+    result = described_class.call(http_url)
+    expect(result.https).to be false
+    expect(result.reachable).to be true
+    expect(result.url).to eq(http_url)
+  end
+
+  it "http, https unreachable" do
+    allow(HTTParty).to receive(:head).with(https_url).and_raise(Errno::ECONNREFUSED)
+    allow(HTTParty).to receive(:head).with(http_url).and_raise(Errno::ECONNREFUSED)
+    result = described_class.call(http_url)
+    expect(result.https).to be false
+    expect(result.reachable).to be false
+    expect(result.url).to eq(http_url)
+  end
+end

spec/system/podcasts/user_visits_podcast_episode_spec.rb

@@ -17,13 +17,20 @@ RSpec.describe "User visits podcast show page", type: :system do
     expect(find("#comment_commentable_id", visible: false).value).to eq(podcast_episode.id.to_s)
   end
 
-  context "when podcast has a status_notice" do
-    let(:podcast) { create(:podcast, status_notice: "This podcast may not be playable in the browser") }
-    let!(:podcast_episode) { create(:podcast_episode, podcast_id: podcast.id) }
-
+  context "when episode may not be playable" do
     it "displays the status_notice" do
+      podcast = create(:podcast, status_notice: "This podcast may not be playable in the browser")
+      podcast_episode = create(:podcast_episode, podcast_id: podcast.id)
       visit podcast_episode.path.to_s
-      expect(page).to have_text(podcast.status_notice)
+      expect(page).to have_text(I18n.t("podcasts.statuses.unplayable"))
+      expect(page).to have_text("Click here to download")
+    end
+
+    it "displays status when episode is not reachable by https" do
+      podcast_episode = create(:podcast_episode, https: false)
+      visit podcast_episode.path.to_s
+      expect(page).to have_text(I18n.t("podcasts.statuses.unplayable"))
+      expect(page).to have_text("Click here to download")
     end
   end
 
@@ -34,6 +41,8 @@ RSpec.describe "User visits podcast show page", type: :system do
     it "doesn't display status_notice" do
       visit podcast_episode.path.to_s
       expect(page).not_to have_text("Random status notice")
+      expect(page).not_to have_text(I18n.t("podcasts.statuses.unplayable"))
+      expect(page).not_to have_text("Click here to download")
     end
   end