deprecate PR edited field (#2692)

* deprecate PR edited field
CODE-2402
Marko Gaćeša 2024-09-16 15:44:17 +00:00 committed by Harness
parent 934b3e8e5a
commit 1e644163d3
16 changed files with 78 additions and 49 deletions

View File

@ -17,7 +17,6 @@ package pullreq
import (
"context"
"fmt"
"time"
"github.com/harness/gitness/app/auth"
"github.com/harness/gitness/app/services/label"
@ -59,13 +58,9 @@ func (c *Controller) AssignLabel(
return out.PullReqLabel, nil
}
pullreq, err = c.pullreqStore.UpdateOptLock(ctx, pullreq, func(pullreq *types.PullReq) error {
pullreq.Edited = time.Now().UnixMilli()
pullreq.ActivitySeq++
return nil
})
pullreq, err = c.pullreqStore.UpdateActivitySeq(ctx, pullreq)
if err != nil {
return nil, fmt.Errorf("failed to update pull request: %w", err)
return nil, fmt.Errorf("failed to update pull request activity sequence: %w", err)
}
payload := activityPayload(out)

View File

@ -17,7 +17,6 @@ package pullreq
import (
"context"
"fmt"
"time"
"github.com/harness/gitness/app/auth"
"github.com/harness/gitness/types"
@ -50,13 +49,9 @@ func (c *Controller) UnassignLabel(
return fmt.Errorf("failed to delete pullreq label: %w", err)
}
pullreq, err = c.pullreqStore.UpdateOptLock(ctx, pullreq, func(pullreq *types.PullReq) error {
pullreq.Edited = time.Now().UnixMilli()
pullreq.ActivitySeq++
return nil
})
pullreq, err = c.pullreqStore.UpdateActivitySeq(ctx, pullreq)
if err != nil {
return fmt.Errorf("failed to update pull request: %w", err)
return fmt.Errorf("failed to update pull request activity sequence: %w", err)
}
var value *string

View File

@ -443,7 +443,6 @@ func (c *Controller) Merge(
nowMilli := now.UnixMilli()
pr.Edited = nowMilli
pr.Merged = &nowMilli
pr.MergedBy = &mergedBy
pr.MergeMethod = &in.Method

View File

@ -140,16 +140,17 @@ func (c *Controller) State(ctx context.Context,
pr, err = c.pullreqStore.UpdateOptLock(ctx, pr, func(pr *types.PullReq) error {
pr.State = in.State
pr.IsDraft = in.IsDraft
pr.Edited = time.Now().UnixMilli()
switch stateChange {
case changeClose:
nowMilli := time.Now().UnixMilli()
// clear all merge (check) related fields
pr.MergeCheckStatus = enum.MergeCheckStatusUnchecked
pr.MergeSHA = nil
pr.MergeConflicts = nil
pr.MergeTargetSHA = nil
pr.Closed = &pr.Edited
pr.Closed = &nowMilli
case changeReopen:
pr.SourceSHA = sourceSHA.String()
pr.MergeBaseSHA = mergeBaseSHA.String()

View File

@ -18,7 +18,6 @@ import (
"context"
"fmt"
"strings"
"time"
apiauth "github.com/harness/gitness/app/api/auth"
"github.com/harness/gitness/app/auth"
@ -96,7 +95,6 @@ func (c *Controller) Update(ctx context.Context,
pr, err = c.pullreqStore.UpdateOptLock(ctx, pr, func(pr *types.PullReq) error {
pr.Title = in.Title
pr.Description = in.Description
pr.Edited = time.Now().UnixMilli()
if needToWriteActivity {
pr.ActivitySeq++
}

View File

@ -127,11 +127,11 @@ var queryParameterCreatedGt = openapi3.ParameterOrRef{
},
}
var queryParameterEditedLt = openapi3.ParameterOrRef{
var queryParameterUpdatedLt = openapi3.ParameterOrRef{
Parameter: &openapi3.Parameter{
Name: request.QueryParamEditedLt,
Name: request.QueryParamUpdatedLt,
In: openapi3.ParameterInQuery,
Description: ptr.String("The result should contain only entries edited before this timestamp (unix millis)."),
Description: ptr.String("The result should contain only entries updated before this timestamp (unix millis)."),
Required: ptr.Bool(false),
Schema: &openapi3.SchemaOrRef{
Schema: &openapi3.Schema{
@ -142,11 +142,11 @@ var queryParameterEditedLt = openapi3.ParameterOrRef{
},
}
var queryParameterEditedGt = openapi3.ParameterOrRef{
var queryParameterUpdatedGt = openapi3.ParameterOrRef{
Parameter: &openapi3.Parameter{
Name: request.QueryParamEditedGt,
Name: request.QueryParamUpdatedGt,
In: openapi3.ParameterInQuery,
Description: ptr.String("The result should contain only entries edited after this timestamp (unix millis)."),
Description: ptr.String("The result should contain only entries updated after this timestamp (unix millis)."),
Required: ptr.Bool(false),
Schema: &openapi3.SchemaOrRef{
Schema: &openapi3.Schema{

View File

@ -493,7 +493,7 @@ func pullReqOperations(reflector *openapi3.Reflector) {
queryParameterSourceBranchPullRequest, queryParameterTargetBranchPullRequest,
queryParameterQueryPullRequest, queryParameterCreatedByPullRequest,
queryParameterOrder, queryParameterSortPullRequest,
queryParameterCreatedLt, queryParameterCreatedGt, queryParameterEditedLt, queryParameterEditedGt,
queryParameterCreatedLt, queryParameterCreatedGt, queryParameterUpdatedLt, queryParameterUpdatedGt,
queryParameterIncludeDescription,
QueryParameterPage, QueryParameterLimit,
QueryParameterLabelID, QueryParameterValueID,

View File

@ -594,7 +594,7 @@ func spaceOperations(reflector *openapi3.Reflector) {
queryParameterStatePullRequest, queryParameterSourceRepoRefPullRequest,
queryParameterSourceBranchPullRequest, queryParameterTargetBranchPullRequest,
queryParameterQueryPullRequest, queryParameterCreatedByPullRequest,
queryParameterCreatedLt, queryParameterCreatedGt, queryParameterEditedLt,
queryParameterCreatedLt, queryParameterCreatedGt, queryParameterUpdatedLt,
queryParameterIncludeDescription, queryParameterIncludeSubspaces,
QueryParameterLimit,
QueryParameterLabelID, QueryParameterValueID,

View File

@ -46,6 +46,8 @@ const (
QueryParamCreatedLt = "created_lt"
QueryParamCreatedGt = "created_gt"
QueryParamUpdatedLt = "updated_lt"
QueryParamUpdatedGt = "updated_gt"
QueryParamEditedLt = "edited_lt"
QueryParamEditedGt = "edited_gt"
@ -152,6 +154,26 @@ func ParseCreated(r *http.Request) (types.CreatedFilter, error) {
return filter, nil
}
// ParseUpdated extracts the updated filter from the url query param.
func ParseUpdated(r *http.Request) (types.UpdatedFilter, error) {
filter := types.UpdatedFilter{}
updatedLt, err := QueryParamAsPositiveInt64OrDefault(r, QueryParamUpdatedLt, 0)
if err != nil {
return filter, fmt.Errorf("encountered error parsing updated lt: %w", err)
}
updatedGt, err := QueryParamAsPositiveInt64OrDefault(r, QueryParamUpdatedGt, 0)
if err != nil {
return filter, fmt.Errorf("encountered error parsing updated gt: %w", err)
}
filter.UpdatedGt = updatedGt
filter.UpdatedLt = updatedLt
return filter, nil
}
// ParseEdited extracts the edited filter from the url query param.
func ParseEdited(r *http.Request) (types.EditedFilter, error) {
filter := types.EditedFilter{}

View File

@ -110,12 +110,17 @@ func ParsePullReqFilter(r *http.Request) (*types.PullReqFilter, error) {
return nil, fmt.Errorf("encountered error parsing valueid filter: %w", err)
}
createdAtFilter, err := ParseCreated(r)
createdFilter, err := ParseCreated(r)
if err != nil {
return nil, fmt.Errorf("encountered error parsing pr created filter: %w", err)
}
editedAtFilter, err := ParseEdited(r)
updatedFilter, err := ParseUpdated(r)
if err != nil {
return nil, fmt.Errorf("encountered error parsing pr updated filter: %w", err)
}
editedFilter, err := ParseEdited(r)
if err != nil {
return nil, fmt.Errorf("encountered error parsing pr edited filter: %w", err)
}
@ -169,8 +174,9 @@ func ParsePullReqFilter(r *http.Request) (*types.PullReqFilter, error) {
ReviewDecisions: reviewDecisions,
MentionedID: mentionedID,
IncludeDescription: includeDescription,
CreatedFilter: createdAtFilter,
EditedFilter: editedAtFilter,
CreatedFilter: createdFilter,
UpdatedFilter: updatedFilter,
EditedFilter: editedFilter,
}, nil
}

View File

@ -272,7 +272,7 @@ func (r *repoImportState) convertPullReq(
case enum.PullReqStateMerged:
// For merged PR's assume the Head.Sha and Base.Sha point to commits at the time of merging.
pr.Merged = &pr.Edited
pr.Merged = &pr.Updated
pr.MergedBy = &author.ID // Don't have real info for this - use the author.
mergeMethod := enum.MergeMethodMerge // Don't know
pr.MergeMethod = &mergeMethod
@ -294,7 +294,7 @@ func (r *repoImportState) convertPullReq(
pr.MergeSHA = nil
pr.MergeConflicts = nil
pr.MergeTargetSHA = nil
pr.Closed = &pr.Edited
pr.Closed = &pr.Updated
case enum.PullReqStateOpen:
// For open PR we need to verify existence of branches and find to merge base.

View File

@ -19,7 +19,6 @@ import (
"errors"
"fmt"
"strings"
"time"
gitevents "github.com/harness/gitness/app/events/git"
pullreqevents "github.com/harness/gitness/app/events/pullreq"
@ -119,7 +118,6 @@ func (s *Service) triggerPREventOnBranchUpdate(ctx context.Context,
pr.Number, event.Payload.NewSHA, event.Payload.OldSHA, pr.SourceSHA)
}
pr.Edited = time.Now().UnixMilli()
pr.SourceSHA = event.Payload.NewSHA
pr.MergeBaseSHA = newMergeBase.String()

View File

@ -77,10 +77,10 @@ func (c *ListService) ListForSpace(
filter *types.PullReqFilter,
) ([]types.PullReqRepo, error) {
// list of unsupported filter options
filter.Sort = enum.PullReqSortEdited // the only supported option, hardcoded in the SQL query
filter.Order = enum.OrderDesc // the only supported option, hardcoded in the SQL query
filter.Page = 0 // unsupported, pagination should be done with the EditedLt parameter
filter.EditedGt = 0 // unsupported
filter.Sort = enum.PullReqSortUpdated // the only supported option, hardcoded in the SQL query
filter.Order = enum.OrderDesc // the only supported option, hardcoded in the SQL query
filter.Page = 0 // unsupported, pagination should be done with the UpdatedLt parameter
filter.UpdatedGt = 0 // unsupported
if includeSubspaces {
subspaces, err := c.spaceStore.GetDescendantsData(ctx, space.ID)
@ -112,7 +112,7 @@ func (c *ListService) ListForSpace(
loadMore = len(pullReqs) == prLimit || len(repoUnchecked) == repoLimit
if loadMore && len(pullReqs) > 0 {
filter.EditedLt = pullReqs[len(pullReqs)-1].Edited
filter.UpdatedLt = pullReqs[len(pullReqs)-1].Updated
}
for repoID := range repoUnchecked {

View File

@ -61,7 +61,7 @@ type pullReq struct {
CreatedBy int64 `db:"pullreq_created_by"`
Created int64 `db:"pullreq_created"`
Updated int64 `db:"pullreq_updated"`
Edited int64 `db:"pullreq_edited"`
Edited int64 `db:"pullreq_edited"` // TODO: Remove
Closed null.Int `db:"pullreq_closed"`
State enum.PullReqState `db:"pullreq_state"`
@ -308,11 +308,12 @@ func (s *PullReqStore) Update(ctx context.Context, pr *types.PullReq) error {
db := dbtx.GetAccessor(ctx, s.db)
updatedAt := time.Now()
updatedAt := time.Now().UnixMilli()
dbPR := mapInternalPullReq(pr)
dbPR.Version++
dbPR.Updated = updatedAt.UnixMilli()
dbPR.Updated = updatedAt
dbPR.Edited = updatedAt
query, arg, err := db.BindNamed(sqlQuery, dbPR)
if err != nil {
@ -495,7 +496,7 @@ func (s *PullReqStore) List(ctx context.Context, opts *types.PullReqFilter) ([]*
func (s *PullReqStore) Stream(ctx context.Context, opts *types.PullReqFilter) (<-chan *types.PullReq, <-chan error) {
stmt := s.listQuery(opts)
stmt = stmt.OrderBy("pullreq_edited desc")
stmt = stmt.OrderBy("pullreq_updated desc")
chPRs := make(chan *types.PullReq)
chErr := make(chan error, 1)
@ -560,7 +561,7 @@ func (s *PullReqStore) listQuery(opts *types.PullReqFilter) squirrel.SelectBuild
return stmt
}
//nolint:cyclop
//nolint:cyclop,gocognit,gocyclo
func (s *PullReqStore) applyFilter(stmt *squirrel.SelectBuilder, opts *types.PullReqFilter) {
if len(opts.States) == 1 {
*stmt = stmt.Where("pullreq_state = ?", opts.States[0])
@ -600,6 +601,14 @@ func (s *PullReqStore) applyFilter(stmt *squirrel.SelectBuilder, opts *types.Pul
*stmt = stmt.Where("pullreq_created > ?", opts.CreatedGt)
}
if opts.UpdatedLt > 0 {
*stmt = stmt.Where("pullreq_updated < ?", opts.UpdatedLt)
}
if opts.UpdatedGt > 0 {
*stmt = stmt.Where("pullreq_updated > ?", opts.UpdatedGt)
}
if opts.EditedLt > 0 {
*stmt = stmt.Where("pullreq_edited < ?", opts.EditedLt)
}
@ -706,7 +715,7 @@ func mapPullReq(pr *pullReq) *types.PullReq {
CreatedBy: pr.CreatedBy,
Created: pr.Created,
Updated: pr.Updated,
Edited: pr.Edited,
Edited: pr.Edited, // TODO: When we remove the DB column, make Edited equal to Updated
Closed: pr.Closed.Ptr(),
State: pr.State,
IsDraft: pr.IsDraft,
@ -752,7 +761,7 @@ func mapInternalPullReq(pr *types.PullReq) *pullReq {
CreatedBy: pr.CreatedBy,
Created: pr.Created,
Updated: pr.Updated,
Edited: pr.Edited,
Edited: pr.Edited, // TODO: When we remove the DB column, make Edited equal to Updated
Closed: null.IntFromPtr(pr.Closed),
State: pr.State,
IsDraft: pr.IsDraft,

View File

@ -25,6 +25,11 @@ type CreatedFilter struct {
CreatedLt int64 `json:"created_lt"`
}
type UpdatedFilter struct {
UpdatedGt int64 `json:"updated_gt"`
UpdatedLt int64 `json:"updated_lt"`
}
type EditedFilter struct {
EditedGt int64 `json:"edited_gt"`
EditedLt int64 `json:"edited_lt"`

View File

@ -26,8 +26,8 @@ type PullReq struct {
CreatedBy int64 `json:"-"` // not returned, because the author info is in the Author field
Created int64 `json:"created"`
Updated int64 `json:"-"` // not returned, it's updated by the server internally. Clients should use EditedAt.
Edited int64 `json:"edited"`
Updated int64 `json:"updated"`
Edited int64 `json:"edited"` // TODO: Remove. Field Edited is equal to Updated
Closed *int64 `json:"closed,omitempty"`
State enum.PullReqState `json:"state"`
@ -115,6 +115,7 @@ type PullReqFilter struct {
MentionedID int64 `json:"mentioned_id"`
IncludeDescription bool `json:"include_description"`
CreatedFilter
UpdatedFilter
EditedFilter
// internal use only