#302

backend/app/controllers/theatre_comments_controller.rb

@@ -45,6 +45,7 @@ class TheatreCommentsController < ApplicationController
 
     comment = TheatreComment.find_by(theatre_id:, no:)
     return head :not_found unless comment
+    return head :forbidden unless comment.user == current_user
 
     comment.discard!
 

backend/app/controllers/theatre_skip_events_controller.rb

@@ -6,7 +6,7 @@ class TheatreSkipEventsController < ApplicationController
     events =
         TheatreSkipEvent
         .where(theatre_id: params[:theatre_id])
-        .includes(:skipped_by_user, :users, :tags, post: { tags: :tag_name })
+        .includes(:tags, post: { tags: :tag_name })
         .order(created_at: :desc)
         .limit(limit)
 
@@ -14,8 +14,6 @@ class TheatreSkipEventsController < ApplicationController
       { id: event.id,
         theatre_id: event.theatre_id,
         post: PostRepr.base(event.post),
-        skipped_by_user: UserRepr.base(event.skipped_by_user),
-        voters: event.users.map { |user| UserRepr.base(user) },
         tags: event.tags.map { |tag| TagRepr.inline(tag) },
         programme_position: event.programme_position,
         created_at: event.created_at }

backend/app/controllers/theatres_controller.rb

@@ -54,8 +54,11 @@ class TheatresController < ApplicationController
 
     theatre = Theatre.find_by(id: params[:id])
     return head :not_found unless theatre
+    requested_post_id = params[:post_id].to_i
+    return head :unprocessable_entity if requested_post_id <= 0
 
     skipped = false
+    conflicted = false
 
     ApplicationRecord.transaction do
       theatre.lock!
@@ -64,7 +67,12 @@ class TheatresController < ApplicationController
           _1.expires_at = 30.seconds.from_now
         }.save!
 
-        TheatreSkipVote.find_or_create_by!(theatre:, post: theatre.current_post, user: current_user)
+        if theatre.current_post_id != requested_post_id
+          conflicted = true
+          next
+        end
+
+        TheatreSkipVote.find_or_create_by!(theatre:, post_id: requested_post_id, user: current_user)
 
         vote_status = skip_vote_status(theatre)
         if vote_status[:votes_count] >= vote_status[:required_count]
@@ -76,6 +84,8 @@ class TheatresController < ApplicationController
     end
 
     theatre.reload
+    return render json: theatre_info_json(theatre, skipped: false), status: :conflict if conflicted
+
     render json: theatre_info_json(theatre, skipped:)
   end
 
@@ -84,11 +94,24 @@ class TheatresController < ApplicationController
 
     theatre = Theatre.find_by(id: params[:id])
     return head :not_found unless theatre
+    requested_post_id = params[:post_id].to_i
+    return head :unprocessable_entity if requested_post_id <= 0
 
-    if theatre.current_post
-      TheatreSkipVote.where(theatre:, post: theatre.current_post, user: current_user).delete_all
+    conflicted = false
+
+    theatre.with_lock do
+      if theatre.current_post
+        if theatre.current_post_id != requested_post_id
+          conflicted = true
+        else
+          TheatreSkipVote.where(theatre:, post_id: requested_post_id, user: current_user).delete_all
+        end
+      end
     end
 
+    theatre.reload
+    return render json: theatre_info_json(theatre, skipped: false), status: :conflict if conflicted
+
     render json: theatre_info_json(theatre, skipped: false)
   end
 

backend/spec/requests/theatre_comments_spec.rb

@@ -147,4 +147,44 @@ RSpec.describe 'TheatreComments', type: :request do
       })
     end
   end
