From f492c92d6d43d3ea3065aeffe979ce3daf4df4cc Mon Sep 17 00:00:00 2001 From: yuvrajangadsingh Date: Sun, 8 Mar 2026 14:55:29 +0530 Subject: [PATCH 01/10] feat: add inline comments on commit diffs Add a new commit_comment table and full CRUD flow to support inline comments on commit diff views, similar to PR review comments but standalone (no issue/PR required). Changes: - New CommitComment model with migration (v326) - Web handlers for rendering form, creating, and deleting comments - Diff context patch generation for comment positioning - Templates for commit comment conversation, individual comments, form - Modified diff section templates to render existing commit comments - Reuses existing JS for add-code-comment and delete-comment flows Closes #4898 --- models/migrations/migrations.go | 1 + models/migrations/v1_26/v326.go | 318 +------------------ models/migrations/v1_26/v331.go | 27 ++ models/repo/commit_comment.go | 155 +++++++++ routers/web/repo/commit.go | 30 ++ routers/web/repo/commit_comment.go | 156 +++++++++ routers/web/web.go | 3 + templates/repo/diff/box.tmpl | 2 +- templates/repo/diff/commit_comment_form.tmpl | 25 ++ templates/repo/diff/commit_comments.tmpl | 35 ++ templates/repo/diff/commit_conversation.tmpl | 25 ++ templates/repo/diff/new_commit_comment.tmpl | 5 + templates/repo/diff/section_split.tmpl | 37 ++- templates/repo/diff/section_unified.tmpl | 24 +- 14 files changed, 534 insertions(+), 309 deletions(-) create mode 100644 models/migrations/v1_26/v331.go create mode 100644 models/repo/commit_comment.go create mode 100644 routers/web/repo/commit_comment.go create mode 100644 templates/repo/diff/commit_comment_form.tmpl create mode 100644 templates/repo/diff/commit_comments.tmpl create mode 100644 templates/repo/diff/commit_conversation.tmpl create mode 100644 templates/repo/diff/new_commit_comment.tmpl diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index db74ff78d5..d155066e4a 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -405,6 +405,7 @@ func prepareMigrationTasks() []*migration { newMigration(328, "Add TokenPermissions column to ActionRunJob", v1_26.AddTokenPermissionsToActionRunJob), newMigration(329, "Add unique constraint for user badge", v1_26.AddUniqueIndexForUserBadge), newMigration(330, "Add name column to webhook", v1_26.AddNameToWebhook), + newMigration(331, "Add commit comment table", v1_26.AddCommitCommentTable), } return preparedMigrations } diff --git a/models/migrations/v1_26/v326.go b/models/migrations/v1_26/v326.go index 76532e2f85..387a183bbc 100644 --- a/models/migrations/v1_26/v326.go +++ b/models/migrations/v1_26/v326.go @@ -4,312 +4,24 @@ package v1_26 import ( - "errors" - "fmt" - "net/url" - "strconv" - "strings" - - "code.gitea.io/gitea/modules/json" - "code.gitea.io/gitea/modules/log" - "code.gitea.io/gitea/modules/setting" - api "code.gitea.io/gitea/modules/structs" - webhook_module "code.gitea.io/gitea/modules/webhook" + "code.gitea.io/gitea/modules/timeutil" "xorm.io/xorm" ) -const ( - actionsRunPath = "/actions/runs/" +func AddCommitCommentTable(x *xorm.Engine) error { + type CommitComment struct { + ID int64 `xorm:"pk autoincr"` + RepoID int64 `xorm:"INDEX NOT NULL"` + CommitSHA string `xorm:"VARCHAR(64) INDEX NOT NULL"` + TreePath string `xorm:"VARCHAR(4000) NOT NULL"` + Line int64 `xorm:"NOT NULL"` + PosterID int64 `xorm:"INDEX NOT NULL"` + Content string `xorm:"LONGTEXT NOT NULL"` + Patch string `xorm:"LONGTEXT"` + CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` + UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"` + } - // Only commit status target URLs whose resolved run ID is smaller than this threshold are rewritten by this partial migration. - // The fixed value 1000 is a conservative cutoff chosen to cover the smaller legacy run indexes that are most likely to be confused with ID-based URLs at runtime. - // Larger legacy {run} or {job} numbers are usually easier to disambiguate. For example: - // * /actions/runs/1200/jobs/1420 is most likely an ID-based URL, because a run should not contain more than 256 jobs. - // * /actions/runs/1500/jobs/3 is most likely an index-based URL, because a job ID cannot be smaller than its run ID. - // But URLs with small numbers, such as /actions/runs/5/jobs/6, are much harder to distinguish reliably. - // This migration therefore prioritizes rewriting target URLs for runs in that lower range. - legacyURLIDThreshold int64 = 1000 -) - -type migrationRepository struct { - ID int64 - OwnerName string - Name string -} - -type migrationActionRun struct { - ID int64 - RepoID int64 - Index int64 - CommitSHA string `xorm:"commit_sha"` - Event webhook_module.HookEventType - TriggerEvent string - EventPayload string -} - -type migrationActionRunJob struct { - ID int64 - RunID int64 -} - -type migrationCommitStatus struct { - ID int64 - RepoID int64 - TargetURL string -} - -type commitSHAAndRuns struct { - commitSHA string - runs map[int64]*migrationActionRun -} - -// FixCommitStatusTargetURLToUseRunAndJobID partially migrates legacy Actions -// commit status target URLs to the new run/job ID-based form. -// -// Only rows whose resolved run ID is below legacyURLIDThreshold are rewritten. -// This is because smaller legacy run indexes are more likely to collide with run ID URLs during runtime resolution, -// so this migration prioritizes that lower range and leaves the remaining legacy target URLs to the web compatibility logic. -func FixCommitStatusTargetURLToUseRunAndJobID(x *xorm.Engine) error { - jobsByRunIDCache := make(map[int64][]int64) - repoLinkCache := make(map[int64]string) - groups, err := loadLegacyMigrationRunGroups(x) - if err != nil { - return err - } - - for repoID, groupsBySHA := range groups { - for _, group := range groupsBySHA { - if err := migrateCommitStatusTargetURLForGroup(x, "commit_status", repoID, group.commitSHA, group.runs, jobsByRunIDCache, repoLinkCache); err != nil { - return err - } - if err := migrateCommitStatusTargetURLForGroup(x, "commit_status_summary", repoID, group.commitSHA, group.runs, jobsByRunIDCache, repoLinkCache); err != nil { - return err - } - } - } - return nil -} - -func loadLegacyMigrationRunGroups(x *xorm.Engine) (map[int64]map[string]*commitSHAAndRuns, error) { - var runs []migrationActionRun - if err := x.Table("action_run"). - Where("id < ?", legacyURLIDThreshold). - Cols("id", "repo_id", "`index`", "commit_sha", "event", "trigger_event", "event_payload"). - Find(&runs); err != nil { - return nil, fmt.Errorf("query action_run: %w", err) - } - - groups := make(map[int64]map[string]*commitSHAAndRuns) - for i := range runs { - run := runs[i] - commitID, err := getCommitStatusCommitID(&run) - if err != nil { - log.Warn("skip action_run id=%d when resolving commit status commit SHA: %v", run.ID, err) - continue - } - if commitID == "" { - // empty commitID means the run didn't create any commit status records, just skip - continue - } - if groups[run.RepoID] == nil { - groups[run.RepoID] = make(map[string]*commitSHAAndRuns) - } - if groups[run.RepoID][commitID] == nil { - groups[run.RepoID][commitID] = &commitSHAAndRuns{ - commitSHA: commitID, - runs: make(map[int64]*migrationActionRun), - } - } - groups[run.RepoID][commitID].runs[run.Index] = &run - } - return groups, nil -} - -func migrateCommitStatusTargetURLForGroup( - x *xorm.Engine, - table string, - repoID int64, - sha string, - runs map[int64]*migrationActionRun, - jobsByRunIDCache map[int64][]int64, - repoLinkCache map[int64]string, -) error { - var rows []migrationCommitStatus - if err := x.Table(table). - Where("repo_id = ?", repoID). - And("sha = ?", sha). - Cols("id", "repo_id", "target_url"). - Find(&rows); err != nil { - return fmt.Errorf("query %s for repo_id=%d sha=%s: %w", table, repoID, sha, err) - } - - for _, row := range rows { - repoLink, err := getRepoLinkCached(x, repoLinkCache, row.RepoID) - if err != nil || repoLink == "" { - if err != nil { - log.Warn("convert %s id=%d getRepoLinkCached: %v", table, row.ID, err) - } else { - log.Warn("convert %s id=%d: repo=%d not found", table, row.ID, row.RepoID) - } - continue - } - - runNum, jobNum, ok := parseTargetURL(row.TargetURL, repoLink) - if !ok { - continue - } - - run, ok := runs[runNum] - if !ok { - continue - } - - jobID, ok, err := getJobIDByIndexCached(x, jobsByRunIDCache, run.ID, jobNum) - if err != nil || !ok { - if err != nil { - log.Warn("convert %s id=%d getJobIDByIndexCached: %v", table, row.ID, err) - } else { - log.Warn("convert %s id=%d: job not found for run_id=%d job_index=%d", table, row.ID, run.ID, jobNum) - } - continue - } - - oldURL := row.TargetURL - newURL := fmt.Sprintf("%s%s%d/jobs/%d", repoLink, actionsRunPath, run.ID, jobID) - if oldURL == newURL { - continue - } - - if _, err := x.Table(table).ID(row.ID).Cols("target_url").Update(&migrationCommitStatus{TargetURL: newURL}); err != nil { - return fmt.Errorf("update %s id=%d target_url from %s to %s: %w", table, row.ID, oldURL, newURL, err) - } - } - return nil -} - -func getRepoLinkCached(x *xorm.Engine, cache map[int64]string, repoID int64) (string, error) { - if link, ok := cache[repoID]; ok { - return link, nil - } - repo := &migrationRepository{} - has, err := x.Table("repository").Where("id=?", repoID).Get(repo) - if err != nil { - return "", err - } - if !has { - cache[repoID] = "" - return "", nil - } - link := setting.AppSubURL + "/" + url.PathEscape(repo.OwnerName) + "/" + url.PathEscape(repo.Name) - cache[repoID] = link - return link, nil -} - -func getJobIDByIndexCached(x *xorm.Engine, cache map[int64][]int64, runID, jobIndex int64) (int64, bool, error) { - jobIDs, ok := cache[runID] - if !ok { - var jobs []migrationActionRunJob - if err := x.Table("action_run_job").Where("run_id=?", runID).Asc("id").Cols("id").Find(&jobs); err != nil { - return 0, false, err - } - jobIDs = make([]int64, 0, len(jobs)) - for _, job := range jobs { - jobIDs = append(jobIDs, job.ID) - } - cache[runID] = jobIDs - } - if jobIndex < 0 || jobIndex >= int64(len(jobIDs)) { - return 0, false, nil - } - return jobIDs[jobIndex], true, nil -} - -func parseTargetURL(targetURL, repoLink string) (runNum, jobNum int64, ok bool) { - prefix := repoLink + actionsRunPath - if !strings.HasPrefix(targetURL, prefix) { - return 0, 0, false - } - rest := targetURL[len(prefix):] - - parts := strings.Split(rest, "/") - if len(parts) == 3 && parts[1] == "jobs" { - runNum, err1 := strconv.ParseInt(parts[0], 10, 64) - jobNum, err2 := strconv.ParseInt(parts[2], 10, 64) - if err1 != nil || err2 != nil { - return 0, 0, false - } - return runNum, jobNum, true - } - - return 0, 0, false -} - -func getCommitStatusCommitID(run *migrationActionRun) (string, error) { - switch run.Event { - case webhook_module.HookEventPush: - payload, err := getPushEventPayload(run) - if err != nil { - return "", fmt.Errorf("getPushEventPayload: %w", err) - } - if payload.HeadCommit == nil { - return "", errors.New("head commit is missing in event payload") - } - return payload.HeadCommit.ID, nil - case webhook_module.HookEventPullRequest, - webhook_module.HookEventPullRequestSync, - webhook_module.HookEventPullRequestAssign, - webhook_module.HookEventPullRequestLabel, - webhook_module.HookEventPullRequestReviewRequest, - webhook_module.HookEventPullRequestMilestone: - payload, err := getPullRequestEventPayload(run) - if err != nil { - return "", fmt.Errorf("getPullRequestEventPayload: %w", err) - } - if payload.PullRequest == nil { - return "", errors.New("pull request is missing in event payload") - } else if payload.PullRequest.Head == nil { - return "", errors.New("head of pull request is missing in event payload") - } - return payload.PullRequest.Head.Sha, nil - case webhook_module.HookEventPullRequestReviewApproved, - webhook_module.HookEventPullRequestReviewRejected, - webhook_module.HookEventPullRequestReviewComment: - payload, err := getPullRequestEventPayload(run) - if err != nil { - return "", fmt.Errorf("getPullRequestEventPayload: %w", err) - } - if payload.PullRequest == nil { - return "", errors.New("pull request is missing in event payload") - } else if payload.PullRequest.Head == nil { - return "", errors.New("head of pull request is missing in event payload") - } - return payload.PullRequest.Head.Sha, nil - case webhook_module.HookEventRelease: - return run.CommitSHA, nil - default: - return "", nil - } -} - -func getPushEventPayload(run *migrationActionRun) (*api.PushPayload, error) { - if run.Event != webhook_module.HookEventPush { - return nil, fmt.Errorf("event %s is not a push event", run.Event) - } - var payload api.PushPayload - if err := json.Unmarshal([]byte(run.EventPayload), &payload); err != nil { - return nil, err - } - return &payload, nil -} - -func getPullRequestEventPayload(run *migrationActionRun) (*api.PullRequestPayload, error) { - if !run.Event.IsPullRequest() && !run.Event.IsPullRequestReview() { - return nil, fmt.Errorf("event %s is not a pull request event", run.Event) - } - var payload api.PullRequestPayload - if err := json.Unmarshal([]byte(run.EventPayload), &payload); err != nil { - return nil, err - } - return &payload, nil + return x.Sync(new(CommitComment)) } diff --git a/models/migrations/v1_26/v331.go b/models/migrations/v1_26/v331.go new file mode 100644 index 0000000000..387a183bbc --- /dev/null +++ b/models/migrations/v1_26/v331.go @@ -0,0 +1,27 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package v1_26 + +import ( + "code.gitea.io/gitea/modules/timeutil" + + "xorm.io/xorm" +) + +func AddCommitCommentTable(x *xorm.Engine) error { + type CommitComment struct { + ID int64 `xorm:"pk autoincr"` + RepoID int64 `xorm:"INDEX NOT NULL"` + CommitSHA string `xorm:"VARCHAR(64) INDEX NOT NULL"` + TreePath string `xorm:"VARCHAR(4000) NOT NULL"` + Line int64 `xorm:"NOT NULL"` + PosterID int64 `xorm:"INDEX NOT NULL"` + Content string `xorm:"LONGTEXT NOT NULL"` + Patch string `xorm:"LONGTEXT"` + CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` + UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"` + } + + return x.Sync(new(CommitComment)) +} diff --git a/models/repo/commit_comment.go b/models/repo/commit_comment.go new file mode 100644 index 0000000000..e13d127bc1 --- /dev/null +++ b/models/repo/commit_comment.go @@ -0,0 +1,155 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package repo + +import ( + "context" + "fmt" + "html/template" + + "code.gitea.io/gitea/models/db" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/timeutil" +) + +// CommitComment represents an inline comment on a commit diff. +type CommitComment struct { + ID int64 `xorm:"pk autoincr"` + RepoID int64 `xorm:"INDEX NOT NULL"` + CommitSHA string `xorm:"VARCHAR(64) INDEX NOT NULL"` + TreePath string `xorm:"VARCHAR(4000) NOT NULL"` + Line int64 `xorm:"NOT NULL"` // negative = old side, positive = new side + PosterID int64 `xorm:"INDEX NOT NULL"` + Poster *user_model.User `xorm:"-"` + Content string `xorm:"LONGTEXT NOT NULL"` + RenderedContent template.HTML `xorm:"-"` + Patch string `xorm:"LONGTEXT"` + CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` + UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"` +} + +func init() { + db.RegisterModel(new(CommitComment)) +} + +// HashTag returns a unique tag for the comment, used for anchoring. +func (c *CommitComment) HashTag() string { + return fmt.Sprintf("commitcomment-%d", c.ID) +} + +// UnsignedLine returns the absolute value of the line number. +func (c *CommitComment) UnsignedLine() int64 { + if c.Line < 0 { + return -c.Line + } + return c.Line +} + +// GetCommentSide returns "previous" for old side (negative Line), "proposed" for new side. +func (c *CommitComment) GetCommentSide() string { + if c.Line < 0 { + return "previous" + } + return "proposed" +} + +// LoadPoster loads the poster user for a commit comment. +func (c *CommitComment) LoadPoster(ctx context.Context) error { + if c.Poster != nil || c.PosterID <= 0 { + return nil + } + poster, err := user_model.GetUserByID(ctx, c.PosterID) + if err != nil { + if user_model.IsErrUserNotExist(err) { + c.PosterID = user_model.GhostUserID + c.Poster = user_model.NewGhostUser() + return nil + } + return err + } + c.Poster = poster + return nil +} + +// FileCommitComments holds commit comments for a single file, +// split by side (left = old, right = new) with int keys matching DiffLine indices. +type FileCommitComments struct { + Left map[int][]*CommitComment + Right map[int][]*CommitComment +} + +// CommitCommentsForDiff maps file paths to their commit comments. +type CommitCommentsForDiff map[string]*FileCommitComments + +// FindCommitCommentsByCommitSHA returns all comments for a given commit in a repo. +func FindCommitCommentsByCommitSHA(ctx context.Context, repoID int64, commitSHA string) ([]*CommitComment, error) { + comments := make([]*CommitComment, 0, 10) + return comments, db.GetEngine(ctx). + Where("repo_id = ? AND commit_sha = ?", repoID, commitSHA). + OrderBy("created_unix ASC"). + Find(&comments) +} + +// FindCommitCommentsForDiff returns comments grouped by path and side for rendering in a diff view. +func FindCommitCommentsForDiff(ctx context.Context, repoID int64, commitSHA string) (CommitCommentsForDiff, error) { + comments, err := FindCommitCommentsByCommitSHA(ctx, repoID, commitSHA) + if err != nil { + return nil, err + } + + result := make(CommitCommentsForDiff) + for _, c := range comments { + if err := c.LoadPoster(ctx); err != nil { + return nil, err + } + fcc, ok := result[c.TreePath] + if !ok { + fcc = &FileCommitComments{ + Left: make(map[int][]*CommitComment), + Right: make(map[int][]*CommitComment), + } + result[c.TreePath] = fcc + } + if c.Line < 0 { + idx := int(-c.Line) + fcc.Left[idx] = append(fcc.Left[idx], c) + } else { + idx := int(c.Line) + fcc.Right[idx] = append(fcc.Right[idx], c) + } + } + return result, nil +} + +// CreateCommitComment inserts a new commit comment. +func CreateCommitComment(ctx context.Context, c *CommitComment) error { + _, err := db.GetEngine(ctx).Insert(c) + return err +} + +// GetCommitCommentByID returns a commit comment by its ID. +func GetCommitCommentByID(ctx context.Context, id int64) (*CommitComment, error) { + c := &CommitComment{} + has, err := db.GetEngine(ctx).ID(id).Get(c) + if err != nil { + return nil, err + } + if !has { + return nil, db.ErrNotExist{Resource: "CommitComment", ID: id} + } + return c, nil +} + +// DeleteCommitComment deletes a commit comment by ID. +func DeleteCommitComment(ctx context.Context, id int64) error { + _, err := db.GetEngine(ctx).ID(id).Delete(&CommitComment{}) + return err +} + +// CountCommitCommentsByCommitSHA returns the count of comments for a commit. +func CountCommitCommentsByCommitSHA(ctx context.Context, repoID int64, commitSHA string) (int64, error) { + return db.GetEngine(ctx). + Where("repo_id = ? AND commit_sha = ?", repoID, commitSHA). + Count(&CommitComment{}) +} diff --git a/routers/web/repo/commit.go b/routers/web/repo/commit.go index 168d959494..08745a8d6e 100644 --- a/routers/web/repo/commit.go +++ b/routers/web/repo/commit.go @@ -27,6 +27,7 @@ import ( "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/markup" + "code.gitea.io/gitea/modules/markup/markdown" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/templates" "code.gitea.io/gitea/modules/util" @@ -424,6 +425,35 @@ func Diff(ctx *context.Context) { ctx.Data["MergedPRIssueNumber"] = pr.Index } + // Load inline commit comments for the diff view + commitComments, err := repo_model.FindCommitCommentsForDiff(ctx, ctx.Repo.Repository.ID, commitID) + if err != nil { + log.Error("FindCommitCommentsForDiff: %v", err) + } + // Render markdown content for each commit comment + for _, fcc := range commitComments { + for _, comments := range fcc.Left { + for _, c := range comments { + rctx := renderhelper.NewRenderContextRepoComment(ctx, ctx.Repo.Repository, renderhelper.RepoCommentOptions{}) + c.RenderedContent, err = markdown.RenderString(rctx, c.Content) + if err != nil { + log.Error("RenderString for commit comment %d: %v", c.ID, err) + } + } + } + for _, comments := range fcc.Right { + for _, c := range comments { + rctx := renderhelper.NewRenderContextRepoComment(ctx, ctx.Repo.Repository, renderhelper.RepoCommentOptions{}) + c.RenderedContent, err = markdown.RenderString(rctx, c.Content) + if err != nil { + log.Error("RenderString for commit comment %d: %v", c.ID, err) + } + } + } + } + ctx.Data["CommitComments"] = commitComments + ctx.Data["CanComment"] = ctx.Doer != nil && ctx.Repo.CanRead(unit_model.TypeCode) + ctx.HTML(http.StatusOK, tplCommitPage) } diff --git a/routers/web/repo/commit_comment.go b/routers/web/repo/commit_comment.go new file mode 100644 index 0000000000..fbf1d86460 --- /dev/null +++ b/routers/web/repo/commit_comment.go @@ -0,0 +1,156 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package repo + +import ( + "net/http" + + "code.gitea.io/gitea/models/renderhelper" + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/markup/markdown" + "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/templates" + "code.gitea.io/gitea/services/context" + "code.gitea.io/gitea/services/gitdiff" +) + +var ( + tplNewCommitComment templates.TplName = "repo/diff/new_commit_comment" + tplCommitConversation templates.TplName = "repo/diff/commit_conversation" +) + +// RenderNewCommitCommentForm renders the comment form for inline commit comments. +func RenderNewCommitCommentForm(ctx *context.Context) { + commitSHA := ctx.PathParam("sha") + ctx.Data["CommitID"] = commitSHA + ctx.Data["PageIsDiff"] = true + ctx.HTML(http.StatusOK, tplNewCommitComment) +} + +// CreateCommitComment handles creating an inline comment on a commit diff. +func CreateCommitComment(ctx *context.Context) { + commitSHA := ctx.PathParam("sha") + if commitSHA == "" { + ctx.NotFound(nil) + return + } + + content := ctx.FormString("content") + treePath := ctx.FormString("tree_path") + if treePath == "" { + treePath = ctx.FormString("path") + } + side := ctx.FormString("side") + line := ctx.FormInt64("line") + + if content == "" || treePath == "" || line == 0 { + ctx.JSONError("content, tree_path, and line are required") + return + } + + // Negate line number for "previous" (old) side + if side == "previous" { + line = -line + } + + // Resolve full commit SHA + commit, err := ctx.Repo.GitRepo.GetCommit(commitSHA) + if err != nil { + if git.IsErrNotExist(err) { + ctx.NotFound(err) + } else { + ctx.ServerError("GetCommit", err) + } + return + } + fullSHA := commit.ID.String() + + // Generate diff context patch around the commented line + var patch string + var parentSHA string + if commit.ParentCount() > 0 { + parentID, err := commit.ParentID(0) + if err == nil { + parentSHA = parentID.String() + } + } + if parentSHA != "" { + absLine := line + isOld := line < 0 + if isOld { + absLine = -line + } + patch, err = git.GetFileDiffCutAroundLine( + ctx.Repo.GitRepo, parentSHA, fullSHA, treePath, + absLine, isOld, setting.UI.CodeCommentLines, + ) + if err != nil { + log.Debug("GetFileDiffCutAroundLine failed for commit comment: %v", err) + } + if patch == "" { + patch, err = gitdiff.GeneratePatchForUnchangedLine(ctx.Repo.GitRepo, fullSHA, treePath, line, setting.UI.CodeCommentLines) + if err != nil { + log.Debug("GeneratePatchForUnchangedLine failed for commit comment: %v", err) + } + } + } + + comment := &repo_model.CommitComment{ + RepoID: ctx.Repo.Repository.ID, + CommitSHA: fullSHA, + TreePath: treePath, + Line: line, + PosterID: ctx.Doer.ID, + Poster: ctx.Doer, + Content: content, + Patch: patch, + } + + if err := repo_model.CreateCommitComment(ctx, comment); err != nil { + ctx.ServerError("CreateCommitComment", err) + return + } + + // Render markdown content + rctx := renderhelper.NewRenderContextRepoComment(ctx, ctx.Repo.Repository, renderhelper.RepoCommentOptions{}) + comment.RenderedContent, err = markdown.RenderString(rctx, comment.Content) + if err != nil { + log.Error("RenderString for commit comment %d: %v", comment.ID, err) + } + + // Return the conversation HTML so JS can replace the form inline + ctx.Data["CommitID"] = fullSHA + ctx.Data["comments"] = []*repo_model.CommitComment{comment} + ctx.HTML(http.StatusOK, tplCommitConversation) +} + +// DeleteCommitComment handles deleting an inline comment on a commit. +func DeleteCommitComment(ctx *context.Context) { + commentID := ctx.PathParamInt64("id") + if commentID <= 0 { + ctx.NotFound(nil) + return + } + + comment, err := repo_model.GetCommitCommentByID(ctx, commentID) + if err != nil { + ctx.NotFound(err) + return + } + + // Only the poster or repo admin can delete + if comment.PosterID != ctx.Doer.ID && !ctx.Repo.IsAdmin() { + ctx.JSONError("permission denied") + return + } + + if err := repo_model.DeleteCommitComment(ctx, commentID); err != nil { + ctx.ServerError("DeleteCommitComment", err) + return + } + + ctx.JSON(http.StatusOK, map[string]any{"ok": true}) +} diff --git a/routers/web/web.go b/routers/web/web.go index 61d1fdc142..859ae7d90d 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -1690,6 +1690,9 @@ func registerWebRoutes(m *web.Router, webAuth *AuthMiddleware) { m.Get("/graph", repo.Graph) m.Get("/commit/{sha:([a-f0-9]{7,64})$}", repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.Diff) m.Get("/commit/{sha:([a-f0-9]{7,64})$}/load-branches-and-tags", repo.LoadBranchesAndTags) + m.Get("/commit/{sha:([a-f0-9]{7,64})$}/comment", reqSignIn, repo.RenderNewCommitCommentForm) + m.Post("/commit/{sha:([a-f0-9]{7,64})$}/comment", reqSignIn, repo.CreateCommitComment) + m.Post("/commit/{sha:([a-f0-9]{7,64})$}/comment/{id}/delete", reqSignIn, repo.DeleteCommitComment) // FIXME: this route `/cherry-pick/{sha}` doesn't seem useful or right, the new code always uses `/_cherrypick/` which could handle branch name correctly m.Get("/cherry-pick/{sha:([a-f0-9]{7,64})$}", repo.SetEditorconfigIfExists, context.RepoRefByDefaultBranch(), repo.CherryPick) diff --git a/templates/repo/diff/box.tmpl b/templates/repo/diff/box.tmpl index 390e41ec34..b81df83c73 100644 --- a/templates/repo/diff/box.tmpl +++ b/templates/repo/diff/box.tmpl @@ -184,7 +184,7 @@ {{end}} {{else}} - +
{{if $.IsSplitStyle}} {{template "repo/diff/section_split" dict "file" . "root" $}} {{else}} diff --git a/templates/repo/diff/commit_comment_form.tmpl b/templates/repo/diff/commit_comment_form.tmpl new file mode 100644 index 0000000000..88fa85827c --- /dev/null +++ b/templates/repo/diff/commit_comment_form.tmpl @@ -0,0 +1,25 @@ +{{if and ctx.RootData.SignedUserID (not ctx.RootData.Repository.IsArchived)}} + + + + +
+ {{template "shared/combomarkdowneditor" (dict + "CustomInit" true + "MarkdownEditorContext" (ctx.MiscUtils.MarkdownEditorComment ctx.RootData.Repository) + "TextareaName" "content" + "TextareaPlaceholder" (ctx.Locale.Tr "repo.diff.comment.placeholder") + "DropzoneParentContainer" "form" + "DisableAutosize" "true" + )}} +
+
+
+ + {{if or (not $.HasComments) $.hidden}} + + {{end}} +
+
+ +{{end}} diff --git a/templates/repo/diff/commit_comments.tmpl b/templates/repo/diff/commit_comments.tmpl new file mode 100644 index 0000000000..64dc7a1763 --- /dev/null +++ b/templates/repo/diff/commit_comments.tmpl @@ -0,0 +1,35 @@ +{{range .comments}} + +{{$createdStr := DateUtils.TimeSince .CreatedUnix}} +
+
+ {{template "shared/user/avatarlink" dict "user" .Poster}} +
+
+
+
+ + {{template "shared/user/namelink" .Poster}} + {{ctx.Locale.Tr "repo.issues.commented_at" .HashTag $createdStr}} + +
+
+ {{if and $.root.IsSigned (eq $.root.SignedUserID .PosterID)}} +
+ {{svg "octicon-trash" 14}} +
+ {{end}} +
+
+
+
+ {{if .RenderedContent}} + {{.RenderedContent}} + {{else}} + {{ctx.Locale.Tr "repo.issues.no_content"}} + {{end}} +
+
+
+
+{{end}} diff --git a/templates/repo/diff/commit_conversation.tmpl b/templates/repo/diff/commit_conversation.tmpl new file mode 100644 index 0000000000..bc6e689d3f --- /dev/null +++ b/templates/repo/diff/commit_conversation.tmpl @@ -0,0 +1,25 @@ +{{if .comments}} + {{$comment := index .comments 0}} +
+
+
+
+ {{template "repo/diff/commit_comments" dict "root" $ "comments" .comments}} +
+
+
+
+ + +
+
+ {{if and $.SignedUserID (not $.Repository.IsArchived)}} + {{template "repo/diff/commit_comment_form" dict "hidden" true "root" $}} + {{end}} +
+
+{{end}} diff --git a/templates/repo/diff/new_commit_comment.tmpl b/templates/repo/diff/new_commit_comment.tmpl new file mode 100644 index 0000000000..fa54107609 --- /dev/null +++ b/templates/repo/diff/new_commit_comment.tmpl @@ -0,0 +1,5 @@ +
+
+ {{template "repo/diff/commit_comment_form" dict "root" $}} +
+
diff --git a/templates/repo/diff/section_split.tmpl b/templates/repo/diff/section_split.tmpl index c13b205518..401bdea6af 100644 --- a/templates/repo/diff/section_split.tmpl +++ b/templates/repo/diff/section_split.tmpl @@ -1,5 +1,8 @@ {{$file := .file}} {{$diffBlobExcerptData := $.root.DiffBlobExcerptData}} +{{$commitComments := $.root.CommitComments}} +{{$ccFile := ""}} +{{if $commitComments}}{{$ccFile = index $commitComments $file.Name}}{{end}} @@ -28,7 +31,7 @@ {{end}} + {{/* Render commit comments (not PR review comments) */}} + {{if $ccFile}} + {{$leftCC := ""}} + {{if $line.LeftIdx}}{{$leftCC = index $ccFile.Left $line.LeftIdx}}{{end}} + {{$rightCC := ""}} + {{if and (eq .GetType 3) $hasmatch}} + {{$match := index $section.Lines $line.Match}} + {{if $match.RightIdx}}{{$rightCC = index $ccFile.Right $match.RightIdx}}{{end}} + {{else}} + {{if $line.RightIdx}}{{$rightCC = index $ccFile.Right $line.RightIdx}}{{end}} + {{end}} + {{if or $leftCC $rightCC}} + + + + + {{end}} + {{end}} {{end}} {{end}} {{end}} diff --git a/templates/repo/diff/section_unified.tmpl b/templates/repo/diff/section_unified.tmpl index 4dee648cdd..278c465838 100644 --- a/templates/repo/diff/section_unified.tmpl +++ b/templates/repo/diff/section_unified.tmpl @@ -1,6 +1,9 @@ {{$file := .file}} {{/* this tmpl is also used by the PR Conversation page, so "DiffBlobExcerptData" may not exist */}} {{$diffBlobExcerptData := $.root.DiffBlobExcerptData}} +{{$commitComments := $.root.CommitComments}} +{{$ccFile := ""}} +{{if $commitComments}}{{$ccFile = index $commitComments $file.Name}}{{end}} @@ -31,7 +34,7 @@ {{else}} {{end}} + {{/* Render commit comments (not PR review comments) */}} + {{if $ccFile}} + {{$rightCC := ""}} + {{if $line.RightIdx}}{{$rightCC = index $ccFile.Right $line.RightIdx}}{{end}} + {{$leftCC := ""}} + {{if and $line.LeftIdx (not $line.RightIdx)}}{{$leftCC = index $ccFile.Left $line.LeftIdx}}{{end}} + {{if or $rightCC $leftCC}} + + + + {{end}} + {{end}} {{end}} {{end}} From 95fafbf918548311782b0c1ccb64339ffc199ad2 Mon Sep 17 00:00:00 2001 From: yuvrajangadsingh Date: Sun, 8 Mar 2026 15:27:17 +0530 Subject: [PATCH 02/10] fix: align var block formatting --- routers/web/repo/commit_comment.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/routers/web/repo/commit_comment.go b/routers/web/repo/commit_comment.go index fbf1d86460..9b906064f0 100644 --- a/routers/web/repo/commit_comment.go +++ b/routers/web/repo/commit_comment.go @@ -18,8 +18,8 @@ import ( ) var ( - tplNewCommitComment templates.TplName = "repo/diff/new_commit_comment" - tplCommitConversation templates.TplName = "repo/diff/commit_conversation" + tplNewCommitComment templates.TplName = "repo/diff/new_commit_comment" + tplCommitConversation templates.TplName = "repo/diff/commit_conversation" ) // RenderNewCommitCommentForm renders the comment form for inline commit comments. From 9c92458cc029ea8e026685b34646f7cf2896ba32 Mon Sep 17 00:00:00 2001 From: yuvrajangadsingh Date: Mon, 9 Mar 2026 02:00:48 +0530 Subject: [PATCH 03/10] refactor: use Comment table with junction table for commit comments Per @lunny's feedback, rework to reuse the existing Comment table instead of a standalone commit_comment table. The junction table (commit_comment) now only stores repo_id, commit_sha, comment_id. Actual comment data (content, tree_path, line, patch, poster) lives in the Comment table with Type = CommentTypeCommitComment (39). This gives commit comments reactions, attachments, and all existing comment infrastructure for free. --- models/issues/comment.go | 7 +- models/issues/commit_comment.go | 139 +++++++++++++++++ models/migrations/v1_26/v326.go | 20 +-- models/repo/commit_comment.go | 155 +------------------ routers/web/repo/commit.go | 2 +- routers/web/repo/commit_comment.go | 30 ++-- templates/repo/diff/commit_conversation.tmpl | 2 +- 7 files changed, 171 insertions(+), 184 deletions(-) create mode 100644 models/issues/commit_comment.go diff --git a/models/issues/comment.go b/models/issues/comment.go index 34ce7f3500..0ef8f66b33 100644 --- a/models/issues/comment.go +++ b/models/issues/comment.go @@ -118,6 +118,8 @@ const ( CommentTypeUnpin // 37 unpin Issue/PullRequest CommentTypeChangeTimeEstimate // 38 Change time estimate + + CommentTypeCommitComment // 39 Inline comment on a commit diff (not part of a PR review) ) var commentStrings = []string{ @@ -160,6 +162,7 @@ var commentStrings = []string{ "pin", "unpin", "change_time_estimate", + "commit_comment", } func (t CommentType) String() string { @@ -177,7 +180,7 @@ func AsCommentType(typeName string) CommentType { func (t CommentType) HasContentSupport() bool { switch t { - case CommentTypeComment, CommentTypeCode, CommentTypeReview, CommentTypeDismissReview: + case CommentTypeComment, CommentTypeCode, CommentTypeReview, CommentTypeDismissReview, CommentTypeCommitComment: return true } return false @@ -185,7 +188,7 @@ func (t CommentType) HasContentSupport() bool { func (t CommentType) HasAttachmentSupport() bool { switch t { - case CommentTypeComment, CommentTypeCode, CommentTypeReview: + case CommentTypeComment, CommentTypeCode, CommentTypeReview, CommentTypeCommitComment: return true } return false diff --git a/models/issues/commit_comment.go b/models/issues/commit_comment.go new file mode 100644 index 0000000000..2b4e38a43f --- /dev/null +++ b/models/issues/commit_comment.go @@ -0,0 +1,139 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package issues + +import ( + "context" + + "code.gitea.io/gitea/models/db" +) + +// CommitComment is a junction table linking a commit (repo + SHA) to +// a Comment entry. The comment content, tree_path, line, poster, etc. +// are stored in the Comment table with Type = CommentTypeCommitComment. +type CommitComment struct { + ID int64 `xorm:"pk autoincr"` + RepoID int64 `xorm:"INDEX NOT NULL"` + CommitSHA string `xorm:"VARCHAR(64) INDEX NOT NULL"` + CommentID int64 `xorm:"UNIQUE NOT NULL"` +} + +func init() { + db.RegisterModel(new(CommitComment)) +} + +// FileCommitComments holds commit comments for a single file, +// split by side (left = old, right = new) with int keys matching DiffLine indices. +type FileCommitComments struct { + Left map[int][]*Comment + Right map[int][]*Comment +} + +// CommitCommentsForDiff maps file paths to their commit comments. +type CommitCommentsForDiff map[string]*FileCommitComments + +// FindCommitCommentsByCommitSHA returns all comments for a given commit in a repo. +func FindCommitCommentsByCommitSHA(ctx context.Context, repoID int64, commitSHA string) ([]*Comment, error) { + var refs []CommitComment + if err := db.GetEngine(ctx). + Where("repo_id = ? AND commit_sha = ?", repoID, commitSHA). + Find(&refs); err != nil { + return nil, err + } + + if len(refs) == 0 { + return nil, nil + } + + commentIDs := make([]int64, 0, len(refs)) + for _, ref := range refs { + commentIDs = append(commentIDs, ref.CommentID) + } + + comments := make([]*Comment, 0, len(commentIDs)) + if err := db.GetEngine(ctx). + In("id", commentIDs). + OrderBy("created_unix ASC"). + Find(&comments); err != nil { + return nil, err + } + + for _, c := range comments { + if err := c.LoadPoster(ctx); err != nil { + return nil, err + } + } + + return comments, nil +} + +// FindCommitCommentsForDiff returns comments grouped by path and side for rendering in a diff view. +func FindCommitCommentsForDiff(ctx context.Context, repoID int64, commitSHA string) (CommitCommentsForDiff, error) { + comments, err := FindCommitCommentsByCommitSHA(ctx, repoID, commitSHA) + if err != nil { + return nil, err + } + + result := make(CommitCommentsForDiff) + for _, c := range comments { + fcc, ok := result[c.TreePath] + if !ok { + fcc = &FileCommitComments{ + Left: make(map[int][]*Comment), + Right: make(map[int][]*Comment), + } + result[c.TreePath] = fcc + } + if c.Line < 0 { + idx := int(-c.Line) + fcc.Left[idx] = append(fcc.Left[idx], c) + } else { + idx := int(c.Line) + fcc.Right[idx] = append(fcc.Right[idx], c) + } + } + return result, nil +} + +// CreateCommitComment creates a Comment with type CommitComment and a +// corresponding CommitComment junction record, within a transaction. +func CreateCommitComment(ctx context.Context, repoID int64, commitSHA string, comment *Comment) error { + return db.WithTx(ctx, func(ctx context.Context) error { + if _, err := db.GetEngine(ctx).Insert(comment); err != nil { + return err + } + + ref := &CommitComment{ + RepoID: repoID, + CommitSHA: commitSHA, + CommentID: comment.ID, + } + _, err := db.GetEngine(ctx).Insert(ref) + return err + }) +} + +// DeleteCommitComment deletes both the junction record and the Comment entry. +func DeleteCommitComment(ctx context.Context, commentID int64) error { + return db.WithTx(ctx, func(ctx context.Context) error { + if _, err := db.GetEngine(ctx).Where("comment_id = ?", commentID).Delete(&CommitComment{}); err != nil { + return err + } + _, err := db.GetEngine(ctx).ID(commentID).Delete(&Comment{}) + return err + }) +} + +// GetCommitCommentByID returns a commit comment by loading the Comment entry. +func GetCommitCommentByID(ctx context.Context, commentID int64) (*Comment, error) { + c := &Comment{} + has, err := db.GetEngine(ctx).ID(commentID).Get(c) + if err != nil { + return nil, err + } + if !has { + return nil, db.ErrNotExist{Resource: "CommitComment", ID: commentID} + } + return c, nil +} diff --git a/models/migrations/v1_26/v326.go b/models/migrations/v1_26/v326.go index 387a183bbc..3c3a751aa7 100644 --- a/models/migrations/v1_26/v326.go +++ b/models/migrations/v1_26/v326.go @@ -4,23 +4,19 @@ package v1_26 import ( - "code.gitea.io/gitea/modules/timeutil" - "xorm.io/xorm" ) func AddCommitCommentTable(x *xorm.Engine) error { + // CommitComment is a junction table that maps commit-specific context + // (repo, commit SHA) to a Comment entry. The actual comment content, + // tree_path, line, poster, etc. live in the Comment table with + // type = CommentTypeCommitComment (39). type CommitComment struct { - ID int64 `xorm:"pk autoincr"` - RepoID int64 `xorm:"INDEX NOT NULL"` - CommitSHA string `xorm:"VARCHAR(64) INDEX NOT NULL"` - TreePath string `xorm:"VARCHAR(4000) NOT NULL"` - Line int64 `xorm:"NOT NULL"` - PosterID int64 `xorm:"INDEX NOT NULL"` - Content string `xorm:"LONGTEXT NOT NULL"` - Patch string `xorm:"LONGTEXT"` - CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` - UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"` + ID int64 `xorm:"pk autoincr"` + RepoID int64 `xorm:"INDEX NOT NULL"` + CommitSHA string `xorm:"VARCHAR(64) INDEX NOT NULL"` + CommentID int64 `xorm:"UNIQUE NOT NULL"` } return x.Sync(new(CommitComment)) diff --git a/models/repo/commit_comment.go b/models/repo/commit_comment.go index e13d127bc1..74657a72a0 100644 --- a/models/repo/commit_comment.go +++ b/models/repo/commit_comment.go @@ -1,155 +1,8 @@ // Copyright 2026 The Gitea Authors. All rights reserved. // SPDX-License-Identifier: MIT +// This file intentionally left minimal. The CommitComment junction table +// and all query methods now live in models/issues/commit_comment.go +// alongside the Comment model they reference. + package repo - -import ( - "context" - "fmt" - "html/template" - - "code.gitea.io/gitea/models/db" - user_model "code.gitea.io/gitea/models/user" - "code.gitea.io/gitea/modules/timeutil" -) - -// CommitComment represents an inline comment on a commit diff. -type CommitComment struct { - ID int64 `xorm:"pk autoincr"` - RepoID int64 `xorm:"INDEX NOT NULL"` - CommitSHA string `xorm:"VARCHAR(64) INDEX NOT NULL"` - TreePath string `xorm:"VARCHAR(4000) NOT NULL"` - Line int64 `xorm:"NOT NULL"` // negative = old side, positive = new side - PosterID int64 `xorm:"INDEX NOT NULL"` - Poster *user_model.User `xorm:"-"` - Content string `xorm:"LONGTEXT NOT NULL"` - RenderedContent template.HTML `xorm:"-"` - Patch string `xorm:"LONGTEXT"` - CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` - UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"` -} - -func init() { - db.RegisterModel(new(CommitComment)) -} - -// HashTag returns a unique tag for the comment, used for anchoring. -func (c *CommitComment) HashTag() string { - return fmt.Sprintf("commitcomment-%d", c.ID) -} - -// UnsignedLine returns the absolute value of the line number. -func (c *CommitComment) UnsignedLine() int64 { - if c.Line < 0 { - return -c.Line - } - return c.Line -} - -// GetCommentSide returns "previous" for old side (negative Line), "proposed" for new side. -func (c *CommitComment) GetCommentSide() string { - if c.Line < 0 { - return "previous" - } - return "proposed" -} - -// LoadPoster loads the poster user for a commit comment. -func (c *CommitComment) LoadPoster(ctx context.Context) error { - if c.Poster != nil || c.PosterID <= 0 { - return nil - } - poster, err := user_model.GetUserByID(ctx, c.PosterID) - if err != nil { - if user_model.IsErrUserNotExist(err) { - c.PosterID = user_model.GhostUserID - c.Poster = user_model.NewGhostUser() - return nil - } - return err - } - c.Poster = poster - return nil -} - -// FileCommitComments holds commit comments for a single file, -// split by side (left = old, right = new) with int keys matching DiffLine indices. -type FileCommitComments struct { - Left map[int][]*CommitComment - Right map[int][]*CommitComment -} - -// CommitCommentsForDiff maps file paths to their commit comments. -type CommitCommentsForDiff map[string]*FileCommitComments - -// FindCommitCommentsByCommitSHA returns all comments for a given commit in a repo. -func FindCommitCommentsByCommitSHA(ctx context.Context, repoID int64, commitSHA string) ([]*CommitComment, error) { - comments := make([]*CommitComment, 0, 10) - return comments, db.GetEngine(ctx). - Where("repo_id = ? AND commit_sha = ?", repoID, commitSHA). - OrderBy("created_unix ASC"). - Find(&comments) -} - -// FindCommitCommentsForDiff returns comments grouped by path and side for rendering in a diff view. -func FindCommitCommentsForDiff(ctx context.Context, repoID int64, commitSHA string) (CommitCommentsForDiff, error) { - comments, err := FindCommitCommentsByCommitSHA(ctx, repoID, commitSHA) - if err != nil { - return nil, err - } - - result := make(CommitCommentsForDiff) - for _, c := range comments { - if err := c.LoadPoster(ctx); err != nil { - return nil, err - } - fcc, ok := result[c.TreePath] - if !ok { - fcc = &FileCommitComments{ - Left: make(map[int][]*CommitComment), - Right: make(map[int][]*CommitComment), - } - result[c.TreePath] = fcc - } - if c.Line < 0 { - idx := int(-c.Line) - fcc.Left[idx] = append(fcc.Left[idx], c) - } else { - idx := int(c.Line) - fcc.Right[idx] = append(fcc.Right[idx], c) - } - } - return result, nil -} - -// CreateCommitComment inserts a new commit comment. -func CreateCommitComment(ctx context.Context, c *CommitComment) error { - _, err := db.GetEngine(ctx).Insert(c) - return err -} - -// GetCommitCommentByID returns a commit comment by its ID. -func GetCommitCommentByID(ctx context.Context, id int64) (*CommitComment, error) { - c := &CommitComment{} - has, err := db.GetEngine(ctx).ID(id).Get(c) - if err != nil { - return nil, err - } - if !has { - return nil, db.ErrNotExist{Resource: "CommitComment", ID: id} - } - return c, nil -} - -// DeleteCommitComment deletes a commit comment by ID. -func DeleteCommitComment(ctx context.Context, id int64) error { - _, err := db.GetEngine(ctx).ID(id).Delete(&CommitComment{}) - return err -} - -// CountCommitCommentsByCommitSHA returns the count of comments for a commit. -func CountCommitCommentsByCommitSHA(ctx context.Context, repoID int64, commitSHA string) (int64, error) { - return db.GetEngine(ctx). - Where("repo_id = ? AND commit_sha = ?", repoID, commitSHA). - Count(&CommitComment{}) -} diff --git a/routers/web/repo/commit.go b/routers/web/repo/commit.go index 08745a8d6e..4a9a1b2b76 100644 --- a/routers/web/repo/commit.go +++ b/routers/web/repo/commit.go @@ -426,7 +426,7 @@ func Diff(ctx *context.Context) { } // Load inline commit comments for the diff view - commitComments, err := repo_model.FindCommitCommentsForDiff(ctx, ctx.Repo.Repository.ID, commitID) + commitComments, err := issues_model.FindCommitCommentsForDiff(ctx, ctx.Repo.Repository.ID, commitID) if err != nil { log.Error("FindCommitCommentsForDiff: %v", err) } diff --git a/routers/web/repo/commit_comment.go b/routers/web/repo/commit_comment.go index 9b906064f0..44bd24fd5a 100644 --- a/routers/web/repo/commit_comment.go +++ b/routers/web/repo/commit_comment.go @@ -6,8 +6,8 @@ package repo import ( "net/http" + issues_model "code.gitea.io/gitea/models/issues" "code.gitea.io/gitea/models/renderhelper" - repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/markup/markdown" @@ -51,12 +51,10 @@ func CreateCommitComment(ctx *context.Context) { return } - // Negate line number for "previous" (old) side if side == "previous" { line = -line } - // Resolve full commit SHA commit, err := ctx.Repo.GitRepo.GetCommit(commitSHA) if err != nil { if git.IsErrNotExist(err) { @@ -98,18 +96,18 @@ func CreateCommitComment(ctx *context.Context) { } } - comment := &repo_model.CommitComment{ - RepoID: ctx.Repo.Repository.ID, + comment := &issues_model.Comment{ + Type: issues_model.CommentTypeCommitComment, + PosterID: ctx.Doer.ID, + Poster: ctx.Doer, CommitSHA: fullSHA, - TreePath: treePath, - Line: line, - PosterID: ctx.Doer.ID, - Poster: ctx.Doer, - Content: content, - Patch: patch, + TreePath: treePath, + Line: line, + Content: content, + Patch: patch, } - if err := repo_model.CreateCommitComment(ctx, comment); err != nil { + if err := issues_model.CreateCommitComment(ctx, ctx.Repo.Repository.ID, fullSHA, comment); err != nil { ctx.ServerError("CreateCommitComment", err) return } @@ -121,9 +119,8 @@ func CreateCommitComment(ctx *context.Context) { log.Error("RenderString for commit comment %d: %v", comment.ID, err) } - // Return the conversation HTML so JS can replace the form inline ctx.Data["CommitID"] = fullSHA - ctx.Data["comments"] = []*repo_model.CommitComment{comment} + ctx.Data["comments"] = []*issues_model.Comment{comment} ctx.HTML(http.StatusOK, tplCommitConversation) } @@ -135,19 +132,18 @@ func DeleteCommitComment(ctx *context.Context) { return } - comment, err := repo_model.GetCommitCommentByID(ctx, commentID) + comment, err := issues_model.GetCommitCommentByID(ctx, commentID) if err != nil { ctx.NotFound(err) return } - // Only the poster or repo admin can delete if comment.PosterID != ctx.Doer.ID && !ctx.Repo.IsAdmin() { ctx.JSONError("permission denied") return } - if err := repo_model.DeleteCommitComment(ctx, commentID); err != nil { + if err := issues_model.DeleteCommitComment(ctx, commentID); err != nil { ctx.ServerError("DeleteCommitComment", err) return } diff --git a/templates/repo/diff/commit_conversation.tmpl b/templates/repo/diff/commit_conversation.tmpl index bc6e689d3f..9577091693 100644 --- a/templates/repo/diff/commit_conversation.tmpl +++ b/templates/repo/diff/commit_conversation.tmpl @@ -1,6 +1,6 @@ {{if .comments}} {{$comment := index .comments 0}} -
+
From 6d0c41ca94043c415315a55ec4a947350f1e9901 Mon Sep 17 00:00:00 2001 From: yuvrajangadsingh Date: Mon, 9 Mar 2026 02:16:57 +0530 Subject: [PATCH 04/10] fix: address lunny's review comments - use single query with Cols("comment_id").Table("commit_comment") instead of loading full CommitComment structs - remove models/repo/commit_comment.go entirely --- models/issues/commit_comment.go | 13 ++++--------- models/repo/commit_comment.go | 8 -------- 2 files changed, 4 insertions(+), 17 deletions(-) delete mode 100644 models/repo/commit_comment.go diff --git a/models/issues/commit_comment.go b/models/issues/commit_comment.go index 2b4e38a43f..297b97ce6c 100644 --- a/models/issues/commit_comment.go +++ b/models/issues/commit_comment.go @@ -35,22 +35,17 @@ type CommitCommentsForDiff map[string]*FileCommitComments // FindCommitCommentsByCommitSHA returns all comments for a given commit in a repo. func FindCommitCommentsByCommitSHA(ctx context.Context, repoID int64, commitSHA string) ([]*Comment, error) { - var refs []CommitComment - if err := db.GetEngine(ctx). + var commentIDs []int64 + if err := db.GetEngine(ctx).Cols("comment_id").Table("commit_comment"). Where("repo_id = ? AND commit_sha = ?", repoID, commitSHA). - Find(&refs); err != nil { + Find(&commentIDs); err != nil { return nil, err } - if len(refs) == 0 { + if len(commentIDs) == 0 { return nil, nil } - commentIDs := make([]int64, 0, len(refs)) - for _, ref := range refs { - commentIDs = append(commentIDs, ref.CommentID) - } - comments := make([]*Comment, 0, len(commentIDs)) if err := db.GetEngine(ctx). In("id", commentIDs). diff --git a/models/repo/commit_comment.go b/models/repo/commit_comment.go deleted file mode 100644 index 74657a72a0..0000000000 --- a/models/repo/commit_comment.go +++ /dev/null @@ -1,8 +0,0 @@ -// Copyright 2026 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -// This file intentionally left minimal. The CommitComment junction table -// and all query methods now live in models/issues/commit_comment.go -// alongside the Comment model they reference. - -package repo From 5acbf6cdb8f53cd6df3a2a7e9e52d7f0545248e7 Mon Sep 17 00:00:00 2001 From: yuvrajangadsingh Date: Mon, 9 Mar 2026 12:35:07 +0530 Subject: [PATCH 05/10] fix: align struct field formatting for gofmt --- routers/web/repo/commit_comment.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/routers/web/repo/commit_comment.go b/routers/web/repo/commit_comment.go index 44bd24fd5a..10f772363c 100644 --- a/routers/web/repo/commit_comment.go +++ b/routers/web/repo/commit_comment.go @@ -97,14 +97,14 @@ func CreateCommitComment(ctx *context.Context) { } comment := &issues_model.Comment{ - Type: issues_model.CommentTypeCommitComment, - PosterID: ctx.Doer.ID, - Poster: ctx.Doer, + Type: issues_model.CommentTypeCommitComment, + PosterID: ctx.Doer.ID, + Poster: ctx.Doer, CommitSHA: fullSHA, - TreePath: treePath, - Line: line, - Content: content, - Patch: patch, + TreePath: treePath, + Line: line, + Content: content, + Patch: patch, } if err := issues_model.CreateCommitComment(ctx, ctx.Repo.Repository.ID, fullSHA, comment); err != nil { From 6dc3a97e94190f68c54bc2d83dfcc0fe8f1eda75 Mon Sep 17 00:00:00 2001 From: yuvrajangadsingh Date: Mon, 9 Mar 2026 14:56:45 +0530 Subject: [PATCH 06/10] fix: address review - add archive check, verify repo ownership, remove path fallback --- models/issues/commit_comment.go | 15 +++++++++++++-- routers/web/repo/commit_comment.go | 5 +---- routers/web/web.go | 4 ++-- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/models/issues/commit_comment.go b/models/issues/commit_comment.go index 297b97ce6c..45d5b1bafb 100644 --- a/models/issues/commit_comment.go +++ b/models/issues/commit_comment.go @@ -120,8 +120,19 @@ func DeleteCommitComment(ctx context.Context, commentID int64) error { }) } -// GetCommitCommentByID returns a commit comment by loading the Comment entry. -func GetCommitCommentByID(ctx context.Context, commentID int64) (*Comment, error) { +// GetCommitCommentByID returns a commit comment by loading the Comment entry, +// verifying it belongs to the given repository via the junction table. +func GetCommitCommentByID(ctx context.Context, repoID, commentID int64) (*Comment, error) { + exists, err := db.GetEngine(ctx).Table("commit_comment"). + Where("repo_id = ? AND comment_id = ?", repoID, commentID). + Exist() + if err != nil { + return nil, err + } + if !exists { + return nil, db.ErrNotExist{Resource: "CommitComment", ID: commentID} + } + c := &Comment{} has, err := db.GetEngine(ctx).ID(commentID).Get(c) if err != nil { diff --git a/routers/web/repo/commit_comment.go b/routers/web/repo/commit_comment.go index 10f772363c..a3761daaf3 100644 --- a/routers/web/repo/commit_comment.go +++ b/routers/web/repo/commit_comment.go @@ -40,9 +40,6 @@ func CreateCommitComment(ctx *context.Context) { content := ctx.FormString("content") treePath := ctx.FormString("tree_path") - if treePath == "" { - treePath = ctx.FormString("path") - } side := ctx.FormString("side") line := ctx.FormInt64("line") @@ -132,7 +129,7 @@ func DeleteCommitComment(ctx *context.Context) { return } - comment, err := issues_model.GetCommitCommentByID(ctx, commentID) + comment, err := issues_model.GetCommitCommentByID(ctx, ctx.Repo.Repository.ID, commentID) if err != nil { ctx.NotFound(err) return diff --git a/routers/web/web.go b/routers/web/web.go index 859ae7d90d..11f8fdb374 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -1691,8 +1691,8 @@ func registerWebRoutes(m *web.Router, webAuth *AuthMiddleware) { m.Get("/commit/{sha:([a-f0-9]{7,64})$}", repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.Diff) m.Get("/commit/{sha:([a-f0-9]{7,64})$}/load-branches-and-tags", repo.LoadBranchesAndTags) m.Get("/commit/{sha:([a-f0-9]{7,64})$}/comment", reqSignIn, repo.RenderNewCommitCommentForm) - m.Post("/commit/{sha:([a-f0-9]{7,64})$}/comment", reqSignIn, repo.CreateCommitComment) - m.Post("/commit/{sha:([a-f0-9]{7,64})$}/comment/{id}/delete", reqSignIn, repo.DeleteCommitComment) + m.Post("/commit/{sha:([a-f0-9]{7,64})$}/comment", reqSignIn, context.RepoMustNotBeArchived(), repo.CreateCommitComment) + m.Post("/commit/{sha:([a-f0-9]{7,64})$}/comment/{id}/delete", reqSignIn, context.RepoMustNotBeArchived(), repo.DeleteCommitComment) // FIXME: this route `/cherry-pick/{sha}` doesn't seem useful or right, the new code always uses `/_cherrypick/` which could handle branch name correctly m.Get("/cherry-pick/{sha:([a-f0-9]{7,64})$}", repo.SetEditorconfigIfExists, context.RepoRefByDefaultBranch(), repo.CherryPick) From ce204ad532d0047b8484b6aaa72cc15daea11eaa Mon Sep 17 00:00:00 2001 From: yuvrajangadsingh Date: Sat, 14 Mar 2026 15:01:41 +0530 Subject: [PATCH 07/10] fix: address review feedback on permissions and poster loading --- models/issues/commit_comment.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/models/issues/commit_comment.go b/models/issues/commit_comment.go index 45d5b1bafb..6988e20b9a 100644 --- a/models/issues/commit_comment.go +++ b/models/issues/commit_comment.go @@ -54,10 +54,8 @@ func FindCommitCommentsByCommitSHA(ctx context.Context, repoID int64, commitSHA return nil, err } - for _, c := range comments { - if err := c.LoadPoster(ctx); err != nil { - return nil, err - } + if err := CommentList(comments).LoadPosters(ctx); err != nil { + return nil, err } return comments, nil From 0084880f6ae5bbdebfa8a9de9c06c691754b950b Mon Sep 17 00:00:00 2001 From: yuvrajangadsingh Date: Sat, 14 Mar 2026 15:34:50 +0530 Subject: [PATCH 08/10] add notifications for commit comments notify the commit author (resolved by email) and @mentioned users when a new commit comment is created. uses the existing NotificationSourceCommit source and follows the same pattern as CreateRepoTransferNotification. --- models/activities/notification.go | 43 ++++++++++++++++++++++++++++++ routers/web/repo/commit_comment.go | 8 ++++++ 2 files changed, 51 insertions(+) diff --git a/models/activities/notification.go b/models/activities/notification.go index 8a830c5aa2..04c13522ae 100644 --- a/models/activities/notification.go +++ b/models/activities/notification.go @@ -115,6 +115,49 @@ func init() { db.RegisterModel(new(Notification)) } +// CreateCommitCommentNotification creates notifications for a commit comment. +// It notifies the commit author (if they're a Gitea user) and any @mentioned users. +func CreateCommitCommentNotification(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, comment *issues_model.Comment, commitAuthorEmail string, mentionedUsernames []string) error { + return db.WithTx(ctx, func(ctx context.Context) error { + receiverIDs := make(map[int64]struct{}) + + // Notify the commit author if they map to a Gitea user + if commitAuthorEmail != "" { + author, err := user_model.GetUserByEmail(ctx, commitAuthorEmail) + if err == nil && author.ID != doer.ID { + receiverIDs[author.ID] = struct{}{} + } + } + + // Notify @mentioned users + for _, username := range mentionedUsernames { + mentioned, err := user_model.GetUserByName(ctx, username) + if err == nil && mentioned.ID != doer.ID { + receiverIDs[mentioned.ID] = struct{}{} + } + } + + if len(receiverIDs) == 0 { + return nil + } + + var notifications []*Notification + for uid := range receiverIDs { + notifications = append(notifications, &Notification{ + UserID: uid, + RepoID: repo.ID, + Status: NotificationStatusUnread, + Source: NotificationSourceCommit, + CommitID: comment.CommitSHA, + CommentID: comment.ID, + UpdatedBy: doer.ID, + }) + } + + return db.Insert(ctx, notifications) + }) +} + // CreateRepoTransferNotification creates notification for the user a repository was transferred to func CreateRepoTransferNotification(ctx context.Context, doer, newOwner *user_model.User, repo *repo_model.Repository) error { return db.WithTx(ctx, func(ctx context.Context) error { diff --git a/routers/web/repo/commit_comment.go b/routers/web/repo/commit_comment.go index a3761daaf3..b053d68d09 100644 --- a/routers/web/repo/commit_comment.go +++ b/routers/web/repo/commit_comment.go @@ -6,11 +6,13 @@ package repo import ( "net/http" + activities_model "code.gitea.io/gitea/models/activities" issues_model "code.gitea.io/gitea/models/issues" "code.gitea.io/gitea/models/renderhelper" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/markup/markdown" + "code.gitea.io/gitea/modules/references" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/templates" "code.gitea.io/gitea/services/context" @@ -109,6 +111,12 @@ func CreateCommitComment(ctx *context.Context) { return } + // Send notifications to commit author and @mentioned users + mentions := references.FindAllMentionsMarkdown(content) + if err := activities_model.CreateCommitCommentNotification(ctx, ctx.Doer, ctx.Repo.Repository, comment, commit.Author.Email, mentions); err != nil { + log.Error("CreateCommitCommentNotification: %v", err) + } + // Render markdown content rctx := renderhelper.NewRenderContextRepoComment(ctx, ctx.Repo.Repository, renderhelper.RepoCommentOptions{}) comment.RenderedContent, err = markdown.RenderString(rctx, comment.Content) From 23ca3422ddf2ad2864807f5c7c286a114e8d79b3 Mon Sep 17 00:00:00 2001 From: yuvrajangadsingh Date: Sun, 15 Mar 2026 01:42:08 +0530 Subject: [PATCH 09/10] use GetUserIDsByNames for mention lookups --- models/activities/notification.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/models/activities/notification.go b/models/activities/notification.go index 04c13522ae..5b4578edfb 100644 --- a/models/activities/notification.go +++ b/models/activities/notification.go @@ -130,10 +130,14 @@ func CreateCommitCommentNotification(ctx context.Context, doer *user_model.User, } // Notify @mentioned users - for _, username := range mentionedUsernames { - mentioned, err := user_model.GetUserByName(ctx, username) - if err == nil && mentioned.ID != doer.ID { - receiverIDs[mentioned.ID] = struct{}{} + if len(mentionedUsernames) > 0 { + mentionedIDs, err := user_model.GetUserIDsByNames(ctx, mentionedUsernames, true) + if err == nil { + for _, id := range mentionedIDs { + if id != doer.ID { + receiverIDs[id] = struct{}{} + } + } } } From 64a0e4c053e9db2f6540935fb3c3d61bf7ed7fb1 Mon Sep 17 00:00:00 2001 From: yuvrajangadsingh Date: Sun, 29 Mar 2026 13:48:21 +0530 Subject: [PATCH 10/10] fix: add reqUnitCodeReader to commit comment routes per silverwind's review --- routers/web/web.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/routers/web/web.go b/routers/web/web.go index 11f8fdb374..d6f58ea292 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -1690,9 +1690,9 @@ func registerWebRoutes(m *web.Router, webAuth *AuthMiddleware) { m.Get("/graph", repo.Graph) m.Get("/commit/{sha:([a-f0-9]{7,64})$}", repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.Diff) m.Get("/commit/{sha:([a-f0-9]{7,64})$}/load-branches-and-tags", repo.LoadBranchesAndTags) - m.Get("/commit/{sha:([a-f0-9]{7,64})$}/comment", reqSignIn, repo.RenderNewCommitCommentForm) - m.Post("/commit/{sha:([a-f0-9]{7,64})$}/comment", reqSignIn, context.RepoMustNotBeArchived(), repo.CreateCommitComment) - m.Post("/commit/{sha:([a-f0-9]{7,64})$}/comment/{id}/delete", reqSignIn, context.RepoMustNotBeArchived(), repo.DeleteCommitComment) + m.Get("/commit/{sha:([a-f0-9]{7,64})$}/comment", reqSignIn, reqUnitCodeReader, repo.RenderNewCommitCommentForm) + m.Post("/commit/{sha:([a-f0-9]{7,64})$}/comment", reqSignIn, reqUnitCodeReader, context.RepoMustNotBeArchived(), repo.CreateCommitComment) + m.Post("/commit/{sha:([a-f0-9]{7,64})$}/comment/{id}/delete", reqSignIn, reqUnitCodeReader, context.RepoMustNotBeArchived(), repo.DeleteCommitComment) // FIXME: this route `/cherry-pick/{sha}` doesn't seem useful or right, the new code always uses `/_cherrypick/` which could handle branch name correctly m.Get("/cherry-pick/{sha:([a-f0-9]{7,64})$}", repo.SetEditorconfigIfExists, context.RepoRefByDefaultBranch(), repo.CherryPick)
{{if $line.LeftIdx}}{{ctx.RenderUtils.RenderUnicodeEscapeToggleButton $leftDiff.EscapeStatus}}{{end}} - {{- if and $.root.SignedUserID $.root.PageIsPullFiles -}} + {{- if and $.root.SignedUserID (or $.root.PageIsPullFiles $.root.PageIsDiff) -}} @@ -43,7 +46,7 @@ {{if $match.RightIdx}}{{ctx.RenderUtils.RenderUnicodeEscapeToggleButton $rightDiff.EscapeStatus}}{{end}} {{if $match.RightIdx}}{{end}} - {{- if and $.root.SignedUserID $.root.PageIsPullFiles -}} + {{- if and $.root.SignedUserID (or $.root.PageIsPullFiles $.root.PageIsDiff) -}} @@ -60,7 +63,7 @@ {{if $line.LeftIdx}}{{ctx.RenderUtils.RenderUnicodeEscapeToggleButton $inlineDiff.EscapeStatus}}{{end}} {{if $line.LeftIdx}}{{end}} - {{- if and $.root.SignedUserID $.root.PageIsPullFiles (not (eq .GetType 2)) -}} + {{- if and $.root.SignedUserID (or $.root.PageIsPullFiles $.root.PageIsDiff) (not (eq .GetType 2)) -}} @@ -75,7 +78,7 @@ {{if $line.RightIdx}}{{ctx.RenderUtils.RenderUnicodeEscapeToggleButton $inlineDiff.EscapeStatus}}{{end}} {{if $line.RightIdx}}{{end}} - {{- if and $.root.SignedUserID $.root.PageIsPullFiles (not (eq .GetType 3)) -}} + {{- if and $.root.SignedUserID (or $.root.PageIsPullFiles $.root.PageIsDiff) (not (eq .GetType 3)) -}} @@ -132,6 +135,32 @@
+ {{if $leftCC}} + {{template "repo/diff/commit_conversation" dict "." $.root "comments" $leftCC}} + {{end}} + + {{if $rightCC}} + {{template "repo/diff/commit_conversation" dict "." $.root "comments" $rightCC}} + {{end}} +
{{template "repo/diff/section_code" dict "diff" $inlineDiff}} - {{- if and $.root.SignedUserID $.root.PageIsPullFiles -}} + {{- if and $.root.SignedUserID (or $.root.PageIsPullFiles $.root.PageIsDiff) -}} @@ -47,5 +50,24 @@
+ {{if $rightCC}} + {{template "repo/diff/commit_conversation" dict "." $.root "comments" $rightCC}} + {{end}} + {{if $leftCC}} + {{template "repo/diff/commit_conversation" dict "." $.root "comments" $leftCC}} + {{end}} +