Skip to content
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

Apple Delegated Delivery UI updates #1139

Merged
merged 23 commits into from
Nov 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
b612b72
Add in an apple id, maybe this is a good place?
kookster Oct 23, 2024
7ef7b93
Simplify params, update feed, use apple api to pick apple show
kookster Oct 23, 2024
c81214e
Add more options to the apple feed
kookster Oct 23, 2024
433774e
Reenable apple feed form
kookster Oct 23, 2024
797de22
lint!
kookster Oct 23, 2024
d068bea
merge main
kookster Oct 23, 2024
6556f19
Have some feeds locked for editing
kookster Oct 23, 2024
606bc1a
Connect the existing apple show on save
kookster Oct 23, 2024
369cab8
Fix spacing on locked message
kookster Oct 23, 2024
541d49b
Add some tests!
kookster Oct 24, 2024
256198e
lint tests
kookster Oct 24, 2024
f7d0139
Reload data after connecting apple id
kookster Oct 24, 2024
a1897f7
Merge branch 'main' into fix/apple_dd
kookster Oct 29, 2024
cc24147
Set apple ids on feeds
kookster Oct 29, 2024
88ce618
Query apple for shows and ids whenever a key is available
kookster Nov 2, 2024
631062f
There are multiple values, the visible text fields do not need this
kookster Nov 2, 2024
343f780
Validate the key works with a remote call
kookster Nov 2, 2024
6badb72
Return after delete, only create a log when none exists
kookster Nov 2, 2024
57e5c09
Filter archived feeds, make slug unique when soft delete
kookster Nov 2, 2024
3fbc474
Allow delete got apple feeds
kookster Nov 2, 2024
13af4d8
Only show other forms when apple feed saved
kookster Nov 2, 2024
dca482b
Only show more options after a valid key is uploaded
kookster Nov 2, 2024
bfad45f
Merge branch 'main' into fix/apple_dd
kookster Nov 2, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 13 additions & 17 deletions app/controllers/feeds_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ def index

# GET /feeds/1
def show
@feed.assign_attributes(feed_params_with_apple)
@feed.assign_attributes(feed_params)
authorize @feed
@apple_show_options = get_apple_show_options(@feed)
end

# GET /feeds/new
Expand All @@ -22,19 +23,25 @@ def new
@feed.clear_attribute_changes(%i[file_name podcast_id private slug])
end

def get_apple_show_options(feed)
kookster marked this conversation as resolved.
Show resolved Hide resolved
if feed.apple? && feed.apple_config&.key
feed.apple_show_options
end
end

def new_apple
@feed = Feeds::AppleSubscription.new(podcast: @podcast, private: true)
@feed.build_apple_config
@feed.apple_config.build_key
authorize @feed

@feed.assign_attributes(feed_params_with_apple)
@feed.assign_attributes(feed_params)
render "new"
end

# POST /feeds
def create
@feed = @podcast.feeds.new(feed_params_with_apple)
@feed = @podcast.feeds.new(feed_params)
@feed.slug = "" if @feed.slug.nil?
authorize @feed

Expand Down Expand Up @@ -114,10 +121,6 @@ def feed_params
params.fetch(:feed, {}).permit(:slug).merge(nilified_feed_params)
end

def feed_params_with_apple
svevang marked this conversation as resolved.
Show resolved Hide resolved
params.fetch(:feed, {}).permit(:slug).merge(nilified_feed_params).merge(apple_params)
end

def nilified_feed_params
nilify params.fetch(:feed, {}).permit(
:lock_version,
Expand All @@ -144,20 +147,13 @@ def nilified_feed_params
:paid,
:sonic_id,
:type,
:apple_show_id,
itunes_category: [],
itunes_subcategory: [],
feed_tokens_attributes: %i[id label token _destroy],
feed_images_attributes: %i[id original_url size alt_text caption credit _destroy _retry],
itunes_images_attributes: %i[id original_url size alt_text caption credit _destroy _retry]
)
end

def apple_params
nilify params.fetch(:feed, {}).permit(
apple_config_attributes: {
id: :id,
key_attributes: %i[id provider_id key_id key_pem_b64]
}
itunes_images_attributes: %i[id original_url size alt_text caption credit _destroy _retry],
apple_config_attributes: [:id, :publish_enabled, :sync_blocks_rss, {key_attributes: %i[id provider_id key_id key_pem_b64]}]
kookster marked this conversation as resolved.
Show resolved Hide resolved
)
end
end
7 changes: 0 additions & 7 deletions app/javascript/controllers/apple_key_controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,7 @@ export default class extends Controller {
const keyId = fields[1].split(".")[0]

this.providerTargets.forEach((target) => (target.value = provider))
this.providerTarget.disabled = false
this.providerTarget.focus()
this.providerTarget.disabled = true

this.keyTargets.forEach((target) => (target.value = keyId))
this.keyTarget.disabled = false
this.keyTarget.focus()
this.keyTarget.disabled = true
svevang marked this conversation as resolved.
Show resolved Hide resolved
}

