diff --git a/app/controllers/api/v1/recordings_controller.rb b/app/controllers/api/v1/recordings_controller.rb index ece4ba6b07a217ff9a6c460f95979ab0d0bdc9d1..7255133ce4e992d8266cacd70465f8ab1077ef59 100644 --- a/app/controllers/api/v1/recordings_controller.rb +++ b/app/controllers/api/v1/recordings_controller.rb @@ -65,17 +65,16 @@ module Api # POST /api/v1/recordings/update_visibility.json # Update's a recordings visibility by setting publish/unpublish and protected/unprotected def update_visibility - new_visibility = params[:visibility] - old_visibility = @recording.visibility - bbb_api = BigBlueButtonApi.new(provider: current_provider) + new_visibility = params[:visibility].to_s - bbb_api.publish_recordings(record_ids: @recording.record_id, publish: true) if old_visibility == 'Unpublished' + new_visibility_params = visibility_params_of(new_visibility) - bbb_api.publish_recordings(record_ids: @recording.record_id, publish: false) if new_visibility == 'Unpublished' + return render_error status: :bad_request if new_visibility_params.nil? - bbb_api.update_recordings(record_id: @recording.record_id, meta_hash: { protect: true }) if new_visibility == 'Protected' + bbb_api = BigBlueButtonApi.new(provider: current_provider) - bbb_api.update_recordings(record_id: @recording.record_id, meta_hash: { protect: false }) if old_visibility == 'Protected' + bbb_api.publish_recordings(record_ids: @recording.record_id, publish: new_visibility_params[:publish]) + bbb_api.update_recordings(record_id: @recording.record_id, meta_hash: new_visibility_params[:meta_hash]) @recording.update!(visibility: new_visibility) @@ -114,6 +113,18 @@ module Api def find_recording @recording = Recording.find_by! record_id: params[:id] end + + def visibility_params_of(visibility) + params_of = { + Recording::VISIBILITIES[:unpublished] => { publish: false, meta_hash: { protect: false, 'meta_gl-listed': false } }, + Recording::VISIBILITIES[:published] => { publish: true, meta_hash: { protect: false, 'meta_gl-listed': false } }, + Recording::VISIBILITIES[:public] => { publish: true, meta_hash: { protect: false, 'meta_gl-listed': true } }, + Recording::VISIBILITIES[:protected] => { publish: true, meta_hash: { protect: true, 'meta_gl-listed': false } }, + Recording::VISIBILITIES[:public_protected] => { publish: true, meta_hash: { protect: true, 'meta_gl-listed': true } } + } + + params_of[visibility.to_s] + end end end end diff --git a/app/models/recording.rb b/app/models/recording.rb index 2de63200889efe03e8841ca61993f4b5f93c197b..41bde789850e95a614d9f27838b3471365210261 100644 --- a/app/models/recording.rb +++ b/app/models/recording.rb @@ -17,6 +17,14 @@ # frozen_string_literal: true class Recording < ApplicationRecord + VISIBILITIES = { + published: 'Published', + unpublished: 'Unpublished', + protected: 'Protected', + public: 'Public', + public_protected: 'Public/Protected' + }.freeze + belongs_to :room has_one :user, through: :room has_many :formats, dependent: :destroy @@ -26,6 +34,7 @@ class Recording < ApplicationRecord validates :visibility, presence: true validates :length, presence: true validates :participants, presence: true + validates :visibility, inclusion: VISIBILITIES.values scope :with_provider, ->(current_provider) { where(user: { provider: current_provider }) } diff --git a/app/services/recording_creator.rb b/app/services/recording_creator.rb index b3cf2bb974c4f18a8a434e137224f7982ff4a0f4..c13cbe56d67ae990e0c093ada31601fa4f8f9f2c 100644 --- a/app/services/recording_creator.rb +++ b/app/services/recording_creator.rb @@ -49,11 +49,11 @@ class RecordingCreator # Returns the visibility of the recording (published, unpublished or protected) def get_recording_visibility(recording:) - return 'Protected' if recording[:protected].to_s == 'true' + return Recording::VISIBILITIES[:protected] if recording[:protected].to_s == 'true' - return 'Published' if recording[:published].to_s == 'true' + return Recording::VISIBILITIES[:published] if recording[:published].to_s == 'true' - 'Unpublished' + Recording::VISIBILITIES[:unpublished] end # Returns the length of presentation recording for the recording given diff --git a/spec/controllers/recordings_controller_spec.rb b/spec/controllers/recordings_controller_spec.rb index 687d53300924472a519829b8fdf58ec217b98a85..a3f4c3830fbf0f8fb6727f6543e6cfca4bc55256 100644 --- a/spec/controllers/recordings_controller_spec.rb +++ b/spec/controllers/recordings_controller_spec.rb @@ -194,66 +194,88 @@ RSpec.describe Api::V1::RecordingsController, type: :controller do describe '#update_visibility' do let(:room) { create(:room, user:) } - let(:published_recording) { create(:recording, room:, visibility: 'Published') } - let(:unpublished_recording) { create(:recording, room:, visibility: 'Unpublished') } - let(:protected_recording) { create(:recording, room:, visibility: 'Protected') } + let(:recording) { create(:recording, room:) } - it 'changes the recording from Published to Unpublished on the bbb server' do - expect_any_instance_of(BigBlueButtonApi).to receive(:publish_recordings).with(record_ids: published_recording.record_id, publish: false) - expect_any_instance_of(BigBlueButtonApi).not_to receive(:update_recordings) - post :update_visibility, params: { visibility: 'Unpublished', id: published_recording.record_id } + def expect_to_update_recording_props_to(publish:, protect:, list:, visibility:) + expect_any_instance_of(BigBlueButtonApi).to receive(:publish_recordings).with(record_ids: recording.record_id, publish:) + expect_any_instance_of(BigBlueButtonApi).to receive(:update_recordings).with(record_id: recording.record_id, + meta_hash: { + protect:, 'meta_gl-listed': list + }) + + post :update_visibility, params: { visibility:, id: recording.record_id } + + expect(recording.reload.visibility).to eq(visibility) + expect(response).to have_http_status(:ok) end - it 'changes the recording from Published to Protected on the bbb server' do - expect_any_instance_of(BigBlueButtonApi).to receive(:update_recordings).with(record_id: published_recording.record_id, - meta_hash: { protect: true }) - expect_any_instance_of(BigBlueButtonApi).not_to receive(:publish_recordings) - post :update_visibility, params: { visibility: 'Protected', id: published_recording.record_id } + it 'changes the recording visibility to "Published"' do + expect_to_update_recording_props_to(publish: true, protect: false, list: false, visibility: Recording::VISIBILITIES[:published]) end - it 'changes the recording from Unpublished to Protected on the bbb server' do - expect_any_instance_of(BigBlueButtonApi).to receive(:publish_recordings).with(record_ids: unpublished_recording.record_id, publish: true) - expect_any_instance_of(BigBlueButtonApi).to receive(:update_recordings).with(record_id: unpublished_recording.record_id, - meta_hash: { protect: true }) - post :update_visibility, params: { visibility: 'Protected', id: unpublished_recording.record_id } + it 'changes the recording visibility to "Unpublished"' do + expect_to_update_recording_props_to(publish: false, protect: false, list: false, visibility: Recording::VISIBILITIES[:unpublished]) end - it 'changes the recording from Unpublished to Published on the bbb server' do - expect_any_instance_of(BigBlueButtonApi).to receive(:publish_recordings).with(record_ids: unpublished_recording.record_id, publish: true) - expect_any_instance_of(BigBlueButtonApi).not_to receive(:update_recordings) - post :update_visibility, params: { visibility: 'Published', id: unpublished_recording.record_id } + it 'changes the recording visibility to "Protected"' do + expect_to_update_recording_props_to(publish: true, protect: true, list: false, visibility: Recording::VISIBILITIES[:protected]) end - it 'changes the recording from Protected to Published on the bbb server' do - expect_any_instance_of(BigBlueButtonApi).not_to receive(:publish_recordings) - expect_any_instance_of(BigBlueButtonApi).to receive(:update_recordings).with(record_id: protected_recording.record_id, - meta_hash: { protect: false }) - post :update_visibility, params: { visibility: 'Published', id: protected_recording.record_id } + it 'changes the recording visibility to "Public"' do + expect_to_update_recording_props_to(publish: true, protect: false, list: true, visibility: Recording::VISIBILITIES[:public]) end - it 'changes the recording from Protected to Unpublished on the bbb server' do - expect_any_instance_of(BigBlueButtonApi).to receive(:publish_recordings).with(record_ids: protected_recording.record_id, publish: false) - expect_any_instance_of(BigBlueButtonApi).to receive(:update_recordings).with(record_id: protected_recording.record_id, - meta_hash: { protect: false }) - post :update_visibility, params: { visibility: 'Unpublished', id: protected_recording.record_id } + it 'changes the recording visibility to "Public/Protected"' do + expect_to_update_recording_props_to(publish: true, protect: true, list: true, visibility: Recording::VISIBILITIES[:public_protected]) end - it 'changes the local recording visibility' do - allow_any_instance_of(BigBlueButtonApi).to receive(:publish_recordings) - post :update_visibility, params: { visibility: 'Unpublished', id: unpublished_recording.record_id } - expect(unpublished_recording.reload.visibility).to eq('Unpublished') + context 'Unkown visibility' do + it 'returns :bad_request and does not update the recording' do + expect_any_instance_of(BigBlueButtonApi).not_to receive(:publish_recordings) + expect_any_instance_of(BigBlueButtonApi).not_to receive(:update_recordings) + + expect do + post :update_visibility, params: { visibility: '404', id: recording.record_id } + end.not_to(change { recording.reload.visibility }) + + expect(response).to have_http_status(:bad_request) + end end - it 'allows a shared user to update a recording visibility' do - shared_user = create(:user) - create(:shared_access, user_id: shared_user.id, room_id: room.id) - sign_in_user(shared_user) + context 'shared access' do + let(:signed_in_user) { create(:user) } + let(:recording) { create(:recording, room:, visibility: Recording::VISIBILITIES[:published]) } - expect_any_instance_of(BigBlueButtonApi).to receive(:publish_recordings).with(record_ids: published_recording.record_id, publish: false) - expect_any_instance_of(BigBlueButtonApi).not_to receive(:update_recordings) - expect do - post :update_visibility, params: { visibility: 'Unpublished', id: published_recording.record_id } - end.to(change { published_recording.reload.visibility }) + before do + sign_in_user(signed_in_user) + end + + it 'allows a shared user to update a recording visibility' do + create(:shared_access, user_id: signed_in_user.id, room_id: room.id) + + expect_any_instance_of(BigBlueButtonApi).to receive(:publish_recordings).with(record_ids: recording.record_id, publish: false) + expect_any_instance_of(BigBlueButtonApi).to receive(:update_recordings).with(record_id: recording.record_id, + meta_hash: { + protect: false, 'meta_gl-listed': false + }) + + expect do + post :update_visibility, params: { visibility: Recording::VISIBILITIES[:unpublished], id: recording.record_id } + end.to(change { recording.reload.visibility }) + + expect(response).to have_http_status(:ok) + end + + it 'disallows a none shared user to update a recording visibility' do + expect_any_instance_of(BigBlueButtonApi).not_to receive(:publish_recordings) + expect_any_instance_of(BigBlueButtonApi).not_to receive(:update_recordings) + + expect do + post :update_visibility, params: { visibility: Recording::VISIBILITIES[:unpublished], id: recording.record_id } + end.not_to(change { recording.reload.visibility }) + + expect(response).to have_http_status(:forbidden) + end end # TODO: samuel - add tests for user_with_manage_recordings_permission diff --git a/spec/factories/recording_factory.rb b/spec/factories/recording_factory.rb index 7c1655bea2753715d2fd3a8dddf9d1c04ec4cc10..c90e7668cc41606b01cbe264c22fa077551dcf5e 100644 --- a/spec/factories/recording_factory.rb +++ b/spec/factories/recording_factory.rb @@ -21,7 +21,7 @@ FactoryBot.define do room name { Faker::Educator.course_name } record_id { Faker::Internet.uuid } - visibility { 'Unpublished' } + visibility { Recording::VISIBILITIES[:unpublished] } length { Faker::Number.within(range: 1..60) } participants { Faker::Number.within(range: 1..100) } recorded_at { Faker::Time.between(from: 2.days.ago, to: Time.zone.now) } diff --git a/spec/models/recording_spec.rb b/spec/models/recording_spec.rb index 7d074b7de0d6e1f1a8d793aa48bf9b591df5908f..952362f3b465690a5ff8c85f1b1aec0fc6fe3403 100644 --- a/spec/models/recording_spec.rb +++ b/spec/models/recording_spec.rb @@ -19,6 +19,20 @@ require 'rails_helper' RSpec.describe Recording, type: :model do + describe 'Constants' do + context 'VISIBILITES' do + it 'matches certain map' do + expect(Recording::VISIBILITIES).to eq({ + published: 'Published', + unpublished: 'Unpublished', + protected: 'Protected', + public: 'Public', + public_protected: 'Public/Protected' + }) + end + end + end + describe 'validations' do subject { create(:recording) } @@ -29,6 +43,7 @@ RSpec.describe Recording, type: :model do it { is_expected.to validate_presence_of(:visibility) } it { is_expected.to validate_presence_of(:length) } it { is_expected.to validate_presence_of(:participants) } + it { is_expected.to validate_inclusion_of(:visibility).in_array(Recording::VISIBILITIES.values) } end describe 'scopes' do