Chore: Make media its own model (#940)

* chore: Setup new Medium model and table This does *not* migrate data just yet, and is intended to record the model creation and associations without being in use yet. * db: Move Story Media migration * chore: switch active storage validator The new gem has more robust features, including that the attachment can be processable and custom proc validations. It's also a 1.0+ release rather than 0.X. From what I can determine, the old validation syntax is still compatible with this one, so no other changes are necessary for the swap. * Enable ActiveStorage 6.1 defaults * Story references new Medium model * refactor: extract media rendering into shared partial This way our media rendering stays consistent * Ensure routes to create, update, and delete Story Media still work * Ensure all API endpoints and usages of Medium are updated * Add file limit size on Media attachments * chore: update all file uploads to have restricted file sizes * Add irregular inflection for singular Media --------- Co-authored-by: Laura Mosher <[email protected]>
Terrastories · Oct 2, 2023 · 4e03e99 · 4e03e99
1 parent 3771942
commit 4e03e99
Show file tree

Hide file tree

Showing 29 changed files with 348 additions and 110 deletions.
diff --git a/rails/Gemfile b/rails/Gemfile
@@ -49,7 +49,7 @@ gem 'devise-i18n', '~> 1.11.0'
 gem 'aws-sdk-s3', '~> 1.119.0'
 
 # ActiveStorage Validation
-gem 'activestorage-validator', '~> 0.2.0'
+gem 'active_storage_validations', '~> 1.0'
 
 # Enable Webpack for javascript application code
 gem 'webpacker', '~> 5.0'

diff --git a/rails/Gemfile.lock b/rails/Gemfile.lock
@@ -39,6 +39,11 @@ GEM
       erubi (~> 1.4)
       rails-dom-testing (~> 2.0)
       rails-html-sanitizer (~> 1.1, >= 1.2.0)
+    active_storage_validations (1.0.4)
+      activejob (>= 5.2.0)
+      activemodel (>= 5.2.0)
+      activestorage (>= 5.2.0)
+      activesupport (>= 5.2.0)
     activejob (6.1.7.4)
       activesupport (= 6.1.7.4)
       globalid (>= 0.3.6)
@@ -54,8 +59,6 @@ GEM
       activesupport (= 6.1.7.4)
       marcel (~> 1.0)
       mini_mime (>= 1.1.0)
-    activestorage-validator (0.2.2)
-      rails (>= 6.0.4.1)
     activesupport (6.1.7.4)
       concurrent-ruby (~> 1.0, >= 1.0.2)
       i18n (>= 1.6, < 2)
@@ -339,7 +342,7 @@ PLATFORMS
   ruby
 
 DEPENDENCIES
-  activestorage-validator (~> 0.2.0)
+  active_storage_validations (~> 1.0)
   annotate
   aws-sdk-s3 (~> 1.119.0)
   better_errors

diff --git a/rails/app/controllers/dashboard/stories_controller.rb b/rails/app/controllers/dashboard/stories_controller.rb
@@ -21,9 +21,12 @@ def new
 
     def create
       authorize Story
-      @story = community_stories.new(story_params)
+      @story = community_stories.new(story_params.except(:media))
 
       if @story.save
+        story_params[:media].each do |media|
+          m = @story.media.create(media: media)
+        end
         redirect_to @story
       else
         render :new
@@ -41,7 +44,11 @@ def edit
     def update
       @story = authorize community_stories.find(params[:id])
 
-      if @story.update(story_params)
+      if @story.update(story_params.except(:media))
+        story_params[:media].each do |media|
+          m = @story.media.create(media: media)
+        end
+
         redirect_to @story
       else
         render :edit
@@ -59,8 +66,8 @@ def destroy
     def delete_media
       @story = authorize community_stories.find(params[:story_id])
 
-      media_blob = @story.media.blobs.find_signed(params[:id])
-      media_blob.attachments.each(&:purge)
+      media = @story.media.find(params[:id])
+      media.destroy
 
       head :ok
     end

diff --git a/rails/app/models/community.rb b/rails/app/models/community.rb
@@ -14,8 +14,8 @@ class Community < ApplicationRecord
 
   accepts_nested_attributes_for :users, limit: 1
 
-  validates :background_img, blob: { content_type: ['image/png', 'image/jpg', 'image/jpeg'] }
-  validates :sponsor_logos, blob: { content_type: ['image/png', 'image/jpg', 'image/jpeg'], size_range: 1..5.megabytes }
+  validates :background_img, content_type: [:png, :jpeg, 'image/jpg'], size: { less_than_or_equal_to: 5.megabytes }
+  validates :sponsor_logos, content_type: [:png, :jpeg, 'image/jpg'], size: { less_than_or_equal_to: 5.megabytes }
 
   validates :slug, presence: true, if: -> { self.public? }
 

diff --git a/rails/app/models/concerns/importable.rb b/rails/app/models/concerns/importable.rb
@@ -104,6 +104,13 @@ def parse
       end
     end
 
+    def attach_files(k, attachment, filename)
+      path = filename.dup.insert(0, IMPORT_PATH)
+      if filename && File.exist?(path)
+        k.send(attachment).attach(io: File.open(path), filename: filename)
+      end
+    end
+
     def save_importable_records
       importable_rows.each do |row|
         # with_indifferent_access to allow key access as strings or symbols
@@ -118,8 +125,17 @@ def save_importable_records
           hash[k] = value
         end
 
+        # Story Media Association
+        # This must be after attachment attributes to ensure files are correctly
+        # matched when "media" is used for attachment key.
+        if attributes["media"].present?
+          story_media = attributes.delete("media")
+        end
+
         # Find or create has_many* relationships
         @klass.associated_attribute_names.each do |association|
+          next if association == :media
+
           values = attributes[association].to_s.split(",")
           attributes[association] = values.map do |name|
             association.to_s.singularize.classify.constantize.find_or_create_by(name: name, community_id: @community_id)
@@ -143,10 +159,15 @@ def save_importable_records
         if record.save
           media.each do |attachment, filenames|
             filenames.split(",").each do |filename|
-              path = filename.dup.insert(0, IMPORT_PATH)
-              if filename && File.exist?(path)
-                record.send(attachment).attach(io: File.open(path), filename: filename)
-              end
+              attach_files(record, attachment, filename)
+            end
+          end
+
+          if story_media
+            story_media.split(",").each do |filename|
+              sm = record.media.new
+              attach_files(sm, "media", filename)
+              sm.save if sm.media.attached?
             end
           end
         else

diff --git a/rails/app/models/media.rb b/rails/app/models/media.rb
@@ -0,0 +1,42 @@
+class Media < ApplicationRecord
+  self.table_name = "media"
+
+  belongs_to :story
+
+  has_one_attached :media do
+    def blob_id
+      blob.id
+    end
+  end
+
+  validates :media,
+    attached: true,
+    # Symbol Content Types must map in Marcel::EXTENSIONS
+    # otherwise, use string for full mime type.
+    content_type: [
+      # image types
+      :png, :jpeg, :svg, 'image/jpg',
+      # video types
+      :mpeg, :mp4, :mov, :webm,
+      # audio types
+      :mp3, :aac, :flac, :mp4a, :wav,
+      'audio/wav', 'audio/m4a', 'audio/x-m4a', 'audio/x-aac', 'audio/x-flac',
+    ],
+    size: { less_than_or_equal_to: 200.megabytes }
+
+  delegate :content_type, :blob_id, :blob, to: :media
+end
+
+# == Schema Information
+#
+# Table name: media
+#
+#  id         :bigint           not null, primary key
+#  created_at :datetime         not null
+#  updated_at :datetime         not null
+#  story_id   :bigint
+#
+# Indexes
+#
+#  index_media_on_story_id  (story_id)
+#
diff --git a/rails/app/models/place.rb b/rails/app/models/place.rb
@@ -6,8 +6,13 @@ class Place < ApplicationRecord
   has_one_attached :photo
   has_one_attached :name_audio
   validates :name, presence: true
-  validates :photo, blob: { content_type: ['image/png', 'image/jpg', 'image/jpeg'] }
-  validates :name_audio, blob: { content_type: ['audio/mpeg', 'audio/wav', 'audio/mp4', 'audio/m4a', 'audio/x-m4a', 'audio/x-aac', 'audio/x-flac'] }
+  validates :photo, content_type: [:png, 'image/jpg', :jpeg], size: { less_than_or_equal_to: 5.megabytes }
+  validates :name_audio,
+    content_type: [
+      :mp3, :aac, :flac, :mp4a, :wav,
+      'audio/wav', 'audio/m4a', 'audio/x-m4a', 'audio/x-aac', 'audio/x-flac',
+    ],
+    size: { less_than_or_equal_to: 10.megabytes }
   validates :lat, numericality: { greater_than_or_equal_to:  -90, less_than_or_equal_to:  90, message: :invalid_latitude }, allow_blank: true
   validates :long, numericality: { greater_than_or_equal_to: -180, less_than_or_equal_to: 180, message: :invalid_longitude}, allow_blank: true
   has_many :interview_stories, class_name: "Story", foreign_key: "interview_location_id"

diff --git a/rails/app/models/speaker.rb b/rails/app/models/speaker.rb
@@ -9,6 +9,8 @@ class Speaker < ApplicationRecord
 
   validates :name, presence: true
 
+  validates :photo, content_type: [:png, :jpeg, "image/jpg"], size: { less_than_or_equal_to: 5.megabytes }
+
   # photo
   def picture_url
     if photo.attached?

diff --git a/rails/app/models/story.rb b/rails/app/models/story.rb
@@ -3,7 +3,9 @@ class Story < ApplicationRecord
 
   has_many :speaker_stories, inverse_of: :story
   has_many :speakers, through: :speaker_stories
-  has_many_attached :media
+
+  has_many :media, inverse_of: :story
+
   has_and_belongs_to_many :places
   belongs_to :community, touch: true
   belongs_to :interview_location, class_name: "Place", foreign_key: "interview_location_id", optional: true
@@ -13,10 +15,15 @@ class Story < ApplicationRecord
   validates :title, presence: true
   validates :speaker_ids, presence: true
   validates :place_ids, presence: true
-  validates :media, blob: { content_type: ['image/png', 'image/jpg', 'image/jpeg', 'video/mpeg', 'video/mp4', 'video/quicktime', 'video/webm', 'audio/mpeg', 'audio/wav', 'audio/mp4', 'audio/m4a', 'audio/x-m4a', 'audio/x-aac', 'audio/x-flac'] }
+
+  accepts_nested_attributes_for :media
 
   def media_types
-    media.flat_map { |media| media.content_type.split('/')[0] }.uniq
+    media.flat_map do |m|
+      registry, kind = m.content_type.split('/')
+
+      registry == "application" ? kind : registry
+    end.uniq
   end
 
   def self.export_sample_csv
@@ -53,9 +60,7 @@ def public_points
 
   EXCLUDE_ATTRIBUTES_FROM_IMPORT = %i[
     speaker_stories
-    media_attachments
     media_links
-    media_blobs
   ]
 end
 

diff --git a/rails/app/models/user.rb b/rails/app/models/user.rb
@@ -7,6 +7,7 @@ class User < ApplicationRecord
   belongs_to :community, optional: true, touch: true
   has_one_attached :photo
 
+  validates :photo, content_type: [:png, :jpeg, "image/jpg"], size: { less_than_or_equal_to: 5.megabytes }
   # Username is required for logging in with Devise. Email is optional.
   # Remove email_required? override if username changes to optional.
   validates :username, uniqueness: true, presence: true, format: {without: /\s/, message: :invalid_username_format}

diff --git a/rails/app/views/api/stories/show.json.jbuilder b/rails/app/views/api/stories/show.json.jbuilder
@@ -1,8 +1,8 @@
 json.(@story, :id, :title, :desc, :topic, :language)
 json.media @story.media do |media|
-  json.contentType media.blob.content_type
-  json.blob media.blob.id
-  json.url rails_blob_url(media)
+  json.contentType media.content_type
+  json.blob media.blob_id
+  json.url rails_blob_url(media.media)
 end
 json.speakers @story.speakers do |speaker|
   json.(speaker, :id, :name)

diff --git a/rails/app/views/dashboard/stories/_form.html.erb b/rails/app/views/dashboard/stories/_form.html.erb
@@ -21,50 +21,12 @@
 
   <% if @story.persisted? %>
   <div class="with-media-list">
-    <% if @story.media.attached? %>
+    <% if @story.media.any? %>
       <% @story.media.each do |media| %>
-        <% if media.image? %>
-          <span class="media-with-controls">
-            <% if media.variable? %>
-              <%= image_tag(media.variant(resize_to_limit: [150, 150])) %>
-            <% else %>
-              <%= image_tag(media, width: 150) %>
-            <% end %>
-            <%= link_to t("dashboard.actions.destroy"), story_delete_media_path(@story, media.signed_id), class: "delete-link", method: :delete, data: {confirm: t("dashboard.actions.confirm"), turbo_confirm: t("dashboard.actions.confirm"), turbo_method: :delete}, remote: true %>
-          </span>
-        <% elsif media.video? %>
-          <span class="media-with-controls">
-            <video
-              controls
-              disablePictureInPicture
-              controlsList="nodownload"
-              id="video-player-<%= media.blob.id %>"
-              poster="<%= url_for(media.preview(resize_to_limit:[150, 150])) if media.previewable? %>"
-            >
-              <source src="<%= url_for(media) %>"/>
-              <%= t("video_unsupported") %>
-            </video>
-            <%= link_to t("dashboard.actions.destroy"), story_delete_media_path(@story, media.signed_id), class: "delete-link", method: :delete, data: {confirm: t("dashboard.actions.confirm"), turbo_confirm: t("dashboard.actions.confirm"), turbo_method: :delete}, remote: true %>
-          </span>
-        <% elsif media.audio? %>
-          <span class="media-with-controls">
-            <audio id="audio-player-<%= media.blob.id %>"
-              controls
-              controlsList="nodownload"
-              ref="audio"
-            >
-              <source src="<%= url_for(media) %>" type="<%= media.blob.content_type %>" />
-            </audio>
-            <%= link_to t("dashboard.actions.destroy"), story_delete_media_path(@story, media.signed_id), class: "delete-link", method: :delete, data: {confirm: t("dashboard.actions.confirm"), turbo_confirm: t("dashboard.actions.confirm"), turbo_method: :delete}, remote: true %>
-          </span>
-        <% elsif media.previewable? %>
-          <span class="media-with-controls">
-            <%= image_tag(media.preview(resize_to_limit:[150, 150])) %>
-            <%= link_to t("dashboard.actions.destroy"), story_delete_media_path(@story, media.signed_id), class: "delete-link", method: :delete, data: {confirm: t("dashboard.actions.confirm"), turbo_confirm: t("dashboard.actions.confirm"), turbo_method: :delete}, remote: true %>
-        <% else %>
-          <%= media.filename %>
-          <%= link_to t("dashboard.actions.destroy"), story_delete_media_path(@story, media.signed_id), class: "delete-link", method: :delete, data: {confirm: t("dashboard.actions.confirm"), turbo_confirm: t("dashboard.actions.confirm"), turbo_method: :delete}, remote: true %>
-        <% end %>
+        <span class="media-with-controls">
+          <%= render partial: "media", locals: {media: media.media, size: 150} %>
+          <%= link_to t("dashboard.actions.destroy"), story_delete_media_path(@story, media.id), class: "delete-link", method: :delete, data: {confirm: t("dashboard.actions.confirm"), turbo_confirm: t("dashboard.actions.confirm"), turbo_method: :delete}, remote: true %>
+        </span>
       <% end %>
     <% end %>
   </div>

diff --git a/rails/app/views/dashboard/stories/_media.html.erb b/rails/app/views/dashboard/stories/_media.html.erb
@@ -0,0 +1,30 @@
+<% if media.image? %>
+  <% if media.variable? %>
+    <%= image_tag(media.variant(resize_to_limit: [size, size])) %>
+  <% else %>
+    <%= image_tag(media, width: size) %>
+  <% end %>
+<% elsif media.video? %>
+  <video
+    controls
+    disablePictureInPicture
+    controlsList="nodownload"
+    id="video-player-<%= media.blob.id %>"
+    poster="<%= url_for(media.preview(resize_to_limit:[nil, size])) if media.previewable? %>"
+  >
+    <source src="<%= url_for(media) %>"/>
+    <%= t("video_unsupported") %>
+  </video>
+<% elsif media.audio? %>
+  <audio id="audio-player-<%= media.blob.id %>"
+    controls
+    controlsList="nodownload"
+    ref="audio"
+  >
+    <source src="<%= url_for(media) %>" type="<%= media.blob.content_type %>" />
+  </audio>
+<% elsif media.previewable? %>
+  <%= image_tag(media.preview(resize_to_limit:[size, size])) %>
+<% else %>
+  <%= media.filename %>
+<% end %>
diff --git a/rails/app/views/dashboard/stories/show.html.erb b/rails/app/views/dashboard/stories/show.html.erb
@@ -87,36 +87,7 @@
 
     <% @story.media.each do |media| %>
       <p class="media">
-        <% if media.image? %>
-          <% if media.variable? %>
-            <%= image_tag(media.variant(resize_to_limit: [400, 400])) %>
-          <% else %>
-            <%= image_tag(media, width: 400) %>
-          <% end %>
-        <% elsif media.video? %>
-          <video
-            controls
-            disablePictureInPicture
-            controlsList="nodownload"
-            id="video-player-<%= media.blob.id %>"
-            poster="<%= url_for(media.preview(resize_to_limit:[400, 400])) if media.previewable? %>"
-          >
-            <source src="<%= url_for(media) %>"/>
-            <%= t("video_unsupported") %>
-          </video>
-        <% elsif media.audio? %>
-          <audio id="audio-player-<%= media.blob.id %>"
-            controls
-            controlsList="nodownload"
-            ref="audio"
-          >
-            <source src="<%= url_for(media) %>" type="<%= media.blob.content_type %>" />
-          </audio>
-        <% elsif media.previewable? %>
-            <%= image_tag(media.preview(resize_to_limit:[150, 150])) %>
-        <% else %>
-          <%= media.filename %>
-        <% end %>
+        <%= render partial: "media", locals: {media: media.media, size: 400} %>
       </p>
     <% end %>
   </div>