From 73b48c5a5b839e91c586237574c1dde10f6563f3 Mon Sep 17 00:00:00 2001
From: Bilal <bilal@saburly.com>
Date: Tue, 25 Aug 2020 00:21:42 +0300
Subject: [PATCH 1/4] allow user to star/unstar broadcast recordings

---
 .../broadcast_recordings_controller.rb        | 11 ++-
 app/policies/broadcast_recording_policy.rb    |  6 +-
 .../_refresh_recordings_list.js.erb           |  6 ++
 app/views/broadcast_recordings/destroy.js.erb |  7 +-
 app/views/broadcast_recordings/update.js.erb  |  1 +
 .../broadcasts/_broadcast_recordings.html.erb |  3 +
 config/routes.rb                              |  2 +-
 ...171649_add_star_to_broadcast_recordings.rb |  5 ++
 db/structure.sql                              | 20 +----
 .../broadcast_recordings_controller_spec.rb   | 39 ++++++++-
 spec/factories/broadcast_recordings.rb        |  4 +
 .../features/user_managing_broadcasts_spec.rb | 85 +++++++++++++++++++
 12 files changed, 160 insertions(+), 29 deletions(-)
 create mode 100644 app/views/broadcast_recordings/_refresh_recordings_list.js.erb
 create mode 100644 app/views/broadcast_recordings/update.js.erb
 create mode 100644 db/migrate/20200824171649_add_star_to_broadcast_recordings.rb

diff --git a/app/controllers/broadcast_recordings_controller.rb b/app/controllers/broadcast_recordings_controller.rb
index 477edda..fd76560 100644
--- a/app/controllers/broadcast_recordings_controller.rb
+++ b/app/controllers/broadcast_recordings_controller.rb
@@ -5,9 +5,14 @@ class BroadcastRecordingsController < ApplicationController
   before_action :set_broadcast
   before_action :set_recording
 
+  def update
+    @recording.update(star: !@recording.star)
+    set_recordings
+  end
+
   def destroy
     @recording.update(hidden: true)
-    @recordings = @broadcast.broadcast_recordings.visible.order_by_recent.paginate(page: params[:page])
+    set_recordings
   end
 
   private
@@ -23,4 +28,8 @@ class BroadcastRecordingsController < ApplicationController
   def set_recording
     @recording = authorize policy_scope(@broadcast.broadcast_recordings).find(params[:id])
   end
+
+  def set_recordings
+    @recordings = @broadcast.broadcast_recordings.visible.order_by_recent.paginate(page: params[:page])
+  end
 end
diff --git a/app/policies/broadcast_recording_policy.rb b/app/policies/broadcast_recording_policy.rb
index 7423f2d..5b23a29 100644
--- a/app/policies/broadcast_recording_policy.rb
+++ b/app/policies/broadcast_recording_policy.rb
@@ -3,7 +3,11 @@ class BroadcastRecordingPolicy < ApplicationPolicy
     if user.nil? || user.user.nil?
       return false
     end
-    
+
     user.manager? || user.account_manager?
   end
+
+  def update?
+    destroy?
+  end
 end
diff --git a/app/views/broadcast_recordings/_refresh_recordings_list.js.erb b/app/views/broadcast_recordings/_refresh_recordings_list.js.erb
new file mode 100644
index 0000000..f1da0b9
--- /dev/null
+++ b/app/views/broadcast_recordings/_refresh_recordings_list.js.erb
@@ -0,0 +1,6 @@
+var dom_id = "<%= dom_id(@recording) %>"
+$('[data-id="' + dom_id + '"]').remove();
+<% if @recordings.empty? %>
+$("#broadcast_recordings_nav").append('<p class="dropdown-item text-muted">Recordings will appear here</p>')
+<% end %>
+$("#broadcast_recordings").html("<%= j render(partial: 'broadcasts/broadcast_recordings', locals: { recordings: @recordings, broadcast: @broadcast }) %>");
\ No newline at end of file
diff --git a/app/views/broadcast_recordings/destroy.js.erb b/app/views/broadcast_recordings/destroy.js.erb
index 0359114..de2425d 100644
--- a/app/views/broadcast_recordings/destroy.js.erb
+++ b/app/views/broadcast_recordings/destroy.js.erb
@@ -1,6 +1 @@
-var dom_id = "<%= dom_id(@recording) %>"
-$('[data-id="' + dom_id + '"]').remove();
-<% if @recordings.empty? %>
-  $("#broadcast_recordings_nav").append('<p class="dropdown-item text-muted">Recordings will appear here</p>')
-<% end %>
-$("#broadcast_recordings").html("<%= j render(partial: 'broadcasts/broadcast_recordings', locals: { recordings: @recordings, broadcast: @broadcast }) %>");
\ No newline at end of file
+<%= render("broadcast_recordings/refresh_recordings_list") %>
\ No newline at end of file
diff --git a/app/views/broadcast_recordings/update.js.erb b/app/views/broadcast_recordings/update.js.erb
new file mode 100644
index 0000000..de2425d
--- /dev/null
+++ b/app/views/broadcast_recordings/update.js.erb
@@ -0,0 +1 @@
+<%= render("broadcast_recordings/refresh_recordings_list") %>
\ No newline at end of file
diff --git a/app/views/broadcasts/_broadcast_recordings.html.erb b/app/views/broadcasts/_broadcast_recordings.html.erb
index 73f6f91..52e6483 100644
--- a/app/views/broadcasts/_broadcast_recordings.html.erb
+++ b/app/views/broadcasts/_broadcast_recordings.html.erb
@@ -7,6 +7,9 @@
         <% if (controller.class.module_parent.to_s != "Public" && policy(BroadcastRecording).destroy?) %>
           <%= link_to "Hide", [broadcast.project, broadcast, recording], class: "btn-sm btn-primary ml-1 text-decoration-none", remote: true, method: :delete, data: { confirm: t('.confirm_hide') } %>
         <% end %>
