* Fixes #7152 - Allow create/update/delete message to be empty, use default message * Linting fix * Fix to delete integration tests
This commit is contained in:
		
							parent
							
								
									462284e2f5
								
							
						
					
					
						commit
						002b597c1f
					
				
					 7 changed files with 104 additions and 27 deletions
				
			
		|  | @ -28,7 +28,7 @@ func getCreateFileOptions() api.CreateFileOptions { | |||
| 		FileOptions: api.FileOptions{ | ||||
| 			BranchName:    "master", | ||||
| 			NewBranchName: "master", | ||||
| 			Message:       "Creates new/file.txt", | ||||
| 			Message:       "Making this new file new/file.txt", | ||||
| 			Author: api.Identity{ | ||||
| 				Name:  "John Doe", | ||||
| 				Email: "johndoe@example.com", | ||||
|  | @ -150,6 +150,19 @@ func TestAPICreateFile(t *testing.T) { | |||
| 		assert.EqualValues(t, expectedSHA, fileResponse.Content.SHA) | ||||
| 		assert.EqualValues(t, expectedHTMLURL, fileResponse.Content.HTMLURL) | ||||
| 		assert.EqualValues(t, expectedDownloadURL, fileResponse.Content.DownloadURL) | ||||
| 		assert.EqualValues(t, createFileOptions.Message+"\n", fileResponse.Commit.Message) | ||||
| 
 | ||||
| 		// Test creating a file without a message
 | ||||
| 		createFileOptions = getCreateFileOptions() | ||||
| 		createFileOptions.Message = "" | ||||
| 		fileID++ | ||||
| 		treePath = fmt.Sprintf("new/file%d.txt", fileID) | ||||
| 		url = fmt.Sprintf("/api/v1/repos/%s/%s/contents/%s?token=%s", user2.Name, repo1.Name, treePath, token2) | ||||
| 		req = NewRequestWithJSON(t, "POST", url, &createFileOptions) | ||||
| 		resp = session.MakeRequest(t, req, http.StatusCreated) | ||||
| 		DecodeJSON(t, resp, &fileResponse) | ||||
| 		expectedMessage := "Add '" + treePath + "'\n" | ||||
| 		assert.EqualValues(t, expectedMessage, fileResponse.Commit.Message) | ||||
| 
 | ||||
| 		// Test trying to create a file that already exists, should fail
 | ||||
| 		createFileOptions = getCreateFileOptions() | ||||
|  |  | |||
|  | @ -23,7 +23,7 @@ func getDeleteFileOptions() *api.DeleteFileOptions { | |||
| 		FileOptions: api.FileOptions{ | ||||
| 			BranchName:    "master", | ||||
| 			NewBranchName: "master", | ||||
| 			Message:       "Updates new/file.txt", | ||||
| 			Message:       "Removing the file new/file.txt", | ||||
| 			Author: api.Identity{ | ||||
| 				Name:  "John Doe", | ||||
| 				Email: "johndoe@example.com", | ||||
|  | @ -89,6 +89,20 @@ func TestAPIDeleteFile(t *testing.T) { | |||
| 		DecodeJSON(t, resp, &fileResponse) | ||||
| 		assert.NotNil(t, fileResponse) | ||||
| 		assert.Nil(t, fileResponse.Content) | ||||
| 		assert.EqualValues(t, deleteFileOptions.Message+"\n", fileResponse.Commit.Message) | ||||
| 
 | ||||
| 		// Test deleting file without a message
 | ||||
| 		fileID++ | ||||
| 		treePath = fmt.Sprintf("delete/file%d.txt", fileID) | ||||
| 		createFile(user2, repo1, treePath) | ||||
| 		deleteFileOptions = getDeleteFileOptions() | ||||
| 		deleteFileOptions.Message = "" | ||||
| 		url = fmt.Sprintf("/api/v1/repos/%s/%s/contents/%s?token=%s", user2.Name, repo1.Name, treePath, token2) | ||||
| 		req = NewRequestWithJSON(t, "DELETE", url, &deleteFileOptions) | ||||
| 		resp = session.MakeRequest(t, req, http.StatusOK) | ||||
| 		DecodeJSON(t, resp, &fileResponse) | ||||
| 		expectedMessage := "Delete '" + treePath + "'\n" | ||||
| 		assert.EqualValues(t, expectedMessage, fileResponse.Commit.Message) | ||||
| 
 | ||||
| 		// Test deleting a file with the wrong SHA
 | ||||
| 		fileID++ | ||||
|  |  | |||
|  | @ -25,8 +25,23 @@ func getUpdateFileOptions() *api.UpdateFileOptions { | |||
| 	content := "This is updated text" | ||||
| 	contentEncoded := base64.StdEncoding.EncodeToString([]byte(content)) | ||||
| 	return &api.UpdateFileOptions{ | ||||
| 		DeleteFileOptions: *getDeleteFileOptions(), | ||||
| 		Content:           contentEncoded, | ||||
| 		DeleteFileOptions: api.DeleteFileOptions{ | ||||
| 			FileOptions: api.FileOptions{ | ||||
| 				BranchName:    "master", | ||||
| 				NewBranchName: "master", | ||||
| 				Message:       "My update of new/file.txt", | ||||
| 				Author: api.Identity{ | ||||
| 					Name:  "John Doe", | ||||
| 					Email: "johndoe@example.com", | ||||
| 				}, | ||||
| 				Committer: api.Identity{ | ||||
| 					Name:  "Jane Doe", | ||||
| 					Email: "janedoe@example.com", | ||||
| 				}, | ||||
| 			}, | ||||
| 			SHA: "103ff9234cefeee5ec5361d22b49fbb04d385885", | ||||
| 		}, | ||||
| 		Content: contentEncoded, | ||||
| 	} | ||||
| } | ||||
| 
 | ||||
|  | @ -67,7 +82,7 @@ func getExpectedFileResponseForUpdate(commitID, treePath string) *api.FileRespon | |||
| 					Email: "johndoe@example.com", | ||||
| 				}, | ||||
| 			}, | ||||
| 			Message: "Updates README.md\n", | ||||
| 			Message: "My update of README.md\n", | ||||
| 		}, | ||||
| 		Verification: &api.PayloadCommitVerification{ | ||||
| 			Verified:  false, | ||||
|  | @ -140,6 +155,7 @@ func TestAPIUpdateFile(t *testing.T) { | |||
| 		assert.EqualValues(t, expectedSHA, fileResponse.Content.SHA) | ||||
| 		assert.EqualValues(t, expectedHTMLURL, fileResponse.Content.HTMLURL) | ||||
| 		assert.EqualValues(t, expectedDownloadURL, fileResponse.Content.DownloadURL) | ||||
| 		assert.EqualValues(t, updateFileOptions.Message+"\n", fileResponse.Commit.Message) | ||||
| 
 | ||||
| 		// Test updating a file and renaming it
 | ||||
| 		updateFileOptions = getUpdateFileOptions() | ||||
|  | @ -160,6 +176,20 @@ func TestAPIUpdateFile(t *testing.T) { | |||
| 		assert.EqualValues(t, expectedHTMLURL, fileResponse.Content.HTMLURL) | ||||
| 		assert.EqualValues(t, expectedDownloadURL, fileResponse.Content.DownloadURL) | ||||
| 
 | ||||
| 		// Test updating a file without a message
 | ||||
| 		updateFileOptions = getUpdateFileOptions() | ||||
| 		updateFileOptions.Message = "" | ||||
| 		updateFileOptions.BranchName = repo1.DefaultBranch | ||||
| 		fileID++ | ||||
| 		treePath = fmt.Sprintf("update/file%d.txt", fileID) | ||||
| 		createFile(user2, repo1, treePath) | ||||
| 		url = fmt.Sprintf("/api/v1/repos/%s/%s/contents/%s?token=%s", user2.Name, repo1.Name, treePath, token2) | ||||
| 		req = NewRequestWithJSON(t, "PUT", url, &updateFileOptions) | ||||
| 		resp = session.MakeRequest(t, req, http.StatusOK) | ||||
| 		DecodeJSON(t, resp, &fileResponse) | ||||
| 		expectedMessage := "Update '" + treePath + "'\n" | ||||
| 		assert.EqualValues(t, expectedMessage, fileResponse.Commit.Message) | ||||
| 
 | ||||
| 		// Test updating a file with the wrong SHA
 | ||||
| 		fileID++ | ||||
| 		treePath = fmt.Sprintf("update/file%d.txt", fileID) | ||||
|  |  | |||
|  | @ -24,40 +24,32 @@ func getDeleteRepoFileOptions(repo *models.Repository) *repofiles.DeleteRepoFile | |||
| 		TreePath:     "README.md", | ||||
| 		Message:      "Deletes README.md", | ||||
| 		SHA:          "4b4851ad51df6a7d9f25c979345979eaeb5b349f", | ||||
| 		Author:       nil, | ||||
| 		Committer:    nil, | ||||
| 		Author: &repofiles.IdentityOptions{ | ||||
| 			Name:  "Bob Smith", | ||||
| 			Email: "bob@smith.com", | ||||
| 		}, | ||||
| 		Committer: nil, | ||||
| 	} | ||||
| } | ||||
| 
 | ||||
| func getExpectedDeleteFileResponse(u *url.URL) *api.FileResponse { | ||||
| 	// Just returns fields that don't change, i.e. fields with commit SHAs and dates can't be determined
 | ||||
| 	return &api.FileResponse{ | ||||
| 		Content: nil, | ||||
| 		Commit: &api.FileCommitResponse{ | ||||
| 			CommitMeta: api.CommitMeta{ | ||||
| 				URL: u.String() + "api/v1/repos/user2/repo1/git/commits/65f1bf27bc3bf70f64657658635e66094edbcb4d", | ||||
| 				SHA: "65f1bf27bc3bf70f64657658635e66094edbcb4d", | ||||
| 			}, | ||||
| 			HTMLURL: u.String() + "user2/repo1/commit/65f1bf27bc3bf70f64657658635e66094edbcb4d", | ||||
| 			Author: &api.CommitUser{ | ||||
| 				Identity: api.Identity{ | ||||
| 					Name:  "user1", | ||||
| 					Email: "address1@example.com", | ||||
| 					Name:  "Bob Smith", | ||||
| 					Email: "bob@smith.com", | ||||
| 				}, | ||||
| 				Date: "2017-03-19T20:47:59Z", | ||||
| 			}, | ||||
| 			Committer: &api.CommitUser{ | ||||
| 				Identity: api.Identity{ | ||||
| 					Name:  "Ethan Koenig", | ||||
| 					Email: "ethantkoenig@gmail.com", | ||||
| 					Name:  "Bob Smith", | ||||
| 					Email: "bob@smith.com", | ||||
| 				}, | ||||
| 				Date: "2017-03-19T20:47:59Z", | ||||
| 			}, | ||||
| 			Parents: []*api.CommitMeta{}, | ||||
| 			Message: "Initial commit\n", | ||||
| 			Tree: &api.CommitMeta{ | ||||
| 				URL: u.String() + "api/v1/repos/user2/repo1/git/trees/2a2f1d4670728a2e10049e345bd7a276468beab6", | ||||
| 				SHA: "2a2f1d4670728a2e10049e345bd7a276468beab6", | ||||
| 			}, | ||||
| 			Message: "Deletes README.md\n", | ||||
| 		}, | ||||
| 		Verification: &api.PayloadCommitVerification{ | ||||
| 			Verified:  false, | ||||
|  | @ -89,7 +81,12 @@ func testDeleteRepoFile(t *testing.T, u *url.URL) { | |||
| 		fileResponse, err := repofiles.DeleteRepoFile(repo, doer, opts) | ||||
| 		assert.Nil(t, err) | ||||
| 		expectedFileResponse := getExpectedDeleteFileResponse(u) | ||||
| 		assert.EqualValues(t, expectedFileResponse, fileResponse) | ||||
| 		assert.NotNil(t, fileResponse) | ||||
| 		assert.Nil(t, fileResponse.Content) | ||||
| 		assert.EqualValues(t, expectedFileResponse.Commit.Message, fileResponse.Commit.Message) | ||||
| 		assert.EqualValues(t, expectedFileResponse.Commit.Author.Identity, fileResponse.Commit.Author.Identity) | ||||
| 		assert.EqualValues(t, expectedFileResponse.Commit.Committer.Identity, fileResponse.Commit.Committer.Identity) | ||||
| 		assert.EqualValues(t, expectedFileResponse.Verification, fileResponse.Verification) | ||||
| 	}) | ||||
| 
 | ||||
| 	t.Run("Verify README.md has been deleted", func(t *testing.T) { | ||||
|  | @ -124,7 +121,12 @@ func testDeleteRepoFileWithoutBranchNames(t *testing.T, u *url.URL) { | |||
| 		fileResponse, err := repofiles.DeleteRepoFile(repo, doer, opts) | ||||
| 		assert.Nil(t, err) | ||||
| 		expectedFileResponse := getExpectedDeleteFileResponse(u) | ||||
| 		assert.EqualValues(t, expectedFileResponse, fileResponse) | ||||
| 		assert.NotNil(t, fileResponse) | ||||
| 		assert.Nil(t, fileResponse.Content) | ||||
| 		assert.EqualValues(t, expectedFileResponse.Commit.Message, fileResponse.Commit.Message) | ||||
| 		assert.EqualValues(t, expectedFileResponse.Commit.Author.Identity, fileResponse.Commit.Author.Identity) | ||||
| 		assert.EqualValues(t, expectedFileResponse.Commit.Committer.Identity, fileResponse.Commit.Committer.Identity) | ||||
| 		assert.EqualValues(t, expectedFileResponse.Verification, fileResponse.Verification) | ||||
| 	}) | ||||
| } | ||||
| 
 | ||||
|  |  | |||
|  | @ -198,6 +198,11 @@ func DeleteRepoFile(repo *models.Repository, doer *models.User, opts *DeleteRepo | |||
| 		return nil, fmt.Errorf("PushUpdate: %v", err) | ||||
| 	} | ||||
| 
 | ||||
| 	commit, err = t.GetCommit(commitHash) | ||||
| 	if err != nil { | ||||
| 		return nil, err | ||||
| 	} | ||||
| 
 | ||||
| 	file, err := GetFileResponseFromCommit(repo, commit, opts.NewBranch, treePath) | ||||
| 	if err != nil { | ||||
| 		return nil, err | ||||
|  |  | |||
|  | @ -8,7 +8,7 @@ package structs | |||
| // FileOptions options for all file APIs
 | ||||
| type FileOptions struct { | ||||
| 	// message (optional) for the commit of this file. if not supplied, a default message will be used
 | ||||
| 	Message string `json:"message" binding:"Required"` | ||||
| 	Message string `json:"message"` | ||||
| 	// branch (optional) to base this file from. if not given, the default branch is used
 | ||||
| 	BranchName string `json:"branch"` | ||||
| 	// new_branch (optional) will make a new branch from `branch` before creating the file
 | ||||
|  |  | |||
|  | @ -204,6 +204,11 @@ func CreateFile(ctx *context.APIContext, apiOpts api.CreateFileOptions) { | |||
| 			Email: apiOpts.Author.Email, | ||||
| 		}, | ||||
| 	} | ||||
| 
 | ||||
| 	if opts.Message == "" { | ||||
| 		opts.Message = ctx.Tr("repo.editor.add", opts.TreePath) | ||||
| 	} | ||||
| 
 | ||||
| 	if fileResponse, err := createOrUpdateFile(ctx, opts); err != nil { | ||||
| 		ctx.Error(http.StatusInternalServerError, "CreateFile", err) | ||||
| 	} else { | ||||
|  | @ -264,6 +269,10 @@ func UpdateFile(ctx *context.APIContext, apiOpts api.UpdateFileOptions) { | |||
| 		}, | ||||
| 	} | ||||
| 
 | ||||
| 	if opts.Message == "" { | ||||
| 		opts.Message = ctx.Tr("repo.editor.update", opts.TreePath) | ||||
| 	} | ||||
| 
 | ||||
| 	if fileResponse, err := createOrUpdateFile(ctx, opts); err != nil { | ||||
| 		ctx.Error(http.StatusInternalServerError, "UpdateFile", err) | ||||
| 	} else { | ||||
|  | @ -346,6 +355,10 @@ func DeleteFile(ctx *context.APIContext, apiOpts api.DeleteFileOptions) { | |||
| 		}, | ||||
| 	} | ||||
| 
 | ||||
| 	if opts.Message == "" { | ||||
| 		opts.Message = ctx.Tr("repo.editor.delete", opts.TreePath) | ||||
| 	} | ||||
| 
 | ||||
| 	if fileResponse, err := repofiles.DeleteRepoFile(ctx.Repo.Repository, ctx.User, opts); err != nil { | ||||
| 		ctx.Error(http.StatusInternalServerError, "DeleteFile", err) | ||||
| 	} else { | ||||
|  |  | |||
		Loading…
	
		Reference in a new issue