From b1362d327c2b7502197cd61e075a633323c3a1f3 Mon Sep 17 00:00:00 2001 From: miteruzo Date: Sat, 6 Jun 2026 20:29:34 +0900 Subject: [PATCH] #302 --- .../theatre_comments_controller.rb | 1 + .../theatre_skip_events_controller.rb | 4 +- .../app/controllers/theatres_controller.rb | 29 +++++++++- .../spec/requests/theatre_comments_spec.rb | 40 +++++++++++++ backend/spec/requests/theatres_spec.rb | 58 ++++++++++++++++++- .../src/pages/theatres/TheatreDetailPage.tsx | 22 ++++--- frontend/src/types.ts | 2 - 7 files changed, 138 insertions(+), 18 deletions(-) diff --git a/backend/app/controllers/theatre_comments_controller.rb b/backend/app/controllers/theatre_comments_controller.rb index 7cbaa40..599a57e 100644 --- a/backend/app/controllers/theatre_comments_controller.rb +++ b/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! diff --git a/backend/app/controllers/theatre_skip_events_controller.rb b/backend/app/controllers/theatre_skip_events_controller.rb index 4ee35da..18ec914 100644 --- a/backend/app/controllers/theatre_skip_events_controller.rb +++ b/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 } diff --git a/backend/app/controllers/theatres_controller.rb b/backend/app/controllers/theatres_controller.rb index 8c864e2..10f4455 100644 --- a/backend/app/controllers/theatres_controller.rb +++ b/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 diff --git a/backend/spec/requests/theatre_comments_spec.rb b/backend/spec/requests/theatre_comments_spec.rb index 856b309..5f41395 100644 --- a/backend/spec/requests/theatre_comments_spec.rb +++ b/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 diff --git a/backend/spec/requests/theatres_spec.rb b/backend/spec/requests/theatres_spec.rb index 16d0de7..00ab143 100644 --- a/backend/spec/requests/theatres_spec.rb +++ b/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 diff --git a/frontend/src/pages/theatres/TheatreDetailPage.tsx b/frontend/src/pages/theatres/TheatreDetailPage.tsx index bf68703..6a7d103 100644 --- a/frontend/src/pages/theatres/TheatreDetailPage.tsx +++ b/frontend/src/pages/theatres/TheatreDetailPage.tsx @@ -55,9 +55,9 @@ const LAYOUT_STORAGE_KEY = 'theatre-layout-mode' const TAG_FLOW_STORAGE_KEY = 'theatre-tag-flow' const LAYOUT_LABELS: Record = { - threeColumns: '3 コラム', - tagsBottom: 'タグ下', - commentsBottom: 'コメント下' } + threeColumns: '3 列', + tagsBottom: '2 列(コメント欄)', + commentsBottom: '2 列(タグ欄)' } const TAG_FLOW_LABELS: Record = { vertical: 'タグ縦', @@ -118,8 +118,9 @@ const tagsByCategory = (tags: Tag[]): Partial> => { } -const TagList: FC<{ tags: Tag[]; compact?: boolean; flow?: TagFlow }> = -({ tags, compact, flow = 'vertical' }) => { +const TagList: FC<{ tags: Tag[]; compact?: boolean; flow?: TagFlow }> = ( + { tags, compact, flow = 'vertical' }, +) => { const grouped = tagsByCategory (tags) if (flow === 'horizontal') @@ -463,8 +464,10 @@ const TheatreDetailPage: FC = ({ user }: Props) => { { const nextInfo = theatreInfo.skipVote.voted - ? await apiDelete (`/theatres/${ id }/skip_vote`) - : await apiPut (`/theatres/${ id }/skip_vote`) + ? await apiDelete ( + `/theatres/${ id }/skip_vote`, { params: { post_id: post.id } }) + : await apiPut ( + `/theatres/${ id }/skip_vote`, { post_id: post.id }) applyTheatreInfo (nextInfo) @@ -477,6 +480,9 @@ const TheatreDetailPage: FC = ({ user }: Props) => { } catch (error) { + if (isApiError (error) && error.response?.status === 409) + applyTheatreInfo (await apiPut (`/theatres/${ id }/watching`)) + console.error (error) } finally @@ -635,7 +641,7 @@ const TheatreDetailPage: FC = ({ user }: Props) => { {theatreInfo.watchingUsers.map (watchingUser => (
{userName (watchingUser)} - {watchingUser.id === user?.id && 自分} + {watchingUser.id === user?.id && お前}
))} ) diff --git a/frontend/src/types.ts b/frontend/src/types.ts index 49dc634..746991c 100644 --- a/frontend/src/types.ts +++ b/frontend/src/types.ts @@ -262,8 +262,6 @@ export type TheatreSkipEvent = { id: number theatreId: number post: Post - skippedByUser: Pick - voters: Pick[] tags: Tag[] programmePosition: number | null createdAt: string }