+        <% if (controller.class.module_parent.to_s != "Public" && policy(BroadcastRecording).destroy?) %>
+          <%= link_to fa_icon("#{recording.star ? 'star' : 'star-o'} fw"), [broadcast.project, broadcast, recording], class: "text-warning", method: :put, remote: true %>
+      <% end %>
       </li>
     <% end %>
   </ul>
diff --git a/config/routes.rb b/config/routes.rb
index 79bb52d..6b853d7 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -101,7 +101,7 @@ Rails.application.routes.draw do
           delete :destroy_file
         end
         resource :zoom_meeting, only: [:show]
-        resources :broadcast_recordings, only: :destroy
+        resources :broadcast_recordings, only: [:destroy, :update]
       end
       resources :directories, except: [:index] do
         member do
diff --git a/db/migrate/20200824171649_add_star_to_broadcast_recordings.rb b/db/migrate/20200824171649_add_star_to_broadcast_recordings.rb
new file mode 100644
index 0000000..a8b115d
--- /dev/null
+++ b/db/migrate/20200824171649_add_star_to_broadcast_recordings.rb
@@ -0,0 +1,5 @@
+class AddStarToBroadcastRecordings < ActiveRecord::Migration[6.0]
+  def change
+    add_column :broadcast_recordings, :star, :boolean
+  end
+end
diff --git a/db/structure.sql b/db/structure.sql
index 6fa0d3d..28a72af 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -9,20 +9,6 @@ SET xmloption = content;
 SET client_min_messages = warning;
 SET row_security = off;
 
---
--- Name: plpgsql; Type: EXTENSION; Schema: -; Owner: -
---
-
-CREATE EXTENSION IF NOT EXISTS plpgsql WITH SCHEMA pg_catalog;
-
-
---
--- Name: EXTENSION plpgsql; Type: COMMENT; Schema: -; Owner: -
---
-
-COMMENT ON EXTENSION plpgsql IS 'PL/pgSQL procedural language';
-
-
 --
 -- Name: fuzzystrmatch; Type: EXTENSION; Schema: -; Owner: -
 --
@@ -555,7 +541,8 @@ CREATE TABLE public.broadcast_recordings (
     created_at timestamp(6) without time zone NOT NULL,
     updated_at timestamp(6) without time zone NOT NULL,
     duration double precision,
-    hidden boolean DEFAULT false
+    hidden boolean DEFAULT false,
+    star boolean
 );
 
 
@@ -4036,6 +4023,7 @@ INSERT INTO "schema_migrations" (version) VALUES
 ('20200807190607'),
 ('20200811102720'),
 ('20200812060406'),
-('20200819070738');
+('20200819070738'),
+('20200824171649');
 
 
diff --git a/spec/controllers/broadcast_recordings_controller_spec.rb b/spec/controllers/broadcast_recordings_controller_spec.rb
index 0254a16..397333b 100644
--- a/spec/controllers/broadcast_recordings_controller_spec.rb
+++ b/spec/controllers/broadcast_recordings_controller_spec.rb
@@ -9,16 +9,47 @@ RSpec.describe BroadcastRecordingsController, type: :controller do
 
   before do
     sign_in user
+    stub_mux_live_stream
+  end
+
+  describe "#update" do
+    let(:broadcast) { create(:broadcast, project: project, name: "New Broadcast") }
+    let(:recordings) { create_list(:broadcast_recording, 5, :with_random_asset_uid, broadcast: broadcast) }
+    let(:starred_recordings) { create_list(:broadcast_recording, 5, :with_random_asset_uid, broadcast: broadcast, star: true) }
+
+    it "sets star property to true when recording is starred" do
+      recordings.each do |recording|
+        expect(recording.star).to be_falsey
+      end
+
+      put :update, params: { project_id: project, broadcast_id: broadcast, id: recordings.first.id }, xhr: true
+
+      expect(recordings.first.reload.star).to eq true
+
+      recordings[1..5].each do |recording|
+        expect(recording.reload.star).to be_falsey
+      end
+    end
+
+    it "sets star property to false when recording is unstarred" do
+      starred_recordings.each do |recording|
+        expect(recording.star).to be_truthy
+      end
+
+      put :update, params: { project_id: project, broadcast_id: broadcast, id: starred_recordings.first.id }, xhr: true
+
+      expect(starred_recordings.first.reload.star).to eq false
+
+      starred_recordings[1..5].each do |recording|
+        expect(recording.reload.star).to eq true
+      end
+    end
   end
 
   describe "#destroy" do
     let(:broadcast) { create(:broadcast, project: project, name: "New Broadcast") }
     let(:recording) { create(:broadcast_recording, broadcast: broadcast) }
 
