You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
feat(billing): write feature title and metadata as given on UpdateFeature - #2045
The UpdateFeature API now saves a feature's title, metadata and product links exactly as sent.
A title sent as empty clears the title.
Metadata sent as empty, or left out, is saved as {}.
Updating a feature name that does not exist now returns NotFound. Before, it quietly created the feature.
The feature is found by body.name only. The request id is ignored, as it was before this change. A name that looks like a UUID is still treated as a name.
Why
Before this change, UpdateFeature kept the old title and metadata when the request sent them empty. So there was no way to clear them through the API.
Declarative reconcile needs that. Its rule is that a field left out of the file goes back to its default (RFC 0001). The new BillingFeature reconcile kind uses this API to set feature titles and tooltips from a file.
UpdateProduct already works this way for product titles.
What does not change
UpsertFeature keeps its old merge behavior.
Creating a product, adding a feature to a product, and CreatePlan still go through UpsertFeature. Those calls send feature names only, so they can't wipe a title or metadata.
Only the UpdateFeature handler uses the new product.Service.UpdateFeature.
For callers
A client that calls UpdateFeature with only some fields filled will now clear the fields it leaves out. No caller in this repo does that.
Tests
Service tests:
a title is cleared
nil metadata is saved as {}
a non-empty title and metadata are saved as given
product links are written
an unknown name returns not found
an empty name is rejected
a name that looks like a UUID is still looked up by name
Handler tests: empty and filled-in fields pass through, NotFound is returned, and InvalidArgument is returned.
Related
The BillingFeature reconcile kind is in #2046. That PR does not depend on this one to build. A reset (a title or metadata left out of the file) only applies after this PR is merged.
No actionable comments were generated in the recent review.
Summary
Summary by CodeRabbit
New Features
Added support for updating existing billing product features. Updates replace the feature’s title, product associations, and metadata with the supplied values, and duplicate product associations are removed.
Updates can clear existing metadata by supplying empty metadata.
Error Handling
Requests for features that don’t exist return a not-found error. Invalid feature details, including an empty feature name, return an invalid-argument error.
The product service now updates existing features by name. The Connect handler calls the update operation and returns its result or an error.
Changes: Feature Update
Layer / File(s)
Summary
Product service update billing/product/service.go, billing/product/service_test.go
Adds UpdateFeature to replace existing feature fields, deduplicate product IDs, initialize nil metadata to an empty metadata map, and persist the result. Service tests cover field replacement, UUID-shaped names, and error cases.
raystack/frontier#2046: Uses UpdateFeature to reconcile feature titles and metadata. This PR’s update operation resets omitted values to empty values.
Suggested reviewers:amangit07
Merge Risk: Minimal · up to 99600
UpdateFeature now writes title, metadata and product links as given, and returns NotFound for an unknown feature name. No remaining merge-blocking risk was identified. Callers that send only some fields will clear the omitted ones, as the author notes.
Pre-merge checks: 2
Check name
Status
Explanation
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Nitpick comments: 2
billing/product/service_test.go: 1
1292-1297: Trivial · Functional Correctness · Low value
Test clearing existing product links.
The nil-input case starts with no product links. A regression that preserves existing links for an empty input would pass. Give the fixture an existing link so the UpdateByName expectation verifies its removal.
1183-1195: Trivial · Maintainability & Code Quality · Low value
Add a populated update case.
The successful handler test uses empty title and metadata values. It cannot detect an UpdateFeature-specific regression that drops populated values. Add a case that checks both fields in the service call and response. Compare the response metadata with the expected value.
Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.
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
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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
What
The
UpdateFeatureAPI now saves a feature's title, metadata and product links exactly as sent.{}.body.nameonly. The requestidis ignored, as it was before this change. A name that looks like a UUID is still treated as a name.Why
Before this change,
UpdateFeaturekept the old title and metadata when the request sent them empty. So there was no way to clear them through the API.Declarative reconcile needs that. Its rule is that a field left out of the file goes back to its default (RFC 0001). The new
BillingFeaturereconcile kind uses this API to set feature titles and tooltips from a file.UpdateProductalready works this way for product titles.What does not change
UpsertFeaturekeeps its old merge behavior.CreatePlanstill go throughUpsertFeature. Those calls send feature names only, so they can't wipe a title or metadata.UpdateFeaturehandler uses the newproduct.Service.UpdateFeature.For callers
A client that calls
UpdateFeaturewith only some fields filled will now clear the fields it leaves out. No caller in this repo does that.Tests
{}Related
The
BillingFeaturereconcile kind is in #2046. That PR does not depend on this one to build. A reset (a title or metadata left out of the file) only applies after this PR is merged.