search

Lit-innosence / tus_yuurikai_system

MIT License
1 stars 0 forks source link

Feature/user-register #8

Closed Dot-P closed 1 month ago

Dot-P commented 1 month ago

Requestの処理

概要

新しく「/locker/user-register」を追加。それに伴い、Request jsonを定義。

テスト状態

swagger-ui上でrequest内の情報が正しく処理できているかを確認

要望

コードレビュー (コードの一部にすぎないので、テスト等は必要ありません)

Dot-P commented 1 month ago

dockerの起動 (初回)

$ docker run --name [任意のコンテナ名] \
             -v [任意のボリューム名]:/var/lib/postgresql/data \
             -e POSTGRES_USER=[任意のユーザ名] \
             -e POSTGRES_PASSWORD=[任意のパスワード] \
             -e POSTGRES_DB=[任意のDB名] \
             -p 5432:5432 \
             postgres:15-bullseye

dockerの起動、停止 (二回目以降)

$ docker start [任意のコンテナ名] $ docker stop [任意のコンテナ名]

SQLを直接叩く方法

$ psql -h localhost -p 5432 -d [任意のDB名]

dieselのセットアップ

この記事を参照 (migrationファイルをもう一回作成しないように注意)

Dot-P commented 1 month ago

Screenshot from 2024-08-13 18-56-49 Screenshot from 2024-08-13 19-04-50 キー制約これでいいかな？ idを何によって定めるのかはリサーチしてまとめておきます。

Lit-innosence commented 1 month ago

キー制約これでいいかな？ idを何によって定めるのかはリサーチしてまとめておきます。

大丈夫だと思います． pair_idは例えば"0000-0000"のようなidをつけることを考えてSTRING型にしたっていう話だったよね

Dot-P commented 1 month ago

autoincrementにするかUUIDにするか、その他にするか、後で考えればいいという結論になった気がする。 ~~今考えると、ULIDが良きだと感じたんだが、どうだろうか。 パフォーマンス的にも劣ってなくて、同時挿入による異常も発生しにくいと思う。~~ postgresにはULIDが実装されていないので面倒。UUIDを選び、パーフォーマンスよりも頑健性を求めたほうがいいと感じた。(データ数が異常までに増加することはここ数年では考えられないため) 参考資料1: ID生成方法についてあれこれ 参考資料2: Postgres と MySQL における id, created_at, updated_at に関するベストプラクティス

Dot-P commented 1 month ago

結論として、

created_atやupdated_atでソート可能であるので、UUID v7である必要がなく、実装を複雑化させる必要がない。postgresの推奨がv4であることを踏まえて選択。 また、timestamptzにしたのは、SELECT でクライアント時刻を考慮して日付時刻文字列表示するなど、timestampの上位互換であることを踏まえて選択。

Dot-P commented 1 month ago

update_atの自動更新はpostgresはサポートしていないみたいなので、rustで関数を作ろうと思ってます

Dot-P commented 1 month ago

Screenshot from 2024-08-13 18-56-49

nameはfamily_nameとgiven_nameに分けたほうがいいよね？ とりあえず、分けておきました

Dot-P commented 1 month ago

DB接続・Insert処理

概要

Lit-innosence commented 1 month ago

id選定とcreated_atの型について，現時点では異論ないです． コード内の変数名は問題ないと思います． 細かいですがレビューしたので確認お願いします． また，SQLのcreateが何を指すのかがわからなかったのでもう少し詳細にお願いします．

フィルターを使って特定のcommitに限定すると少し見やすいかも🦆

すごく見やすくなった．ありがとう

Dot-P commented 1 month ago

また，SQLのcreateが何を指すのかがわからなかったのでもう少し詳細にお願いします.

create table ~ のファイル群のこと。migrationフォルダの中の up.sql (2つ) が該当します。

Lit-innosence commented 1 month ago

create table ~ のファイル群のこと。migrationフォルダの中の up.sql (2つ) が該当します。

ありがとうございます． create文確認しました．問題ないと思います．

Dot-P commented 1 month ago

Upsert処理・ステータスコード・エラーハンドリング

概要

テスト状態

目視での確認。同じ学籍番号の情報がPOSTされたとき、updated_atが更新されていることを確認。 Student_Pairの処理を追加後、ユニットテストを行う予定。

要望

コードレビュー

参考資料：upsert処理-公式ドキュメント 参考資料：ステータスコード-公式ドキュメント

Dot-P commented 1 month ago

student_pairのInsert処理

概要

Lit-innosence commented 1 month ago

上記コードレビューは確認と質問なのでコードの変更は必要ありません． 構成変更によって既存のデータベースやORM処理に影響が及ぶ部分がないため，構成変更へ移って問題ない状態だと判断します．

Dot-P commented 1 month ago

構成変更に移ろうと思ったのですが、メール認証を完成させる必要があります。 メール認証に必要な情報が何か分からない、一時データベースに何が入るか分からないといった問題を抱えていて、データベースを構築できないといった問題があります。 そのため、先にメール認証を作成しようと思っているのですがいかがでしょうか？

時間的に次のミーティングまでの完成は難しいと思っています。 APIごとに分担したいので、一度マージさせ、メール認証(送受信)が完成しマージした後にリファクタ要件として行うのがベストであると思っています。

Lit-innosence commented 1 month ago

メール認証で変更されるファイルも多いと思うので、コンフリクトを避けるためにも一度マージするべきだと思います。

Dot-P commented 1 month ago

検討事項