-    before do
-      stub_mux_live_stream
-    end
-
     it "hides the broadcast recording" do
       expect(recording.hidden).to be false
 
diff --git a/spec/factories/broadcast_recordings.rb b/spec/factories/broadcast_recordings.rb
index a30bf80..89c2e3c 100644
--- a/spec/factories/broadcast_recordings.rb
+++ b/spec/factories/broadcast_recordings.rb
@@ -5,5 +5,9 @@ FactoryBot.define do
     asset_uid "asset_uid"
     asset_playback_uid "asset_playback_uid"
     hidden { false }
+
+    trait :with_random_asset_uid do
+      sequence(:asset_uid, 'a')
+    end
   end
 end
diff --git a/spec/features/user_managing_broadcasts_spec.rb b/spec/features/user_managing_broadcasts_spec.rb
index 9c40b71..0707e75 100644
--- a/spec/features/user_managing_broadcasts_spec.rb
+++ b/spec/features/user_managing_broadcasts_spec.rb
@@ -223,6 +223,42 @@ feature 'User managing broadcasts' do
       expect(page).to have_content("Recording of the live stream will appear here")
     end
 
+    scenario 'project manager can star broadcast recordings', js: true do
+      broadcast = create(:broadcast, :with_stream, :with_files, project: project)
+      recording = create(:broadcast_recording, broadcast: broadcast, asset_uid: "another_asset_uid")
+
+      visit project_broadcast_path(project, broadcast)
+
+      click_on 'Previous Sessions'
+
+      expect(page).to have_content(recording.download_file_name)
+      expect(page).to have_css('i.fa-star-o')
+      expect(page).not_to have_css('i.fa-star')
+
+      find('i.fa-star-o').find(:xpath, '..').click
+
+      expect(page).not_to have_css('i.fa-star-o')
+      expect(page).to have_css('i.fa-star')
+    end
+
+    scenario 'project manager can unstar broadcast recordings', js: true do
+      broadcast = create(:broadcast, :with_stream, :with_files, project: project)
+      recording = create(:broadcast_recording, broadcast: broadcast, asset_uid: "another_asset_uid", star: true)
+
+      visit project_broadcast_path(project, broadcast)
+
+      click_on 'Previous Sessions'
+
+      expect(page).to have_content(recording.download_file_name)
+      expect(page).to have_css('i.fa-star')
+      expect(page).not_to have_css('i.fa-star-o')
+
+      find('i.fa-star').find(:xpath, '..').click
+
+      expect(page).not_to have_css('i.fa-star')
+      expect(page).to have_css('i.fa-star-o')
+    end
+
     context 'When the user is associate' do
       let(:current_user) { create(:user, :associate) }
 
@@ -251,6 +287,19 @@ feature 'User managing broadcasts' do
         expect(page).to have_content(recording.download_file_name)
         expect(page).not_to have_content('Hide')
       end
+
+      scenario 'associate does not see broadcast recordings star', js: true do
+        broadcast = create(:broadcast, :with_stream, :with_files, project: project)
+        recording = create(:broadcast_recording, broadcast: broadcast, asset_uid: "another_asset_uid", star: true)
+
+        visit project_broadcast_path(project, broadcast)
+
+        click_on 'Previous Sessions'
+
+        expect(page).to have_content(recording.download_file_name)
+        expect(page).not_to have_css('i.fa-star')
+        expect(page).not_to have_css('i.fa-star-o')
+      end
     end
 
     context 'When the user is account manager' do
@@ -295,6 +344,42 @@ feature 'User managing broadcasts' do
         expect(page).not_to have_content(recording.download_file_name)
         expect(page).to have_content("Recording of the live stream will appear here")
       end
+
+      scenario 'account manager can star broadcast recordings', js: true do
+        broadcast = create(:broadcast, :with_stream, :with_files, project: project)
+        recording = create(:broadcast_recording, broadcast: broadcast, asset_uid: "another_asset_uid")
+
+        visit project_broadcast_path(project, broadcast)
+
+        click_on 'Previous Sessions'
+
+        expect(page).to have_content(recording.download_file_name)
+        expect(page).to have_css('i.fa-star-o')
+        expect(page).not_to have_css('i.fa-star')
+
+        find('i.fa-star-o').find(:xpath, '..').click
+
+        expect(page).not_to have_css('i.fa-star-o')
+        expect(page).to have_css('i.fa-star')
+      end
+
+      scenario 'account manager can unstar broadcast recordings', js: true do
+        broadcast = create(:broadcast, :with_stream, :with_files, project: project)
+        recording = create(:broadcast_recording, broadcast: broadcast, asset_uid: "another_asset_uid", star: true)
+
+        visit project_broadcast_path(project, broadcast)
+
+        click_on 'Previous Sessions'
+
+        expect(page).to have_content(recording.download_file_name)
+        expect(page).to have_css('i.fa-star')
+        expect(page).not_to have_css('i.fa-star-o')
+
+        find('i.fa-star').find(:xpath, '..').click
+
+        expect(page).not_to have_css('i.fa-star')
+        expect(page).to have_css('i.fa-star-o')
+      end
     end
 
   end
