fix MR comments

app/controllers/broadcasts_controller.rb

@@ -3,7 +3,7 @@ class BroadcastsController < ApplicationController
 
   before_action :set_project
   before_action :build_broadcast, only: [:new, :create]
-  before_action :set_broadcast, only: [:show, :destroy, :update]
+  before_action :set_broadcast, only: [:show, :destroy, :update, :destroy_file]
   before_action :set_multi_view_broadcasts, only: [:show]
   before_action :show_splash_screen, only: :index
 
@@ -39,10 +39,7 @@ class BroadcastsController < ApplicationController
     end
 
     @broadcast.update(broadcast_params)
-    @files = @broadcast.files.order("created_at DESC").paginate(page: 1)
+    update_files_section
-
-    pagination_content = ApplicationController.render html: helpers.will_paginate(@files, params: { active_tab: params[:active_tab], page: params[:page], active_files_tab: params[:active_files_tab] })
-    BroadcastsChannel.broadcast_file_upload_updates(@broadcast, @files, pagination_content)
   end
 
   def destroy
@@ -53,8 +50,23 @@ class BroadcastsController < ApplicationController
     end
   end
 
+  def destroy_file
+    authorize Broadcast
+    file = ActiveStorage::Attachment.find(params[:file_id])
+    file.destroy
+
+    update_files_section
+  end
+
   private
 
+  def update_files_section
+    @files = @broadcast.files.order("created_at DESC").paginate(page: 1)
+
+    pagination_content = ApplicationController.render html: helpers.will_paginate(@files, params: { active_tab: params[:active_tab], page: params[:page], active_files_tab: params[:active_files_tab] })
+    BroadcastsChannel.broadcast_file_upload_updates(@broadcast, @files, pagination_content)
+  end
+
   def show_splash_screen
     render :splash if broadcasts.count.zero?
   end

app/policies/broadcast_policy.rb

@@ -18,4 +18,12 @@ class BroadcastPolicy < ApplicationPolicy
   def update?
     true
   end
+
+  def destroy_file?
+    if user.nil? || user.user.nil?
+      return false
+    end
+
+    user.manager? || user.account_manager?
+  end
 end

app/views/broadcasts/_file.html.erb

@@ -1,12 +1,25 @@
-<li class="my-2" id="<%= dom_id(file) %>">
+<div class="row">
-  <% if file.variable? %>
+  <% broadcast = file.record %>
-    <%= link_to image_tag(file.variant(resize_and_pad: [300, 300, background: "#F7F8F9"]), class: "bg-light img-thumbnail img-fluid"), file, target: "_blank" %>
+  <% show_delete = controller.class.module_parent.to_s == "Public" ? false : policy(broadcast).destroy_file? %>
-  <% else %>
+  <% file_class = show_delete ? "col-8" : "col-12" %>
-    <div class="border rounded bg-light text-muted d-flex justify-content-center align-items-center fix-h-and-w">
+  <div class="<%= file_class %>">
-      <%= link_to file, target: "_blank" do %>
+    <li class="my-2" id="<%= dom_id(file) %>">
-        <%= fa_icon("file", style: "font-size: 2rem") %>
+      <% if file.variable? %>
-        <div class="mt-2"><%= file.filename %></div>
+        <%= link_to image_tag(file.variant(resize_and_pad: [300, 300, background: "#F7F8F9"]), class: "bg-light img-thumbnail img-fluid"), file, target: "_blank" %>
+      <% else %>
+        <div class="border rounded bg-light text-muted d-flex justify-content-center align-items-center img-fluid fix-h-and-w">
+          <%= link_to file, target: "_blank" do %>
+            <%= fa_icon("file", style: "font-size: 2rem") %>
+            <div class="mt-2"><%= file.filename %></div>
+          <% end %>
+        </div>
       <% end %>
+    </li>
+  </div>
+  <% if show_delete %>
+    <div class="col-4 align-self-center p-0 m-0">
+      <% url = url_for [:destroy_file, broadcast.project, broadcast, { file_id: file.id }] %>
+      <%= link_to fa_icon("trash fw", text: t('.actions.delete_file')), url, class: "btn btn-danger", remote: true, method: :delete, data: { confirm: t('.confirm_delete') } %>
     </div>
   <% end %>
-</li>
+</div>

config/locales/en.yml

@@ -214,6 +214,10 @@ en:
     destroy:
       alert: A live stream has been deleted
       api_error: Something went wrong, please try again later after some time
+    file:
+      actions:
+        delete_file: Delete
+      confirm_delete: Are you sure?
     index:
       actions:
         new: Create New Live Stream

config/locales/es.yml

