-
Notifications
You must be signed in to change notification settings - Fork 0
Improvement - use sqlx methods #19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…y service (in future this will be a cron job that will be scheduled to run daily)
…andling while fetching repositoryDetails
…ndled in frontend
Repo layer code looks good to me. |
/review |
Changelist by BitoThis pull request implements the following key changes.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review Agent Run #16718b
Actionable Suggestions - 2
-
internal/repository/repository.go - 1
- SQL parameter mismatch in query execution · Line 57-57
-
internal/db/migrations/1750328591_add_column_contributors_url.down.sql - 1
- Migration rollback breaks application queries · Line 1-1
Additional Suggestions - 3
-
internal/db/migrations/1750328591_add_column_contributors_url.up.sql - 1
-
Redundant WHERE clause in UPDATE statement · Line 3-3The UPDATE statement uses `WHERE contributors_url IS NULL` but this condition will always be true for all existing rows since the column was just added without a default value. Remove the WHERE clause to update all existing rows: `UPDATE repositories SET contributors_url = '';`
Code suggestion
@@ -3,1 +3,1 @@ -UPDATE repositories SET contributors_url = '' WHERE contributors_url IS NULL; +UPDATE repositories SET contributors_url = '';
-
-
internal/pkg/apperrors/errors.go - 1
-
Typo in error message text · Line 40-40Typo in error message: `ErrContributionCreationFailed` contains "contrbitution" instead of "contribution". This will display incorrect error messages to users. Fix: Change to `errors.New("failed to create contribution")`
Code suggestion
@@ -40,1 +40,1 @@ - ErrContributionCreationFailed = errors.New("failed to create contrbitution") + ErrContributionCreationFailed = errors.New("failed to create contribution")
-
-
internal/app/bigquery/domain.go - 1
-
Duplicate struct definition across packages · Line 5-5Duplicate struct definition: `ContributionResponse` is already defined in `internal/app/contribution/domain.go` with identical fields. This creates type confusion and potential import conflicts when both packages are used together. Consider using a shared domain model or renaming one of the structs to avoid conflicts.
Code suggestion
@@ -5,1 +5,1 @@ -type ContributionResponse struct { +type BigQueryContributionResponse struct {
-
Review Details
-
Files reviewed - 24 · Commit Range:
3834565..187dc89
- .gitignore
- cmd/main.go
- go.mod
- go.sum
- internal/app/bigquery/domain.go
- internal/app/bigquery/service.go
- internal/app/contribution/domain.go
- internal/app/contribution/handler.go
- internal/app/contribution/service.go
- internal/app/dependencies.go
- internal/app/repository/domain.go
- internal/app/repository/handler.go
- internal/app/repository/service.go
- internal/app/router.go
- internal/config/app.go
- internal/config/bigquery.go
- internal/db/migrations/1750328591_add_column_contributors_url.down.sql
- internal/db/migrations/1750328591_add_column_contributors_url.up.sql
- internal/pkg/apperrors/errors.go
- internal/repository/base.go
- internal/repository/contribution.go
- internal/repository/domain.go
- internal/repository/repository.go
- internal/repository/user.go
-
Files skipped - 0
-
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- GOVULNCHECK (Security Vulnerability) - ✖︎ Failed
- OWASP (Security Vulnerability) - ✔︎ Successful
- SNYK (Security Vulnerability) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review
- Manually triggers a full AI review. -
/pause
- Pauses automatic reviews on this pull request. -
/resume
- Resumes automatic reviews. -
/resolve
- Marks all Bito-posted review comments as resolved. -
/abort
- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Default Agent
You can customize the agent settings here or contact your Bito workspace admin at [email protected].
Documentation & Help
|
||
fetchUsersContributedReposQuery = `SELECT * from repositories where id in (SELECT repository_id from contributions where user_id=$1);` | ||
|
||
fetchUserContributionsInRepoQuery = `SELECT * from contributions where repository_id=$1 and user_id=$2;` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SQL parameter mismatch: The query fetchUserContributionsInRepoQuery
expects repository_id
as the first parameter, but repoGithubId
is passed. This will cause incorrect query results or database errors. Fix by either modifying the query to use github_repo_id
or first converting repoGithubId
to repository_id
.
Code suggestion
Check the AI-generated fix before applying
fetchUserContributionsInRepoQuery = `SELECT * from contributions where repository_id=$1 and user_id=$2;` | |
fetchUserContributionsInRepoQuery = `SELECT c.* from contributions c JOIN repositories r ON c.repository_id = r.id WHERE r.github_repo_id=$1 and c.user_id=$2;` |
Code Review Run #16718b
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
@@ -0,0 +1 @@ | |||
ALTER TABLE repositories DROP COLUMN contributors_url; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The down migration drops the contributors_url
column, but the Go application code still references this column in SQL queries (createRepositoryQuery
) and struct definitions. This will cause runtime SQL errors when the migration is rolled back and the application tries to insert or query the non-existent column. Consider adding application code changes to handle the missing column or ensure migrations are coordinated with code deployments.
Code suggestion
Check the AI-generated fix before applying
ALTER TABLE repositories DROP COLUMN contributors_url; | |
-- WARNING: Rolling back this migration will break the application | |
-- Ensure application code is updated to remove contributors_url references before running this migration | |
ALTER TABLE repositories DROP COLUMN contributors_url; |
Code Review Run #16718b
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
Use sqlx methods instead of sql methods for simplicity.
Summary by Bito
This pull request enhances the application by integrating BigQuery functionality, refactoring database access with SQLx methods, and improving repository management components. It adds new endpoints for contributions and repository details while updating configuration structures and database migrations for better maintainability.