-- 
2.47.3


From 4a09406120b7d60d7cc723430af89e3a4e6ce88b Mon Sep 17 00:00:00 2001
From: Bilal <bilal@saburly.com>
Date: Tue, 25 Aug 2020 00:21:42 +0300
Subject: [PATCH 2/4] allow user to star/unstar broadcast recordings

---
 .../broadcast_recordings_controller.rb        | 11 ++-
 app/policies/broadcast_recording_policy.rb    |  6 +-
 .../_refresh_recordings_list.js.erb           |  6 ++
 app/views/broadcast_recordings/destroy.js.erb |  7 +-
 app/views/broadcast_recordings/update.js.erb  |  1 +
 .../broadcasts/_broadcast_recordings.html.erb |  3 +
 config/routes.rb                              |  2 +-
 ...171649_add_star_to_broadcast_recordings.rb |  5 ++
 db/structure.sql                              | 20 +----
 .../broadcast_recordings_controller_spec.rb   | 39 ++++++++-
 spec/factories/broadcast_recordings.rb        |  4 +
 .../features/user_managing_broadcasts_spec.rb | 85 +++++++++++++++++++
 12 files changed, 160 insertions(+), 29 deletions(-)
 create mode 100644 app/views/broadcast_recordings/_refresh_recordings_list.js.erb
 create mode 100644 app/views/broadcast_recordings/update.js.erb
 create mode 100644 db/migrate/20200824171649_add_star_to_broadcast_recordings.rb

diff --git a/app/controllers/broadcast_recordings_controller.rb b/app/controllers/broadcast_recordings_controller.rb
index 477edda..fd76560 100644
--- a/app/controllers/broadcast_recordings_controller.rb
+++ b/app/controllers/broadcast_recordings_controller.rb
@@ -5,9 +5,14 @@ class BroadcastRecordingsController < ApplicationController
   before_action :set_broadcast
   before_action :set_recording
 
+  def update
+    @recording.update(star: !@recording.star)
+    set_recordings
+  end
+
   def destroy
     @recording.update(hidden: true)
-    @recordings = @broadcast.broadcast_recordings.visible.order_by_recent.paginate(page: params[:page])
+    set_recordings
   end
 
   private
@@ -23,4 +28,8 @@ class BroadcastRecordingsController < ApplicationController
   def set_recording
     @recording = authorize policy_scope(@broadcast.broadcast_recordings).find(params[:id])
   end
+
+  def set_recordings
+    @recordings = @broadcast.broadcast_recordings.visible.order_by_recent.paginate(page: params[:page])
+  end
 end
diff --git a/app/policies/broadcast_recording_policy.rb b/app/policies/broadcast_recording_policy.rb
index 7423f2d..5b23a29 100644
--- a/app/policies/broadcast_recording_policy.rb
+++ b/app/policies/broadcast_recording_policy.rb
@@ -3,7 +3,11 @@ class BroadcastRecordingPolicy < ApplicationPolicy
     if user.nil? || user.user.nil?
       return false
     end
-    
+
     user.manager? || user.account_manager?
   end
+
+  def update?
+    destroy?
+  end
 end
diff --git a/app/views/broadcast_recordings/_refresh_recordings_list.js.erb b/app/views/broadcast_recordings/_refresh_recordings_list.js.erb
new file mode 100644
index 0000000..f1da0b9
--- /dev/null
+++ b/app/views/broadcast_recordings/_refresh_recordings_list.js.erb
@@ -0,0 +1,6 @@
+var dom_id = "<%= dom_id(@recording) %>"
+$('[data-id="' + dom_id + '"]').remove();
+<% if @recordings.empty? %>
+$("#broadcast_recordings_nav").append('<p class="dropdown-item text-muted">Recordings will appear here</p>')
+<% end %>
+$("#broadcast_recordings").html("<%= j render(partial: 'broadcasts/broadcast_recordings', locals: { recordings: @recordings, broadcast: @broadcast }) %>");
\ No newline at end of file
diff --git a/app/views/broadcast_recordings/destroy.js.erb b/app/views/broadcast_recordings/destroy.js.erb
index 0359114..de2425d 100644
--- a/app/views/broadcast_recordings/destroy.js.erb
+++ b/app/views/broadcast_recordings/destroy.js.erb
@@ -1,6 +1 @@
-var dom_id = "<%= dom_id(@recording) %>"
-$('[data-id="' + dom_id + '"]').remove();
-<% if @recordings.empty? %>
-  $("#broadcast_recordings_nav").append('<p class="dropdown-item text-muted">Recordings will appear here</p>')
-<% end %>
-$("#broadcast_recordings").html("<%= j render(partial: 'broadcasts/broadcast_recordings', locals: { recordings: @recordings, broadcast: @broadcast }) %>");
\ No newline at end of file
+<%= render("broadcast_recordings/refresh_recordings_list") %>
\ No newline at end of file
diff --git a/app/views/broadcast_recordings/update.js.erb b/app/views/broadcast_recordings/update.js.erb
new file mode 100644
index 0000000..de2425d
--- /dev/null
+++ b/app/views/broadcast_recordings/update.js.erb
@@ -0,0 +1 @@
+<%= render("broadcast_recordings/refresh_recordings_list") %>
\ No newline at end of file
diff --git a/app/views/broadcasts/_broadcast_recordings.html.erb b/app/views/broadcasts/_broadcast_recordings.html.erb
index 73f6f91..52e6483 100644
--- a/app/views/broadcasts/_broadcast_recordings.html.erb
+++ b/app/views/broadcasts/_broadcast_recordings.html.erb
@@ -7,6 +7,9 @@
         <% if (controller.class.module_parent.to_s != "Public" && policy(BroadcastRecording).destroy?) %>
           <%= link_to "Hide", [broadcast.project, broadcast, recording], class: "btn-sm btn-primary ml-1 text-decoration-none", remote: true, method: :delete, data: { confirm: t('.confirm_hide') } %>
         <% end %>
