[command] import issues from file#16
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (18)
📝 WalkthroughWalkthroughAdds a Bitbucket issue importer CLI subcommand, refactors CLI command wiring into ChangesImporter + Task Kind + CLI refactor
Sequence DiagramsequenceDiagram
actor CLI as CLI User
participant CLIApp as CLI App
participant FX as FX Application
participant Importer as Importer Runnable
participant Parser as Bitbucket Parser
participant TaskSvc as Task Service
participant CommentSvc as Comment Service
participant DB as Database
CLI->>CLIApp: run "import --file=export.json --project=slug --default-user=1"
CLIApp->>FX: start import command (supplies config)
FX->>Importer: Run(ctx)
Importer->>Parser: parseExportFile(export.json)
Parser-->>Importer: Export{Issues, Comments}
Importer->>Importer: resolveUser(defaultUser)
loop For each Issue
Importer->>Importer: normalizePriority/Status/Kind
Importer->>TaskSvc: Import(task with explicit metadata)
TaskSvc->>DB: INSERT task (preserve timestamps/number)
DB-->>TaskSvc: inserted task
TaskSvc-->>Importer: created task (store mapping)
end
loop For each Comment
Importer->>Importer: lookup task ID by issue ID
alt task found
Importer->>CommentSvc: Import(comment)
CommentSvc->>DB: INSERT comment (preserve timestamps)
DB-->>CommentSvc: inserted comment
CommentSvc-->>Importer: comment created
else task missing
Importer->>Importer: log warning and skip
end
end
Importer->>FX: signal Shutdown
FX-->>CLIApp: return import counts
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Review rate limit: 0/1 reviews remaining, refill in 42 minutes and 5 seconds.Comment |
🤖 Pull request artifacts
|
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/tasks/repository.go (1)
179-207:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
kindupdates are accepted but never persisted
TaskUpdate.Kindis now wired through DTO/domain, butRepository.Updatenever applieskindin the SQLSETclause, soPATCHrequests that only changekindare silently ignored.💡 Proposed fix
if update.Status != nil { query = query.Set("status = ?", *update.Status) } + if update.Kind != nil { + query = query.Set("kind = ?", *update.Kind) + } if update.AssigneeID != nil { if *update.AssigneeID == 0 { query = query.Set("assignee_id = NULL")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/tasks/repository.go` around lines 179 - 207, The Update implementation in Repository (in internal/tasks/repository.go, around the code constructing query in Update) never applies TaskUpdate.Kind to the SQL SET clause so kind changes are not persisted; add a branch similar to Title/Description that checks if update.Kind != nil and then call query = query.Set("kind = ?", *update.Kind) (or if your domain allows an empty/nil string to clear the field, handle that by setting "kind = NULL" when *update.Kind == ""), ensuring the property is included before executing the update.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/commands/importer/import.go`:
- Around line 74-87: Replace the current select that listens on app.Done() with
code that reads the shutdown signal from app.Wait(): block on ctx.Done() or sig
:= <-app.Wait(), then after sig is received save its ShutdownSignal.ExitCode;
call stop via app.Stop(stopCtx) as before, and finally if
sig.ShutdownSignal.ExitCode is non-zero return an error representing that exit
code (e.g., fmt.Errorf with the code) so importer.Run() failures are propagated
instead of returning nil. Ensure you reference ctx, app.Wait(), sig (the value
from app.Wait()), sig.ShutdownSignal.ExitCode, and app.Stop(...) when
implementing this change.
In `@internal/commands/importer/importer.go`:
- Around line 224-237: The resolveUser function currently only supports numeric
IDs but its comment claims it supports username/email; make the behavior and
docs consistent by updating the function to be ID-only: change the doc comment
for resolveUser to state "finds a user by ID only", and when
strconv.Atoi(userStr) fails return a clear error like "user must be an ID"
(preserve wrapping of the parse error), and keep using usersSvc.GetByID(ctx,
int64(userID)) to fetch the user; refer to resolveUser and users.Service.GetByID
in your changes so callers and docs are no longer misleading.
In `@internal/db/migrations/20260429000000_add_task_kind.sql`:
- Line 4: The new column addition ADD COLUMN `kind` VARCHAR(255) NOT NULL
DEFAULT 'Task' is too permissive; change the migration to constrain `kind` to
the allowed domain (like existing `status`/`priority`) by using a SQL ENUM
(e.g., CREATE TYPE task_kind AS ENUM(...)) or a strict CHECK constraint (e.g.,
CHECK(kind IN (...))) and set the DEFAULT to a valid member; update the ALTER
TABLE/ADD COLUMN statement to reference that enum/type or include the CHECK, and
ensure any backfill/rollback logic in the migration handles existing rows and
uses the same constraint name (reference the `kind` column and the migration
that adds it).
In `@internal/server/tasks/dto.go`:
- Around line 209-218: The toTaskInput conversion is passing an empty kind
string into tasks.TaskInput when req.Kind is omitted; update the mapping so Kind
is set to the documented default (tasks.Task) when req.Kind is empty — for
example, after computing kind := tasks.Kind(req.Kind) coalesce or check
emptiness and set Kind: lo.CoalesceOrEmpty(kind, tasks.Task) (or equivalent
conditional) so the returned tasks.TaskInput.Kind never remains "" and defaults
to tasks.Task.
In `@internal/tasks/domain.go`:
- Around line 139-143: TaskInput.Validate currently only checks Status; add
validation for the Kind enum in the same validation block by calling the enum
validator (e.g., i.Kind.IsValid()) and returning fmt.Errorf("%w: invalid kind
value", ErrValidationFailed) on failure; place this check adjacent to the
existing Status validation in the TaskInput.Validate method so create-time
domain validation enforces Kind from non-HTTP code paths as well.
In `@pkg/bitbucket/issues.go`:
- Around line 36-37: Change the Comment model to make UpdatedOn nullable (use
*time.Time instead of time.Time) so JSON nulls are preserved during
unmarshalling (update the struct field UpdatedOn in the Comment type). Then
update the importer logic that reads comment timestamps (where it currently
assumes Comment.UpdatedOn is a time.Time) to handle a nil UpdatedOn by falling
back to the comment's CreatedOn value when setting the import audit timestamp.
Ensure any uses of Comment.UpdatedOn check for nil before dereferencing.
---
Outside diff comments:
In `@internal/tasks/repository.go`:
- Around line 179-207: The Update implementation in Repository (in
internal/tasks/repository.go, around the code constructing query in Update)
never applies TaskUpdate.Kind to the SQL SET clause so kind changes are not
persisted; add a branch similar to Title/Description that checks if update.Kind
!= nil and then call query = query.Set("kind = ?", *update.Kind) (or if your
domain allows an empty/nil string to clear the field, handle that by setting
"kind = NULL" when *update.Kind == ""), ensuring the property is included before
executing the update.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 862408b5-b679-404a-9318-74c0d9fecebf
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (18)
go.modinternal/app.gointernal/commands/commands.gointernal/commands/importer/config.gointernal/commands/importer/import.gointernal/commands/importer/importer.gointernal/commands/serve/serve.gointernal/comments/models.gointernal/comments/repository.gointernal/comments/service.gointernal/db/migrations/20260429000000_add_task_kind.sqlinternal/server/docs/docs.gointernal/server/tasks/dto.gointernal/tasks/domain.gointernal/tasks/models.gointernal/tasks/repository.gointernal/tasks/service.gopkg/bitbucket/issues.go
d254621 to
26a9deb
Compare
Summary by CodeRabbit