POC of the auto unsubscribe functionality #376
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request updates the unsubscribe functionality across both backend and frontend components. The primary change is modifying the function signature of the unsubscribe process to accept an object with both Changes
Sequence Diagram(s)sequenceDiagram
participant HTTP Request
participant Server
participant AutoUnsubscribe
HTTP Request->>Server: POST /unsubscribe with {url, email}
Server->>AutoUnsubscribe: autoUnsubscribe({url, email})
AutoUnsubscribe-->>Server: returns Boolean
Server-->>HTTP Request: Response (success/failure)
sequenceDiagram
participant Client
participant UnsubscribeAction
participant AuthService
participant API
Client->>UnsubscribeAction: Invoke unsubscribeAction with URL
UnsubscribeAction->>AuthService: Retrieve user session
AuthService-->>UnsubscribeAction: Return session (with email)
UnsubscribeAction->>API: Send fetch request with email & URL
API-->>UnsubscribeAction: Return response status
UnsubscribeAction-->>Client: Return success/error message
Possibly related PRs
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
apps/unsubscriber/src/main.tsOops! Something went wrong! :( ESLint: 9.23.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by apps/web/env.tsOops! Something went wrong! :( ESLint: 9.23.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by apps/web/app/(app)/bulk-unsubscribe/hooks.tsOops! Something went wrong! :( ESLint: 9.23.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
apps/unsubscriber/src/server.ts (1)
37-39: Verify error message to reflect both parameters.The error message only mentions "Invalid URL provided" but now that the schema validates both URL and email, consider updating the error message to reflect both validation requirements.
- return reply.status(400).send({ error: "Invalid URL provided" }); + return reply.status(400).send({ error: "Invalid URL or email provided" });apps/web/utils/actions/unsubscriber.ts (1)
47-49: Add more detailed error handling.The current error handling only returns a generic "Failed to unsubscribe" message without including any details from the response.
- if (!response.ok) { - return { error: "Failed to unsubscribe" }; - } + if (!response.ok) { + try { + const errorData = await response.json(); + return { error: errorData.message || "Failed to unsubscribe" }; + } catch (e) { + return { error: `Failed to unsubscribe: ${response.status} ${response.statusText}` }; + } + }apps/web/app/(app)/bulk-unsubscribe/hooks.ts (1)
51-86: POC implementation needs testing and error handlingAccording to the PR objectives, this is a POC that hasn't been tested. Before merging, consider adding proper testing for this functionality.
Improve the error handling by checking the result of the unsubscribe action:
if (item.lastUnsubscribeLink) { - await unsubscribeAction({ url: item.lastUnsubscribeLink }); + const result = await unsubscribeAction({ url: item.lastUnsubscribeLink }); + if (result.error) { + toast.error(`Failed to unsubscribe: ${result.error}`); + return; + } }Also, consider adding a loading indicator or toast notification to indicate that an unsubscribe action is in progress.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/unsubscriber/src/main.ts(1 hunks)apps/unsubscriber/src/server.ts(2 hunks)apps/web/app/(app)/bulk-unsubscribe/hooks.ts(3 hunks)apps/web/env.ts(1 hunks)apps/web/utils/actions/unsubscriber.ts(2 hunks)
🧰 Additional context used
🧬 Code Definitions (3)
apps/web/app/(app)/bulk-unsubscribe/hooks.ts (1)
apps/web/utils/actions/unsubscriber.ts (1)
unsubscribeAction(35-53)
apps/web/utils/actions/unsubscriber.ts (1)
apps/web/env.ts (1)
env(5-215)
apps/unsubscriber/src/server.ts (1)
apps/unsubscriber/src/main.ts (1)
autoUnsubscribe(206-276)
🔇 Additional comments (8)
apps/web/env.ts (1)
82-82: Added new environment variable for unsubscriber service.This new optional environment variable
UNSUBSCRIBER_API_URLallows configuring the endpoint for the unsubscribe service, making the system more flexible for different deployment configurations.apps/unsubscriber/src/server.ts (2)
17-20: Updated schema to include email parameter.The unsubscribe schema now requires both URL and email fields, which aligns with the updated function signature in the main module.
28-29: Updated function call to use the new parameter structure.The route handler now correctly destructures both
urlandautoUnsubscribefunction, maintaining consistency with the updated function signature.apps/unsubscriber/src/main.ts (1)
206-212:❓ Verification inconclusive
Updated function signature to include email parameter.
The
autoUnsubscribefunction now accepts an object with bothurlandWhile the function signature has been updated to include the email parameter, I notice that the email isn't being used in the function body. This might be intentional as part of the POC, but it's worth confirming whether this is expected behavior.
🏁 Script executed:
#!/bin/bash # Check if the email parameter is used anywhere in the function body grep -A 70 "export async function autoUnsubscribe" apps/unsubscriber/src/main.ts | grep -E "email\s*[=:]"Length of output: 124
Rewritten Review Comment
The updated function signature now includes an
urlas part of an object parameter forautoUnsubscribe. The grep check confirms that
- Location:
apps/unsubscriber/src/main.ts, around lines 206-212- Observation: The
apps/web/utils/actions/unsubscriber.ts (1)
7-7: Added env import for API URL configuration.The env import is required to access the UNSUBSCRIBER_API_URL configuration value.
apps/web/app/(app)/bulk-unsubscribe/hooks.ts (3)
7-10: Import structure updated to include unsubscribeActionThe import now correctly includes the new
unsubscribeActionfunction alongside the existingsetNewsletterStatusAction. This is necessary for the auto-unsubscribe functionality being implemented.
82-82: Added lastUnsubscribeLink to dependency arrayCorrectly added
item.lastUnsubscribeLinkto the dependency array of the useCallback, ensuring the callback is updated when this property changes.
58-70:❓ Verification inconclusive
Potential logic flow issue with status updates
The code automatically attempts to unsubscribe when
item.status === NewsletterStatus.UNSUBSCRIBEDand then sets the status to null. This might be counter-intuitive as you're taking an action on an already unsubscribed newsletter.Verify if this is the intended behavior. Should the auto-unsubscribe action happen when:
- The user is marking a newsletter as unsubscribed (else branch)
- Or when they're resetting the status of an already unsubscribed newsletter (current implementation)
🌐 Web query:
What does setting a newsletter status to null mean in the context of email unsubscribe systems?💡 Result:
Setting a newsletter status to
nullin email unsubscribe systems typically indicates removing or invalidating the subscription record, but its exact implications depend on the system's architecture and data handling. Here's a breakdown:
Key Implications
Global Unsubscription
In systems like Salesforce Marketing Cloud[4], setting a status tonullmay trigger a master unsubscribe, blocking all emails across an organization. This differs from list-specific unsubscribes and affects:
- Business unit-level sends
- Enterprise-wide campaigns
- Automated workflows tied to subscriber status
SPF Authentication and Bounce Handling
Null sender addresses (e.g.,<>) are used for emails where delivery notifications aren’t required (e.g., bounce messages)[1]. However, in unsubscribe systems,nullstatuses might bypass standard SPF checks if the domain lacks proper HELO configurations[1].Data View Conflicts
_UnsubscribeData Views track events (e.g., clicks, API calls)[4], while_Subscribersreflects current status. Setting a status tonullmay disconnect event logs from active records, requiring manual reconciliation.Technical Risks
- Errors in Automation: Missing email addresses (
TargetEmail = null) disrupt processes like indemnification workflows[5].- List-Unsubscribe Headers: Improperly configured
nullstates can break unsubscribe headers, leading to spam complaints[12].
Common Use Cases
- Bulk Removal: Clearing subscription data during migrations or compliance audits.
- Opt-Out Enforcement: Resetting users to a "never subscribed" state instead of retaining explicit unsubscription records[2].
- System Cleanup: Deleting inactive or invalid email addresses from databases[5].
Best Practices
- Explicit Statuses: Use
Unsubscribed,Active, orBlockedinstead ofnullto avoid ambiguity[8].- Data Validation: Ensure emails aren’t
nullbefore processing unsubscriptions[5].- Audit Logs: Track
nullstatus changes to troubleshoot delivery failures[4].
In most systems,
nullis a blunt tool for subscription management. Prefer granular controls (e.g., list-level unsubscribes or global opt-outs) to maintain deliverability and compliance.Citations:
- 1: https://wordtothewise.com/2019/09/null-sender-address/
- 2: https://docs.kentico.com/k12sp/on-line-marketing-features/configuring-and-customizing-your-on-line-marketing-features/configuring-email-marketing/handling-newsletter-subscriptions-on-mvc-sites
- 3: https://customcratingandlogistics.com/what-does-null-mean-shipping/
- 4: https://mateuszdabrowski.pl/docs/config/sfmc-system-data-views/
- 5: https://shpbeds.org/ufaq/target-email-is-null-what-does-this-mean/
- 6: https://discussions.apple.com/thread/253542468
- 7: https://answers.microsoft.com/en-us/windows/forum/all/email-accounts-what-does-null-mean/ed83fca1-55d2-4fab-81b6-0e5dcaa93fba
- 8: https://wordpress.org/support/topic/what-does-a-status-of-block-mean/
- 9: https://discussions.apple.com/thread/8384331
- 10: https://discussions.apple.com/thread/5934010
- 11: https://community.solutionreach.com/s/article/Why-does-the-word-null-appear-next-to-a-patient-s-name-on-the-schedule-tab
- 12: https://developer.salesforce.com/docs/atlas.en-us.apexref.meta/apexref/apex_classes_email_outbound_single.htm
- 13: https://www.iea.org
Action Required: Verify Unsubscribe Flow and Status Update Logic
The current implementation automatically triggers an unsubscribe action for newsletters already marked as unsubscribed. This is immediately followed by updating the status to
null, which—according to common practices in email unsubscribe systems—typically serves to remove or invalidate the subscription record. However, this behavior could be counterintuitive if the intent was to perform the unsubscribe action only when changing the status from an active state.Please verify whether the intended behavior is to:
- Clean up unsubscribed records: Automatically remove the subscription (by setting the status to
null) for newsletters that are already unsubscribed, potentially as a data cleanup mechanism.- User-triggered unsubscription: Execute the unsubscribe action only when the user is marking a newsletter as unsubscribed (i.e., via the else branch), rather than performing this action on an already unsubscribed record.
The code in question (located at
apps/web/app/(app)/bulk-unsubscribe/hooks.ts, lines 58–70) currently handles the two branches as follows:
- If
item.status === NewsletterStatus.UNSUBSCRIBED:
- Calls
unsubscribeActionif alastUnsubscribeLinkis available.- Updates the newsletter status to
null(effectively removing the record).- Calls
mutate()to refresh the state.- Else:
- Calls
unsubscribeAndArchive, handling both unsubscription and archival for newsletters not yet marked as unsubscribed.A brief clarification in the code comments might help future maintainers understand the rationale behind this flow.
| const response = await fetch( | ||
| `${env.UNSUBSCRIBER_API_URL}/unsubscribe?url=${url}&email=${session.user.email}`, | ||
| ); | ||
|
|
There was a problem hiding this comment.
API endpoint inconsistency with server implementation.
The server implementation in apps/unsubscriber/src/server.ts expects the URL and email in the request body, but this client is sending them as query parameters.
The request format should match what the server expects. The server is set up to parse the request body, not query parameters.
| export const unsubscribeAction = withActionInstrumentation( | ||
| "unsubscribe", | ||
| async (options: { url: string }) => { | ||
| const session = await auth(); | ||
| if (!session?.user.email) return { error: "Not logged in" }; | ||
|
|
||
| const { url } = options; | ||
|
|
||
| const response = await fetch( | ||
| `${env.UNSUBSCRIBER_API_URL}/unsubscribe?url=${url}&email=${session.user.email}`, | ||
| ); | ||
|
|
||
| if (!response.ok) { | ||
| return { error: "Failed to unsubscribe" }; | ||
| } | ||
|
|
||
| return { success: true }; | ||
| }, | ||
| ); |
There was a problem hiding this comment.
Implemented new server action for unsubscription.
The new unsubscribeAction server action correctly:
- Verifies user authentication
- Extracts the URL from options
- Makes a request to the unsubscriber API
- Handles the response appropriately
The URL parameters in the fetch request are not being encoded, which could lead to issues with special characters in URLs or emails:
- const response = await fetch(
- `${env.UNSUBSCRIBER_API_URL}/unsubscribe?url=${url}&email=${session.user.email}`,
- );
+ const response = await fetch(
+ `${env.UNSUBSCRIBER_API_URL}/unsubscribe`, {
+ method: 'POST',
+ headers: {
+ 'Content-Type': 'application/json',
+ },
+ body: JSON.stringify({ url, email: session.user.email }),
+ }
+ );This change:
- Uses POST with a JSON body instead of query parameters
- Avoids URL encoding issues
- Aligns with how the server is expecting to receive the data (in the request body)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const unsubscribeAction = withActionInstrumentation( | |
| "unsubscribe", | |
| async (options: { url: string }) => { | |
| const session = await auth(); | |
| if (!session?.user.email) return { error: "Not logged in" }; | |
| const { url } = options; | |
| const response = await fetch( | |
| `${env.UNSUBSCRIBER_API_URL}/unsubscribe?url=${url}&email=${session.user.email}`, | |
| ); | |
| if (!response.ok) { | |
| return { error: "Failed to unsubscribe" }; | |
| } | |
| return { success: true }; | |
| }, | |
| ); | |
| export const unsubscribeAction = withActionInstrumentation( | |
| "unsubscribe", | |
| async (options: { url: string }) => { | |
| const session = await auth(); | |
| if (!session?.user.email) return { error: "Not logged in" }; | |
| const { url } = options; | |
| const response = await fetch( | |
| `${env.UNSUBSCRIBER_API_URL}/unsubscribe`, { | |
| method: 'POST', | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| }, | |
| body: JSON.stringify({ url, email: session.user.email }), | |
| } | |
| ); | |
| if (!response.ok) { | |
| return { error: "Failed to unsubscribe" }; | |
| } | |
| return { success: true }; | |
| }, | |
| ); |
| if (item.lastUnsubscribeLink) { | ||
| await unsubscribeAction({ url: item.lastUnsubscribeLink }); | ||
| } | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Added auto-unsubscribe functionality using the unsubscribe link
This change adds automatic unsubscription via API when a user has an existing unsubscribe link. However, there's a potential inconsistency between the implementation and expected behavior.
While the code passes only the URL to unsubscribeAction, the AI summary suggests the function should accept both url and email parameters. Let's verify the actual implementation:
Also, there's no handling for potential failures from the unsubscribe API call. Consider adding a check for the response and only setting the newsletter status to null if the unsubscribe action succeeds.
🏁 Script executed:
#!/bin/bash
# Check if unsubscribeAction accepts both url and email or just url
cat apps/web/utils/actions/unsubscriber.ts | grep -A 10 "unsubscribeAction"Length of output: 451
Action Required: Improve error handling in auto-unsubscribe flow
- The
unsubscribeActionfunction is correctly implemented to accept only aurlparameter, with the email automatically retrieved from the session. There’s no need to pass the email explicitly. - However, the current implementation in
apps/web/app/(app)/bulk-unsubscribe/hooks.tsdoes not handle failures from the unsubscribe API call. Consider checking the response fromunsubscribeActionand only updating the newsletter status to null if the action indicates success.
Hasn't been tested. Reference point to start the task yourself.
Summary by CodeRabbit