+        <% if (controller.class.module_parent.to_s != "Public" && policy(BroadcastRecording).destroy?) %>
+          <%= link_to fa_icon("#{recording.star ? 'star' : 'star-o'} fw"), [broadcast.project, broadcast, recording], class: "text-warning", method: :put, remote: true %>
+      <% end %>
       </li>
     <% end %>
   </ul>
diff --git a/config/routes.rb b/config/routes.rb
index 79bb52d..6b853d7 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -101,7 +101,7 @@ Rails.application.routes.draw do
           delete :destroy_file
         end
         resource :zoom_meeting, only: [:show]
-        resources :broadcast_recordings, only: :destroy
+        resources :broadcast_recordings, only: [:destroy, :update]
       end
       resources :directories, except: [:index] do
         member do
diff --git a/db/migrate/20200824171649_add_star_to_broadcast_recordings.rb b/db/migrate/20200824171649_add_star_to_broadcast_recordings.rb
new file mode 100644
index 0000000..a8b115d
--- /dev/null
+++ b/db/migrate/20200824171649_add_star_to_broadcast_recordings.rb
@@ -0,0 +1,5 @@
+class AddStarToBroadcastRecordings < ActiveRecord::Migration[6.0]
+  def change
+    add_column :broadcast_recordings, :star, :boolean
+  end
+end
diff --git a/db/structure.sql b/db/structure.sql
index 6fa0d3d..28a72af 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -9,20 +9,6 @@ SET xmloption = content;
 SET client_min_messages = warning;
 SET row_security = off;
 
---
--- Name: plpgsql; Type: EXTENSION; Schema: -; Owner: -
---
-
-CREATE EXTENSION IF NOT EXISTS plpgsql WITH SCHEMA pg_catalog;
-
-
---
--- Name: EXTENSION plpgsql; Type: COMMENT; Schema: -; Owner: -
---
-
-COMMENT ON EXTENSION plpgsql IS 'PL/pgSQL procedural language';
-
-
 --
 -- Name: fuzzystrmatch; Type: EXTENSION; Schema: -; Owner: -
 --
@@ -555,7 +541,8 @@ CREATE TABLE public.broadcast_recordings (
     created_at timestamp(6) without time zone NOT NULL,
     updated_at timestamp(6) without time zone NOT NULL,
     duration double precision,
-    hidden boolean DEFAULT false
+    hidden boolean DEFAULT false,
+    star boolean
 );
 
 
@@ -4036,6 +4023,7 @@ INSERT INTO "schema_migrations" (version) VALUES
 ('20200807190607'),
 ('20200811102720'),
 ('20200812060406'),
-('20200819070738');
+('20200819070738'),
+('20200824171649');
 
 
diff --git a/spec/controllers/broadcast_recordings_controller_spec.rb b/spec/controllers/broadcast_recordings_controller_spec.rb
index 0254a16..397333b 100644
--- a/spec/controllers/broadcast_recordings_controller_spec.rb
+++ b/spec/controllers/broadcast_recordings_controller_spec.rb
@@ -9,16 +9,47 @@ RSpec.describe BroadcastRecordingsController, type: :controller do
 
   before do
     sign_in user
+    stub_mux_live_stream
+  end
+
+  describe "#update" do
+    let(:broadcast) { create(:broadcast, project: project, name: "New Broadcast") }
+    let(:recordings) { create_list(:broadcast_recording, 5, :with_random_asset_uid, broadcast: broadcast) }
+    let(:starred_recordings) { create_list(:broadcast_recording, 5, :with_random_asset_uid, broadcast: broadcast, star: true) }
+
+    it "sets star property to true when recording is starred" do
+      recordings.each do |recording|
+        expect(recording.star).to be_falsey
+      end
+
+      put :update, params: { project_id: project, broadcast_id: broadcast, id: recordings.first.id }, xhr: true
+
+      expect(recordings.first.reload.star).to eq true
+
+      recordings[1..5].each do |recording|
+        expect(recording.reload.star).to be_falsey
+      end
+    end
+
+    it "sets star property to false when recording is unstarred" do
+      starred_recordings.each do |recording|
+        expect(recording.star).to be_truthy
+      end
+
+      put :update, params: { project_id: project, broadcast_id: broadcast, id: starred_recordings.first.id }, xhr: true
+
+      expect(starred_recordings.first.reload.star).to eq false
+
+      starred_recordings[1..5].each do |recording|
+        expect(recording.reload.star).to eq true
+      end
+    end
   end
 
   describe "#destroy" do
     let(:broadcast) { create(:broadcast, project: project, name: "New Broadcast") }
     let(:recording) { create(:broadcast_recording, broadcast: broadcast) }
 