@@ -81,6 +81,10 @@ es:
       do_not_copy_warning: "Do not copy (ES)"
       serial_number_label: "Serial Number (ES)"
   broadcasts:
+    file:
+      actions:
+        delete_file: Delete
+      confirm_delete: Are you sure? (ES)
     show:
       actions:
         reset_url: Reset URL (ES)

config/routes.rb

@@ -96,6 +96,9 @@ Rails.application.routes.draw do
     end
     resources :projects, only: [] do
       resources :broadcasts, except: [:edit] do
+        member do
+          delete :destroy_file
+        end
         resource :zoom_meeting, only: [:show]
       end
       resources :directories, except: [:index] do

spec/controllers/api/broadcasts_controller_spec.rb

@@ -106,7 +106,7 @@ RSpec.describe Api::BroadcastsController, type: :controller do
       included = JSON.parse(response.body).dig('included')
 
       expect(relationships.keys).to include('files')
-      expect(included.size).to eq 1
+      expect(included.size).to eq 3
       expect(included.first.dig("id")).to eq broadcast.files.first.id.to_s
       expect(included.first.dig("type")).to eq 'active_storage_attachment'
     end

spec/controllers/public/broadcasts_controller_spec.rb

@@ -87,6 +87,7 @@ RSpec.describe Public::BroadcastsController, type: :controller do
     let!(:broadcast) { create(:broadcast, :with_stream, skip_create_callback: true, project: project ) }
   
     it "uploads files to broadcast" do
+      allow(BroadcastsChannel).to receive(:broadcast_file_upload_updates)
       patch :update, params: { token: broadcast.token, broadcast: file_params }, xhr: true
 
       expect(broadcast.files.count).to eq(1)

spec/factories/broadcasts.rb

@@ -17,7 +17,13 @@ FactoryBot.define do
     end
 
     trait :with_files do
-      files { [Rack::Test::UploadedFile.new('spec/fixtures/files/contract.pdf', 'application/pdf')] }
+      files do
+        [
+            Rack::Test::UploadedFile.new('spec/fixtures/files/contract.pdf', 'application/pdf'),
+            Rack::Test::UploadedFile.new('spec/fixtures/files/audio.mp3', 'audio/mpeg'),
+            Rack::Test::UploadedFile.new('spec/fixtures/files/video_file.mp4', 'video/mp4')
+        ]
+      end
     end
 
     after(:build) do |broadcast, evaluator|

spec/features/user_managing_broadcasts_spec.rb

@@ -164,6 +164,21 @@ feature 'User managing broadcasts' do
       click_on add_file_button
     end
 
+    scenario 'manager user can click delete button next to the file and delete file', js: true do
+      broadcast = create(:broadcast, :with_stream, :with_files, project: project)
+
+      visit project_broadcast_path(project, broadcast)
+
+      expect(page).to have_content delete_file_button, count: 3
+
+      accept_alert do
+        first('a', text: delete_file_button).click
+      end
+
+      expect(page).to have_content delete_file_button, count: 2
+      expect(Broadcast.find(broadcast.id).files.count).to eq 2
+    end
+
     scenario 'visit multi-view broadcast page', js: true do
       broadcast_one = create(:broadcast, :with_stream, :with_files, name: 'Broadcast 1', project: project)
       broadcast_two = create(:broadcast, :with_stream, :with_files, name: 'Broadcast 2', project: project)
@@ -198,6 +213,14 @@ feature 'User managing broadcasts' do
         expect(page).to have_content schedule_demo
         expect(page).not_to have_content create_stream
       end
+
+      scenario 'associate user does not see delete button next to the file', js: true do
+        broadcast = create(:broadcast, :with_stream, :with_files, project: project)
+
+        visit project_broadcast_path(project, broadcast)
+
+        expect(page).to have_content delete_file_button, count: 0
+      end
     end
 
     context 'When the user is account manager' do
@@ -209,6 +232,21 @@ feature 'User managing broadcasts' do
         expect(page).to have_content schedule_demo
         expect(page).to have_content create_stream
       end
+
+      scenario 'account manager user can click delete button next to the file and delete file', js: true do
+        broadcast = create(:broadcast, :with_stream, :with_files, project: project)
+
+        visit project_broadcast_path(project, broadcast)
+
+        expect(page).to have_content delete_file_button, count: 3
+
+        accept_alert do
+          first('a', text: delete_file_button).click
+        end
+
+        expect(page).to have_content delete_file_button, count: 2
+        expect(Broadcast.find(broadcast.id).files.count).to eq 2
+      end
     end
 
   end
@@ -262,5 +300,9 @@ feature 'User managing broadcasts' do
     'Live stream is waiting to begin'
   end
 
+  def delete_file_button
+    t 'broadcasts.file.actions.delete_file'
+  end
+
 
 end