Address PR review comments

Signed-off-by: Juan Antonio Osorio <[email protected]>

internal/providers/gitlab/manager/webhook_handlers.go

@@ -1,4 +1,3 @@
-// Copyright 2024 Stacklok, Inc.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -111,24 +110,25 @@ func (m *providerClassManager) handleMergeRequest(l zerolog.Logger, r *http.Requ
 
 	mrID := mergeRequestEvent.ObjectAttributes.IID
 	if mrID == 0 {
-		l.Error().Msg("merge request event missing IID")
 		return fmt.Errorf("merge request event missing IID")
 	}
 
 	rawID := mergeRequestEvent.Project.ID
 	if rawID == 0 {
-		l.Error().Msg("merge request event missing project ID")
 		return fmt.Errorf("merge request event missing project ID")
 	}
 
 	switch {
 	case mergeRequestEvent.ObjectAttributes.Action == "open",
 		mergeRequestEvent.ObjectAttributes.Action == "reopen":
-		return m.publishOriginatingOpenMergeRequest(l, rawID, mrID)
+		return m.publishMergeRequestMessage(l, rawID, mrID,
+			events.TopicQueueOriginatingEntityAdd)
 	case mergeRequestEvent.ObjectAttributes.Action == "close":
-		return m.publishClosedMergeRequest(l, rawID, mrID)
+		return m.publishMergeRequestMessage(l, rawID, mrID,
+			events.TopicQueueOriginatingEntityDelete)
 	case mergeRequestEvent.ObjectAttributes.Action == "update":
-		return m.publishRefreshAndEvalForMergeRequest(l, rawID, mrID)
+		return m.publishMergeRequestMessage(l, rawID, mrID,
+			events.TopicQueueRefreshEntityAndEvaluate)
 	default:
 		return nil
 	}
@@ -170,21 +170,6 @@ func (m *providerClassManager) publishRefreshAndEvalForGitlabProject(
 	return nil
 }
 
-func (m *providerClassManager) publishOriginatingOpenMergeRequest(
-	l zerolog.Logger, rawProjectID int, mrID int) error {
-	return m.publishMergeRequestMessage(l, mrID, rawProjectID, events.TopicQueueOriginatingEntityAdd)
-}
-
-func (m *providerClassManager) publishClosedMergeRequest(
-	l zerolog.Logger, rawProjectID int, mrID int) error {
-	return m.publishMergeRequestMessage(l, mrID, rawProjectID, events.TopicQueueOriginatingEntityDelete)
-}
-
-func (m *providerClassManager) publishRefreshAndEvalForMergeRequest(
-	l zerolog.Logger, rawProjectID int, mrID int) error {
-	return m.publishMergeRequestMessage(l, mrID, rawProjectID, events.TopicQueueRefreshEntityAndEvaluate)
-}
-
 func (m *providerClassManager) publishMergeRequestMessage(
 	l zerolog.Logger, mrID int, rawProjectID int, queueTopic string) error {
 	mrUpstreamID := gitlab.FormatPullRequestUpstreamID(mrID)
@@ -196,15 +181,13 @@ func (m *providerClassManager) publishMergeRequestMessage(
 		gitlab.PullRequestProjectID:   mrProjectID,
 	})
 	if err != nil {
-		l.Error().Err(err).Msg("error creating identifying properties")
 		return fmt.Errorf("error creating identifying properties: %w", err)
 	}
 
 	repoIdentifyingProps, err := properties.NewProperties(map[string]any{
 		properties.PropertyUpstreamID: mrProjectID,
 	})
 	if err != nil {
-		l.Error().Err(err).Msg("error creating repo identifying properties")
 		return fmt.Errorf("error creating repo identifying properties: %w", err)
 	}
 
@@ -218,14 +201,11 @@ func (m *providerClassManager) publishMergeRequestMessage(
 	msgID := uuid.New().String()
 	msg := message.NewMessage(msgID, nil)
 	if err := outm.ToMessage(msg); err != nil {
-		l.Error().Err(err).Msg("error converting message to protobuf")
 		return fmt.Errorf("error converting message to protobuf: %w", err)
 	}
 
 	// Publish message
-	l.Debug().Str("msg_id", msgID).Msg("publishing refresh and eval message")
 	if err := m.pub.Publish(queueTopic, msg); err != nil {
-		l.Error().Err(err).Msg("error publishing refresh and eval message")
 		return fmt.Errorf("error publishing refresh and eval message: %w", err)
 	}
 

internal/providers/gitlab/pull_request_properties.go

@@ -83,8 +83,8 @@ func (c *gitlabClient) getPropertiesForPullRequest(
 	}
 
 	// Validate - merge request upstream ID must match the one we requested
-	if FormatPullRequestUpstreamID(mr.ID) != uid {
-		return nil, fmt.Errorf("merge request ID mismatch: %s != %s", FormatPullRequestUpstreamID(mr.ID), uid)
+	if res := FormatPullRequestUpstreamID(mr.ID); res != uid {
+		return nil, fmt.Errorf("merge request ID mismatch: %s != %s", res, uid)
 	}
 
 	proj, err := c.getGitLabProject(ctx, pid)