Skip to content

Commit

Permalink
Merge pull request #728 from PRX/chore/retryable_media
Browse files Browse the repository at this point in the history
Retryable media
  • Loading branch information
cavis authored Aug 2, 2023
2 parents 86c67e5 + 811b155 commit b5b5c7d
Show file tree
Hide file tree
Showing 18 changed files with 90 additions and 65 deletions.
4 changes: 4 additions & 0 deletions app/helpers/uploads_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,10 @@ def upload_processing?(rec)
%w[started created processing retrying].include?(rec&.status)
end

def upload_stalled?(rec)
upload_processing?(rec) && rec.retryable?
end

def upload_complete?(rec)
%w[complete].include?(rec.status)
end
Expand Down
13 changes: 9 additions & 4 deletions app/javascript/controllers/polling_controller.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
import { Controller } from "@hotwired/stimulus"

const DEFAULT_DEBOUNCE = 1000
const DEFAULT_MAX = 120

export default class extends Controller {
static values = { debounce: Number }
static values = { debounce: Number, max: Number }

connect() {
this.count = 0
this.frame = this.element.closest("turbo-frame")
this.interval = setInterval(() => {
this.clickOrTurbo()
}, this.debounceValue || DEFAULT_DEBOUNCE)
this.interval = setInterval(() => this.clickOrTurbo(), this.debounceValue || DEFAULT_DEBOUNCE)
}

disconnect() {
Expand All @@ -18,6 +18,11 @@ export default class extends Controller {

// use a turbo-frame to reload if possible, to dodge document.click events
clickOrTurbo() {
this.count += 1
if (this.count > (this.maxValue || DEFAULT_MAX)) {
return this.disconnect()
}

if (this.frame && this.frame.src) {
this.frame.reload()
} else if (this.frame && this.element.href) {
Expand Down
5 changes: 3 additions & 2 deletions app/models/concerns/image_file.rb
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,9 @@ def update_image(img)
end

def retryable?
if status_started? || status_created? || status_processing?
(Time.now - updated_at) > 30
if %w[started created processing retrying].include?(status)
last_event = task&.updated_at || updated_at
Time.now - last_event > 100
else
false
end
Expand Down
5 changes: 3 additions & 2 deletions app/models/media_resource.rb
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,9 @@ def update_resource(res)
end

def retryable?
if status_started? || status_created? || status_processing?
(Time.now - updated_at) > 30
if %w[started created processing retrying].include?(status)
last_event = task&.updated_at || updated_at
Time.now - last_event > 100
else
false
end
Expand Down
2 changes: 1 addition & 1 deletion app/views/episodes/media/_complete.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
</button>

<div class="form-control d-flex align-items-center">
<div class="text-secondary me-2 flex-grow-1 overflow-hidden text-truncate"><%= media.file_name %></div>
<div class="text-secondary me-2 overflow-hidden text-truncate"><%= media.file_name %></div>

<small class="text-danger flex-grow-1 me-2"><%= t(".unable") %></small>

Expand Down
11 changes: 9 additions & 2 deletions app/views/episodes/media/_error.html.erb
Original file line number Diff line number Diff line change
@@ -1,9 +1,16 @@
<div class="col-12 mb-4">
<div class="form-floating input-group">
<div class="form-control d-flex align-items-center is-invalid">
<span class="material-icons text-primary mx-2"><%= episode.medium_video? ? "video_file" : "audio_file" %></span>
<span class="material-icons text-danger"><%= episode.medium_video? ? "video_file" : "audio_file" %></span>
<div class="text-secondary mx-2 flex-grow-1 overflow-hidden text-truncate"><%= media.file_name %></div>
<small class="text-muted">(<%= number_to_human_size(media.file_size) %>)</small>

<% if retryable %>
<%= link_to edit_episode_path(media.episode, uploads_retry_params(form)), class: "btn btn-sm btn-warning" do %>
<span class="material-icons">restart_alt</span> Retry
<% end %>
<% else %>
<small class="text-muted">(<%= number_to_human_size(media.file_size) %>)</small>
<% end %>
</div>

<%= link_to edit_episode_path(media.episode, uploads_destroy_params(form)), class: "input-text-group prx-input-text-group" do %>
Expand Down
4 changes: 3 additions & 1 deletion app/views/episodes/media/_media.html.erb
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
<% if upload_new?(media) %>
<%= render "episodes/media/new", form: form, episode: episode, media: media %>
<% elsif upload_stalled?(media) %>
<%= render "episodes/media/error", form: form, episode: episode, media: media, retryable: true %>
<% elsif upload_processing?(media) %>
<%= render "episodes/media/processing", form: form, episode: episode, media: media %>
<% elsif upload_complete?(media) %>
<%= render "episodes/media/complete", form: form, episode: episode, media: media %>
<% else %>
<%= render "episodes/media/error", form: form, episode: episode, media: media %>
<%= render "episodes/media/error", form: form, episode: episode, media: media, retryable: false %>
<% end %>
4 changes: 2 additions & 2 deletions app/views/episodes/media/_new.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@
<%= form.hidden_field :position %>

<div class="form-control d-flex align-items-center is-changed if-visible">
<span class="material-icons text-primary ms-2"><%= episode.medium_video? ? "video_file" : "audio_file" %></span>
<span class="material-icons text-primary"><%= episode.medium_video? ? "video_file" : "audio_file" %></span>
<div class="text-secondary mx-2 flex-grow-1 overflow-hidden text-truncate" data-upload-target="fileName"></div>
<small class="text-muted">(<span data-upload-target="fileSize"></span>)</small>
</div>
Expand All @@ -76,7 +76,7 @@
<%# step 3b: something went horribly wrong! %>
<div class="form-floating d-none" data-upload-target="error">
<div class="form-control d-flex align-items-center is-invalid">
<span class="material-icons text-primary mx-2"><%= episode.medium_video? ? "video_file" : "audio_file" %></span>
<span class="material-icons text-primary"><%= episode.medium_video? ? "video_file" : "audio_file" %></span>
<div class="text-secondary mx-2 flex-grow-1 overflow-hidden text-truncate" data-upload-target="fileName"></div>
<small class="text-muted">(<span data-upload-target="fileSize"></span>)</small>

Expand Down
13 changes: 2 additions & 11 deletions app/views/episodes/media/_processing.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,8 @@
<span class="visually-hidden"><%= t(".hint") %>...</span>
</div>

<div class="text-secondary mx-2 flex-grow-1 overflow-hidden text-truncate"><%= media.file_name %></div>

<% if media.retryable? %>
<div class="flex-grow-1">
<%= link_to edit_episode_path(media.episode, uploads_retry_params(form)), class: "btn btn-sm btn-warning" do %>
<span class="material-icons">restart_alt</span> Retry
<% end %>
</div>
<% else %>
<small class="text-muted flex-grow-1">(<%= t(".hint") %>)</small>
<% end %>
<div class="text-secondary mx-2 overflow-hidden text-truncate"><%= media.file_name %></div>
<small class="text-muted flex-grow-1">(<%= t(".hint") %>)</small>

<% if media.file_size.present? %>
<small class="text-muted">(<%= number_to_human_size(media.file_size) %>)</small>
Expand Down
17 changes: 12 additions & 5 deletions app/views/images/_error.html.erb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<div class="card shadow prx-uploads mb-4">
<div class="card shadow border-0 prx-uploads">
<div class="card-header-info d-flex justify-content-between align-items-center">
<h2 class="card-title h5"><%= t("images.title.#{image.model_name.singular}") %></h5>

Expand All @@ -10,14 +10,21 @@
<div class="card-body">
<div class="form-floating mb-4">
<div class="form-control d-flex align-items-center is-invalid">
<span class="material-icons text-primary mx-2">image</span>
<span class="material-icons text-danger">image</span>
<div class="text-secondary mx-2 flex-grow-1 overflow-hidden text-truncate"><%= image.file_name %></div>
<small class="text-muted">(<%= number_to_human_size(image.size) %>)</small>

<% if retry_path %>
<%= link_to retry_path, class: "btn btn-sm btn-warning" do %>
<span class="material-icons">restart_alt</span> <%= t("images.label.retry") %>
<% end %>
<% else %>
<small class="text-muted">(<%= number_to_human_size(image.size) %>)</small>
<% end %>
</div>

<div class="invalid-feedback"><%= upload_invalid_messages(image) || "There was a problem processing your image" %></div>
<div class="invalid-feedback"><%= upload_invalid_messages(image) || t("images.error.processing") %></div>

<label class="is-invalid">Image File</label>
<label class="is-invalid"><%= t("images.label.image_file") %></label>
</div>

<%= form.hidden_field :id %>
Expand Down
8 changes: 5 additions & 3 deletions app/views/images/_form.html.erb
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
<% if upload_new?(image) %>
<%= render "images/new", form: form, image: image %>
<% elsif upload_stalled?(image) %>
<%= render "images/error", form: form, image: image, delete_path: delete_path, retry_path: retry_path %>
<% elsif upload_processing?(image) %>
<%= render "images/processing", form: form, image: image, delete_path: delete_path, retry_path: retry_path %>
<%= render "images/processing", form: form, image: image, delete_path: delete_path %>
<% elsif upload_complete?(image) %>
<%= render "images/complete", form: form, image: image, delete_path: delete_path, retry_path: retry_path %>
<%= render "images/complete", form: form, image: image, delete_path: delete_path %>
<% else %>
<%= render "images/error", form: form, image: image, delete_path: delete_path, retry_path: retry_path %>
<%= render "images/error", form: form, image: image, delete_path: delete_path, retry_path: nil %>
<% end %>
2 changes: 1 addition & 1 deletion app/views/images/_meta.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
</div>
</div>

<div class="col mb-4" data-morph="false">
<div class="col mb-2" data-morph="false">
<div class="form-floating">
<%= form.text_field :alt_text %>
<%= form.label :alt_text %>
Expand Down
12 changes: 6 additions & 6 deletions app/views/images/_modal.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@
<div class="modal-content">
<div class="modal-header">
<h5 class="modal-title"><%= title %></h5>
<button type="button" class="btn-close" data-bs-dismiss="modal" aria-label="Close"></button>
<button type="button" class="btn-close" data-bs-dismiss="modal" aria-label="<%= t("images.label.close") %>"></button>
</div>
<div class="modal-body">
<table class="table table-sm m-0">
<tbody>
<tr>
<th scope="row"><strong>File:</strong></th>
<th scope="row"><strong><%= t("images.label.file") %>:</strong></th>
<td>
<% if image.status_complete? %>
<%= link_to image.file_name, image.url, target: "_blank", rel: "noopener" %>
Expand All @@ -19,21 +19,21 @@
</td>
</tr>
<tr>
<th scope="row"><strong>Status:</strong></th>
<th scope="row"><strong><%= t("images.label.status") %>:</strong></th>
<td>
<span class="badge bg-<%= upload_status_class(image) %>"><%= image.status.capitalize %></span>
</td>
</tr>
<tr>
<th scope="row"><strong>Size:</strong></th>
<th scope="row"><strong><%= t("images.label.size") %>:</strong></th>
<td><%= blank_dash(image.size) { number_to_human_size(image.size) } %></td>
</tr>
<tr>
<th scope="row"><strong>Format:</strong></th>
<th scope="row"><strong><%= t("images.label.format") %>:</strong></th>
<td><%= blank_dash(image.format) %></td>
</tr>
<tr>
<th scope="row"><strong>Dimensions:</strong></th>
<th scope="row"><strong><%= t("images.label.dimensions") %>:</strong></th>
<td><%= blank_dash(image.width, image.height) { "#{image.width} x #{image.height}" } %></td>
</tr>
</tbody>
Expand Down
14 changes: 7 additions & 7 deletions app/views/images/_new.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@
<%= help_text t("images.help.#{image.model_name.singular}") %>
<%# delete buttons in header, after upload completes %>
<button type="button" class="btn btn-icon p-0 d-none" data-action="upload#cancelUpload" data-upload-target="success">
<button type="button" class="prx-btn-help d-none" data-action="upload#cancelUpload" data-upload-target="success">
<span class="material-icons">delete</span>
</button>
<button type="button" class="btn btn-icon p-0 d-none" data-action="upload#cancelUpload" data-upload-target="error">
<button type="button" class="prx-btn-help d-none" data-action="upload#cancelUpload" data-upload-target="error">
<span class="material-icons">delete</span>
</button>
</div>
Expand All @@ -31,10 +31,10 @@
<%# fake display field; real text area is opacity-0 on top of it %>
<div class="form-control d-flex align-items-center <%= "is-changed" if image.marked_for_destruction? %>">
<span class="material-icons text-primary">upload</span>
<div class="text-secondary mx-2 flex-grow-1 overflow-hidden text-truncate">Upload a file</div>
<div class="text-secondary mx-2 flex-grow-1 overflow-hidden text-truncate"><%= t("images.label.upload") %></div>
</div>

<%= form.label :image_file, "Image File" %>
<%= form.label :image_file, t("images.label.image_file") %>
</div>

<%# step 2: upload progressbar %>
Expand All @@ -52,7 +52,7 @@
</button>
</div>

<label>Image File</label>
<label><%= t("images.label.image_file") %></label>
</div>

<%# step 3a: hidden original_url field pointing to the uploaded temporary s3 file %>
Expand All @@ -66,7 +66,7 @@
<small class="text-muted">(<span data-upload-target="fileSize"></span>)</small>
</div>

<label>Image File</label>
<label><%= t("images.label.image_file") %></label>
</div>
<div class="d-none" data-upload-target="success">
<%= render "images/meta", form: form %>
Expand All @@ -82,7 +82,7 @@

<div class="invalid-feedback" data-upload-target="errorMessage"></div>

<label class="is-invalid">Image File</label>
<label class="is-invalid"><%= t("images.label.image_file") %></label>
</div>

</div>
Expand Down
19 changes: 5 additions & 14 deletions app/views/images/_processing.html.erb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<div class="card shadow border-0 prx-uploads mb-4">
<div class="card shadow border-0 prx-uploads">
<div class="card-header-info d-flex justify-content-between align-items-center">
<h2 class="card-title h5"><%= t("images.title.#{image.model_name.singular}") %></h5>

Expand All @@ -11,27 +11,18 @@
<div class="form-floating mb-4">
<div class="form-control d-flex align-items-center">
<div class="spinner-border text-primary ms-2" role="status">
<span class="visually-hidden">Loading...</span>
<span class="visually-hidden"><%= t("images.label.loading") %></span>
</div>

<div class="text-secondary mx-2 flex-grow-1 overflow-hidden text-truncate"><%= image.file_name %></div>

<% if image.retryable? %>
<div class="flex-grow-1">
<%= link_to retry_path, class: "btn btn-sm btn-warning" do %>
<span class="material-icons">restart_alt</span> Retry
<% end %>
</div>
<% else %>
<small class="text-muted flex-grow-1">(Processing)</small>
<% end %>
<div class="text-secondary mx-2 overflow-hidden text-truncate"><%= image.file_name %></div>
<small class="text-muted flex-grow-1">(<%= t("images.label.processing") %>)</small>

<% if image.size.present? %>
<small class="text-muted">(<%= number_to_human_size(image.size) %>)</small>
<% end %>
</div>

<label>Image File</label>
<label><%= t("images.label.image_file") %></label>
</div>

<%# just keep polling until the file finishes processing %>
Expand Down
14 changes: 14 additions & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -404,10 +404,24 @@ en:
update:
<<: *update
images:
error:
processing: There was a problem processing your image
help:
episode_image: Provide an image for your episode, if desired. Image dimensions must be square.
feed_image: Optionally provide an alternate image to use for smaller display purposes.
itunes_image: This image will be used as the cover image for this series. Image dimensions must be square.
label:
close: Close
dimensions: Dimensions
file: File
format: Format
image_file: Image File
loading: Loading...
processing: Processing
retry: Retry
size: Size
status: Status
upload: Upload a file
title:
episode_image: Cover Image
feed_image: Thumbnail Image
Expand Down
4 changes: 2 additions & 2 deletions test/models/concerns/image_file_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,8 @@
refute image.tap { |i| i.status = "processing" }.retryable?
refute image.tap { |i| i.status = "complete" }.retryable?

# updated 1 minute ago
image.updated_at = Time.now - 60
# updated 2 minutes ago
image.updated_at = Time.now - 120
assert image.tap { |i| i.status = "started" }.retryable?
assert image.tap { |i| i.status = "processing" }.retryable?
refute image.tap { |i| i.status = "complete" }.retryable?
Expand Down
4 changes: 2 additions & 2 deletions test/models/media_resource_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@
refute mr.tap { |i| i.status = "processing" }.retryable?
refute mr.tap { |i| i.status = "complete" }.retryable?

# updated 1 minute ago
mr.updated_at = Time.now - 60
# updated 2 minutes ago
mr.updated_at = Time.now - 120
assert mr.tap { |i| i.status = "started" }.retryable?
assert mr.tap { |i| i.status = "processing" }.retryable?
refute mr.tap { |i| i.status = "complete" }.retryable?
Expand Down

0 comments on commit b5b5c7d

Please sign in to comment.