-    before do
-      stub_mux_live_stream
-    end
-
     it "hides the broadcast recording" do
       expect(recording.hidden).to be false
 
diff --git a/spec/factories/broadcast_recordings.rb b/spec/factories/broadcast_recordings.rb
index a30bf80..89c2e3c 100644
--- a/spec/factories/broadcast_recordings.rb
+++ b/spec/factories/broadcast_recordings.rb
@@ -5,5 +5,9 @@ FactoryBot.define do
     asset_uid "asset_uid"
     asset_playback_uid "asset_playback_uid"
     hidden { false }
+
+    trait :with_random_asset_uid do
+      sequence(:asset_uid, 'a')
+    end
   end
 end
diff --git a/spec/features/user_managing_broadcasts_spec.rb b/spec/features/user_managing_broadcasts_spec.rb
index 9c40b71..0707e75 100644
--- a/spec/features/user_managing_broadcasts_spec.rb
+++ b/spec/features/user_managing_broadcasts_spec.rb
@@ -223,6 +223,42 @@ feature 'User managing broadcasts' do
       expect(page).to have_content("Recording of the live stream will appear here")
     end
 
+    scenario 'project manager can star broadcast recordings', js: true do
+      broadcast = create(:broadcast, :with_stream, :with_files, project: project)
+      recording = create(:broadcast_recording, broadcast: broadcast, asset_uid: "another_asset_uid")
+
+      visit project_broadcast_path(project, broadcast)
+
+      click_on 'Previous Sessions'
+
+      expect(page).to have_content(recording.download_file_name)
+      expect(page).to have_css('i.fa-star-o')
+      expect(page).not_to have_css('i.fa-star')
+
+      find('i.fa-star-o').find(:xpath, '..').click
+
+      expect(page).not_to have_css('i.fa-star-o')
+      expect(page).to have_css('i.fa-star')
+    end
+
+    scenario 'project manager can unstar broadcast recordings', js: true do
+      broadcast = create(:broadcast, :with_stream, :with_files, project: project)
+      recording = create(:broadcast_recording, broadcast: broadcast, asset_uid: "another_asset_uid", star: true)
+
+      visit project_broadcast_path(project, broadcast)
+
+      click_on 'Previous Sessions'
+
+      expect(page).to have_content(recording.download_file_name)
+      expect(page).to have_css('i.fa-star')
+      expect(page).not_to have_css('i.fa-star-o')
+
+      find('i.fa-star').find(:xpath, '..').click
+
+      expect(page).not_to have_css('i.fa-star')
+      expect(page).to have_css('i.fa-star-o')
+    end
+
     context 'When the user is associate' do
       let(:current_user) { create(:user, :associate) }
 
@@ -251,6 +287,19 @@ feature 'User managing broadcasts' do
         expect(page).to have_content(recording.download_file_name)
         expect(page).not_to have_content('Hide')
       end
+
+      scenario 'associate does not see broadcast recordings star', js: true do
+        broadcast = create(:broadcast, :with_stream, :with_files, project: project)
+        recording = create(:broadcast_recording, broadcast: broadcast, asset_uid: "another_asset_uid", star: true)
+
+        visit project_broadcast_path(project, broadcast)
+
+        click_on 'Previous Sessions'
+
+        expect(page).to have_content(recording.download_file_name)
+        expect(page).not_to have_css('i.fa-star')
+        expect(page).not_to have_css('i.fa-star-o')
+      end
     end
 
     context 'When the user is account manager' do
@@ -295,6 +344,42 @@ feature 'User managing broadcasts' do
         expect(page).not_to have_content(recording.download_file_name)
         expect(page).to have_content("Recording of the live stream will appear here")
       end
+
+      scenario 'account manager can star broadcast recordings', js: true do
+        broadcast = create(:broadcast, :with_stream, :with_files, project: project)
+        recording = create(:broadcast_recording, broadcast: broadcast, asset_uid: "another_asset_uid")
+
+        visit project_broadcast_path(project, broadcast)
+
+        click_on 'Previous Sessions'
+
+        expect(page).to have_content(recording.download_file_name)
+        expect(page).to have_css('i.fa-star-o')
+        expect(page).not_to have_css('i.fa-star')
+
+        find('i.fa-star-o').find(:xpath, '..').click
+
+        expect(page).not_to have_css('i.fa-star-o')
+        expect(page).to have_css('i.fa-star')
+      end
+
+      scenario 'account manager can unstar broadcast recordings', js: true do
+        broadcast = create(:broadcast, :with_stream, :with_files, project: project)
+        recording = create(:broadcast_recording, broadcast: broadcast, asset_uid: "another_asset_uid", star: true)
+
+        visit project_broadcast_path(project, broadcast)
+
+        click_on 'Previous Sessions'
+
+        expect(page).to have_content(recording.download_file_name)
+        expect(page).to have_css('i.fa-star')
+        expect(page).not_to have_css('i.fa-star-o')
+
+        find('i.fa-star').find(:xpath, '..').click
+
+        expect(page).not_to have_css('i.fa-star')
+        expect(page).to have_css('i.fa-star-o')
+      end
     end
 
   end
