fix(newsletter): set first_name on signup so issues stop greeting "Namaste ," - #473
Open
damo-da wants to merge 1 commit into
Open
fix(newsletter): set first_name on signup so issues stop greeting "Namaste ,"#473damo-da wants to merge 1 commit into
damo-da wants to merge 1 commit into
Conversation
…maste ,"
Every newsletter template greets with {{first_name}}, but the subscribe
endpoint never set that variable. It joined firstName and lastName into a
display name and passed it as `name`, which SendPulse stores as its own
"Name" field. Nothing reads that.
So every organic signup landed with a name SendPulse could show in its
dashboard column and no name any issue could greet with. SendPulse renders
a missing merge variable as empty, not as a fallback, so those contacts
received "Namaste ," with the comma still attached.
This is not hypothetical and it is not historical. The four most recent
website signups before this change (2026-08-25 through 08-28) all arrived
with Name and no first_name, and had to be backfilled by hand before the
event-postponement issue could go out. Earlier contacts look correct only
because someone already swept them.
The serializer has carried firstName as its own validated field the whole
time, so the given name just needs forwarding into the variable the
templates actually read. The joined display name still rides along as
`name` for the dashboard column.
Two tests cover it, both of which fail without the change: the main
subscribe path asserts first_name and the display name are populated
independently, and a second asserts first_name survives when lastName is
omitted, since lastName is optional.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
🛠️ Relevant configurations:
[config] enable_ai_metadata: False
model: openai/cx/gpt-5.5
git_provider: github
custom_reasoning_model: False
output_relevant_configurations: True
custom_model_max_tokens: 200000
fallback_models: ['openai/cx/gpt-5.4-mini']
ENABLE_AUTO_APPROVAL: True
is_auto_command: True
publish_output: True
publish_output_progress: True
progress_gif_url:
progress_gif_width: 48
verbosity_level: 0
use_extra_bad_extensions: False
log_level: DEBUG
use_wiki_settings_file: True
use_repo_settings_file: True
use_global_settings_file: True
extra_config_url:
disable_auto_feedback: False
ai_timeout: 120
response_language: en-US
repo_context_files: ['AGENTS.md']
repo_context_from_default_branch: True
repo_context_max_lines: 500
max_description_tokens: 500
max_commits_tokens: 500
max_model_tokens: 32000
model_token_count_estimate_factor: 0.3
patch_extension_skip_types: ['.md', '.txt']
allow_dynamic_context: True
max_extra_lines_before_dynamic_context: 10
patch_extra_lines_before: 5
patch_extra_lines_after: 1
cli_mode: False
large_patch_policy: clip
duplicate_prompt_examples: False
seed: -1
temperature: 0.2
ignore_pr_title: ['^\\[Auto\\]', '^Auto', '^Bump ', '^chore\\(deps\\)']
ignore_pr_target_branches: []
ignore_pr_source_branches: []
ignore_pr_labels: []
ignore_pr_authors: []
ignore_repositories: []
ignore_language_framework: []
restricted_mode: False
reasoning_effort: medium
enable_claude_extended_thinking: False
extended_thinking_budget_tokens: 2048
extended_thinking_max_output_tokens: 4096
claude_extended_thinking_models_override: []
extract_issue_from_branch: True
branch_issue_regex:
enable_custom_labels: False
[pr_reviewer] require_ticket_analysis_review: False
require_score_review: False
require_tests_review: True
require_estimate_effort_to_review: True
require_can_be_split_review: False
require_security_review: True
require_estimate_contribution_time_cost: False
require_todo_scan: False
publish_output_no_suggestions: True
persistent_comment: True
extra_instructions: Focus on: logic errors and edge cases; security/authz regressions; missing error handling;
Django/DRF correctness (migrations, N+1 queries, transaction/atomicity, serializer & permission gaps).
Do NOT comment on formatting, import order, or naming — ruff handles those in CI.
num_max_findings: 3
final_update_message: True
enable_review_labels_security: True
enable_review_labels_effort: True
require_all_thresholds_for_incremental_review: False
minimal_commits_for_incremental_review: 0
minimal_minutes_for_incremental_review: 0
enable_intro_text: True
enable_help_text: False
|
PR Code Suggestions ✨No code suggestions found for the PR. |
PR Agent Walkthrough 🤖Welcome to the PR Agent, an AI-powered tool for automated pull request analysis, feedback, suggestions and more. Here is a list of tools you can use to interact with the PR Agent:
(1) Note that each tool can be triggered automatically when a new PR is opened, or called manually by commenting on a PR. (2) Tools marked with [*] require additional parameters to be passed. For example, to invoke the |
|
Auto-approved PR |
![jawafdehi-pr-agent[bot]](https://avatars.githubusercontent.com/u/241969530?s=60&v=4){kind=small}
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe newsletter subscribe view now forwards the subscriber’s first name to SendPulse as ChangesNewsletter subscription
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The signup flow now populates the first name used by newsletter greetings while preserving the existing display name, with no actionable merge-blocking risk remaining after normal checks and review. Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
What
The subscribe endpoint never set the
first_nameSendPulse variable. It joinedfirstNameandlastNameinto a display name and passed it asname, which SendPulse stores as its own Name field. No newsletter template reads that field.Every newsletter greets with
{{first_name}}. SendPulse renders a missing merge variable as empty, not as a fallback, so organic signups received:comma still attached.
Why now
Not hypothetical, and not historical. The four most recent website signups before this change all arrived with
Nameand nofirst_name:root@awafdehi.orgkgurung@yahoo.comsujeshsah01@gmail.comkamalghimire121@gmail.comAll four had to be backfilled by hand before the event-postponement issue could go out. Contacts from 2026-07-22 to 08-20 look correct only because someone already swept them, which is why this reads as fixed when you glance at the book.
The fix
NewsletterSubscriptionSerializerhas carriedfirstNameas its own validated field the whole time, so the given name only needs forwarding into the variable the templates read. The joined display name still rides along asnamefor SendPulse's dashboard column.Tests
Two, both verified to fail with the change reverted:
test_subscribe_success_calls_sendpulsenow assertsfirst_nameand the display name are populated independently, so a future refactor cannot collapse one into the other unnoticedtest_subscribe_sets_first_name_without_last_namecovers the optional-lastNamepathnewsletter/suite: 34 passed.ruff checkclean.ruff formatis not run: it wants to reformat four files including ones this PR does not touch, so the repo is not format-clean to begin with and reformatting would bury the change.Not covered here
first_nameas of 2026-08-29). This only stops new ones arriving broken.first_nameis still worth setting as a safety net, since it also covers contacts added by any other route. That is a dashboard setting under Personalisation, not reachable from the API.🤖 Generated with Claude Code
PR Type
Bug fix, Tests
Description
SendPulse
first_namepopulatedDisplay
namepreserved separatelyOptional
lastNamecoveredDiagram Walkthrough
flowchart LR signup["Signup payload"] view["Newsletter view"] sendpulse["SendPulse contact"] template["Newsletter greeting"] signup -- "firstName" --> view view -- "first_name variable" --> sendpulse sendpulse -- "{{first_name}}" --> templateFile Walkthrough
views.py
Forward firstName into SendPulse variablenewsletter/views.py
first_nameto SendPulse variables.data["firstName"].namefor dashboard display.test_api.py
Cover newsletter first_name subscription behaviornewsletter/tests/test_api.py
first_namesent on subscription.nameremains full display name.lastNamecoverage.🛠️ Relevant configurations:
These are the relevant configurations for this tool:
[config]
[pr_description]
Summary by CodeRabbit