convertKeyToB64(fileText) {
Expand Down
10 changes: 10 additions & 0 deletions app/models/apple/key.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,16 @@ class Key < ApplicationRecord

validate :provider_id_is_valid, if: :provider_id?
validate :ec_key_format, if: :key_pem_b64?
validate :must_have_working_key
svevang marked this conversation as resolved.
Show resolved Hide resolved

def must_have_working_key
return if Rails.env.test?
api = Apple::Api.from_key(self)
Apple::Show.apple_shows_json(api)
rescue => err
logger.error(err)
errors.add(:key_id, "must have a working Apple key")
end

def provider_id_is_valid
if provider_id.include?("_")
Expand Down
23 changes: 17 additions & 6 deletions app/models/apple/show.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,29 @@ class Show
:private_feed,
:api

def self.apple_shows_json(api)
api.get_paged_collection("shows")
end

def self.apple_episode_json(api, show_id)
api.get_paged_collection("shows/#{show_id}/episodes")
end

def self.connect_existing(apple_show_id, apple_config)
api = Apple::Api.from_apple_config(apple_config)

SyncLog.log!(feeder_id: apple_config.public_feed.id,
feeder_type: :feeds,
sync_completed_at: Time.now.utc,
external_id: apple_show_id)
if (sl = SyncLog.find_by(feeder_id: apple_config.public_feed.id, feeder_type: :feeds))
if apple_show_id.blank?
return sl.destroy!
elsif sl.external_id != apple_show_id
sl.update!(external_id: apple_show_id)
end
else
SyncLog.log!(feeder_id: apple_config.public_feed.id,
feeder_type: :feeds,
sync_completed_at: Time.now.utc,
external_id: apple_show_id)
end
kookster marked this conversation as resolved.
Show resolved Hide resolved

api = Apple::Api.from_apple_config(apple_config)
new(api: api,
public_feed: apple_config.public_feed,
private_feed: apple_config.private_feed)
Expand Down
29 changes: 29 additions & 0 deletions app/models/feeds/apple_subscription.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ class Feeds::AppleSubscription < Feed

after_create :republish_public_feed

after_save_commit :update_apple_show

has_one :apple_config, class_name: "::Apple::Config", dependent: :destroy, autosave: true, validate: true, inverse_of: :feed

accepts_nested_attributes_for :apple_config, allow_destroy: true, reject_if: :all_blank
Expand All @@ -23,6 +25,14 @@ class Feeds::AppleSubscription < Feed
validate :must_be_private
validate :must_have_token

# for soft delete, need a unique slug to be able to make another
def paranoia_destroy_attributes
{
deleted_at: current_time_from_proper_timezone,
slug: "#{slug} - #{Time.now.to_i}"
svevang marked this conversation as resolved.
Show resolved Hide resolved
}
end

def set_defaults
self.slug ||= DEFAULT_FEED_SLUG
self.title ||= DEFAULT_TITLE
Expand All @@ -35,6 +45,25 @@ def set_defaults
super
end

def update_apple_show
if previous_changes[:apple_show_id]
Apple::Show.connect_existing(apple_show_id, apple_config)
end
end

def apple_show_options
used_ids = Feed.apple.distinct.where("id != ?", id).pluck(:apple_show_id).compact
api = Apple::Api.from_apple_config(apple_config)
shows_json = Apple::Show.apple_shows_json(api) || []
kookster marked this conversation as resolved.
Show resolved Hide resolved
shows_json
.filter { |sj| sj["attributes"]["publishingState"] != "ARCHIVED" }
svevang marked this conversation as resolved.
Show resolved Hide resolved
.filter { |sj| !used_ids.include?(sj["id"]) }
.map { |sj| ["#{sj["id"]} (#{sj["attributes"]["title"]})", sj["id"]] }
rescue => err
logger.error(err)
[]
end

def guess_audio_format
default_feed_audio_format || episode_audio_format || DEFAULT_AUDIO_FORMAT
end
Expand Down
2 changes: 1 addition & 1 deletion app/policies/apple/config_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@ def create?
end

def update?
false
FeedPolicy.new(token, resource.feed).update?
svevang marked this conversation as resolved.
Show resolved Hide resolved
end
end
6 changes: 3 additions & 3 deletions app/policies/feed_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,14 @@ def create?
end

def new_apple?
create?
update?
end

def update?
PodcastPolicy.new(token, resource.podcast).update?
PodcastPolicy.new(token, resource.podcast).update? && !resource.edit_locked?
end

def destroy?
resource.custom? && update? && !resource.apple?
resource.custom? && update?
svevang marked this conversation as resolved.
Show resolved Hide resolved
end
end
14 changes: 12 additions & 2 deletions app/views/feeds/_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,24 @@
<%= form_with(url: url, model: model, method: method, html: {autocomplete: "off"}, data: data) do |form| %>
<div class="row my-4 mx-2" data-controller="feed-tokens" data-feed-tokens-unsaved-outlet="form">
<div class="col-lg-8">
<% if feed.edit_locked? %>
<div class="col-12">
<div class="alert alert-danger" role="alert">
<b><%= t(".title.warning") %></b>
<%= t(".help.locked").html_safe %>
</div>
</div>
<% end %>
<div class="row" data-controller="feed-link">
<% if feed.default? %>
<%= render "form_main", podcast: podcast, feed: feed, form: form %>
<%= render "form_audio_format", podcast: podcast, feed: feed, form: form %>
<% elsif apple_feed?(feed) %>
<%= render "form_apple_config", podcast: podcast, feed: feed, form: form %>
<%= render "form_audio_format", podcast: podcast, feed: feed, form: form %>
<%= render "form_ad_zones", podcast: podcast, feed: feed, form: form %>
<% if feed.persisted? %>
kookster marked this conversation as resolved.
Show resolved Hide resolved
<%= render "form_audio_format", podcast: podcast, feed: feed, form: form %>
<%= render "form_ad_zones", podcast: podcast, feed: feed, form: form %>
<% end %>
<% else %> <%# custom feeds %>
<%= render "form_main", podcast: podcast, feed: feed, form: form %>
<%= render "form_auth", podcast: podcast, feed: feed, form: form %>
Expand Down
62 changes: 45 additions & 17 deletions app/views/feeds/_form_apple_config.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -6,38 +6,66 @@
<div class="card-body" data-controller="apple-key">
<div class="mb-4">
<p><%= t(".description") %></p>
<a href="https://help.prx.org/hc/en-us/articles/26602803552155-Setting-up-Apple-Podcast-Subscriptions-with-Apple-Delegated-Delivery"><%= t(".guide_link") %></a>
<%= t(".guide_link").html_safe %>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a cosmetic change- help urls should not be embedded in the code, moved it to the translations, and allowed a link with html_safe

</div>
<%= form.fields_for :apple_config do |config_form| %>
<%= config_form.fields_for :key do |key_fields| %>
<% unless config_form.object.persisted? %>
<%= key_fields.file_field :key_file, class: "mb-4 form-control", accept: ".p8, .pem", data: {action: "apple-key#convertFileToKey"} %>
<%= key_fields.file_field :key_file, class: "mb-4 form-control", accept: ".p8, .pem", data: {action: "apple-key#convertFileToKey"}, required: true %>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now this is required so you can't create the apple feed without adding a file


<%= key_fields.hidden_field :key_pem_b64, data: {apple_key_target: "pem"} %>
<% else %>
<div class="mb-2"><%= t(".key_uploaded") %></div>
<div class="mb-4 form-floating">
<%= key_fields.text_field :provider_id, disabled: true, data: {apple_key_target: "provider"} %>
<%= key_fields.label :provider_id, "Provider ID" %>
</div>
<div class="form-floating">
<%= key_fields.text_field :key_id, disabled: true, data: {apple_key_target: "key"} %>
<%= key_fields.label :key_id, "Apple Key" %>
</div>
<% end %>
<div class="mb-4 form-floating">
<%= key_fields.text_field :provider_id, disabled: true, data: {apple_key_target: "provider"} %>
<%= key_fields.label :provider_id, "Provider ID" %>
<%= key_fields.hidden_field :provider_id, data: {apple_key_target: "provider"} %>
<%= key_fields.hidden_field :key_id, data: {apple_key_target: "key"} %>
<% end %>
<% if config_form.object.persisted? %>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

within this form, also hides all the other fields until a valid key is uploaded and saved

<div class="col-12 mt-4">
<div class="form-check">
<%= config_form.check_box :publish_enabled %>
<div class="d-flex align-items-center">
<%= config_form.label :publish_enabled %>
<%= help_text t(".help.publish_enabled") %>
</div>
</div>
<div class="form-check">
<%= config_form.check_box :sync_blocks_rss %>
<div class="d-flex align-items-center">
<%= config_form.label :sync_blocks_rss %>
<%= help_text t(".help.sync_blocks_rss") %>
</div>
</div>
</div>
<% end %>
<% end %>
<% if form.object.apple_config&.persisted? %>
<div class="col-12 mt-4">
<div class="form-floating">
<%= key_fields.text_field :key_id, disabled: true, data: {apple_key_target: "key"} %>
<%= key_fields.label :key_id, "Apple Key" %>
<%= form.select :apple_show_id, @apple_show_options, {include_blank: true} %>
svevang marked this conversation as resolved.
Show resolved Hide resolved
<%= form.label :apple_show_id, "Apple Show ID" %>
</div>
</div>

<%= key_fields.hidden_field :provider_id, data: {apple_key_target: "provider"} %>
<%= key_fields.hidden_field :key_id, data: {apple_key_target: "key"} %>
<% end %>
<div class="col-6 mt-4">
<div class="form-floating input-group">
<%= form.number_field :display_episodes_count %>
<%= form.label :display_episodes_count %>
<%= field_help_text t(".help.display_episodes_count") %>
</div>
</div>
<% end %>

<%= form.hidden_field :type, value: "Feeds::AppleSubscription" %>

<div class="col-6 mt-4">
<div class="form-floating input-group">
<%= form.number_field :display_episodes_count %>
<%= form.label :display_episodes_count %>
<%= field_help_text t(".help.display_episodes_count") %>
</div>
</div>
</div>
</div>
</div>
3 changes: 1 addition & 2 deletions app/views/feeds/_tabs.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@
<div class="fixed-bottom col-sm-12 col-md-3 col-xl-2">
<div class="btn-group dropup d-flex sticky-md-bottom m-4 shadow">
<%= link_to "Add a Feed", new_podcast_feed_path(podcast), class: "btn btn-success flex-grow-1" %>
<%# FIXME %>
<% if false && @feeds.none?(&:apple?) %>
<% if @feeds.none?(&:apple?) %>
<button type="button" class="btn btn-success dropdown-toggle dropdown-toggle-split flex-grow-0 px-3 border-0 border-start" data-bs-toggle="dropdown" aria-expanded="false">
<span class="visually-hidden">Toggle Dropdown</span>
</button>
Expand Down
12 changes: 10 additions & 2 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,9 @@ en:
application:
field_copy_tooltip: Copied!
label:
apple/config:
publish_enabled: Enable publishing to Apple Podcasts Connect
sync_blocks_rss: Episodes must publish to Apple before the public feed
svevang marked this conversation as resolved.
Show resolved Hide resolved
episode:
ad_breaks: Ad Breaks
author_email: Author Email
Expand Down Expand Up @@ -779,10 +782,13 @@ en:
ad_zones: Control the types of ads that should be stitched into episodes in this feed.
form_apple_config:
title: Apple Subscriptions
description: In order to publish episodes through Apple's Podcast Connect API, ensure you've delegated access to the PRX account in Podcast Connect.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this isn't true - you need an API key from whatever apple account, doesn't have to be the PRX account

guide_link: Check out our guide to step you through the setup.
description: Configure your credentials to publish episodes through Apple's Podcast Connect API
guide_link: <a href="https://help.prx.org/hc/en-us/articles/26602803552155-Setting-up-Apple-Podcast-Subscriptions-with-Apple-Delegated-Delivery" target="_blank">Check out our guide to step you through the setup.</a>
key_uploaded: Apple API Key Uploaded
help:
display_episodes_count: You can optionally limit the number of episodes that Paid Subscribers will see in Apple Podcasts. Leave this field blank to include all.
publish_enabled: Enable automatically publishing and updating episodes in this feed to Apple?
sync_blocks_rss: Block publishing to the default feed RSS until the Apple episode is published?
form_audio_format:
title: Audio Format
help:
Expand Down Expand Up @@ -831,6 +837,8 @@ en:
title: Feed Status
form:
<<: *form
help:
locked: This feed is locked, to make changes please <a href="https://help.prx.org/hc/en-us">contact PRX support.</a>
helper:
episode_offset_options:
"-300": 5 minutes early
Expand Down
15 changes: 15 additions & 0 deletions db/migrate/20241023012600_add_apple_show_id_to_feeds.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
class AddAppleShowIdToFeeds < ActiveRecord::Migration[7.2]
def change
add_column :feeds, :apple_show_id, :string
svevang marked this conversation as resolved.
Show resolved Hide resolved
add_index :feeds, :apple_show_id

reversible do |dir|
dir.up do
Feeds::AppleSubscription.all.each do |feed|
apple_id = feed.apple_sync_log&.external_id
feed.update_attribute!(:apple_show_id, apple_id)
end
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now this migration adds the apple show id, and fills it from the sync log entries

end
end
end
end
5 changes: 5 additions & 0 deletions db/migrate/20241023211041_add_edit_locked_to_feeds.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddEditLockedToFeeds < ActiveRecord::Migration[7.2]
def change
add_column :feeds, :edit_locked, :boolean
end
end
Loading
Loading