DB seederを追加#2979
Conversation
📝 WalkthroughWalkthroughThis pull request introduces a new database seeding feature to the application. It adds a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/seed.go`:
- Around line 191-203: The seed command currently hard-codes creation of an
admin user "traq"/"traq" using file.GenerateIconFile and repo.CreateUser
(CreateUserArgs with role.Admin); change this so the admin user is only created
in non-production/dev environments and the password is not embedded: add a check
for the runtime environment (e.g., config.Environment or an
IsDevelopment/isLocal guard) and skip creating the default admin unless that
check passes OR an explicit admin password is provided via a CLI flag (e.g.,
--admin-password) or env var (e.g., ADMIN_PASSWORD); read the password from that
flag/env var and pass it into repo.CreateUser instead of the hard-coded "traq"
value, and ensure role.Admin is only assigned under the gated condition.
- Around line 406-416: The backfill can return multiple rows per channel when
several messages share MAX(created_at); change the INSERT SELECT so each channel
yields exactly one row by choosing a deterministic tie-breaker (e.g., the
largest message id among rows with the max created_at). Replace the SELECT block
with a grouped query that joins the max created_at per channel and then GROUPs
BY m.channel_id selecting MAX(m.id) AS message_id and MAX(m.created_at) AS
date_time (so each channel contributes a single (channel_id, message_id,
date_time) tuple), then keep the ON DUPLICATE KEY UPDATE as-is; refer to the
channel_latest_messages table and the messages table/columns used in this diff
and note the primary key on channel_id defined in model/messages.go for locating
where to apply the change.
- Around line 154-156: Replace the detached context creation in the RunE of the
seed command with the Cobra-provided context (use cmd.Context()) and ensure that
all GORM operations use db.WithContext(ctx) rather than raw db, and that any
helper functions invoked from RunE that perform I/O accept and propagate this
ctx; specifically update the RunE closure to obtain ctx := cmd.Context(), change
direct db.Exec/raw writes and batch inserts to db.WithContext(ctx).Exec /
db.WithContext(ctx).Create (or equivalent), and if functions called from this
block (including any wrappers around batch inserts) don't accept context, add
context.Context as the first parameter and pass ctx through. Ensure
getCLILoggers() usage remains unchanged but that logger-using calls also take
ctx where applicable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4fbe8e0e-74f6-482a-ba15-ca0ceb03f408
📒 Files selected for processing (3)
Makefilecmd/root.gocmd/seed.go
| RunE: func(_ *cobra.Command, _ []string) error { | ||
| ctx := context.Background() | ||
| logger, gormLogger := getCLILoggers() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the file exists and get its line count
wc -l cmd/seed.goRepository: traPtitech/traQ
Length of output: 74
🏁 Script executed:
# Now let's look at the specific sections mentioned in the review
# Lines 154-156 (RunE definition)
sed -n '154,156p' cmd/seed.goRepository: traPtitech/traQ
Length of output: 181
🏁 Script executed:
# Lines around 154-156 for broader context
sed -n '150,165p' cmd/seed.goRepository: traPtitech/traQ
Length of output: 564
🏁 Script executed:
# Lines 385-417 (first batch of DB operations)
sed -n '385,420p' cmd/seed.goRepository: traPtitech/traQ
Length of output: 1468
🏁 Script executed:
# Lines 468-481 (second batch of DB operations)
sed -n '465,485p' cmd/seed.goRepository: traPtitech/traQ
Length of output: 757
🏁 Script executed:
# Verify this is not a test file
file cmd/seed.go | grep -i testRepository: traPtitech/traQ
Length of output: 101
Propagate the Cobra context into the raw GORM writes.
ctx := context.Background() detaches this command from caller cancellation, and the largest DB operations below use db directly instead of db.WithContext(ctx). If this command is wrapped with ExecuteContext or cancelled by a higher layer, the batch inserts and raw Exec will still run uncancelled.
Suggested shape
- RunE: func(_ *cobra.Command, _ []string) error {
- ctx := context.Background()
+ RunE: func(cmd *cobra.Command, _ []string) error {
+ ctx := cmd.Context()
logger, gormLogger := getCLILoggers()
...
+ seedDB := db.WithContext(ctx)
...
- if err := db.Omit(clause.Associations).Create(&batch).Error; err != nil {
+ if err := seedDB.Omit(clause.Associations).Create(&batch).Error; err != nil {
...
- if err := db.Exec(`...`).Error; err != nil {
+ if err := seedDB.Exec(`...`).Error; err != nil {
...
- if err := db.Omit(clause.Associations).Create(&reactionBatch).Error; err != nil {
+ if err := seedDB.Omit(clause.Associations).Create(&reactionBatch).Error; err != nil {Per coding guidelines for **/*.go: Functions performing I/O or network requests must accept context.Context as their first parameter, and the received context must be passed down to all subsequent calls. Applies to lines 155, 385–417, and 468–481.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/seed.go` around lines 154 - 156, Replace the detached context creation in
the RunE of the seed command with the Cobra-provided context (use cmd.Context())
and ensure that all GORM operations use db.WithContext(ctx) rather than raw db,
and that any helper functions invoked from RunE that perform I/O accept and
propagate this ctx; specifically update the RunE closure to obtain ctx :=
cmd.Context(), change direct db.Exec/raw writes and batch inserts to
db.WithContext(ctx).Exec / db.WithContext(ctx).Create (or equivalent), and if
functions called from this block (including any wrappers around batch inserts)
don't accept context, add context.Context as the first parameter and pass ctx
through. Ensure getCLILoggers() usage remains unchanged but that logger-using
calls also take ctx where applicable.
| // 管理者ユーザー traq/traq の作成 | ||
| adminIconID, err := file.GenerateIconFile(ctx, fm, "traq") | ||
| if err != nil { | ||
| return fmt.Errorf("failed to generate admin icon: %w", err) | ||
| } | ||
| if u, err := repo.CreateUser(ctx, repository.CreateUserArgs{ | ||
| Name: "traq", | ||
| Password: "traq", | ||
| Role: role.Admin, | ||
| IconFileID: adminIconID, | ||
| }); err != nil { | ||
| logger.Warn("failed to create traq admin user (may already exist)", zap.Error(err)) | ||
| } else { |
There was a problem hiding this comment.
Don't hard-code a known admin login in a general-purpose seed command.
This creates traq / traq with role.Admin against whatever database the active config points at. If seed is ever run outside an isolated local DB, it introduces a predictable privileged credential. Please gate this to dev-only environments and require the password from an explicit flag or env var instead of embedding it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/seed.go` around lines 191 - 203, The seed command currently hard-codes
creation of an admin user "traq"/"traq" using file.GenerateIconFile and
repo.CreateUser (CreateUserArgs with role.Admin); change this so the admin user
is only created in non-production/dev environments and the password is not
embedded: add a check for the runtime environment (e.g., config.Environment or
an IsDevelopment/isLocal guard) and skip creating the default admin unless that
check passes OR an explicit admin password is provided via a CLI flag (e.g.,
--admin-password) or env var (e.g., ADMIN_PASSWORD); read the password from that
flag/env var and pass it into repo.CreateUser instead of the hard-coded "traq"
value, and ensure role.Admin is only assigned under the gated condition.
| if err := db.Exec(` | ||
| INSERT INTO channel_latest_messages (channel_id, message_id, date_time) | ||
| SELECT m.channel_id, m.id, m.created_at | ||
| FROM messages m | ||
| INNER JOIN ( | ||
| SELECT channel_id, MAX(created_at) AS max_created | ||
| FROM messages WHERE deleted_at IS NULL GROUP BY channel_id | ||
| ) latest ON m.channel_id = latest.channel_id AND m.created_at = latest.max_created | ||
| WHERE m.deleted_at IS NULL | ||
| ON DUPLICATE KEY UPDATE message_id = VALUES(message_id), date_time = VALUES(date_time) | ||
| `).Error; err != nil { |
There was a problem hiding this comment.
Make the channel_latest_messages backfill deterministic.
This query only keys on MAX(created_at). With bulk inserts, multiple messages in a channel can share that max timestamp; then the SELECT returns multiple rows for the same channel_id, and because channel_latest_messages is keyed only by channel_id in model/messages.go:31-41, the final message_id becomes arbitrary. Please add a deterministic tie-breaker so each channel contributes exactly one row.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/seed.go` around lines 406 - 416, The backfill can return multiple rows
per channel when several messages share MAX(created_at); change the INSERT
SELECT so each channel yields exactly one row by choosing a deterministic
tie-breaker (e.g., the largest message id among rows with the max created_at).
Replace the SELECT block with a grouped query that joins the max created_at per
channel and then GROUPs BY m.channel_id selecting MAX(m.id) AS message_id and
MAX(m.created_at) AS date_time (so each channel contributes a single
(channel_id, message_id, date_time) tuple), then keep the ON DUPLICATE KEY
UPDATE as-is; refer to the channel_latest_messages table and the messages
table/columns used in this diff and note the primary key on channel_id defined
in model/messages.go for locating where to apply the change.
1 participant
本番と同じぐらいの重さにするためのseederを追加
Summary by CodeRabbit
seedcommand to populate the database with synthetic test data.