Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Offair talks having same track id and different conference_day_id #2002

Merged
merged 3 commits into from
Aug 12, 2023

Conversation

jacopen
Copy link
Collaborator

@jacopen jacopen commented Aug 11, 2023

Fix #2001

2001の修正方法として

  • 既に別日でOnAirのTalkがある場合は受け付けない
  • 別日でOnAirがあった場合は、そちらをオフにしてリクエスト側をオンにする

の2つのアプローチがあるが、UIは前者のアプローチでバリデーションしている。これはUIの構成上誤操作のリスクがあるので好ましい動作。

しかしAPIの利用シーン的には後者のほうが便利だし、誤操作のリスクはそれほど高くない(talk_idを明示するので曜日を誤指定することは考えづらい)
そのため、フィルタリング条件を変更して別日であっても自動的にオフになるようにした

@github-actions
Copy link

@github-actions github-actions bot added the reviewapps Build ReviewApp environment automatically if this label is granted label Aug 11, 2023
@github-actions
Copy link

Simplecov Report

Covered Threshold
61.43% 60%

@jacopen jacopen requested review from takaishi and a team August 11, 2023 08:19
Copy link
Contributor

@takaishi takaishi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

start_on_airメソッドで行っている他のconference dayに紐付くTalkでOnAirならエラーにする処理を消す必要がありそう

@@ -345,7 +345,7 @@ def start_streaming
ActiveRecord::Base.transaction do
other_talks_in_track = conference.tracks.find_by(name: track.name).talks
.accepted_and_intermission
.select { |t| t.conference_day.id == conference_day.id && t.id != id }
.select { |t| t.conference.id == conference.id && t.id != id }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

元々conferenceに紐付くtalkしか取得していないのでconference idの比較は不要な気がする。

Suggested change
.select { |t| t.conference.id == conference.id && t.id != id }
.select { |t| t.id != id }

@jacopen
Copy link
Collaborator Author

jacopen commented Aug 11, 2023

start_on_airメソッドで行っている他のconference dayに紐付くTalkでOnAirならエラーにする処理を消す必要がありそう

UI側に関しては挙動を変えなくていいので、そこはそのままでいいかなと

@jacopen jacopen requested a review from takaishi August 11, 2023 10:50
@takaishi
Copy link
Contributor

UI側に関しては挙動を変えなくていいので、そこはそのままでいいかなと

start_on_airではstart_streamingの実行前に他の日のOnAirをチェックしていて、OnAirならstart_streamingが実行されないのでstart_on_airを直さないといけないと思う。

https://github.com/cloudnativedaysjp/dreamkast/blob/main/app/controllers/admin/talks_controller.rb#L31-L54

@jacopen
Copy link
Collaborator Author

jacopen commented Aug 12, 2023

OnAirならstart_streamingが実行されないのでstart_on_airを直さないといけないと思う。

これは期待している挙動通りなので問題ないような?

@takaishi
Copy link
Contributor

あ、APIの時の挙動についての変更か。勘違いしていた。

@takaishi takaishi merged commit 95da6f7 into main Aug 12, 2023
4 checks passed
@takaishi takaishi deleted the fix-on-air-status branch August 12, 2023 04:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
reviewapps Build ReviewApp environment automatically if this label is granted
Projects
None yet
3 participants