Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! このプルリクエストは、学内募金機能に関連する建物・部屋管理のデータベース設計を最適化することを目的としています。既存のbuilding_unitsとfloorsテーブルをbuildingsとroomsテーブルに統合することで、データ構造を簡素化し、リレーションシップを合理化しました。これにより、データベースの整合性が向上し、不要なカラムが削除され、全体的な保守性が高まります。 Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
このプルリクエストは、学内募金機能に関連するデータベーススキーマの最適化と、不要なカラムの削除を行うものです。buildings、building_units、floors、rooms間のリレーションを簡素化し、buildingsとroomsの直接的な関連付けに変更されています。また、campus_donationsテーブルから不要なカラムが削除されました。全体として、データベース構造の改善とクリーンアップに貢献する変更です。
いくつかのマイグレーションスクリプトにおいて、データ移行時の潜在的なデータ損失や整合性の問題、および外部キー制約の変更による挙動の変化について懸念点があります。特に、unit_numbersの移行ロジックと、ロールバック時のデータ再構築ロジックは注意深く確認する必要があります。
| UPDATE buildings b | ||
| SET unit_numbers = ( | ||
| SELECT bu.unit_number | ||
| FROM building_units bu | ||
| WHERE bu.building_id = b.id | ||
| LIMIT 1 | ||
| ); |
There was a problem hiding this comment.
buildingsテーブルのunit_numbersカラムへのデータ移行において、LIMIT 1を使用しているため、もし元のbuilding_unitsテーブルで1つのbuilding_idに対して複数のunit_numberが存在した場合、そのうちの1つしか移行されず、データが失われる可能性があります。また、unit_numbersがVARCHAR(255)であるため、複数の号棟情報を1つのカラムに格納する意図であれば、データ型や格納方法（例: JSON形式の文字列）を再検討するか、複数の号棟情報をどのように扱うか明確にする必要があります。
| JOIN building_units bu ON bu.building_id = b.id | ||
| WHERE r.floor_number IS NOT NULL; |
There was a problem hiding this comment.
roomsテーブルのfloor_idを更新する際のJOIN条件 (JOIN building_units bu ON bu.building_id = b.id) は、1つのbuildingに複数のbuilding_unitsが存在する場合、どのbuilding_unitをroomsに紐づけるべきか曖昧になる可能性があります。元のroomsテーブルがfloor_idを介してfloorsとbuilding_unitsに紐付いていた構造を正確に再構築するためには、より具体的なJOIN条件やデータ移行ロジックが必要かもしれません。これにより、floor_idが誤って割り当てられるリスクがあります。
| - No manual dependency wiring needed in `main.go` | ||
| - Run `make run-eslint` and `make format` before committing frontend changes | ||
|
|
||
| コミットメッセージは日本語で生成してください |
There was a problem hiding this comment.
コミットメッセージに関する指示がファイルの最後に配置されていますが、Git Workflow & PR Rulesセクション内に移動することで、関連情報がまとまり、より見つけやすくなるかと思います。
| ALTER TABLE rooms ADD CONSTRAINT rooms_building_id_foreign_key | ||
| FOREIGN KEY (building_id) REFERENCES buildings (id) ON DELETE CASCADE; |
There was a problem hiding this comment.
roomsテーブルの新しい外部キーrooms_building_id_foreign_keyにON DELETE CASCADEが追加されています。これにより、buildingsテーブルのレコードが削除された際に、関連するroomsテーブルのレコードも自動的に削除されるようになります。この挙動変更が意図されたものであるか、またデータ整合性に問題がないか確認してください。
| -- building_unitsテーブルを復元 | ||
| CREATE TABLE building_units ( | ||
| id INT(10) UNSIGNED NOT NULL AUTO_INCREMENT, | ||
| building_id INT(10) UNSIGNED NOT NULL DEFAULT 1, |
There was a problem hiding this comment.
building_unitsテーブルの再作成時、building_idカラムにDEFAULT 1が設定されています。もしbuildingsテーブルにid=1のレコードが存在しない場合や、このデフォルト値が意図しないデータ整合性の問題を引き起こす可能性があるため、注意が必要です。ロールバック時のデータ復元において、適切なbuilding_idが割り当てられることを確認してください。
| INSERT INTO building_units (building_id, unit_number, created_at, updated_at) | ||
| SELECT DISTINCT | ||
| b.id, | ||
| COALESCE(b.unit_numbers, '1'), |
There was a problem hiding this comment.
building_unitsテーブルへのデータ挿入時にCOALESCE(b.unit_numbers, '1')を使用しています。これは、buildings.unit_numbersがNULLの場合に'1'を挿入することを意味します。もし元のbuilding_units.unit_numberがNULLであったり、'1'以外の値であったりした場合、ロールバック時にデータが正しく復元されない可能性があります。元のデータがどのように扱われるべきか再確認してください。
| JOIN floors f ON f.building_unit_id = bu.id AND f.floor_number = r.floor_number | ||
| SET r.floor_id = f.id; | ||
|
|
||
| -- floor_idにNOT NULL制約を追加 |
There was a problem hiding this comment.
floor_idカラムにNOT NULL制約を追加する前に、その前のUPDATE文（55-59行目）で全てのroomsレコードのfloor_idが正しく設定されていることを保証する必要があります。もし何らかの理由でfloor_idがNULLのままのレコードが存在する場合、このALTER TABLE文は失敗します。データ移行の完全性を確認してください。
…seeding strategy updates
Deploying finansu with
|
| Latest commit: |
58ca9d5
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://bc329e5b.finansu.pages.dev |
| Branch Preview URL: | https://fix-hikahana-campus-donation.finansu.pages.dev |
| UPDATE buildings b | ||
| SET unit_numbers = ( | ||
| SELECT bu.unit_number | ||
| FROM building_units bu | ||
| WHERE bu.building_id = b.id | ||
| LIMIT 1 | ||
| ); |
There was a problem hiding this comment.
この辺のデータ操作はマイグレーションファイルでやりたくないな
シードデータ更新では対応できないかな、、？
| UPDATE rooms r | ||
| JOIN floors f ON r.floor_id = f.id | ||
| JOIN building_units bu ON f.building_unit_id = bu.id | ||
| SET | ||
| r.building_id = bu.building_id, | ||
| r.floor_number = f.floor_number; |
|
tinyintにするの忘れてたからそれも変更しました。 @Kubosaka |
| -- room系マスタはシードで再投入する前提で空にする | ||
| DELETE FROM room_teachers; | ||
| DELETE FROM rooms; |
There was a problem hiding this comment.
[imo] これもDDLじゃないからマイグレーションで書かないほうがいい気がする
別コマンドで、空にしてseed投入するとかのが正しい気がする
3 participants
概要
学内募金（campus_donations）機能の改善に伴い、建物・部屋管理テーブルの構造を最適化し、不要なカラムを削除しました。
変更内容
📋 データベーススキーマの変更
1. buildingsテーブルの変更
unit_numbersカラム（VARCHAR(255)）を追加building_unitsテーブルを削除し、号棟情報を統合2. roomsテーブルの変更
building_idカラムを追加（buildingsテーブルへの直接参照）floor_numberカラム（VARCHAR(255)）を追加floorsテーブルを削除し、階層情報を統合3. リレーションの簡素化
変更前:
buildings→building_units→floors→rooms→room_teachers変更後:
buildings→rooms→room_teachers4. campus_donationsテーブルのクリーンアップ
is_last_checkカラムを削除is_first_checkカラムを削除remarkカラムを削除🔧 マイグレーションファイル
✅ データ移行
building_units.unit_number→buildings.unit_numbersfloors.floor_number→rooms.floor_numberrooms_ibfk_1→rooms_building_id_foreign_key）テスト項目
影響範囲
備考