back round 0 view raw

appview/pulls: do not mutate submissions in stacked resubmits #663

merged
opened by oppi.li targeting master from push-yzlvxoyxmuyp

patch equality is cool, but insufficent to represent the change of a change. the underlying context could have changed.

Signed-off-by: oppiliappan me@oppi.li

options
unified split
Changed files
+1 -63
appview
pulls
pulls.go
+1 -63
appview/pulls/pulls.go 
···

       
30

       
 

       
	"tangled.org/core/tid"


     

       
31

       
 

       
	"tangled.org/core/types"


     

       
32

       
 

       



     

       
33

       
-

       
	"github.com/bluekeyes/go-gitdiff/gitdiff"


     

       
34

       
 

       
	comatproto "github.com/bluesky-social/indigo/api/atproto"


     

       
35

       
 

       
	lexutil "github.com/bluesky-social/indigo/lex/util"


     

       
36

       
 

       
	indigoxrpc "github.com/bluesky-social/indigo/xrpc"


     
···

       
1927

       
 

       
	// commits that got deleted: corresponding pull is closed


     

       
1928

       
 

       
	// commits that got added: new pull is created


     

       
1929

       
 

       
	// commits that got updated: corresponding pull is resubmitted & new round begins


     

       
1930

       
-

       
	//


     

       
1931

       
-

       
	// for commits that were unchanged: no changes, parent-change-id is updated as necessary


     

       
1932

       
 

       
	additions := make(map[string]*models.Pull)


     

       
1933

       
 

       
	deletions := make(map[string]*models.Pull)


     

       
1934

       
-

       
	unchanged := make(map[string]struct{})


     

       
1935

       
 

       
	updated := make(map[string]struct{})


     

       
1936

       
 

       



     

       
1937

       
 

       
	// pulls in orignal stack but not in new one


     
···

       
1953

       
 

       
	for _, np := range newStack {


     

       
1954

       
 

       
		if op, ok := origById[np.ChangeId]; ok {


     

       
1955

       
 

       
			// pull exists in both stacks


     

       
1956

       
-

       
			// TODO: can we avoid reparse?


     

       
1957

       
-

       
			origFiles, origHeaderStr, _ := gitdiff.Parse(strings.NewReader(op.LatestPatch()))


     

       
1958

       
-

       
			newFiles, newHeaderStr, _ := gitdiff.Parse(strings.NewReader(np.LatestPatch()))


     

       
1959

       
-

       



     

       
1960

       
-

       
			origHeader, _ := gitdiff.ParsePatchHeader(origHeaderStr)


     

       
1961

       
-

       
			newHeader, _ := gitdiff.ParsePatchHeader(newHeaderStr)


     

       
1962

       
-

       



     

       
1963

       
-

       
			patchutil.SortPatch(newFiles)


     

       
1964

       
-

       
			patchutil.SortPatch(origFiles)


     

       
1965

       
-

       



     

       
1966

       
-

       
			// text content of patch may be identical, but a jj rebase might have forwarded it


     

       
1967

       
-

       
			//


     

       
1968

       
-

       
			// we still need to update the hash in submission.Patch and submission.SourceRev


     

       
1969

       
-

       
			if patchutil.Equal(newFiles, origFiles) &&


     

       
1970

       
-

       
				origHeader.Title == newHeader.Title &&


     

       
1971

       
-

       
				origHeader.Body == newHeader.Body {


     

       
1972

       
-

       
				unchanged[op.ChangeId] = struct{}{}


     

       
1973

       
-

       
			} else {


     

       
1974

       
-

       
				updated[op.ChangeId] = struct{}{}


     

       
1975

       
-

       
			}


     

       
1976

       
 

       
		}


     

       
1977

       
 

       
	}


     

       
1978

       
 

       



     
···

       
2064

       
 

       
		})


     

       
2065

       
 

       
	}


     

       
2066

       
 

       



     

       
2067

       
-

       
	// unchanged pulls are edited without starting a new round


     

       
2068

       
-

       
	//


     

       
2069

       
-

       
	// update source-revs & patches without advancing rounds


     

       
