improvement: detect previous published release for notes base

Author: dawidgora

models/repo/release.go

@@ -6,6 +6,7 @@ package repo
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"html/template"
 	"net/url"
@@ -322,6 +323,40 @@ func GetLatestReleaseByRepoID(ctx context.Context, repoID int64) (*Release, erro
 	return rel, nil
 }
 
+// GetPreviousPublishedRelease returns the most recent published release created before the provided release.
+func GetPreviousPublishedRelease(ctx context.Context, repoID int64, current *Release) (*Release, error) {
+	if current == nil {
+		return nil, errors.New("current release must not be nil")
+	}
+
+	cond := builder.NewCond().
+		And(builder.Eq{"repo_id": repoID}).
+		And(builder.Eq{"is_draft": false}).
+		And(builder.Eq{"is_prerelease": false}).
+		And(builder.Eq{"is_tag": false}).
+		And(builder.Or(
+			builder.Lt{"created_unix": current.CreatedUnix},
+			builder.And(
+				builder.Eq{"created_unix": current.CreatedUnix},
+				builder.Lt{"id": current.ID},
+			),
+		))
+
+	rel := new(Release)
+	has, err := db.GetEngine(ctx).
+		Desc("created_unix", "id").
+		Where(cond).
+		Get(rel)
+	if err != nil {
+		return nil, err
+	}
+	if !has {
+		return nil, ErrReleaseNotExist{0, "previous"}
+	}
+
+	return rel, nil
+}
+
 type releaseMetaSearch struct {
 	ID  []int64
 	Rel []*Release

models/repo/release_test.go

@@ -7,6 +7,7 @@ import (
 	"testing"
 
 	"code.gitea.io/gitea/models/unittest"
+	"code.gitea.io/gitea/modules/timeutil"
 
 	"github.com/stretchr/testify/assert"
 )
@@ -37,3 +38,40 @@ func Test_FindTagsByCommitIDs(t *testing.T) {
 	assert.Equal(t, "delete-tag", rels[1].TagName)
 	assert.Equal(t, "v1.0", rels[2].TagName)
 }
+
+func TestGetPreviousPublishedRelease(t *testing.T) {
+	assert.NoError(t, unittest.PrepareTestDatabase())
+
+	current := unittest.AssertExistsAndLoadBean(t, &Release{ID: 8})
+	prev, err := GetPreviousPublishedRelease(t.Context(), current.RepoID, current)
+	assert.NoError(t, err)
+	assert.EqualValues(t, 7, prev.ID)
+}
+
+func TestGetPreviousPublishedRelease_NoPublishedCandidate(t *testing.T) {
+	assert.NoError(t, unittest.PrepareTestDatabase())
+
+	repoID := int64(1)
+	draft := &Release{
+		RepoID:       repoID,
+		PublisherID:  1,
+		TagName:      "draft-prev",
+		LowerTagName: "draft-prev",
+		IsDraft:      true,
+		CreatedUnix:  timeutil.TimeStamp(2),
+	}
+	current := &Release{
+		RepoID:       repoID,
+		PublisherID:  1,
+		TagName:      "published-current",
+		LowerTagName: "published-current",
+		CreatedUnix:  timeutil.TimeStamp(3),
+	}
+
+	err := InsertReleases(t.Context(), draft, current)
+	assert.NoError(t, err)
+
+	_, err = GetPreviousPublishedRelease(t.Context(), repoID, current)
+	assert.Error(t, err)
+	assert.True(t, IsErrReleaseNotExist(err))
+}

services/release/notes.go

@@ -141,61 +141,26 @@ func resolveBaseTag(ctx context.Context, repo *repo_model.Repository, gitRepo *g
 	requestedBase = strings.TrimSpace(requestedBase)
 	if requestedBase != "" {
 		if gitRepo.IsTagExist(requestedBase) {
-			baseCommit, err := gitRepo.GetCommit(requestedBase)
-			if err != nil {
-				return nil, newErrReleaseNotesTagNotFound(requestedBase)
-			}
-			return &baseSelection{
-				CompareBase: requestedBase,
-				PreviousTag: requestedBase,
-				Commit:      baseCommit,
-			}, nil
+			return buildBaseSelectionForTag(gitRepo, requestedBase)
 		}
 		return nil, newErrReleaseNotesTagNotFound(requestedBase)
 	}
 
-	rel, err := repo_model.GetLatestReleaseByRepoID(ctx, repo.ID)
-	switch {
-	case err == nil:
-		candidate := strings.TrimSpace(rel.TagName)
-		if !strings.EqualFold(candidate, tagName) {
-			if gitRepo.IsTagExist(candidate) {
-				baseCommit, err := gitRepo.GetCommit(candidate)
-				if err != nil {
-					return nil, newErrReleaseNotesTagNotFound(candidate)
-				}
-				return &baseSelection{
-					CompareBase: candidate,
-					PreviousTag: candidate,
-					Commit:      baseCommit,
-				}, nil
-			}
-			return nil, newErrReleaseNotesTagNotFound(candidate)
-		}
-	case repo_model.IsErrReleaseNotExist(err):
-		// fall back to tags below
-	default:
-		return nil, fmt.Errorf("GetLatestReleaseByRepoID: %w", err)
+	candidate, err := autoPreviousReleaseTag(ctx, repo, tagName)
+	if err != nil {
+		return nil, err
+	}
+	if candidate != "" {
+		return buildBaseSelectionForTag(gitRepo, candidate)
 	}
 
 	tagInfos, _, err := gitRepo.GetTagInfos(0, 0)
 	if err != nil {
 		return nil, fmt.Errorf("GetTagInfos: %w", err)
 	}
 
-	for _, tag := range tagInfos {
-		if strings.EqualFold(tag.Name, tagName) {
-			continue
-		}
-		baseCommit, err := gitRepo.GetCommit(tag.Name)
-		if err != nil {
-			return nil, newErrReleaseNotesTagNotFound(tag.Name)
-		}
-		return &baseSelection{
-			CompareBase: tag.Name,
-			PreviousTag: tag.Name,
-			Commit:      baseCommit,
-		}, nil
+	if previousTag, ok := findPreviousTagName(tagInfos, tagName); ok {
+		return buildBaseSelectionForTag(gitRepo, previousTag)
 	}
 
 	initialCommit, err := findInitialCommit(headCommit)
@@ -209,6 +174,97 @@ func resolveBaseTag(ctx context.Context, repo *repo_model.Repository, gitRepo *g
 	}, nil
 }
 
+func buildBaseSelectionForTag(gitRepo *git.Repository, tagName string) (*baseSelection, error) {
+	tagName = strings.TrimSpace(tagName)
+	if tagName == "" {
+		return nil, ErrReleaseNotesNoBaseTag{}
+	}
+
+	baseCommit, err := gitRepo.GetCommit(tagName)
+	if err != nil {
+		return nil, newErrReleaseNotesTagNotFound(tagName)
+	}
+	return &baseSelection{
+		CompareBase: tagName,
+		PreviousTag: tagName,
+		Commit:      baseCommit,
+	}, nil
+}
+
+func autoPreviousReleaseTag(ctx context.Context, repo *repo_model.Repository, tagName string) (string, error) {
+	tagName = strings.TrimSpace(tagName)
+	if tagName == "" {
+		return "", nil
+	}
+
+	currentRelease, err := repo_model.GetRelease(ctx, repo.ID, tagName)
+	switch {
+	case err == nil:
+		return findPreviousPublishedReleaseTag(ctx, repo, currentRelease)
+	case repo_model.IsErrReleaseNotExist(err):
+		// this tag has no stored release, fall back to latest release below
+	default:
+		return "", fmt.Errorf("GetRelease: %w", err)
+	}
+
+	rel, err := repo_model.GetLatestReleaseByRepoID(ctx, repo.ID)
+	switch {
+	case err == nil:
+		candidate := strings.TrimSpace(rel.TagName)
+		if candidate == "" || strings.EqualFold(candidate, tagName) {
+			return "", nil
+		}
+		return candidate, nil
+	case repo_model.IsErrReleaseNotExist(err):
+		return "", nil
+	default:
+		return "", fmt.Errorf("GetLatestReleaseByRepoID: %w", err)
+	}
+}
+
+func findPreviousPublishedReleaseTag(ctx context.Context, repo *repo_model.Repository, current *repo_model.Release) (string, error) {
+	target := strings.TrimSpace(current.TagName)
+	prev, err := repo_model.GetPreviousPublishedRelease(ctx, repo.ID, current)
+	switch {
+	case err == nil:
+	case repo_model.IsErrReleaseNotExist(err):
+		return "", nil
+	default:
+		return "", fmt.Errorf("GetPreviousPublishedRelease: %w", err)
+	}
+
+	candidate := strings.TrimSpace(prev.TagName)
+	if candidate == "" || strings.EqualFold(candidate, target) {
+		return "", nil
+	}
+	return candidate, nil
+}
+
+func findPreviousTagName(tags []*git.Tag, target string) (string, bool) {
+	target = strings.TrimSpace(target)
+	foundTarget := false
+	for _, tag := range tags {
+		name := strings.TrimSpace(tag.Name)
+		if name == "" {
+			continue
+		}
+		if strings.EqualFold(name, target) {
+			foundTarget = true
+			continue
+		}
+		if foundTarget {
+			return name, true
+		}
+	}
+	if !foundTarget && len(tags) > 0 {
+		name := strings.TrimSpace(tags[0].Name)
+		if name != "" {
+			return name, true
+		}
+	}
+	return "", false
+}
+
 func findInitialCommit(commit *git.Commit) (*git.Commit, error) {
 	current := commit
 	for current.ParentCount() > 0 {

services/release/notes_test.go

@@ -4,14 +4,18 @@
 package release
 
 import (
+	"context"
 	"fmt"
+	"strings"
 	"testing"
+	"time"
 
 	"code.gitea.io/gitea/models/db"
 	issues_model "code.gitea.io/gitea/models/issues"
 	repo_model "code.gitea.io/gitea/models/repo"
 	"code.gitea.io/gitea/models/unittest"
 	user_model "code.gitea.io/gitea/models/user"
+	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/gitrepo"
 	"code.gitea.io/gitea/modules/timeutil"
 
@@ -69,6 +73,14 @@ func TestGenerateReleaseNotes_NoReleaseFallsBackToTags(t *testing.T) {
 		Where("repo_id=?", repo.ID).
 		Delete(new(repo_model.Release))
 	require.NoError(t, err)
+	t.Cleanup(func() {
+		if len(releases) == 0 {
+			return
+		}
+		ctx := context.Background()
+		_, err := db.GetEngine(ctx).Insert(&releases)
+		require.NoError(t, err)
+	})
 
 	result, err := GenerateReleaseNotes(t.Context(), repo, gitRepo, GenerateReleaseNotesOptions{
 		TagName: "v1.2.0",
@@ -79,6 +91,52 @@ func TestGenerateReleaseNotes_NoReleaseFallsBackToTags(t *testing.T) {
 	assert.Contains(t, result.Content, "@user5")
 }
 
+func TestAutoPreviousReleaseTag_UsesPrevPublishedRelease(t *testing.T) {
+	unittest.PrepareTestEnv(t)
+	ctx := t.Context()
+
+	repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
+	prev := insertTestRelease(ctx, t, repo, "auto-prev", timeutil.TimeStamp(100), releaseInsertOptions{})
+	insertTestRelease(ctx, t, repo, "auto-draft", timeutil.TimeStamp(150), releaseInsertOptions{IsDraft: true})
+	insertTestRelease(ctx, t, repo, "auto-pre", timeutil.TimeStamp(175), releaseInsertOptions{IsPrerelease: true})
+	current := insertTestRelease(ctx, t, repo, "auto-current", timeutil.TimeStamp(200), releaseInsertOptions{})
+
+	candidate, err := autoPreviousReleaseTag(ctx, repo, current.TagName)
+	require.NoError(t, err)
+	assert.Equal(t, prev.TagName, candidate)
+}
+
+func TestAutoPreviousReleaseTag_LatestReleaseFallback(t *testing.T) {
+	unittest.PrepareTestEnv(t)
+	ctx := t.Context()
+
+	repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
+	latest := insertTestRelease(ctx, t, repo, "auto-latest", timeutil.TimeStampNow(), releaseInsertOptions{})
+
+	candidate, err := autoPreviousReleaseTag(ctx, repo, "missing-tag")
+	require.NoError(t, err)
+	assert.Equal(t, latest.TagName, candidate)
+}
+
+func TestFindPreviousTagName(t *testing.T) {
+	tags := []*git.Tag{
+		{Name: "v2.0.0"},
+		{Name: "v1.1.0"},
+		{Name: "v1.0.0"},
+	}
+
+	prev, ok := findPreviousTagName(tags, "v1.1.0")
+	require.True(t, ok)
+	assert.Equal(t, "v1.0.0", prev)
+
+	prev, ok = findPreviousTagName(tags, "v9.9.9")
+	require.True(t, ok)
+	assert.Equal(t, "v2.0.0", prev)
+
+	_, ok = findPreviousTagName([]*git.Tag{{Name: ""}}, "v1.0.0")
+	assert.False(t, ok)
+}
+
 func createMergedPullRequest(t *testing.T, repo *repo_model.Repository, mergeCommit string, posterID int64) *issues_model.PullRequest {
 	user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: posterID})
 
@@ -115,3 +173,37 @@ func createMergedPullRequest(t *testing.T, repo *repo_model.Repository, mergeCom
 	require.NoError(t, pr.Issue.LoadAttributes(t.Context()))
 	return pr
 }
+
+type releaseInsertOptions struct {
+	IsDraft      bool
+	IsPrerelease bool
+	IsTag        bool
+}
+
+func insertTestRelease(ctx context.Context, t *testing.T, repo *repo_model.Repository, tag string, created timeutil.TimeStamp, opts releaseInsertOptions) *repo_model.Release {
+	t.Helper()
+	lower := strings.ToLower(tag)
+
+	release := &repo_model.Release{
+		RepoID:       repo.ID,
+		PublisherID:  repo.OwnerID,
+		TagName:      tag,
+		LowerTagName: lower,
+		Target:       repo.DefaultBranch,
+		Title:        tag,
+		Sha1:         fmt.Sprintf("%040d", int64(created)+time.Now().UnixNano()),
+		IsDraft:      opts.IsDraft,
+		IsPrerelease: opts.IsPrerelease,
+		IsTag:        opts.IsTag,
+		CreatedUnix:  created,
+	}
+
+	_, err := db.GetEngine(ctx).Insert(release)
+	require.NoError(t, err)
+	t.Cleanup(func() {
+		_, err := db.GetEngine(context.Background()).ID(release.ID).Delete(new(repo_model.Release))
+		require.NoError(t, err)
+	})
+
+	return release
+}