重複したイベント/部屋の作成禁止

[ras0q]

close #369

[ras0q]

マイグレーションがまだ

[ras0q]

↓

[ras0q]

これ進捗部屋じゃなくてイベントだ

[hijiki51]

いま見かけたから聞くんですがこれってZoom/Discordの時部屋重複したりしますよね

イベントはいいけど部屋はいまの設計だとDBじゃなくてアプリケーション側で重複管理しないといけないと思います

[ras0q]

確かに全く同じ時間に違う名前のイベントが同じ部屋(zoom等含む)で開催されてたらそうなるか

[hijiki51]

[hijiki51]

あとこういう風に「未定」があります

[ras0q]

type Room struct {
	ID             uuid.UUID `gorm:"type:char(36);primaryKey"`
	Place          string    `gorm:"type:varchar(32); uniqueIndex:idx_place_time_start_time_end"`
	Verified       bool
	TimeStart      time.Time `gorm:"type:DATETIME; index; uniqueIndex:idx_place_time_start_time_end"`
	TimeEnd        time.Time `gorm:"type:DATETIME; index; uniqueIndex:idx_place_time_start_time_end"`
	Events         []Event   `gorm:"->; constraint:-"` // readOnly
	Admins         []RoomAdmin
	CreatedByRefer uuid.UUID `gorm:"type:char(36);" cvt:"CreatedBy, <-"`
	CreatedBy      User      `gorm:"->; foreignKey:CreatedByRefer; constraint:OnDelete:CASCADE;" cvt:"->"`
	Model          `cvt:"->"`
}

Eventsあるけど同じ時間のイベントはここに入って部屋自体が重複することはないみたいなロジックがあればよさそう
まだ見てないけど今は多分進捗部屋のときしかここ複数になりえない気がする

[hijiki51]

Roomに対してイベントの重複がないってロジックならよいと思います。
ただし、述べてもらっている通り、進捗部屋以外という条件が必要です。

いま進捗部屋以外の部屋って登録してましたっけ？ OR 手動で(CSVではなく)簡単に登録できるようになってましたっけ
（ないなら今登録されてるのは進捗部屋だけなので重複管理いらないかなと）

[ras0q]

進捗部屋以外の部屋もDBには登録されてます
フロントエンドからは部屋名だけ渡されるのでその都度部屋が作成されてるっぽい

[ras0q]

手元で動かしてたけどこれm.InitSchema動かなくてインデックス貼れてないな
なぜさっき貼れてたかわからないけど

→ 手元で繰り返しv14実行してたからでした、migrationテーブルから14消したらできた

→ 動いてるけどv14()する前にインデックス貼ろうとして失敗してそう

[ras0q]

v13() v14()の中にインデックス追加する処理書くのって許されますか書いた

[ras0q]

• 名前、開始時刻、終了時刻が重複するイベントは作れない
• 名前、開始時刻、終了時刻が重複するイベントは作れない
• 名前(場所)、開始時刻、終了時刻 が重複する即時部屋は新規作成する代わりに既にある部屋を使う
• ↑のときに部屋のadminをどうにかする

[ras0q]

進捗部屋はIDを指定するからadminの変更等は考えなくていい

[ras0q]

部屋重複させてadmin増やしたらさすがに脆弱性生みそうだけどどうするのがいいんだろ
最初に作った人固定(今のまま)でいい？

[ras0q]

これは多分作成者が未定って指定しただけ(デフォルトではない)ので他の部屋と同じ扱いでよさそう

あとこういう風に「未定」があります

[oribe1115]

migrationの方はまだ見れてないのですが、現状で気になったところを書きました

[oribe1115]

これ、重複イベント/部屋が作成された際に最終的に返るstatus codeで500になってそうです？
アプリケーションのロジックで弾いているものなので、Internal Server ErrorではなくBad Requestを返すようにするのが適切かと思います

[ras0q]

レビューありがとうございます:bow:
イベント重複のときは400が返るようになっているはずですね:thiking:

knoq_development_app         | 2023-05-19T15:27:13.220+0900     INFO    router/middleware.go:63 client error    {"field": {"requestMethod": "POST", "requestUrl": "/api/events", "requestSize": "260", "status": 400, "responseSize": "54", "userAgent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/115.0.0.0 Safari/537.36 Edg/115.0.0.0", "remoteIp": "172.26.0.1", "serverIp": "", "referer": "http://localhost:6006/events/new", "latency": "0.220820805s", "protocol": "HTTP/1.1", "error": "/srv/knoq/router/errors.go:117: bad request: duplicate entry"}}

[oribe1115]

遅くなりましたがmigrationの実装読みました
実行は試していませんが、記述とテスト結果的にはロジックよさそうです
v13の方のみにコメントしましたが、v14も同じコメントになりそうです

[ras0q]

@oribe1115 修正しました:pray:

[oribe1115]

よさそうです！

[ras0q]

(n)を付けたせいで文字数オーバーになるイベントがある