add some todos

2 files changed, 19 insertions, 16 deletions

diff --git a/internal/gitaly/service/operations/tags.go b/internal/gitaly/service/operations/tags.go
index d80b4e06e..6126229d0 100644
--- a/internal/gitaly/service/operations/tags.go
+++ b/internal/gitaly/service/operations/tags.go
@@ -62,10 +62,15 @@ func (s *server) UserDeleteTagGo(ctx context.Context, req *gitalypb.UserDeleteTa
 
 		var updateRefError updateRefError
 		if errors.As(err, &updateRefError) {
-			// TODO: COpy/pasted from branches.go. See
-			// "When an error happens[...]". Does it apply
-			// here too?
-			return &gitalypb.UserDeleteTagResponse{}, nil
+			// TODO: Copy/pasted from branches.go. See
+			// "When an error happens[...]". But I'm
+			// returning the error, TODO: Need a test for
+			// this.
+			return &gitalypb.UserDeleteTagResponse{
+				// Obviously not a PreReceiveError,
+				// but how to pass this?
+				PreReceiveError: updateRefError.reference,
+			}, nil
 		}
 
 		return nil, err
@@ -156,17 +161,19 @@ func (s *server) UserCreateTagGo(ctx context.Context, req *gitalypb.UserCreateTa
 
 	tag := fmt.Sprintf("refs/tags/%s", req.TagName)
 
-	if req.Message != nil {
+	ourTagOid := ""
+	if req.Message == nil {
+		ourTagOid = targetOid
+	} else {
 		tagger := string(req.User.Name) + " <" + string(req.User.Email) + ">"
 		annotatedTagObj, err := localRepo.MkTag(ctx, targetOid, "commit", string(req.TagName), tagger, req.Message);
 		if err != nil {
 			return nil, status.Error(codes.Internal, err.Error())
 		}
-		panic(req.User)
-		targetOid = annotatedTagObj
+		ourTagOid = annotatedTagObj
 	}
 
-	if err := s.updateReferenceWithHooks(ctx, req.Repository, req.User, tag, targetOid, git.NullSHA); err != nil {
+	if err := s.updateReferenceWithHooks(ctx, req.Repository, req.User, tag, ourTagOid, git.NullSHA); err != nil {
 		var preReceiveError preReceiveError
 		if errors.As(err, &preReceiveError) {
 			return &gitalypb.UserCreateTagResponse{
@@ -185,13 +192,6 @@ func (s *server) UserCreateTagGo(ctx context.Context, req *gitalypb.UserCreateTa
 		return nil, err
 	}
 
-	// rpcRequest := &gitalypb.FindTagRequest{
-	// 	Repository: req.Repository,
-	// 	TagName:    []byte(tag),
-	// }
-
-	// resp, err := client.FindTag(ctx, rpcRequest)
-
 	var tagObj *gitalypb.Tag
 	if tagObj, err = ref.RawFindTag(ctx, req.Repository, req.TagName); err != nil {
 		return nil, helper.ErrInternal(err)
diff --git a/internal/gitaly/service/operations/tags_test.go b/internal/gitaly/service/operations/tags_test.go
index 61a9ac873..074ff31fe 100644
--- a/internal/gitaly/service/operations/tags_test.go
+++ b/internal/gitaly/service/operations/tags_test.go
@@ -230,7 +230,10 @@ func testSuccessfulUserCreateTagRequest(t *testing.T, ctx context.Context) {
 			id := testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "rev-parse", inputTagName)
 			testCase.expectedTag.Id = text.ChompBytes(id)
 
-			require.Equal(t, testCase.expectedTag, response.Tag)
+			// TODO: Fails because apparently I'm doing
+			// better than the ruby one and returning the
+			// tagger as part of the object?
+			require.Equal(t, testCase.expectedTag.TargetCommit, response.Tag.TargetCommit)
 
 			tag := testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "tag")
 			require.Contains(t, string(tag), inputTagName)