Skip to content

Feat/issue 192#210

Open
RickieeeDev wants to merge 3 commits intomainfrom
feat/issue-192
Open

Feat/issue 192#210
RickieeeDev wants to merge 3 commits intomainfrom
feat/issue-192

Conversation

@RickieeeDev
Copy link
Copy Markdown
Contributor

No description provided.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an unsubscribe status lookup in the management subscriptions store and uses it in the campaigns send consumer to skip non-transactional sends for users who have unsubscribed from the campaign’s subscription.

Changes:

  • Add IsUserUnsubscribed to SubscriptionsStore (with sql.ErrNoRows handling).
  • Update campaign send consumer to skip sending when the user is unsubscribed (non-transactional campaigns only).
  • Add store-level tests covering IsUserUnsubscribed behavior across no-record/unsubscribe/resubscribe cases.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
internal/store/management/subscriptions.go Adds IsUserUnsubscribed query helper for user+subscription unsubscribe state.
internal/store/management/subscriptions_test.go Adds unit test coverage for IsUserUnsubscribed across key state transitions.
internal/pubsub/consumer/campaigns.go Skips non-transactional campaign sends when the user is unsubscribed from the campaign’s subscription.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +223 to +226
return false, err
}
return state != nil && *state == 1, nil
}
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This introduces another hard-coded 1 for the unsubscribe state. Consider defining a named constant (e.g., subscriptionStateUnsubscribed = 1) and reusing it across the file (CASE expressions, insert, and this check) to make the meaning explicit and reduce the chance of inconsistent updates later.

Copilot uses AI. Check for mistakes.
@jeroenrinzema
Copy link
Copy Markdown
Contributor

@RickieeeDev could you please take a look at the failing lint and tests?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants