fix(billing): cleanup 4 Copilot nits on subscription_changed analytics (#3772)

[PierreBrisorgueil]

Summary

Addresses the 4 Copilot nits from issue #3772 left over from the subscription_changed analytics PR (#3769):

• Remove redundant try/catch around AnalyticsService.capture() — the service wraps client.capture in its own catch (_) {} and is documented to never throw; the outer catch was dead code
• Fix comment naming mismatch — comment said billing.plan.changed but the code emits plan.changed via billingEvents.emit('plan.changed', …); updated to match for grep/trace correctness
• Fix stale-event test fixture — the "stale event" test was using _mkEvent() with no previous_attributes.items, which skipped the plan-change branch before reaching the stale guard; fixture now includes previous_attributes.items so the updateIfEventNewer → null guard is what prevents capture
• Add same-plan test coverage — the "no emit on same plan" block only covered absent previous_attributes.items; added a second case where items IS present and previousPlan === newPlan is the guard that fires

Test plan

• npm run test:unit -- modules/billing/tests/billing.webhook — all 6 tests in the analytics suite pass (verified locally: 1753/1753 total)
• CI green
• CodeRabbit 0 unresolved threads

Summary by CodeRabbit

• Tests
  • Expanded test coverage for subscription change analytics events, including additional scenarios for improved reliability assurance.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 4ac66070-cd46-4e4f-ba8f-ec3e075632f4

📥 Commits

Reviewing files that changed from the base of the PR and between c860b0f and d1b6e2d.

📒 Files selected for processing (2)

• modules/billing/services/billing.webhook.service.js
• modules/billing/tests/billing.webhook.subscription-changed-analytics.unit.tests.js

---

Walkthrough

This PR removes local error handling from the subscription_changed analytics capture call in the subscription webhook handler and shifts error resilience responsibility to the AnalyticsService itself. The test suite is expanded with three new scenarios that validate when analytics capture is suppressed: missing prior plan data, unchanged plan, and stale webhook events.

Changes

Subscription update analytics capture

Layer / File(s)	Summary
Analytics capture error handling refactor modules/billing/services/billing.webhook.service.js	The handleSubscriptionUpdated webhook handler removes its surrounding try/catch block around AnalyticsService.capture, delegating error handling to the service itself. The call is now a direct invocation within the existing AnalyticsService.isConfigured() guard.
Plan-change detection and analytics capture test coverage modules/billing/tests/billing.webhook.subscription-changed-analytics.unit.tests.js	Extended test suite with three new scenarios: no capture when previous_attributes.items is absent (plan-change detection skipped); no capture when prior and current plans are identical (same-plan guard); reworked stale-event test with explicit subscription and prior-plan fixtures to confirm the guard is why capture is suppressed.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related issues

• pierreb-devkit/Node#3772: This PR directly addresses the Copilot code review nits listed in the issue by removing the redundant try/catch around AnalyticsService.capture and adding test coverage for the plan-change guards and stale-event paths.

Possibly related PRs

• pierreb-devkit/Node#3758: Both PRs modify plan-change handling and analytics emission gating within handleSubscriptionUpdated; the related PR strengthens plan-change detection that affects when the analytics capture flow runs.
• pierreb-devkit/Node#3582: Both PRs modify handleSubscriptionUpdated in the same webhook service file; the related PR adjusts plan-change handling logic that interacts with the analytics capture conditionals.
• pierreb-devkit/Node#3361: This PR removes webhook-level error handling and relies on AnalyticsService.capture to swallow its own errors, mirroring the error-resilience behavior introduced in the related PR's analytics service.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The pull request title clearly and concisely summarizes the main change: fixing four Copilot linting nits on subscription_changed analytics functionality.
Description check	✅ Passed	The pull request description provides comprehensive coverage of the summary, scope, validation steps, guardrails checks, and notes for reviewers following the repository template.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/billing-webhook-analytics-nits

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Pull request overview

Cleans up four Copilot nits left from PR #3769's subscription_changed analytics emit: removes a dead try/catch, fixes a comment naming mismatch, and tightens two unit tests so they actually exercise the intended guard branches.

Changes:

• Drop redundant try/catch around AnalyticsService.capture() (the service swallows its own errors) and correct the adjacent comment from billing.plan.changed to plan.changed.
• Update the stale-event test fixture to include previous_attributes.items so the updateIfEventNewer → null guard is what actually prevents capture.
• Add a same-plan test case where previous_attributes.items is present but previousPlan === newPlan, exercising that explicit guard.

Reviewed changes

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

File	Description
modules/billing/services/billing.webhook.service.js	Remove dead try/catch around AnalyticsService.capture(); fix comment to reference plan.changed.
modules/billing/tests/billing.webhook.subscription-changed-analytics.unit.tests.js	Tighten stale-event fixture and add same-plan guard coverage.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.18%. Comparing base (c860b0f) to head (d1b6e2d).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3773      +/-   ##
==========================================
+ Coverage   90.15%   90.18%   +0.03%     
==========================================
  Files         151      151              
  Lines        5006     5003       -3     
  Branches     1590     1589       -1     
==========================================
- Hits         4513     4512       -1     
+ Misses        388      386       -2     
  Partials      105      105              

Flag	Coverage Δ
integration	59.26% <0.00%> (+0.03%)	⬆️
unit	69.25% <100.00%> (+0.02%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c860b0f...d1b6e2d. Read the comment docs.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.