Add endpoint for accessing variant screenshots #233
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
smudge
commented
Nov 13, 2024
| @@ -25,6 +25,7 @@ | |||
| /.powenv | |||
| /.env.local | |||
| /db/seed_apps.yml | |||
| /db/seeds/*.yml | |||
There was a problem hiding this comment.
I added a step to seeds.rb that will conditionally read from any Rails fixture files placed in this folder.
smudge
commented
Nov 13, 2024
| @@ -1,6 +1,8 @@ | |||
| Rails.application.configure do | |||
| # Settings specified here will take precedence over those in config/application.rb. | |||
|
|
|||
| config.hosts << "testtrack.test" | |||
There was a problem hiding this comment.
A conventional hostname for local dev. (I used puma-dev to link the DNS to the running server.)
smudge
commented
Nov 13, 2024
Comment on lines
+27
to
+30
| Admin.find_or_create_by!(email: '[email protected]') do |user| | ||
| user.password = 'password' | ||
| user.password_confirmation = 'password' | ||
| end |
There was a problem hiding this comment.
Seed the development DB with an admin user.
smudge
commented
Nov 13, 2024
| @@ -19,7 +19,7 @@ def storage_settings | |||
| end | |||
|
|
|||
| def max_size | |||
| ENV['ATTACHMENT_MAX_SIZE'] || 512.kilobytes | |||
| ENV['ATTACHMENT_MAX_SIZE']&.to_i || 1.megabyte | |||
There was a problem hiding this comment.
Increasing the default to 1 megabyte (screen resolutions have gotten a bit bigger since 2017!)
Also casting the max size value to an int, since it would otherwise come in as a string.
There was a problem hiding this comment.
is 1mb a reasonable size for gifs? any cost to making this higher?
smudge
commented
Nov 15, 2024
Comment on lines
+12
to
+16
| require 'active_record/fixtures' | ||
| fixtures_dir = File.join(Rails.root, 'db/seeds') | ||
| fixture_files = Dir.glob('db/seeds/*.yml').map { |f| File.basename(f, '.yml') } | ||
|
|
||
| ActiveRecord::FixtureSet.create_fixtures(fixtures_dir, fixture_files) |
jonmauney
reviewed
Nov 18, 2024
| variant_detail = split.variant_details.find_by!(variant: variant) | ||
| raise ActiveRecord::RecordNotFound unless variant_detail.screenshot_file_name? | ||
|
|
||
| redirect_to variant_detail.screenshot.expiring_url(300) |
jonmauney
reviewed
Nov 18, 2024
|
|
||
| describe 'GET /variant.ext' do | ||
| it 'returns a 404' do | ||
| get "/admin/splits/#{split.id}/screenshots/hammer_time.jpg" |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This adds a new unauthenticated endpoint for accessing variant screenshots:
A split's UUID must be known in order to construct this path, and when requested it will 302 redirect to a signed, expiring S3 URL (via paperclip's
expiring_urlmethod).This this also adds a column to the splits show page that links out to the screenshot:
Other Information
A made a few other changes to improve the local dev experience, which I'll comment on below.