-
Notifications
You must be signed in to change notification settings - Fork 1
Feat/episode sparklines #1386
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat/episode sparklines #1386
Changes from all commits
fffd32a
1e807d4
c2237e2
d4fa1cf
caf92ad
65237da
d767717
2c4452c
7f056b4
5a68772
fc723ad
d6e5b62
84330c3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,14 +2,33 @@ class PodcastMetricsController < ApplicationController | |
| include MetricsUtils | ||
|
|
||
| before_action :set_podcast | ||
| before_action :check_clickhouse, except: %i[show] | ||
| before_action :set_date_range, except: %i[dropdays] | ||
| before_action :set_uniques, only: %i[show uniques] | ||
| before_action :set_dropday_range, only: %i[show dropdays] | ||
| # before_action :check_clickhouse, except: %i[show] | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. kind of an aside (for me) for later, but i frequently (but not consistently always) get errors when trying to refresh the view and get a |
||
|
|
||
| def show | ||
| end | ||
|
|
||
| def episode_sparkline | ||
cavis marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| @episode = Episode.find_by(guid: metrics_params[:episode_id]) | ||
| @prev_episode = Episode.find_by(guid: metrics_params[:prev_episode_id]) | ||
|
|
||
| @episode_trend = calculate_episode_trend(@episode, @prev_episode) | ||
|
|
||
| @sparkline_downloads = | ||
radical-ube marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| Rollups::HourlyDownload | ||
| .where(episode_id: @episode[:guid], hour: (publish_hour(@episode)..publish_hour(@episode) + 6.months)) | ||
| .final | ||
| .select(:episode_id, "DATE_TRUNC('DAY', hour) AS hour", "SUM(count) AS count") | ||
| .group(:episode_id, "DATE_TRUNC('DAY', hour) AS hour") | ||
| .order(Arel.sql("DATE_TRUNC('DAY', hour) ASC")) | ||
| .load_async | ||
cavis marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| render partial: "metrics/episode_sparkline", locals: { | ||
| episode: @episode, | ||
| downloads: @sparkline_downloads, | ||
| episode_trend: @episode_trend | ||
| } | ||
| end | ||
|
|
||
| def downloads | ||
| @downloads_within_date_range = | ||
| Rollups::HourlyDownload | ||
|
|
@@ -195,46 +214,39 @@ def set_podcast | |
| render_not_found(e) | ||
| end | ||
|
|
||
| def set_date_range | ||
| @date_start = metrics_params[:date_start] | ||
| @date_end = metrics_params[:date_end] | ||
| @interval = metrics_params[:interval] | ||
| @date_range = generate_date_range(@date_start, @date_end, @interval) | ||
| def metrics_params | ||
| params | ||
| .permit(:podcast_id, :episode_id, :prev_episode_id) | ||
| end | ||
|
|
||
| def set_uniques | ||
| @uniques_selection = uniques_params[:uniques_selection] | ||
| end | ||
| def calculate_episode_trend(episode, prev_episode) | ||
| return nil unless episode.first_rss_published_at.present? && prev_episode.present? | ||
| return nil if (episode.first_rss_published_at + 1.day) > Time.now | ||
|
|
||
| def set_dropday_range | ||
| @dropday_range = dropdays_params[:dropday_range] | ||
| end | ||
| ep_dropday_sum = episode_dropday_query(episode) | ||
| previous_ep_dropday_sum = episode_dropday_query(prev_episode) | ||
|
|
||
| def metrics_params | ||
| params | ||
| .permit(:podcast_id, :date_start, :date_end, :interval) | ||
| .with_defaults( | ||
| date_start: 30.days.ago.utc_date, | ||
| date_end: Date.utc_today, | ||
| interval: "DAY" | ||
| ) | ||
| return nil if ep_dropday_sum <= 0 || previous_ep_dropday_sum <= 0 | ||
|
|
||
| ((ep_dropday_sum.to_f / previous_ep_dropday_sum.to_f) - 1).round(3) | ||
| end | ||
|
|
||
| def uniques_params | ||
| params | ||
| .permit(:uniques_selection) | ||
| .with_defaults( | ||
| uniques_selection: "last_7_rolling" | ||
| ) | ||
| .merge(metrics_params) | ||
| def episode_dropday_query(ep) | ||
| lowerbound = publish_hour(ep) | ||
| upperbound = lowerbound + 24.hours | ||
|
|
||
| Rollups::HourlyDownload | ||
| .where(episode_id: ep[:guid], hour: (lowerbound...upperbound)) | ||
| .final | ||
| .load_async | ||
| .sum(:count) | ||
| end | ||
|
|
||
| def dropdays_params | ||
| params | ||
| .permit(:dropday_range, :interval) | ||
| .with_defaults( | ||
| dropday_range: 7 | ||
| ) | ||
| .merge(metrics_params) | ||
| def publish_hour(episode) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this accurate? trying to address the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would probably just return a 0 / empty array, instead of querying clickhouse at all, if that's nil. The implication is, this episode was never written to the default feed. So you're not going to find any data in Clickhouse. Kind of surprised it got to querying clickhouse - i thought you had some early-returns when
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added to #1400 |
||
| if episode.first_rss_published_at.present? | ||
| episode.first_rss_published_at.beginning_of_hour | ||
| else | ||
| episode.published_at.beginning_of_hour | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,7 +17,12 @@ def index | |
| def show | ||
| authorize @podcast | ||
|
|
||
| @recently_published = @podcast.episodes.published.dropdate_desc.limit(3) | ||
| @recently_published_episodes = @podcast.episodes.published.dropdate_desc.limit(4) | ||
| @trend_episodes = @podcast.default_feed.episodes.published.dropdate_desc.where.not(first_rss_published_at: nil).offset(1).limit(4) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. here i am pulling the episodes to be compared against in the trend calculation. offset by 1 because i don't want the most recent one. |
||
| @episode_trend_pairs = episode_trend_pairs(@recently_published_episodes, @trend_episodes) | ||
|
|
||
| # @recently_published is used for the prod branch | ||
| @recently_published = @recently_published_episodes[0..2] | ||
| @next_scheduled = @podcast.episodes.draft_or_scheduled.dropdate_asc.limit(3) | ||
|
|
||
| @metrics_jwt = prx_jwt | ||
|
|
@@ -189,4 +194,24 @@ def compose_message(podcast) | |
| def sub_escapes(text) | ||
| text.gsub(/[&<>]/, "&" => "&", "<" => "<", ">" => ">") | ||
| end | ||
|
|
||
| def episode_trend_pairs(episodes, trend_episodes) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. and here i realized that i didn't want to necessarily pair episode and trend episode 1:1. i still needed to check if an episode should have a trend number at all, and then count which
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since these are in separate turbo requests, you could just find which episode you should compare to in that other request? It avoids any kind of N+1, not attempting to pair them all upfront. And makes the outer request quicker. Episode.where(podcast_id: ep.podcast_id, first_rss_published_at: ..ep.first_rss_published_at).order(first_rss_published_at: :desc).first(Not a request for change in this PR, just something to think about ... optimize for the outer/first GET request).
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added to #1400 |
||
| paired_trend_episodes = [] | ||
|
|
||
| episodes.map.with_index do |ep, i| | ||
| if ep.in_default_feed? | ||
| paired_trend_episodes << trend_episodes[paired_trend_episodes.length] | ||
|
|
||
| { | ||
| episode: ep, | ||
| prev_episode: paired_trend_episodes.last | ||
| } | ||
| else | ||
| { | ||
| episode: ep, | ||
| prev_episode: nil | ||
| } | ||
| end | ||
| end | ||
| end | ||
| end | ||
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| import { Controller } from "@hotwired/stimulus" | ||
| import { buildSparklineChart, destroyChart } from "util/apex" | ||
|
|
||
| export default class extends Controller { | ||
| static values = { | ||
| id: String, | ||
| downloads: Array, | ||
| } | ||
|
|
||
| static targets = ["chart"] | ||
|
|
||
| connect() { | ||
| const seriesData = this.downloadsValue.map((rollup) => { | ||
| return { | ||
| x: rollup.hour, | ||
| y: rollup.count, | ||
| } | ||
| }) | ||
| const series = [ | ||
| { | ||
| name: "Downloads", | ||
| data: seriesData, | ||
| }, | ||
| ] | ||
|
|
||
| const chart = buildSparklineChart(this.idValue, series, this.chartTarget) | ||
|
|
||
| chart.render() | ||
| } | ||
|
|
||
| disconnect() { | ||
| destroyChart(this.idValue) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| <%= turbo_frame_tag "episode_sparkline" do %> | ||
| <% trend = parse_trend(episode_trend) %> | ||
|
|
||
| <p class="d-flex align-items-center <%= trend[:color] if trend.present? %>"> | ||
| <span class="z-1"><%= trend[:percent] if trend.present? %></span> | ||
| <span class="material-icons ms-1 z-1" aria-hidden="true"><%= trend[:direction] if trend.present? %></span> | ||
| </p> | ||
| <div class="position-absolute top-50 start-50 translate-middle w-100 h-100 z-0" | ||
| data-controller="apex-sparkline" | ||
| data-apex-sparkline-id-value="<%= episode.guid %>" | ||
| data-apex-sparkline-downloads-value="<%= downloads.to_json %>"> | ||
| <div data-apex-sparkline-target="chart"></div> | ||
| </div> | ||
| <% end %> |
Uh oh!
There was an error while loading. Please reload this page.