-- 
2.47.3


From 70a0c7705458160297145a56039636574a409427 Mon Sep 17 00:00:00 2001
From: Bilal <bilal@saburly.com>
Date: Wed, 26 Aug 2020 14:35:56 +0300
Subject: [PATCH 3/4] fix MR comments

---
 app/controllers/broadcast_recordings_controller.rb            | 2 +-
 app/models/broadcast_recording.rb                             | 4 ++++
 .../broadcast_recordings/_refresh_recordings_list.js.erb      | 3 +--
 app/views/broadcasts/_broadcast_recordings.html.erb           | 4 ++--
 db/migrate/20200824171649_add_star_to_broadcast_recordings.rb | 2 +-
 db/structure.sql                                              | 2 +-
 6 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/app/controllers/broadcast_recordings_controller.rb b/app/controllers/broadcast_recordings_controller.rb
index fd76560..f238e54 100644
--- a/app/controllers/broadcast_recordings_controller.rb
+++ b/app/controllers/broadcast_recordings_controller.rb
@@ -6,7 +6,7 @@ class BroadcastRecordingsController < ApplicationController
   before_action :set_recording
 
   def update
-    @recording.update(star: !@recording.star)
+    @recording.toggle_star
     set_recordings
   end
 
diff --git a/app/models/broadcast_recording.rb b/app/models/broadcast_recording.rb
index a7add7d..cbbe8fd 100644
--- a/app/models/broadcast_recording.rb
+++ b/app/models/broadcast_recording.rb
@@ -18,4 +18,8 @@ class BroadcastRecording < ApplicationRecord
   def download_file_name
     "#{broadcast_name}_Date_#{created_at.in_time_zone(broadcast.shoot_location_time_zone).strftime("%Y-%m-%d")}_Time_#{created_at.in_time_zone(broadcast.shoot_location_time_zone).strftime("%T")}".parameterize
   end
+
+  def toggle_star
+    toggle! :starred
+  end
 end
diff --git a/app/views/broadcast_recordings/_refresh_recordings_list.js.erb b/app/views/broadcast_recordings/_refresh_recordings_list.js.erb
index f1da0b9..ad0463b 100644
--- a/app/views/broadcast_recordings/_refresh_recordings_list.js.erb
+++ b/app/views/broadcast_recordings/_refresh_recordings_list.js.erb
@@ -1,5 +1,4 @@
-var dom_id = "<%= dom_id(@recording) %>"
-$('[data-id="' + dom_id + '"]').remove();
+$('[data-id="<%= dom_id(@recording) %>"]').remove();
 <% if @recordings.empty? %>
 $("#broadcast_recordings_nav").append('<p class="dropdown-item text-muted">Recordings will appear here</p>')
 <% end %>
diff --git a/app/views/broadcasts/_broadcast_recordings.html.erb b/app/views/broadcasts/_broadcast_recordings.html.erb
index 52e6483..cb542b4 100644
--- a/app/views/broadcasts/_broadcast_recordings.html.erb
+++ b/app/views/broadcasts/_broadcast_recordings.html.erb
@@ -7,8 +7,8 @@
         <% if (controller.class.module_parent.to_s != "Public" && policy(BroadcastRecording).destroy?) %>
           <%= link_to "Hide", [broadcast.project, broadcast, recording], class: "btn-sm btn-primary ml-1 text-decoration-none", remote: true, method: :delete, data: { confirm: t('.confirm_hide') } %>
         <% end %>
-        <% if (controller.class.module_parent.to_s != "Public" && policy(BroadcastRecording).destroy?) %>
-          <%= link_to fa_icon("#{recording.star ? 'star' : 'star-o'} fw"), [broadcast.project, broadcast, recording], class: "text-warning", method: :put, remote: true %>
+        <% if (controller.class.module_parent.to_s != "Public" && policy(BroadcastRecording).update?) %>
+          <%= link_to fa_icon("#{recording.starred ? 'star' : 'star-o'} fw"), [broadcast.project, broadcast, recording], class: "text-warning", method: :put, remote: true %>
       <% end %>
       </li>
     <% end %>
diff --git a/db/migrate/20200824171649_add_star_to_broadcast_recordings.rb b/db/migrate/20200824171649_add_star_to_broadcast_recordings.rb
index a8b115d..d5424d5 100644
--- a/db/migrate/20200824171649_add_star_to_broadcast_recordings.rb
+++ b/db/migrate/20200824171649_add_star_to_broadcast_recordings.rb
@@ -1,5 +1,5 @@
 class AddStarToBroadcastRecordings < ActiveRecord::Migration[6.0]
   def change