2070

       
-

       
	for changeId := range unchanged {


     

       
2071

       
-

       
		op, _ := origById[changeId]


     

       
2072

       
-

       
		np, _ := newById[changeId]


     

       
2073

       
-

       



     

       
2074

       
-

       
		origSubmission := op.Submissions[op.LastRoundNumber()]


     

       
2075

       
-

       
		newSubmission := np.Submissions[np.LastRoundNumber()]


     

       
2076

       
-

       



     

       
2077

       
-

       
		log.Println("moving unchanged change id : ", changeId)


     

       
2078

       
-

       



     

       
2079

       
-

       
		err := db.UpdatePull(


     

       
2080

       
-

       
			tx,


     

       
2081

       
-

       
			newSubmission.Patch,


     

       
2082

       
-

       
			newSubmission.SourceRev,


     

       
2083

       
-

       
			db.FilterEq("id", origSubmission.ID),


     

       
2084

       
-

       
		)


     

       
2085

       
-

       



     

       
2086

       
-

       
		if err != nil {


     

       
2087

       
-

       
			log.Println("failed to update pull", err, op.PullId)


     

       
2088

       
-

       
			s.pages.Notice(w, "pull-resubmit-error", "Failed to resubmit pull request. Try again later.")


     

       
2089

       
-

       
			return


     

       
2090

       
-

       
		}


     

       
2091

       
-

       



     

       
2092

       
-

       
		record := op.AsRecord()


     

       
2093

       
-

       
		record.Patch = newSubmission.Patch


     

       
2094

       
-

       



     

       
2095

       
-

       
		writes = append(writes, &comatproto.RepoApplyWrites_Input_Writes_Elem{


     

       
2096

       
-

       
			RepoApplyWrites_Update: &comatproto.RepoApplyWrites_Update{


     

       
2097

       
-

       
				Collection: tangled.RepoPullNSID,


     

       
2098

       
-

       
				Rkey:       op.Rkey,


     

       
2099

       
-

       
				Value: &lexutil.LexiconTypeDecoder{


     

       
2100

       
-

       
					Val: &record,


     

       
2101

       
-

       
				},


     

       
2102

       
-

       
			},


     

       
2103

       
-

       
		})


     

       
2104

       
-

       
	}


     

       
2105

       
-

       



     

       
2106

       
 

       
	// update parent-change-id relations for the entire stack


     

       
2107

       
 

       
	for _, p := range newStack {


     

       
2108

       
 

       
		err := db.SetPullParentChangeId(


     
···

       
30

       
 

       
	"tangled.org/core/tid"


     

       
31

       
 

       
	"tangled.org/core/types"


     

       
32

       
 

       



     

       

       

       
     

       
33

       
 

       
	comatproto "github.com/bluesky-social/indigo/api/atproto"


     

       
34

       
 

       
	lexutil "github.com/bluesky-social/indigo/lex/util"


     

       
35

       
 

       
	indigoxrpc "github.com/bluesky-social/indigo/xrpc"


     
···

       
1926

       
 

       
	// commits that got deleted: corresponding pull is closed


     

       
1927

       
 

       
	// commits that got added: new pull is created


     

       
1928

       
 

       
	// commits that got updated: corresponding pull is resubmitted & new round begins


     

       

       

       
     

       

       

       
     

       
1929

       
 

       
	additions := make(map[string]*models.Pull)


     

       
1930

       
 

       
	deletions := make(map[string]*models.Pull)


     

       

       

       
     

       
1931

       
 

       
	updated := make(map[string]struct{})


     

       
1932

       
 

       



     

       
1933

       
 

       
	// pulls in orignal stack but not in new one


     
···

       
1949

       
 

       
	for _, np := range newStack {


     

       
1950

       
 

       
		if op, ok := origById[np.ChangeId]; ok {


     

       
1951

       
 

       
			// pull exists in both stacks


     

       
1952

       
+

       
			updated[op.ChangeId] = struct{}{}


     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       
1953

       
 

       
		}


     

       
1954

       
 

       
	}


     

       
1955

       
 

       



     
···

       
2041

       
 

       
		})


     

       
2042

       
 

       
	}


     

       
2043

       
 

       



     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       

       

       
     

       
2044

       
 

       
	// update parent-change-id relations for the entire stack


     

       
2045

       
 

       
	for _, p := range newStack {


     

       
2046

       
 

       
		err := db.SetPullParentChangeId(