Skip to content

Commit 65e9e1b

Browse files
committed
fix(lint): gocritic/unparam/unused + migration renumber in notifications PR
Address four linter findings in the mute/unsubscribe feature: - smtp_sender: use %q instead of escaped-quote "%s" (sprintfQuotedString) - sender: remove unused mailtoURL return from buildUnsubscribeURL (unparam) - templates: remove dead sendPurchaseApprovalRequestVia function (unused) - migration: renumber 000078_muted_recipients to 000091 to resolve conflict with 000078_monthly_summary_nested_rollup already on main
1 parent 92ff6d7 commit 65e9e1b

7 files changed

Lines changed: 7 additions & 31 deletions

File tree

‎internal/config/interfaces.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -437,7 +437,7 @@ type StoreInterface interface {
437437
LatestLadderRunStartedAt(ctx context.Context, configID string) (*time.Time, error)
438438
TransitionLadderRunStatus(ctx context.Context, id string, fromStatuses []ladder.RunStatus, toStatus ladder.RunStatus) (*LadderRunDB, error)
439439

440-
// Notification mutes (issue #297 / migration 000078).
440+
// Notification mutes (issue #297 / migration 000091).
441441
// UpsertNotificationMute inserts or updates a mute row for (email, scope).
442442
// Idempotent for row existence: calling it again for an already-muted
443443
// address refreshes muted_at and replaces unmute_token if the token changes.

internal/database/postgres/migrations/000078_muted_recipients.down.sql renamed to internal/database/postgres/migrations/000091_muted_recipients.down.sql

File renamed without changes.

internal/database/postgres/migrations/000078_muted_recipients.up.sql renamed to internal/database/postgres/migrations/000091_muted_recipients.up.sql

File renamed without changes.

‎internal/email/mute_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -181,13 +181,13 @@ func TestSendPurchaseApprovalRequest_WithCC_SuppressesListUnsubscribe(t *testing
181181

182182
func TestBuildUnsubscribeURL_EmptyBaseURL_ReturnsEmpty(t *testing.T) {
183183
s := &Sender{}
184-
u, _ := s.buildUnsubscribeURL("user@example.com", "purchase_approvals")
184+
u := s.buildUnsubscribeURL("user@example.com", "purchase_approvals")
185185
assert.Empty(t, u)
186186
}
187187

188188
func TestBuildUnsubscribeURL_WithBaseURL_ContainsParams(t *testing.T) {
189189
s := &Sender{unsubscribeBaseURL: "https://dash.example.com"}
190-
u, _ := s.buildUnsubscribeURL("user@example.com", "purchase_approvals")
190+
u := s.buildUnsubscribeURL("user@example.com", "purchase_approvals")
191191
assert.Contains(t, u, "email=user%40example.com")
192192
assert.Contains(t, u, "scope=purchase_approvals")
193193
assert.Contains(t, u, "token=")

‎internal/email/sender.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -142,9 +142,9 @@ func muteKey() []byte {
142142
}
143143

144144
// buildUnsubscribeURL constructs the one-click unsubscribe URL for the given
145-
// (email, scope) pair. Returns ("", "") when unsubscribeBaseURL is empty.
146-
func (s *Sender) buildUnsubscribeURL(email, scope string) (unsubURL, mailtoURL string) {
147-
return unsubscribeURLFor(s.unsubscribeBaseURL, email, scope), ""
145+
// (email, scope) pair. Returns "" when unsubscribeBaseURL is empty.
146+
func (s *Sender) buildUnsubscribeURL(email, scope string) string {
147+
return unsubscribeURLFor(s.unsubscribeBaseURL, email, scope)
148148
}
149149

150150
// listUnsubscribeHeaders returns the List-Unsubscribe and List-Unsubscribe-Post

‎internal/email/smtp_sender.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,7 @@ func (s *SMTPSender) buildSMTPMessageMultipartWithHeaders(toEmail string, cc []s
248248
if len(cc) > 0 {
249249
headers += fmt.Sprintf("Cc: %s\r\n", strings.Join(cc, ", "))
250250
}
251-
headers += fmt.Sprintf("Subject: %s\r\nMIME-Version: 1.0\r\nContent-Type: multipart/alternative; boundary=\"%s\"\r\n", subject, boundary)
251+
headers += fmt.Sprintf("Subject: %s\r\nMIME-Version: 1.0\r\nContent-Type: multipart/alternative; boundary=%q\r\n", subject, boundary)
252252
headers += extraHeaders
253253
headers += "\r\n"
254254

‎internal/email/templates.go‎

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -672,30 +672,6 @@ const purchaseApprovalRequestHTMLTemplate = `<!DOCTYPE html>
672672
</td></tr></table>
673673
</body></html>`
674674

675-
// sendPurchaseApprovalRequestVia composes the plain-text + HTML approval-request
676-
// bodies and ships them through s.SendToEmailWithCCMultipart. HTML render
677-
// failures are non-fatal and degrade to single-part text so a template bug
678-
// never drops the approval email. Shared by Sender and SMTPSender — see
679-
// issue #287 / PR #298 dedup follow-up.
680-
func sendPurchaseApprovalRequestVia(ctx context.Context, s SenderInterface, recipient, subject string, data NotificationData) error {
681-
textBody, err := RenderPurchaseApprovalRequestEmail(data)
682-
if err != nil {
683-
return fmt.Errorf("failed to render purchase approval request email (text): %w", err)
684-
}
685-
// HTML render failure is non-fatal: degrade to single-part text.
686-
// SendToEmailWithCCMultipart already handles htmlBody=="" by delegating
687-
// to the single-part path on each transport.
688-
htmlBody, htmlErr := RenderPurchaseApprovalRequestEmailHTML(data)
689-
if htmlErr != nil {
690-
// Surface the render failure for production diagnosis. We deliberately
691-
// don't return — text-only delivery is the safer fallback than dropping
692-
// the approval email entirely.
693-
logging.Warnf("email: HTML approval-request render failed, falling back to text-only: %v", htmlErr)
694-
htmlBody = ""
695-
}
696-
return s.SendToEmailWithCCMultipart(ctx, recipient, data.CCEmails, subject, textBody, htmlBody)
697-
}
698-
699675
// sendMultipartVia is the generic dual-render send helper used by the
700676
// invite / password-reset / welcome flows. The two render closures are
701677
// invoked back-to-back; an HTML render failure is non-fatal and degrades

0 commit comments

Comments
 (0)