+
+  describe 'DELETE /theatres/:theatre_id/comments/:id' do
+    let(:theatre) { create(:theatre) }
+    let(:alice) { create(:user, name: 'Alice') }
+    let(:bob) { create(:user, name: 'Bob') }
+    let!(:comment) do
+      create(
+        :theatre_comment,
+        theatre: theatre,
+        no: 1,
+        user: alice,
+        content: 'delete target'
+      )
+    end
+
+    it 'returns 401 when not logged in' do
+      delete "/theatres/#{theatre.id}/comments/#{comment.no}"
+
+      expect(response).to have_http_status(:unauthorized)
+      expect(comment.reload.discarded?).to eq(false)
+    end
+
+    it 'allows the comment owner to delete it' do
+      sign_in_as(alice)
+
+      delete "/theatres/#{theatre.id}/comments/#{comment.no}"
+
+      expect(response).to have_http_status(:no_content)
+      expect(comment.reload.discarded?).to eq(true)
+    end
+
+    it 'returns 403 when another user tries to delete it' do
+      sign_in_as(bob)
+
+      delete "/theatres/#{theatre.id}/comments/#{comment.no}"
+
+      expect(response).to have_http_status(:forbidden)
+      expect(comment.reload.discarded?).to eq(false)
+    end
+  end
 end

backend/spec/requests/theatres_spec.rb

@@ -320,10 +320,11 @@ RSpec.describe 'Theatres API', type: :request do
 
   describe 'PUT /theatres/:id/skip_vote' do
     subject(:do_request) do
-      put "/theatres/#{theatre.id}/skip_vote"
+      put "/theatres/#{theatre.id}/skip_vote", params: { post_id: requested_post_id }
     end
 
     let(:third_user) { create(:user, :member, name: 'third user') }
+    let(:requested_post_id) { niconico_post.id }
 
     before do
       theatre.update!(current_post: niconico_post, current_post_started_at: 10.seconds.ago)
@@ -374,9 +375,25 @@ RSpec.describe 'Theatres API', type: :request do
       expect(event.tags).to contain_exactly(tag)
       expect(TheatreSkipVote.where(theatre:, post: niconico_post)).to be_empty
     end
+
+    it 'does not record a vote when requested post is no longer current' do
+      theatre.update!(current_post: second_niconico_post)
+      sign_in_as(member)
+
+      expect { do_request }.not_to change(TheatreSkipVote, :count)
+
+      expect(response).to have_http_status(:conflict)
+      expect(json['post_id']).to eq(second_niconico_post.id)
+      expect(json['skip_vote']).to include(
+        'votes_count' => 0,
+        'voted' => false
+      )
+    end
   end
 
   describe 'DELETE /theatres/:id/skip_vote' do
+    let(:requested_post_id) { niconico_post.id }
+
     before do
       theatre.update!(current_post: niconico_post, current_post_started_at: 10.seconds.ago)
       TheatreWatchingUser.create!(theatre:, user: member, expires_at: 10.seconds.from_now)
@@ -386,7 +403,7 @@ RSpec.describe 'Theatres API', type: :request do
 
     it 'removes the current user vote' do
       expect {
-        delete "/theatres/#{theatre.id}/skip_vote"
+        delete "/theatres/#{theatre.id}/skip_vote", params: { post_id: requested_post_id }
       }.to change(TheatreSkipVote, :count).by(-1)
 
       expect(response).to have_http_status(:ok)
@@ -397,6 +414,43 @@ RSpec.describe 'Theatres API', type: :request do
         'voted' => false
       )
     end
+
+    it 'does not remove a vote when requested post is no longer current' do
+      theatre.update!(current_post: second_niconico_post)
+
+      expect {
+        delete "/theatres/#{theatre.id}/skip_vote", params: { post_id: requested_post_id }
+      }.not_to change(TheatreSkipVote, :count)
+
+      expect(response).to have_http_status(:conflict)
+      expect(json['post_id']).to eq(second_niconico_post.id)
+    end
+  end
+
+  describe 'GET /theatres/:id/skip_events' do
+    before do
+      sign_in_as(member)
+    end
+
+    it 'does not expose skip voters' do
+      event = TheatreSkipEvent.create!(
+        theatre:,
+        post: niconico_post,
+        skipped_by_user: member,
+        created_at: Time.current
+      )
+      TheatreSkipEventVoter.create!(theatre_skip_event: event, user: member)
+
+      get "/theatres/#{theatre.id}/skip_events"
+
+      expect(response).to have_http_status(:ok)
+      expect(json.first).to include(
+        'id' => event.id,
+        'theatre_id' => theatre.id
+      )
+      expect(json.first).not_to have_key('voters')
+      expect(json.first).not_to have_key('skipped_by_user')
+    end
   end
 
   describe 'GET /theatres/:id/post_selection_weights' do