-    add_column :broadcast_recordings, :star, :boolean
+    add_column :broadcast_recordings, :starred, :boolean, default: false
   end
 end
diff --git a/db/structure.sql b/db/structure.sql
index 28a72af..857b303 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -542,7 +542,7 @@ CREATE TABLE public.broadcast_recordings (
     updated_at timestamp(6) without time zone NOT NULL,
     duration double precision,
     hidden boolean DEFAULT false,
-    star boolean
+    starred boolean DEFAULT false
 );
 
 
-- 
2.47.3


From 99fa8471b2f6b343d45df5097e3460ce89a715c2 Mon Sep 17 00:00:00 2001
From: Bilal <bilal@saburly.com>
Date: Wed, 26 Aug 2020 15:05:34 +0300
Subject: [PATCH 4/4] fix specs

---
 .../broadcast_recordings_controller_spec.rb        | 14 +++++++-------
 spec/features/user_managing_broadcasts_spec.rb     |  6 +++---
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/spec/controllers/broadcast_recordings_controller_spec.rb b/spec/controllers/broadcast_recordings_controller_spec.rb
index 397333b..9dcd655 100644
--- a/spec/controllers/broadcast_recordings_controller_spec.rb
+++ b/spec/controllers/broadcast_recordings_controller_spec.rb
@@ -15,33 +15,33 @@ RSpec.describe BroadcastRecordingsController, type: :controller do
   describe "#update" do
     let(:broadcast) { create(:broadcast, project: project, name: "New Broadcast") }
     let(:recordings) { create_list(:broadcast_recording, 5, :with_random_asset_uid, broadcast: broadcast) }
-    let(:starred_recordings) { create_list(:broadcast_recording, 5, :with_random_asset_uid, broadcast: broadcast, star: true) }
+    let(:starred_recordings) { create_list(:broadcast_recording, 5, :with_random_asset_uid, broadcast: broadcast, starred: true) }
 
     it "sets star property to true when recording is starred" do
       recordings.each do |recording|
-        expect(recording.star).to be_falsey
+        expect(recording.starred).to be_falsey
       end
 
       put :update, params: { project_id: project, broadcast_id: broadcast, id: recordings.first.id }, xhr: true
 
-      expect(recordings.first.reload.star).to eq true
+      expect(recordings.first.reload.starred).to eq true
 
       recordings[1..5].each do |recording|
-        expect(recording.reload.star).to be_falsey
+        expect(recording.reload.starred).to be_falsey
       end
     end
 
     it "sets star property to false when recording is unstarred" do
       starred_recordings.each do |recording|
-        expect(recording.star).to be_truthy
+        expect(recording.starred).to be_truthy
       end
 
       put :update, params: { project_id: project, broadcast_id: broadcast, id: starred_recordings.first.id }, xhr: true
 
-      expect(starred_recordings.first.reload.star).to eq false
+      expect(starred_recordings.first.reload.starred).to eq false
 
       starred_recordings[1..5].each do |recording|
-        expect(recording.reload.star).to eq true
+        expect(recording.reload.starred).to eq true
       end
     end
   end
diff --git a/spec/features/user_managing_broadcasts_spec.rb b/spec/features/user_managing_broadcasts_spec.rb
index 0707e75..d476901 100644
--- a/spec/features/user_managing_broadcasts_spec.rb
+++ b/spec/features/user_managing_broadcasts_spec.rb
@@ -243,7 +243,7 @@ feature 'User managing broadcasts' do
 
     scenario 'project manager can unstar broadcast recordings', js: true do
       broadcast = create(:broadcast, :with_stream, :with_files, project: project)
-      recording = create(:broadcast_recording, broadcast: broadcast, asset_uid: "another_asset_uid", star: true)
+      recording = create(:broadcast_recording, broadcast: broadcast, asset_uid: "another_asset_uid", starred: true)
 
       visit project_broadcast_path(project, broadcast)
 
@@ -290,7 +290,7 @@ feature 'User managing broadcasts' do
 
       scenario 'associate does not see broadcast recordings star', js: true do
         broadcast = create(:broadcast, :with_stream, :with_files, project: project)
-        recording = create(:broadcast_recording, broadcast: broadcast, asset_uid: "another_asset_uid", star: true)
+        recording = create(:broadcast_recording, broadcast: broadcast, asset_uid: "another_asset_uid", starred: true)
 
         visit project_broadcast_path(project, broadcast)
 
@@ -365,7 +365,7 @@ feature 'User managing broadcasts' do
 
       scenario 'account manager can unstar broadcast recordings', js: true do
         broadcast = create(:broadcast, :with_stream, :with_files, project: project)
-        recording = create(:broadcast_recording, broadcast: broadcast, asset_uid: "another_asset_uid", star: true)
+        recording = create(:broadcast_recording, broadcast: broadcast, asset_uid: "another_asset_uid", starred: true)
 
         visit project_broadcast_path(project, broadcast)
 
-- 
2.47.3