fix(billing): omit empty product description for Stripe - #1898
fix(billing): omit empty product description for Stripe#1898RushikeshBhavsar3605 wants to merge 1 commit into
Conversation
|
@RushikeshBhavsar3605 is attempting to deploy a commit to the Raystack Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review. 📝 WalkthroughSummary by CodeRabbit
WalkthroughProduct creation now omits the Stripe description parameter when the description is empty. Tests verify successful persistence and a nil Stripe description parameter. ChangesProduct creation
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: 🔵 Low · up to The change correctly omits an empty Stripe description, while the regression test name does not match its successful expectation and could cause minor maintenance confusion. The PR is mergeable with explicit owner awareness or a follow-up rename. Suggested reviewers: 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8aa6608a-710c-4afa-8829-01626fb79b52
📒 Files selected for processing (2)
billing/product/service.gobilling/product/service_test.go
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.
| setup func() *product.Service | ||
| }{ | ||
| { | ||
| name: "should fail to create product with empty description", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Rename the test to describe success.
The case expects successful creation with wantErr: false. Rename it to "should create product with empty description" so the test name matches the behavior under test.
3fa7284 to
8ddebeb
Compare
| Context: ctx, | ||
| }, | ||
| Name: &existingProduct.Title, | ||
| Description: &existingProduct.Description, |
There was a problem hiding this comment.
We need the same check here as well.
There was a problem hiding this comment.
I found the same pointer usage in Update() and looked into applying the same guard there.
One difference I verified: Stripe accepts description="" on product update and clears the existing description, while omitting the field leaves it unchanged. So applying the same check would prevent clearing an existing description.
Should Update() preserve the ability to clear the description, or should an empty description be treated as omitted here as well?
|
Are there any other stripe fields that have similar behaviour and we need to take care of? |
|
I checked the Stripe product/price fields for the same empty-description behavior and didn't find another occurrence. |
Summary
Fix product creation failing when the product description is empty.
Changes
descriptionparameter when the product description is empty.Test Plan
go test ./billing/productCloses #1853