From 8cad33a303e39a461fca4a6f9726d50d522c40da Mon Sep 17 00:00:00 2001 From: Seth Call Date: Fri, 7 Nov 2014 16:12:36 -0600 Subject: [PATCH] * recording file/manager fixes and aging of recordings --- db/up/deletable_recordings.sql | 2 + ruby/lib/jam_ruby/models/user_sync.rb | 10 +++- ruby/lib/jam_ruby/resque/quick_mixer.rb | 3 +- ruby/spec/jam_ruby/models/user_sync_spec.rb | 29 ++++++++--- web/app/assets/javascripts/findBand.js | 2 +- .../assets/javascripts/sync_viewer.js.coffee | 20 ++++++-- web/app/views/clients/_help.html.erb | 8 +++- .../api_user_syncs_controller_spec.rb | 48 +++++-------------- 8 files changed, 70 insertions(+), 52 deletions(-) diff --git a/db/up/deletable_recordings.sql b/db/up/deletable_recordings.sql index 648b45e19..933f30611 100644 --- a/db/up/deletable_recordings.sql +++ b/db/up/deletable_recordings.sql @@ -1,8 +1,10 @@ + -- this is to make sure we don't delete any recordings for 7 days UPDATE recordings SET updated_at = NOW(); ALTER TABLE recordings ADD COLUMN deleted BOOLEAN DEFAULT FALSE NOT NULL; + DROP VIEW user_syncs; CREATE VIEW user_syncs AS diff --git a/ruby/lib/jam_ruby/models/user_sync.rb b/ruby/lib/jam_ruby/models/user_sync.rb index 03e4b938a..a84dadb6c 100644 --- a/ruby/lib/jam_ruby/models/user_sync.rb +++ b/ruby/lib/jam_ruby/models/user_sync.rb @@ -21,7 +21,15 @@ module JamRuby raise 'no user id specified' if user_id.blank? - query = UserSync.includes(recorded_track: [{recording: [:owner, {claimed_recordings: [:share_token]}, {recorded_tracks: [:user]}, {comments:[:user]}, :likes, :plays, :mixes]}, user: [], instrument:[]], mix: [], quick_mix:[]).where(user_id: user_id).paginate(:page => page, :per_page => limit).order('created_at DESC, unified_id') + query = UserSync + .includes(recorded_track: [{recording: [:owner, {claimed_recordings: [:share_token]}, {recorded_tracks: [:user]}, {comments:[:user]}, :likes, :plays, :mixes]}, user: [], instrument:[]], mix: [], quick_mix:[]) + .joins("LEFT OUTER JOIN claimed_recordings ON claimed_recordings.user_id = user_syncs.user_id AND claimed_recordings.recording_id = user_syncs.recording_id") + .where(user_id: user_id) + .where(%Q{ + ((claimed_recordings IS NULL OR claimed_recordings.discarded = TRUE) AND fully_uploaded = FALSE) OR (claimed_recordings IS NOT NULL AND claimed_recordings.discarded = FALSE) + }) + .paginate(:page => page, :per_page => limit) + .order('created_at DESC, unified_id') # allow selection of single user_sync, by ID diff --git a/ruby/lib/jam_ruby/resque/quick_mixer.rb b/ruby/lib/jam_ruby/resque/quick_mixer.rb index 1c3793c8f..7216610e4 100644 --- a/ruby/lib/jam_ruby/resque/quick_mixer.rb +++ b/ruby/lib/jam_ruby/resque/quick_mixer.rb @@ -65,10 +65,9 @@ module JamRuby cleanup_files - @@log.info("audiomixer job successful. mix_id #{quick_mix_id}") + @@log.info("quickmixer job successful. mix_id #{quick_mix_id}") rescue Exception => e - puts "EEOUOU #{e}" post_error(@quick_mix, e) raise end diff --git a/ruby/spec/jam_ruby/models/user_sync_spec.rb b/ruby/spec/jam_ruby/models/user_sync_spec.rb index 83b9e4465..8699d0e12 100644 --- a/ruby/spec/jam_ruby/models/user_sync_spec.rb +++ b/ruby/spec/jam_ruby/models/user_sync_spec.rb @@ -41,6 +41,22 @@ describe UserSync do user_syncs[2].quick_mix.should eq(quick_mix) end + # https://jamkazam.atlassian.net/browse/VRFS-2450 + it "no longer returned after fully uploaded and unclaimed" do + mix = FactoryGirl.create(:mix) + mix.recording.duration = 1 + mix.recording.save! + quick_mix = FactoryGirl.create(:quick_mix_completed, recording:mix.recording, user: mix.recording.recorded_tracks[0].user, autowire:false, fully_uploaded: true) + mix.recording.recorded_tracks[0].fully_uploaded = true + mix.recording.recorded_tracks[0].save! + mix.recording.claimed_recordings[0].discarded = true + mix.recording.claimed_recordings[0].save! + + data = UserSync.index({user_id: mix.recording.recorded_tracks[0].user.id}) + user_syncs = data[:query] + user_syncs.length.should == 0 + end + it "two mixes, one not belonging to querier" do mix1 = FactoryGirl.create(:mix) mix2 = FactoryGirl.create(:mix) @@ -71,8 +87,8 @@ describe UserSync do describe "one recording with two users" do let!(:recording1) { recording = FactoryGirl.create(:recording, owner: user1, band: nil, duration:1) - recording.recorded_tracks << FactoryGirl.create(:recorded_track, recording: recording, user: recording.owner) - recording.recorded_tracks << FactoryGirl.create(:recorded_track, recording: recording, user: user2) + recording.recorded_tracks << FactoryGirl.create(:recorded_track, recording: recording, user: recording.owner, fully_uploaded:false) + recording.recorded_tracks << FactoryGirl.create(:recorded_track, recording: recording, user: user2, fully_uploaded:false) recording.save! recording.reload recording @@ -177,10 +193,10 @@ describe UserSync do describe "one recording with multi-track users" do let!(:recording1) { recording = FactoryGirl.create(:recording, owner: user1, band: nil, duration:1) - recording.recorded_tracks << FactoryGirl.create(:recorded_track, recording: recording, user: recording.owner) - recording.recorded_tracks << FactoryGirl.create(:recorded_track, recording: recording, user: recording.owner) - recording.recorded_tracks << FactoryGirl.create(:recorded_track, recording: recording, user: user2) - recording.recorded_tracks << FactoryGirl.create(:recorded_track, recording: recording, user: user2) + recording.recorded_tracks << FactoryGirl.create(:recorded_track, recording: recording, user: recording.owner, fully_uploaded:false) + recording.recorded_tracks << FactoryGirl.create(:recorded_track, recording: recording, user: recording.owner, fully_uploaded:false) + recording.recorded_tracks << FactoryGirl.create(:recorded_track, recording: recording, user: user2, fully_uploaded:false) + recording.recorded_tracks << FactoryGirl.create(:recorded_track, recording: recording, user: user2, fully_uploaded:false) recording.save! recording.reload recording @@ -228,6 +244,7 @@ describe UserSync do recording = FactoryGirl.create(:recording, owner: user1, band: nil, duration:1) recording.recorded_tracks << FactoryGirl.create(:recorded_track, recording: recording, user: recording.owner) recording.recorded_tracks << FactoryGirl.create(:recorded_track, recording: recording, user: user2) + claimed_recording = FactoryGirl.create(:claimed_recording, user: recording.owner, recording: recording, discarded:false) recording.save! recording.reload recording diff --git a/web/app/assets/javascripts/findBand.js b/web/app/assets/javascripts/findBand.js index fd3f1358d..df498dca7 100644 --- a/web/app/assets/javascripts/findBand.js +++ b/web/app/assets/javascripts/findBand.js @@ -204,7 +204,7 @@ // @FIXME -- this will need to be tweaked when we allow unfollowing $('div[data-band-id='+newFollowing.band_id+'] .search-m-follow').removeClass('button-orange').addClass('button-grey'); }, - error: app.ajaxError + error: app.ajaxError(arguments) }); } diff --git a/web/app/assets/javascripts/sync_viewer.js.coffee b/web/app/assets/javascripts/sync_viewer.js.coffee index 98f43ec18..3215359e2 100644 --- a/web/app/assets/javascripts/sync_viewer.js.coffee +++ b/web/app/assets/javascripts/sync_viewer.js.coffee @@ -603,16 +603,26 @@ context.JK.SyncViewer = class SyncViewer title: "Confirm Deletion", html: "Are you sure you want to delete this recording?", yes: => - @rest.deleteRecordingClaim(recordingId).done((response)-> + @rest.deleteRecordingClaim(recordingId).done((response)=> cmd = { type: 'recording_directory', action: 'delete', queue: 'cleanup', recording_id: recordingId} - context.JK.ackBubble($delete, 'command-enqueued', {}, {offsetParent: $delete.closest('.dialog')}) - - logger.debug("enqueueing delete #{recordingId}") + # now check if the sync is gone entirely, allowing us to delete it from the UI + @rest.getUserSync({user_sync_id: recordingId}).done((userSync) => + # the user sync is still here. tell user it'll be done as soon as they've uploaded their files + context.JK.ackBubble($delete, 'file-sync-delayed-deletion', {}, {offsetParent: $delete.closest('.dialog')}) + ) + .fail((xhr) => + if xhr.status == 404 + # the userSync is gone; remove from file manager dynamically + $recordingHolder = $details.closest('.recording-holder') + $recordingHolder.slideUp() + else + @app.ajaxError(arguments) + ) context.jamClient.OnTrySyncCommand(cmd) ) @@ -622,7 +632,7 @@ context.JK.SyncViewer = class SyncViewer return false; displaySize: (size) => - # size is in bytes. divide by million, and round to one decimal place + # size is in bytes. divide by million, anxosd round to one decimal place megs = Math.round(size * 10 / (1024 * 1024) ) / 10 "#{megs}M" diff --git a/web/app/views/clients/_help.html.erb b/web/app/views/clients/_help.html.erb index 38409096c..66ac73135 100644 --- a/web/app/views/clients/_help.html.erb +++ b/web/app/views/clients/_help.html.erb @@ -148,8 +148,12 @@ - + \ No newline at end of file diff --git a/web/spec/controllers/api_user_syncs_controller_spec.rb b/web/spec/controllers/api_user_syncs_controller_spec.rb index fb7c0a423..a1b039f2f 100644 --- a/web/spec/controllers/api_user_syncs_controller_spec.rb +++ b/web/spec/controllers/api_user_syncs_controller_spec.rb @@ -46,23 +46,12 @@ describe ApiUserSyncsController do } it "no claimed_recordings" do - # every is supposed to upload immediately, but no downloads until you try claim it. The assertions below validate this + + # if there are no claims, then no one wants the recording. no usersyncs get :index, { :format => 'json', :id => user1.id } json = JSON.parse(response.body, :symbolize_names => true) json[:next].should be_nil - json[:entries].length.should == 2 - - recorded_track1 = json[:entries][0] - recorded_track1[:upload][:should_upload].should be_true - recorded_track1[:upload][:too_many_upload_failures].should be_false - recorded_track1[:download][:should_download].should be_false - recorded_track1[:download][:too_many_downloads].should be_false - - recorded_track2 = json[:entries][1] - recorded_track2[:upload][:should_upload].should be_true - recorded_track2[:upload][:too_many_upload_failures].should be_false - recorded_track2[:download][:should_download].should be_false - recorded_track2[:download][:too_many_downloads].should be_false + json[:entries].length.should == 0 end it "recording isn't over" do @@ -79,6 +68,9 @@ describe ApiUserSyncsController do FactoryGirl.create(:claimed_recording, user: user1, recording: recording1, discarded:false) + recording1.recorded_tracks[0].fully_uploaded = false + recording1.recorded_tracks[0].save! + get :index, { :format => 'json', :id => user1.id } json = JSON.parse(response.body, :symbolize_names => true) json[:next].should be_nil @@ -100,7 +92,7 @@ describe ApiUserSyncsController do get :index, { :format => 'json', :id => user2.id } json = JSON.parse(response.body, :symbolize_names => true) json[:next].should be_nil - json[:entries].length.should == 2 + json[:entries].length.should == 1 recorded_track1 = json[:entries][0] recorded_track1[:upload][:should_upload].should be_true @@ -108,32 +100,17 @@ describe ApiUserSyncsController do recorded_track1[:download][:should_download].should be_false recorded_track1[:download][:too_many_downloads].should be_false - recorded_track2 = json[:entries][1] - recorded_track2[:upload][:should_upload].should be_true - recorded_track2[:upload][:too_many_upload_failures].should be_false - recorded_track2[:download][:should_download].should be_false - recorded_track2[:download][:too_many_downloads].should be_false end it "one user decides to discard the recording" do FactoryGirl.create(:claimed_recording, user: user1, recording: recording1, discarded:true) + FactoryGirl.create(:claimed_recording, user: user2, recording: recording1, discarded:false) + # it's a length of zero because recorded_tracks default to 'fully_uploaded = true'. so nothing to do for this user get :index, { :format => 'json', :id => user1.id } json = JSON.parse(response.body, :symbolize_names => true) json[:next].should be_nil - json[:entries].length.should == 2 - - recorded_track1 = json[:entries][0] - recorded_track1[:upload][:should_upload].should be_true - recorded_track1[:upload][:too_many_upload_failures].should be_false - recorded_track1[:download][:should_download].should be_false - recorded_track1[:download][:too_many_downloads].should be_false - - recorded_track2 = json[:entries][1] - recorded_track2[:upload][:should_upload].should be_true - recorded_track2[:upload][:too_many_upload_failures].should be_false - recorded_track2[:download][:should_download].should be_false - recorded_track2[:download][:too_many_downloads].should be_false + json[:entries].length.should == 0 controller.current_user = user2 get :index, { :format => 'json', :id => user2.id } @@ -141,16 +118,17 @@ describe ApiUserSyncsController do json[:next].should be_nil json[:entries].length.should == 2 + recorded_track1 = json[:entries][0] recorded_track1[:upload][:should_upload].should be_true recorded_track1[:upload][:too_many_upload_failures].should be_false - recorded_track1[:download][:should_download].should be_false + recorded_track1[:download][:should_download].should be_true recorded_track1[:download][:too_many_downloads].should be_false recorded_track2 = json[:entries][1] recorded_track2[:upload][:should_upload].should be_true recorded_track2[:upload][:too_many_upload_failures].should be_false - recorded_track2[:download][:should_download].should be_false + recorded_track2[:download][:should_download].should be_true recorded_track2[:download][:too_many_downloads].should be_false end