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 9 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.edit_locked? && feed.apple? && feed.apple_show_id.blank? && feed.apple_config
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
13 changes: 12 additions & 1 deletion 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)
if (sl = SyncLog.find_by(feeder_id: apple_config.public_feed.id, feeder_type: :feeds))
if apple_show_id.blank?
sl.destroy!
elsif sl.external_id != apple_show_id
sl.update(external_id: apple_show_id)
end
end
kookster marked this conversation as resolved.
Show resolved Hide resolved

SyncLog.log!(feeder_id: apple_config.public_feed.id,
feeder_type: :feeds,
sync_completed_at: Time.now.utc,
external_id: apple_show_id)

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
21 changes: 21 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 @@ -35,6 +37,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.map do |sj|
if used_ids.include?(sj["id"])
nil
else
["#{sj["id"]} (#{sj["attributes"]["title"]})", sj["id"]]
end
end.compact
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
4 changes: 2 additions & 2 deletions app/policies/feed_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ 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?
Expand Down
8 changes: 8 additions & 0 deletions app/views/feeds/_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,14 @@
<%= 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 %>
Expand Down
32 changes: 30 additions & 2 deletions app/views/feeds/_form_apple_config.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,34 @@
<%= key_fields.hidden_field :provider_id, data: {apple_key_target: "provider"} %>
<%= key_fields.hidden_field :key_id, data: {apple_key_target: "key"} %>
<% end %>
<% end %>

<%= form.hidden_field :type, value: "Feeds::AppleSubscription" %>
<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 %>
<div class="col-12 mt-4">
<div class="form-floating">
<% if !@apple_show_options.blank? %>
<%= form.select :apple_show_id, @apple_show_options, {include_blank: true} %>
svevang marked this conversation as resolved.
Show resolved Hide resolved
<% else %>
<%= form.text_field :apple_show_id %>
<% end %>
<%= form.label :apple_show_id, "Apple Show ID" %>
</div>
</div>
Copy link
Member

Choose a reason for hiding this comment

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

It seems that we almost have a two step process here. Upload the key, and then select and connect the show. Another way to say it is that the show drowdown will be empty until the key is populated.

Should we hide the apple config stuff (apple show id, sync blocs rss etc) until the key is uploaded?

If we did that, then the show's paged query could serve as a quick validation of the key as well.
Screenshot 2024-10-29 at 11 39 43 AM

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I thought about that, and I was torn on whether or not it was worth it - in the case where a user already knows the apple id, we don't need to make it two steps, and since almost all shows will have an apple id whether they have a subscription channel or not, I didn't want to prevent using a known value until it could be looked up.

I'll try making this conditional/two-step, and see how it goes!

Copy link
Member Author

Choose a reason for hiding this comment

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

Another problem with this design is you can't delete or replace the API keys.


<div class="col-6 mt-4">
<div class="form-floating input-group">
Expand All @@ -38,6 +63,9 @@
<%= field_help_text t(".help.display_episodes_count") %>
</div>
</div>

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

</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
7 changes: 7 additions & 0 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 @@ -783,6 +786,8 @@ en:
guide_link: Check out our guide to step you through the setup.
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 +836,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
6 changes: 6 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,6 @@
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
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
5 changes: 4 additions & 1 deletion db/schema.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading