From ab70d15eaf2cab0e61108657c11f5636eb9e7326 Mon Sep 17 00:00:00 2001 From: Seth Call Date: Mon, 30 Dec 2013 18:34:15 +0000 Subject: [PATCH] * the form VRFS-922, VRFS-918 (mix checks user permissions), VRFS-945 (verify download permissions for tracks) --- db/manifest | 1 + db/up/discardable_recorded_tracks.sql | 5 + ruby/lib/jam_ruby/models/claimed_recording.rb | 9 +- ruby/lib/jam_ruby/models/recorded_track.rb | 6 + ruby/lib/jam_ruby/models/recording.rb | 55 ++----- .../jam_ruby/models/claimed_recording_spec.rb | 102 ++++++++++++ ruby/spec/jam_ruby/models/mix_spec.rb | 18 ++ .../jam_ruby/models/recorded_track_spec.rb | 12 ++ ruby/spec/jam_ruby/models/recording_spec.rb | 46 ++++-- web/Gemfile | 2 +- web/app/assets/javascripts/genreSelector.js | 1 + web/app/assets/javascripts/jam_rest.js | 26 +++ .../javascripts/recordingFinishedDialog.js | 155 ++++++++++++++++++ web/app/assets/javascripts/recordingModel.js | 5 + web/app/assets/javascripts/sessionModel.js | 8 + web/app/assets/javascripts/utils.js | 2 +- web/app/assets/stylesheets/client/client.css | 1 + .../client/recordingFinishedDialog.css.scss | 12 ++ .../controllers/api_recordings_controller.rb | 30 ++-- web/app/responders/api_responder.rb | 1 - .../clients/_recordingFinishedDialog.html.erb | 118 +++++++------ web/app/views/clients/index.html.erb | 2 + web/config/routes.rb | 1 + .../controllers/recordings_controller_spec.rb | 10 ++ web/spec/features/recordings_spec.rb | 117 +++++++++++-- web/spec/spec_helper.rb | 5 +- web/spec/support/utilities.rb | 2 +- 27 files changed, 606 insertions(+), 146 deletions(-) create mode 100644 db/up/discardable_recorded_tracks.sql create mode 100644 ruby/spec/jam_ruby/models/claimed_recording_spec.rb create mode 100644 web/app/assets/javascripts/recordingFinishedDialog.js create mode 100644 web/app/assets/stylesheets/client/recordingFinishedDialog.css.scss diff --git a/db/manifest b/db/manifest index 6ddf7a6aa..672f89c10 100755 --- a/db/manifest +++ b/db/manifest @@ -81,3 +81,4 @@ notification_band_invite.sql band_photo_filepicker.sql bands_geocoding.sql store_s3_filenames.sql +discardable_recorded_tracks.sql diff --git a/db/up/discardable_recorded_tracks.sql b/db/up/discardable_recorded_tracks.sql new file mode 100644 index 000000000..8e30f6528 --- /dev/null +++ b/db/up/discardable_recorded_tracks.sql @@ -0,0 +1,5 @@ +-- there are no valid recordings and mixes at this time +DELETE FROM recorded_tracks; +DELETE FROM mixes; + +ALTER TABLE recorded_tracks ADD COLUMN discard BOOLEAN DEFAULT FALSE NOT NULL; \ No newline at end of file diff --git a/ruby/lib/jam_ruby/models/claimed_recording.rb b/ruby/lib/jam_ruby/models/claimed_recording.rb index aa8d0764d..bfe3d934a 100644 --- a/ruby/lib/jam_ruby/models/claimed_recording.rb +++ b/ruby/lib/jam_ruby/models/claimed_recording.rb @@ -1,8 +1,13 @@ module JamRuby class ClaimedRecording < ActiveRecord::Base - validates :name, no_profanity: true - validates :description, no_profanity: true + validates :name, no_profanity: true, length: {minimum: 3, maximum: 64}, presence: true + validates :description, no_profanity: true, length: {maximum: 8000} + validates :is_public, :inclusion => {:in => [true, false]} + validates :is_downloadable, :inclusion => {:in => [true, false]} + validates :genre, presence: true + validates_uniqueness_of :recording_id, :scope => :user_id + belongs_to :recording, :class_name => "JamRuby::Recording", :inverse_of => :claimed_recordings belongs_to :user, :class_name => "JamRuby::User", :inverse_of => :claimed_recordings diff --git a/ruby/lib/jam_ruby/models/recorded_track.rb b/ruby/lib/jam_ruby/models/recorded_track.rb index 8b22e86a2..f56bbd335 100644 --- a/ruby/lib/jam_ruby/models/recorded_track.rb +++ b/ruby/lib/jam_ruby/models/recorded_track.rb @@ -8,6 +8,8 @@ module JamRuby self.table_name = "recorded_tracks" self.primary_key = 'id' + attr_accessible :discard + SOUND = %w(mono stereo) MAX_PART_FAILURES = 3 MAX_UPLOAD_FAILURES = 10 @@ -28,6 +30,10 @@ module JamRuby validate :validate_too_many_upload_failures + def can_download?(some_user) + !ClaimedRecording.find_by_user_id_and_recording_id(some_user.id, recording.id).nil? + end + def upload_starting? next_part_to_upload_was == 0 && next_part_to_upload == 1 end diff --git a/ruby/lib/jam_ruby/models/recording.rb b/ruby/lib/jam_ruby/models/recording.rb index 18b2054de..46714fd1c 100644 --- a/ruby/lib/jam_ruby/models/recording.rb +++ b/ruby/lib/jam_ruby/models/recording.rb @@ -28,6 +28,17 @@ module JamRuby end end + def recorded_tracks_for_user(user) + unless self.users.exists?(user) + raise PermissionError, "user was not in this session" + end + RecordedTrack.where(:recording_id=> self.id).where(:user_id=> user.id) + end + + def has_access?(user) + return users.exists?(user) + end + # Start recording a session. def self.start(music_session, owner) recording = nil @@ -75,22 +86,11 @@ module JamRuby # Called when a user wants to "claim" a recording. To do this, the user must have been one of the tracks in the recording. def claim(user, name, description, genre, is_public, is_downloadable) - # if self.users.include?(user) - # raise PermissionError, "user already claimed this recording" - # end unless self.users.exists?(user) raise PermissionError, "user was not in this session" end - if self.music_session.is_recording? - raise PermissionError, "recording cannot be claimed while it is being recorded" - end - - if name.nil? || genre.nil? || is_public.nil? || is_downloadable.nil? - raise PermissionError, "recording must have name, genre and flags" - end - claimed_recording = ClaimedRecording.new claimed_recording.user = user claimed_recording.recording = self @@ -117,41 +117,22 @@ module JamRuby self.destroy end - # Returns the list of files the user needs to upload. This will only ever be recordings - def self.upload_file_list(user) - files = [] - User.joins(:recordings).joins(:recordings => :recorded_tracks) - .where(%Q{ recordings.duration IS NOT NULL }) - .where("recorded_tracks.user_id = '#{user.id}'") - .where(%Q{ recorded_tracks.fully_uploaded = FALSE }).each do |user| - user.recordings.each.do |recording| - recording.recorded_tracks.each do |recorded_track| - files.push( - { - :type => "recorded_track", - :id => recorded_track.client_track_id, - :url => recorded_track.url # FIXME IS THIS RIGHT? - } - ) - end - end - files - end - def self.list_downloads(user, limit = 100, since = 0) since = 0 unless since || since == '' # guard against nil downloads = [] # That second join is important. It's saying join off of recordings, NOT user. If you take out the # ":recordings =>" part, you'll just get the recorded_tracks that I played. Very different! - User.joins(:recordings).joins(:recordings => :recorded_tracks) + + # we also only allow you to be told about downloads if you have claimed the recording + User.joins(:recordings).joins(:recordings => :recorded_tracks).joins(:recordings => :claimed_recordings) .order(%Q{ recorded_tracks.id }) .where(%Q{ recorded_tracks.fully_uploaded = TRUE }) .where('recorded_tracks.id > ?', since) + .where('claimed_recordings.user_id = ?', user) .where(:id => user.id).limit(limit).each do |theuser| theuser.recordings.each do |recording| recording.recorded_tracks.each do |recorded_track| - # recorded_track = user.claimed_recordings.first.recording.recorded_tracks.first downloads.push( { :type => "recorded_track", @@ -159,7 +140,7 @@ module JamRuby :recording_id => recording.id, :length => recorded_track.length, :md5 => recorded_track.md5, - :url => recorded_track.filename, + :url => recorded_track.url, :next => recorded_track.id } ) @@ -169,11 +150,11 @@ module JamRuby latest_recorded_track = downloads[-1][:next] if downloads.length > 0 - # HOW TO LIMIT ON USER User.joins(:recordings).joins(:recordings => :mixes) .order('mixes.id') .where('mixes.completed_at IS NOT NULL') .where('mixes.id > ?', since) + .where(:id => user.id) .limit(limit).each do |theuser| theuser.recordings.each do |recording| recording.mixes.each do |mix| @@ -184,7 +165,7 @@ module JamRuby :recording_id => recording.id, :length => mix.length, :md5 => mix.md5, - :url => mix.filename, + :url => mix.url, :created_at => mix.created_at, :next => mix.id } diff --git a/ruby/spec/jam_ruby/models/claimed_recording_spec.rb b/ruby/spec/jam_ruby/models/claimed_recording_spec.rb new file mode 100644 index 000000000..dec39796a --- /dev/null +++ b/ruby/spec/jam_ruby/models/claimed_recording_spec.rb @@ -0,0 +1,102 @@ +require 'spec_helper' + +def valid_claimed_recording + @claimed_recording.name = "hello" + @claimed_recording.description = "description" + @claimed_recording.genre = Genre.first + @claimed_recording.is_public = true + @claimed_recording.is_downloadable = true + @claimed_recording +end + +describe ClaimedRecording do + + before(:each) do + @user = FactoryGirl.create(:user) + @connection = FactoryGirl.create(:connection, :user => @user) + @instrument = FactoryGirl.create(:instrument, :description => 'a great instrument') + @track = FactoryGirl.create(:track, :connection => @connection, :instrument => @instrument) + @music_session = FactoryGirl.create(:music_session, :creator => @user, :musician_access => true) + @music_session.connections << @connection + @music_session.save + @recording = Recording.start(@music_session, @user) + @recording.stop + @recording.reload + @genre = FactoryGirl.create(:genre) + @recording.claim(@user, "name", "description", @genre, true, true) + @recording.reload + @claimed_recording = @recording.claimed_recordings.first + end + + describe "sucessful save" do + it "with default case" do + valid_claimed_recording + @claimed_recording.save.should be_true + end + end + + describe "update name" do + it "with nil" do + valid_claimed_recording + @claimed_recording.name = nil + @claimed_recording.save.should be_false + @claimed_recording.errors[:name].length.should == 2 + @claimed_recording.errors[:name].select { |value| value.include?("can't be blank") }.length.should == 1 + @claimed_recording.errors[:name].select { |value| value.include?("is too short") }.length.should == 1 + end + + it "too short" do + valid_claimed_recording + @claimed_recording.name = "a" + @claimed_recording.save.should be_false + @claimed_recording.errors[:name].length.should == 1 + @claimed_recording.errors[:name].select { |value| value.include?("is too short") }.length.should == 1 + end + end + + describe "update description" do + it "with nil" do + valid_claimed_recording + @claimed_recording.description = nil + @claimed_recording.save.should be_true + end + end + + describe "update is_public" do + it "with nil" do + valid_claimed_recording + @claimed_recording.is_public = nil + @claimed_recording.save.should be_false + @claimed_recording.errors[:is_public].length.should == 1 + @claimed_recording.errors[:is_public].should == ["is not included in the list"] + end + end + + describe "update is_downloadable" do + it "with nil" do + valid_claimed_recording + @claimed_recording.is_downloadable = nil + @claimed_recording.save.should be_false + @claimed_recording.errors[:is_downloadable].length.should == 1 + @claimed_recording.errors[:is_downloadable].should == ["is not included in the list"] + end + end + + describe "update genre" do + it "with nil" do + valid_claimed_recording + @claimed_recording.genre = nil + @claimed_recording.save.should be_false + @claimed_recording.errors[:genre].length.should == 1 + @claimed_recording.errors[:genre].should == ["can't be blank"] + end + end + + describe "multiple claims" do + it "not valid" do + duplicate = @recording.claim(@user, "name", "description", @genre, true, true) + duplicate.valid?.should be_false + duplicate.errors[:recording_id].should == ['has already been taken'] + end + end +end \ No newline at end of file diff --git a/ruby/spec/jam_ruby/models/mix_spec.rb b/ruby/spec/jam_ruby/models/mix_spec.rb index c0d58e398..12f549bd6 100755 --- a/ruby/spec/jam_ruby/models/mix_spec.rb +++ b/ruby/spec/jam_ruby/models/mix_spec.rb @@ -11,6 +11,7 @@ describe Mix do @music_session.save @recording = Recording.start(@music_session, @user) @recording.stop + @recording.claim(@user, "name", "description", Genre.first, true, true) @mix = Mix.schedule(@recording, "{}") end @@ -62,6 +63,23 @@ describe Mix do @mix.sign_url.should_not be_nil end + it "mixes are restricted by user" do + + @mix.finish(1, "abc") + @mix.reload + @mix.errors.any?.should be_false + + @user2 = FactoryGirl.create(:user) + + recordings = Recording.list_downloads(@user)["downloads"] + recordings.length.should == 1 + recordings[0][:type].should == "mix" + recordings[0][:id].should == @mix.id + + recordings = Recording.list_downloads(@user2)["downloads"] + recordings.length.should == 0 + end + end diff --git a/ruby/spec/jam_ruby/models/recorded_track_spec.rb b/ruby/spec/jam_ruby/models/recorded_track_spec.rb index 8a0e5ae12..e77ae5e92 100644 --- a/ruby/spec/jam_ruby/models/recorded_track_spec.rb +++ b/ruby/spec/jam_ruby/models/recorded_track_spec.rb @@ -51,6 +51,18 @@ describe RecordedTrack do @recorded_track.sign_url.should_not be_nil end + it "can not be downloaded if no claimed recording" do + user2 = FactoryGirl.create(:user) + @recorded_track = RecordedTrack.create_from_track(@track, @recording) + @recorded_track.can_download?(user2).should be_false + @recorded_track.can_download?(@user).should be_false + end + + it "can be downloaded if there is a claimed recording" do + @recorded_track = RecordedTrack.create_from_track(@track, @recording) + @recording.claim(@user, "my recording", "my description", Genre.first, true, true).errors.any?.should be_false + @recorded_track.can_download?(@user).should be_true + end describe "aws-based operations", :aws => true do diff --git a/ruby/spec/jam_ruby/models/recording_spec.rb b/ruby/spec/jam_ruby/models/recording_spec.rb index b77c72492..7300d0637 100644 --- a/ruby/spec/jam_ruby/models/recording_spec.rb +++ b/ruby/spec/jam_ruby/models/recording_spec.rb @@ -8,7 +8,26 @@ describe Recording do @music_session = FactoryGirl.create(:music_session, :creator => @user, :musician_access => true) @connection = FactoryGirl.create(:connection, :user => @user, :music_session => @music_session) @track = FactoryGirl.create(:track, :connection => @connection, :instrument => @instrument) - end + end + + + it "should allow finding of recorded tracks" do + user2 = FactoryGirl.create(:user) + connection2 = FactoryGirl.create(:connection, :user => user2, :music_session => @music_session) + track2 = FactoryGirl.create(:track, :connection => connection2, :instrument => @instrument) + + @recording = Recording.start(@music_session, @user) + user1_recorded_tracks = @recording.recorded_tracks_for_user(@user) + user1_recorded_tracks[0].should == @user.recorded_tracks[0] + user1_recorded_tracks.length.should == 1 + user2_recorded_tracks = @recording.recorded_tracks_for_user(user2) + user2_recorded_tracks.length.should == 1 + user2_recorded_tracks[0].should == user2.recorded_tracks[0] + + RecordedTrack.update(user1_recorded_tracks, :discard => true) + user1_recorded_tracks[0].reload + user1_recorded_tracks[0].discard.should be_true + end it "should set up the recording properly when recording is started with 1 user in the session" do @music_session.is_recording?.should be_false @@ -118,16 +137,6 @@ describe Recording do expect { @recording.claim(user2, "name", "description", @genre, true, true) }.to raise_error end - it "should fail if a user tries to claim a recording twice" do - @recording = Recording.start(@music_session, @user) - @recording.stop - @recording.reload - @genre = FactoryGirl.create(:genre) - @recording.claim(@user, "name", "description", @genre, true, true) - @recording.reload - expect { @recording.claim(@user, "name", "description", @genre, true, true) }.to raise_error - end - it "should allow editing metadata for claimed recordings" do @recording = Recording.start(@music_session, @user) @recording.stop @@ -206,6 +215,20 @@ describe Recording do Recording.list_uploads(@user, 10, uploads["next"])["uploads"].length.should == 0 end + it "should return a download only if claimed" do + @recording = Recording.start(@music_session, @user) + @recording.stop + @recording.reload + @genre = FactoryGirl.create(:genre) + @recording.claim(@user, "Recording", "Recording Description", @genre, true, true) + downloads = Recording.list_downloads(@user) + downloads["downloads"].length.should == 0 + @recorded_track = RecordedTrack.where(:recording_id => @recording.id)[0] + @recorded_track.update_attribute(:fully_uploaded, true) + downloads = Recording.list_downloads(@user) + downloads["downloads"].length.should == 1 + end + it "should return a file list for a user properly" do pending stub_const("APP_CONFIG", app_config) @@ -292,6 +315,7 @@ describe Recording do timeline.last["timestamp"].should == @recording.duration timeline.last["end"].should == true end + end diff --git a/web/Gemfile b/web/Gemfile index 0f5f3066e..2b441b842 100644 --- a/web/Gemfile +++ b/web/Gemfile @@ -91,7 +91,7 @@ if ENV['JAMWEB_QT5'] == '1' else gem "capybara-webkit" end - gem 'capybara-screenshot' + gem 'capybara-screenshot', :path => '/Users/seth/workspace/capybara-screenshot' gem 'cucumber-rails', :require => false #, '1.3.0', :require => false gem 'guard-spork', '0.3.2' gem 'spork', '0.9.0' diff --git a/web/app/assets/javascripts/genreSelector.js b/web/app/assets/javascripts/genreSelector.js index 0c75e8575..7022cee5a 100644 --- a/web/app/assets/javascripts/genreSelector.js +++ b/web/app/assets/javascripts/genreSelector.js @@ -65,6 +65,7 @@ $.each(genreList, function(index, value) { values.push(value.toLowerCase()); }); + console.log("OH HAI O") var selectedVal = $('select', parentSelector).val(values); } diff --git a/web/app/assets/javascripts/jam_rest.js b/web/app/assets/javascripts/jam_rest.js index f0ad75ebc..08faf7794 100644 --- a/web/app/assets/javascripts/jam_rest.js +++ b/web/app/assets/javascripts/jam_rest.js @@ -507,6 +507,30 @@ }) } + function claimRecording(options) { + var recordingId = options["id"]; + + return $.ajax({ + type: "POST", + dataType: "json", + contentType: 'application/json', + url: "/api/recordings/" + recordingId + "/claim", + data: JSON.stringify(options) + }) + } + + function discardRecording(options) { + var recordingId = options["id"]; + + return $.ajax({ + type: "POST", + dataType: "json", + contentType: 'application/json', + url: "/api/recordings/" + recordingId + "/discard", + data: JSON.stringify(options) + }) + } + function putTrackSyncChange(options) { var musicSessionId = options["id"] delete options["id"]; @@ -556,6 +580,8 @@ this.startRecording = startRecording; this.stopRecording = stopRecording; this.getRecording = getRecording; + this.claimRecording = claimRecording; + this.discardRecording = discardRecording; this.putTrackSyncChange = putTrackSyncChange; this.createBand = createBand; this.updateBand = updateBand; diff --git a/web/app/assets/javascripts/recordingFinishedDialog.js b/web/app/assets/javascripts/recordingFinishedDialog.js new file mode 100644 index 000000000..57f6505c6 --- /dev/null +++ b/web/app/assets/javascripts/recordingFinishedDialog.js @@ -0,0 +1,155 @@ +(function(context,$) { + + "use strict"; + context.JK = context.JK || {}; + context.JK.RecordingFinishedDialog = function(app) { + var logger = context.JK.logger; + var rest = context.JK.Rest(); + + function resetForm() { + // remove all display errors + $('#recording-finished-dialog form .error-text').remove() + $('#recording-finished-dialog form .error').removeClass("error") + } + + function beforeShow() { + resetForm(); + + var parentSelector = '#recording-finished-dialog div.genre-selector'; + context.JK.GenreSelectorHelper.render(parentSelector); + + // preset genre from 1st genre in current session + var currentOrLastSession = JK.CurrentSessionModel.getCurrentOrLastSession(); + if(currentOrLastSession && currentOrLastSession.genres.length > 0) { + var genreDescription = currentOrLastSession.genres[0]; + context.JK.GenreSelectorHelper.setSelectedGenres(parentSelector, [genreDescription]); + } + } + + function afterHide() { + + } + + function discardRecording(e) { + + resetForm(); + registerDiscardRecordingHandlers(false); + var recordingId = JK.CurrentSessionModel.recordingModel.currentOrLastRecordingId(); + + rest.discardRecording({ + id: recordingId + }) + .done(function() { + console.error("recording discarded by user. recordingId=%o", recordingId); + }) + .fail(function(jqXHR){ + console.error("recording discard by user failed. recordingId=%o. reason: %o", recordingId, jqXHR.responseText); + }) + .always(function() { + app.layout.closeDialog('recordingFinished') + registerDiscardRecordingHandlers(true); + }) + return false; + } + function claimRecording(e){ + + resetForm(); + registerClaimRecordingHandlers(false); + var recordingId = JK.CurrentSessionModel.recordingModel.currentOrLastRecordingId(); + + var name = $('#recording-finished-dialog form input[name=name]').val(); + var description = $('#recording-finished-dialog form textarea[name=description]').val(); + var genre = $('#recording-finished-dialog form select[name=genre]').val(); + var is_public = $('#recording-finished-dialog form input[name=is_public]').is(':checked') + var is_downloadable = $('#recording-finished-dialog form input[name=is_downloadable]').is(':checked'); + + rest.claimRecording({ + id : recordingId, + name: name, + description: description, + genre: genre, + is_public: is_public, + is_downloadable: is_downloadable + }) + .done(function() { + app.layout.closeDialog('recordingFinished') + }) + .fail(function(jqXHR) { + if(jqXHR.status == 422) { + var errors = JSON.parse(jqXHR.responseText); + + var $name_errors = context.JK.format_errors('name', errors); + if($name_errors) $('#recording-finished-dialog form input[name=name]').closest('div.field').addClass('error').end().after($name_errors); + + var $description_errors = context.JK.format_errors('description', errors); + if($description_errors) $('#recording-finished-dialog form input[name=description]').closest('div.field').addClass('error').end().after($description_errors); + + var $genre_errors = context.JK.format_errors('genre', errors); + if($genre_errors) $('#recording-finished-dialog form select[name=genre]').closest('div.field').addClass('error').end().after($genre_errors); + + var $is_public_errors = context.JK.format_errors('is_public', errors); + if($is_public_errors) $('#recording-finished-dialog form input[name=is_public]').closest('div.field').addClass('error').end().after($is_public_errors); + + var $is_downloadable_errors = context.JK.format_errors('is_downloadable', errors); + if($is_downloadable_errors) $('#recording-finished-dialog form input[name=is_downloadable]').closest('div.field').addClass('error').end().after($is_downloadable_errors); + + var recording_error = context.JK.get_first_error('recording_id', errors); + + if (recording_error) context.JK.showErrorDialog(app, "Unable to claim recording.", recording_error); + } + else { + logger.error("unable to claim recording %o", arguments); + + context.JK.showErrorDialog(app, "Unable to claim recording.", jqXHR.responseText); + } + }) + .always(function() { + registerClaimRecordingHandlers(true); + }); + return false; + } + + function registerClaimRecordingHandlers(onOff) { + if(onOff) { + $('#keep-session-recording').on('click', claimRecording); + $('#recording-finished-dialog form').on('submit', claimRecording); + } + else { + $('#keep-session-recording').off('click', claimRecording) + $('#recording-finished-dialog form').off('submit', claimRecording); + } + + } + + function registerDiscardRecordingHandlers(onOff) { + if(onOff) { + $('#discard-session-recording').on('click', discardRecording); + } + else { + $('#discard-session-recording').off('click', discardRecording); + } + } + + function registerStaticEvents() { + registerClaimRecordingHandlers(true); + registerDiscardRecordingHandlers(true); + + } + + function initialize(){ + var dialogBindings = { + 'beforeShow' : beforeShow, + 'afterHide': afterHide + }; + + app.bindDialog('recordingFinished', dialogBindings); + + registerStaticEvents(); + }; + + + this.initialize = initialize; + } + + return this; +})(window,jQuery); \ No newline at end of file diff --git a/web/app/assets/javascripts/recordingModel.js b/web/app/assets/javascripts/recordingModel.js index 19bf02fe2..59c76c286 100644 --- a/web/app/assets/javascripts/recordingModel.js +++ b/web/app/assets/javascripts/recordingModel.js @@ -20,6 +20,7 @@ context.JK.RecordingModel = function(app, sessionModel, _rest, _jamClient) { var currentRecording = null; // the JSON response from the server for a recording + var currentOrLastRecordingId = null; var currentRecordingId = null; var rest = _rest; var currentlyRecording = false; @@ -57,6 +58,7 @@ currentRecordingId = null; } + function groupTracksToClient(recording) { // group N tracks to the same client Id var groupedTracks = {}; @@ -83,6 +85,7 @@ currentRecording = rest.startRecording({"music_session_id": sessionModel.id()}) .done(function(recording) { currentRecordingId = recording.id; + currentOrLastRecordingId = recording.id; // ask the backend to start the session. var groupedTracks = groupTracksToClient(recording); @@ -220,6 +223,7 @@ }) .done(function(recording) { currentRecordingId = recording.id; + currentOrLastRecordingId = recording.id; }); $self.triggerHandler('startingRecording', {recordingId: recordingId}); @@ -320,6 +324,7 @@ this.isRecording = isRecording; this.reset = reset; this.stopRecordingIfNeeded = stopRecordingIfNeeded; + this.currentOrLastRecordingId = function () { return currentOrLastRecordingId; }; context.JK.HandleRecordingStartResult = handleRecordingStartResult; context.JK.HandleRecordingStopResult = handleRecordingStopResult; diff --git a/web/app/assets/javascripts/sessionModel.js b/web/app/assets/javascripts/sessionModel.js index c4b5148c6..5775f5dc2 100644 --- a/web/app/assets/javascripts/sessionModel.js +++ b/web/app/assets/javascripts/sessionModel.js @@ -11,6 +11,7 @@ var clientId = client.clientID; var currentSessionId = null; // Set on join, prior to setting currentSession. var currentSession = null; + var currentOrLastSession = null; var subscribers = {}; var users = {}; // User info for session participants var rest = context.JK.Rest(); @@ -146,7 +147,11 @@ } } + // you should only update currentSession with this function function updateCurrentSession(sessionData) { + if(sessionData != null) { + currentOrLastSession = sessionData; + } currentSession = sessionData; } @@ -464,6 +469,9 @@ this.getCurrentSession = function() { return currentSession; }; + this.getCurrentOrLastSession = function() { + return currentOrLastSession; + }; }; })(window,jQuery); \ No newline at end of file diff --git a/web/app/assets/javascripts/utils.js b/web/app/assets/javascripts/utils.js index 8f676b221..9b1e2cbd2 100644 --- a/web/app/assets/javascripts/utils.js +++ b/web/app/assets/javascripts/utils.js @@ -270,7 +270,7 @@ return null; } - var ul = $('') + var ul = $(''); $.each(field_errors, function(index, item) { ul.append(context.JK.fillTemplate("
  • {message}
    • ", {message: item})) diff --git a/web/app/assets/stylesheets/client/client.css b/web/app/assets/stylesheets/client/client.css index 2fb0e0a17..ae183b721 100644 --- a/web/app/assets/stylesheets/client/client.css +++ b/web/app/assets/stylesheets/client/client.css @@ -29,6 +29,7 @@ *= require ./search *= require ./ftue *= require ./invitationDialog + *= require ./recordingFinishedDialog *= require ./createSession *= require ./genreSelector *= require ./sessionList diff --git a/web/app/assets/stylesheets/client/recordingFinishedDialog.css.scss b/web/app/assets/stylesheets/client/recordingFinishedDialog.css.scss new file mode 100644 index 000000000..a185d0dfa --- /dev/null +++ b/web/app/assets/stylesheets/client/recordingFinishedDialog.css.scss @@ -0,0 +1,12 @@ +#recording-finished-dialog { + width:1000px; + + div[purpose=description], div[purpose=is_public], div[purpose=is_downloadable] { + margin-top:20px; + } + + label[for=is_downloadable], label[for=is_public] { + display:inline; + } +} + diff --git a/web/app/controllers/api_recordings_controller.rb b/web/app/controllers/api_recordings_controller.rb index 40d2aae14..6cdba007c 100644 --- a/web/app/controllers/api_recordings_controller.rb +++ b/web/app/controllers/api_recordings_controller.rb @@ -1,7 +1,7 @@ class ApiRecordingsController < ApiController before_filter :api_signed_in_user - before_filter :look_up_recording, :only => [ :show, :stop, :claim, :keep ] + before_filter :look_up_recording, :only => [ :show, :stop, :claim, :discard, :keep ] before_filter :parse_filename, :only => [ :download, :upload_next_part, :upload_sign, :upload_part_complete, :upload_complete ] respond_to :json @@ -32,15 +32,15 @@ class ApiRecordingsController < ApiController end def download + raise PermissionError, ValidationMessages::PERMISSION_VALIDATION_ERROR unless @recorded_track.can_download?(current_user) + redirect_to @recorded_track.sign_url end def start music_session = MusicSession.find(params[:music_session_id]) - unless music_session.users.exists?(current_user) - raise PermissionError, ValidationMessages::PERMISSION_VALIDATION_ERROR - end + raise PermissionError, ValidationMessages::PERMISSION_VALIDATION_ERROR unless music_session.users.exists?(current_user) @recording = Recording.start(music_session, current_user) @@ -54,10 +54,6 @@ class ApiRecordingsController < ApiController def stop - unless @recording.users.exists?(current_user) - raise PermissionError, ValidationMessages::PERMISSION_VALIDATION_ERROR - end - @recording.stop if @recording.errors.any? @@ -68,9 +64,9 @@ class ApiRecordingsController < ApiController end end - # keep will kick off a mix, as well as create a claimed recording for the creator + # claim will create a claimed recording for the creator def claim - claim = @recording.claim(current_user, params[:name], params[:description], Genre.find(params[:genre_id]), params[:is_public], params[:is_downloadable]) + claim = @recording.claim(current_user, params[:name], params[:description], Genre.find_by_id(params[:genre]), params[:is_public], params[:is_downloadable]) claim.save if claim.errors.any? @@ -81,6 +77,13 @@ class ApiRecordingsController < ApiController end end + # discard will tell the server the user has no interest in the recording + def discard + @recorded_tracks = @recording.recorded_tracks_for_user(current_user) + RecordedTrack.update(@recorded_tracks, :discard => true) + render :json => {}, :status => 200 + end + def upload_next_part length = params[:length] md5 = params[:md5] @@ -141,14 +144,13 @@ class ApiRecordingsController < ApiController private def parse_filename - @recorded_track = RecordedTrack.find_by_recording_id_and_client_track_id(params[:id], params[:track_id]) - unless @recorded_track - render :json => { :message => ValidationMessages::RECORDING_NOT_FOUND }, :status => 404 - end + @recorded_track = RecordedTrack.find_by_recording_id_and_client_track_id!(params[:id], params[:track_id]) + raise PermissionError, ValidationMessages::PERMISSION_VALIDATION_ERROR unless @recorded_track.recording.has_access?(current_user) end def look_up_recording @recording = Recording.find(params[:id]) + raise PermissionError, ValidationMessages::PERMISSION_VALIDATION_ERROR unless @recording.has_access?(current_user) end end diff --git a/web/app/responders/api_responder.rb b/web/app/responders/api_responder.rb index 64df5fd69..0520f6d88 100644 --- a/web/app/responders/api_responder.rb +++ b/web/app/responders/api_responder.rb @@ -5,7 +5,6 @@ class ApiResponder < ActionController::Responder logger.debug("REST API entity has error: #{resource.errors.inspect}") controller.response.status = :unprocessable_entity when post? - logger.debug("REST API post") controller.response.status = :created end diff --git a/web/app/views/clients/_recordingFinishedDialog.html.erb b/web/app/views/clients/_recordingFinishedDialog.html.erb index 4193782a2..165fd2cd7 100644 --- a/web/app/views/clients/_recordingFinishedDialog.html.erb +++ b/web/app/views/clients/_recordingFinishedDialog.html.erb @@ -7,89 +7,83 @@
    - Fill out the fields below and click the "SAVE" button to save this recording to your library. If you do not want to keep the recording, click the "DISCARD" button. -
    -
    + Fill out the fields below and click the "SAVE" button to save this recording to your library. If you do not want to + keep the recording, click the "DISCARD" button. +
    +
    -
    -
    - Recording name:
    - +
    +
    +
    +
    + +
    -
    - - Genre:
    +
    +
    + +
    + +
    +
    +
    + + +
    +
    + +
    +
    + +
    + - -
    -
    -
    - Description:
    - -
    +
    + Preview Recording:
    + +
    -
    - Preview Recording:
    - -
    + + <%= image_tag "content/icon_playbutton.png", {:height => 20, :width => 20} %> - - <%= image_tag "content/icon_playbutton.png", {:height => 20, :width => 20} %> + +
    - -
    + +
    0:00
    - -
    0:00
    + +
    +
    <%= image_tag "content/slider_playcontrols.png", {:height => 16, :width => 5} %>
    +
    - -
    -
    <%= image_tag "content/slider_playcontrols.png", {:height => 16, :width => 5} %>
    + +
    4:59
    +
    + + + +
    + 1:23
    - -
    4:59
    -
    - - - -
    - 1:23
    + +
    +
    - -
    -
    - Make this Recording Public
    - Make this Recording Downloadable
    -
    - -

    +

    - DISCARD SAVE + DISCARD SAVE
    -
    +
    - - - - DISCARD
    diff --git a/web/app/views/clients/index.html.erb b/web/app/views/clients/index.html.erb index 1e8a95051..320499564 100644 --- a/web/app/views/clients/index.html.erb +++ b/web/app/views/clients/index.html.erb @@ -163,6 +163,8 @@ var sessionSettingsDialog = new JK.SessionSettingsDialog(JK.app, sessionScreen); sessionSettingsDialog.initialize(); + var recordingFinishedDialog = new JK.RecordingFinishedDialog(JK.app); + recordingFinishedDialog.initialize(); var whatsNextDialog = new JK.WhatsNextDialog(JK.app); whatsNextDialog.initialize(invitationDialog); diff --git a/web/config/routes.rb b/web/config/routes.rb index 1bfb079dd..4f4aef161 100644 --- a/web/config/routes.rb +++ b/web/config/routes.rb @@ -274,6 +274,7 @@ SampleApp::Application.routes.draw do match '/recordings/:id' => 'api_recordings#show', :via => :get, :as => 'api_recordings_detail' match '/recordings/:id/stop' => 'api_recordings#stop', :via => :post, :as => 'api_recordings_stop' match '/recordings/:id/claim' => 'api_recordings#claim', :via => :post, :as => 'api_recordings_claim' + match '/recordings/:id/discard' => 'api_recordings#discard', :via => :post, :as => 'api_recordings_discard' match '/recordings/:id/tracks/:track_id/download' => 'api_recordings#download', :via => :get, :as => 'api_recordings_download' match '/recordings/:id/tracks/:track_id/upload_next_part' => 'api_recordings#upload_next_part', :via => :get match '/recordings/:id/tracks/:track_id/upload_sign' => 'api_recordings#upload_sign', :via => :get diff --git a/web/spec/controllers/recordings_controller_spec.rb b/web/spec/controllers/recordings_controller_spec.rb index 4e8601d57..b9447fffc 100644 --- a/web/spec/controllers/recordings_controller_spec.rb +++ b/web/spec/controllers/recordings_controller_spec.rb @@ -86,4 +86,14 @@ describe ApiRecordingsController do response.status.should == 403 end end + + describe "download" do + it "should only allow a user to download a track if they have claimed the recording" do + post :start, { :format => 'json', :music_session_id => @music_session.id } + response_body = JSON.parse(response.body) + recording = Recording.find(response_body['id']) + post :stop, { :format => 'json', :id => recording.id } + response.should be_success + end + end end diff --git a/web/spec/features/recordings_spec.rb b/web/spec/features/recordings_spec.rb index b37891118..54845f0be 100644 --- a/web/spec/features/recordings_spec.rb +++ b/web/spec/features/recordings_spec.rb @@ -34,12 +34,12 @@ describe "Session Recordings", :js => true, :type => :feature, :capybara_feature in_client(creator) do find('#recording-start-stop').trigger(:click) find('#recording-status').should have_content 'Make a Recording' - should have_selector('h1', 'Recording Finished') + should have_selector('h1', text: 'recording finished') end in_client(joiner1) do find('#recording-status').should have_content 'Make a Recording' - should have_selector('h1', 'Recording Finished') + should have_selector('h1', text: 'recording finished') end end @@ -60,12 +60,12 @@ describe "Session Recordings", :js => true, :type => :feature, :capybara_feature in_client(joiner1) do find('#recording-start-stop').trigger(:click) find('#recording-status').should have_content 'Make a Recording' - should have_selector('h1', 'Recording Finished') + should have_selector('h1', text: 'recording finished') end in_client(creator) do find('#recording-status').should have_content 'Make a Recording' - should have_selector('h1', 'Recording Finished') + should have_selector('h1', text: 'recording finished') end end @@ -87,12 +87,12 @@ describe "Session Recordings", :js => true, :type => :feature, :capybara_feature find('#session-leave').trigger(:click) expect(page).to have_selector('h2', text: 'feed') find('#recording-status').should have_content 'Make a Recording' - should have_selector('h1', 'Recording Finished') + should have_selector('h1', text: 'recording finished') end in_client(joiner1) do find('#recording-status').should have_content 'Make a Recording' - should have_selector('h1', 'Recording Finished') + should have_selector('h1', text: 'recording finished') end end @@ -111,7 +111,7 @@ describe "Session Recordings", :js => true, :type => :feature, :capybara_feature end in_client(creator) do - visit "http://www.google.com" # kills websocket, looking like an abrupt leave + visit "/downloads" # kills websocket, looking like an abrupt leave end in_client(joiner1) do @@ -142,17 +142,17 @@ describe "Session Recordings", :js => true, :type => :feature, :capybara_feature in_client(creator) do find('#recording-start-stop').trigger(:click) find('#recording-status').should have_content 'Make a Recording' - should have_selector('h1', 'Recording Finished') + should have_selector('h1', text: 'recording finished') end in_client(joiner1) do find('#recording-status').should have_content 'Make a Recording' - should have_selector('h1', 'Recording Finished') + should have_selector('h1', text: 'recording finished') end in_client(joiner2) do find('#recording-status').should have_content 'Make a Recording' - should have_selector('h1', 'Recording Finished') + should have_selector('h1', text: 'recording finished') end end @@ -177,17 +177,17 @@ describe "Session Recordings", :js => true, :type => :feature, :capybara_feature find('#session-leave').trigger(:click) expect(page).to have_selector('h2', text: 'feed') find('#recording-status').should have_content 'Make a Recording' - should have_selector('h1', 'Recording Finished') + should have_selector('h1', text: 'recording finished') end in_client(joiner1) do find('#recording-status').should have_content 'Make a Recording' - should have_selector('h1', 'Recording Finished') + should have_selector('h1', text: 'recording finished') end in_client(joiner2) do find('#recording-status').should have_content 'Make a Recording' - should have_selector('h1', 'Recording Finished') + should have_selector('h1', text: 'recording finished') end end @@ -211,7 +211,7 @@ describe "Session Recordings", :js => true, :type => :feature, :capybara_feature end in_client(creator) do - visit "http://www.google.com" # kills websocket, looking like an abrupt leave + visit "/downloads" # kills websocket, looking like an abrupt leave end in_client(joiner1) do @@ -226,5 +226,94 @@ describe "Session Recordings", :js => true, :type => :feature, :capybara_feature find('#recording-status').should have_content 'Make a Recording' end end + + # creates a recording, and stops it, and confirms the 'Finished Recording' dialog shows for both + describe "Finished Recording Dialog" do + before(:each) do + + RecordedTrack.delete_all + ClaimedRecording.delete_all + + create_join_session(creator, [joiner1]) + + in_client(creator) do + find('#recording-start-stop').trigger(:click) + find('#recording-status').should have_content 'Stop Recording' + end + + in_client(joiner1) do + find('#notification').should have_content 'started a recording' + find('#recording-status').should have_content 'Stop Recording' + end + + in_client(creator) do + find('#recording-start-stop').trigger(:click) + find('#recording-status').should have_content 'Make a Recording' + should have_selector('h1', text: 'recording finished') + end + end + + it "discard the recording" do + in_client(creator) do + find('#recording-finished-dialog h1') + find('#discard-session-recording').trigger(:click) + should have_no_selector('h1', text: 'recording finished') + music_session = MusicSession.first() + recording = music_session.recordings.first() + tracks = recording.recorded_tracks_for_user(creator) + tracks.length.should == 1 + tracks[0].discard.should be_true + end + + in_client(joiner1) do + find('#recording-finished-dialog h1') + find('#discard-session-recording').trigger(:click) + should have_no_selector('h1', text: 'recording finished') + music_session = MusicSession.first() + recording = music_session.recordings.first() + tracks = recording.recorded_tracks_for_user(joiner1) + tracks.length.should == 1 + tracks[0].discard.should be_true + end + end + + it "claim recording" do + in_client(creator) do + find('#recording-finished-dialog h1') + fill_in "claim-recording-name", with: "my recording" + fill_in "claim-recording-description", with: "my description" + find('#keep-session-recording').trigger(:click) + should have_no_selector('h1', text: 'recording finished') + + music_session = MusicSession.first() + recording = music_session.recordings.first() + claimed_recording = ClaimedRecording.find_by_user_id(creator.id) + claimed_recording.name.should == "my recording" + claimed_recording.description.should == "my description" + claimed_recording.is_public.should be_true + claimed_recording.is_downloadable.should be_true + claimed_recording.genre = music_session.genres[0] + + end + + in_client(joiner1) do + find('#recording-finished-dialog h1') + fill_in "claim-recording-name", with: "my recording" + fill_in "claim-recording-description", with: "my description" + find('#keep-session-recording').trigger(:click) + should have_no_selector('h1', text: 'recording finished') + + music_session = MusicSession.first() + recording = music_session.recordings.first() + claimed_recording = ClaimedRecording.find_by_user_id(joiner1.id) + claimed_recording.name.should == "my recording" + claimed_recording.description.should == "my description" + claimed_recording.is_public.should be_true + claimed_recording.is_downloadable.should be_true + claimed_recording.genre = music_session.genres[0] + end + end + + end end diff --git a/web/spec/spec_helper.rb b/web/spec/spec_helper.rb index 67d835ba3..3e637b07e 100644 --- a/web/spec/spec_helper.rb +++ b/web/spec/spec_helper.rb @@ -117,11 +117,12 @@ Spork.prefork do config.append_after(:each) do + Capybara.reset_sessions! + reset_session_mapper + end config.after(:each) do - Capybara.reset_sessions! - reset_session_mapper if example.metadata[:js] end diff --git a/web/spec/support/utilities.rb b/web/spec/support/utilities.rb index a067e81e6..10f37a345 100644 --- a/web/spec/support/utilities.rb +++ b/web/spec/support/utilities.rb @@ -121,7 +121,7 @@ def create_session(creator = FactoryGirl.create(:user), unique_session_desc = "c # create session in one client in_client(creator) do - page.driver.resize(1500, 600) # makes sure all the elements are visible + page.driver.resize(1500, 800) # makes sure all the elements are visible sign_in_poltergeist creator wait_until_curtain_gone visit "/client#/createSession"