feat(api): add initial support for Addons #2522
Conversation
📝 WalkthroughWalkthroughThe changes introduce extensive support for managing addons through the API and the TypeScript-based product catalog. The OpenAPI specification now includes endpoints for listing, creating, updating, retrieving, deleting, archiving, and publishing addons, along with detailed data schemas and response handling. New TypeScript modules define addon models, enumerations, and ordering options, while an interface for addon operations enhances routing. Additionally, an import statement integrates these modules, ensuring cohesive functionality between the API and the product catalog domain. Changes
🪧 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 (
|
chrisgacsal
force-pushed
the
feat/addons-api
branch
2 times, most recently
from
c817889 to
71fa2ca
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (13)
api/spec/src/productcatalog/addon.tsp (2)
24-28: Minor grammar fix in documentation.The doc says “more then once” instead of “more than once”. Consider correcting to ensure clarity.
- * Single instance addons can be added to subscription only once while addons with multiple type can be added more then once. + * Single instance addons can be added to a subscription only once, while addons with the "multiple" type can be added more than once.
66-68: Minor doc mismatch referencing “plan” instead of “addon”.The doc string states “When not specified, the plan is effective indefinitely,” but this file is for addons. Consider updating to avoid confusion.
- * The date and time when the plan is no longer effective. When not specified, the plan is effective indefinitely. + * The date and time when the addon is no longer effective. When not specified, the addon is effective indefinitely.api/spec/src/productcatalog/routes.tsp (6)
224-231: Use “an addon” for proper grammar.“Create a addon” → “Create an addon.”
- @summary("Create a addon") + @summary("Create an addon")
235-242: Use “an addon” for proper grammar.“Update a addon” → “Update an addon.”
- @summary("Update a addon") + @summary("Update an addon")
269-275: Use “an addon” for proper grammar.“Once a addon is deleted...” → “Once an addon is deleted...”
279-286: Use “an addon” for proper grammar.“Publish a addon version” → “Publish an addon version.”
290-297: Use “an addon” for proper grammar.“Archive a addon version” → “Archive an addon version.”
302-314: Use “an addon” for proper grammar.“Create a new draft version from addon” → “Create a new draft version from an addon.”
api/openapi.yaml (5)
36-38: Clarify Tag Changes for Apps and Addons
The removal of the “Apps” tag (line 36–37) while still retaining an/api/v1/appsendpoint at the end (line 671) may cause confusion. Please confirm that this change is intentional and that the tags are being restructured consistently across the API specification.
305-373: Review /api/v1/addons/{addonId} - Update (PUT) and Retrieval (GET) Endpoints
• The PUT endpoint for updating an addon properly requires theaddonIdpath parameter and references theAddonReplaceUpdateschema.
• In the GET endpoint (starting at line 380), the summary currently reads “Get plan” (line 382) even though the description and context refer to an addon. Consider updating the summary to “Get addon” for consistency.
A suggested diff for the GET summary:- summary: Get plan + summary: Get addon
9471-9591: Review Addon Schema Definition
TheAddonschema is comprehensive and includes all necessary properties. However, the description (“Addon provide a template for extending Plans.”, line 9591) could be rephrased for clarity (e.g. “Addons provide a template for extending plans”).
9644-9653: Ensure Naming Consistency in AddonOrderBy Enum
TheAddonOrderByenum values includeid,key,type,version,create_at, andupdated_at. Note that elsewhere in the schema properties the naming convention uses camelCase (e.g.,createdAt,updatedAt). For consistency, consider updating these enum values tocreatedAtandupdatedAt.
9725-9732: Review AddonType Schema and Grammar
TheAddonTypeschema is defined correctly with allowed valuessingleandmultiple. Please correct the minor grammatical error in the description (“more then once” should be “more than once”).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
api/openapi.yaml(3 hunks)api/spec/src/productcatalog/addon.tsp(1 hunks)api/spec/src/productcatalog/main.tsp(1 hunks)api/spec/src/productcatalog/routes.tsp(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: CI
- GitHub Check: Developer environment
- GitHub Check: Lint
- GitHub Check: Test
- GitHub Check: Analyze (go)
🔇 Additional comments (14)
api/spec/src/productcatalog/main.tsp (1)
16-16: No concerns with the new import statement.Importing
./addon.tspcleanly integrates addon definitions into the product catalog.api/spec/src/productcatalog/addon.tsp (1)
13-18: Inconsistent status enumeration with doc mentioning “scheduled”.The doc comments (lines 75–79) reference a “scheduled” status, but only
draft,active, andarchivedexist in the enum. Confirm whether “scheduled” should be added or removed from the documentation.api/spec/src/productcatalog/routes.tsp (1)
163-219: Looks good overall.The new
listmethod parameters mirror the established pattern from Plans. No format issues or inconsistencies noted.api/openapi.yaml (11)
39-164: Review GET /api/v1/addons Endpoint Design
The GET endpoint for/api/v1/addonsis very comprehensive with well-defined query parameters (e.g.,includeDeleted,id,key,keyVersion,status,currency, as well as pagination and ordering). Please verify that the referenced ordering parameters (#/components/parameters/AddonOrderByOrdering.orderand...orderBy) are defined correctly in the components section.
167-229: Review POST /api/v1/addons Endpoint
The creation endpoint correctly specifies an emptyparametersarray, defines the response for status code 201, and uses a requestBody referencing theAddonCreateschema. This design appears sound.
230-304: Review POST /api/v1/addons/{addonIdOrKey}/next Endpoint
The new draft creation endpoint is clearly defined, with validation on theaddonIdOrKeyparameter and detailed response statuses. Ensure that the business rule—returning an error when there is already a draft or if the ID/key is not the latest published version—is implemented in the service logic.
464-530: Review DELETE /api/v1/addons/{addonId} Endpoint
The DELETE operation is defined to perform a soft delete and returns a 204 status on success. The descriptions and response schemas are clear. Confirm that the irreversible nature of the deletion is documented elsewhere if needed.
531-601: Review POST /api/v1/addons/{addonId}/archive Endpoint
The archive endpoint includes detailed response handling for various client and server errors, which is excellent. Ensure that archiving behavior (and any subsequent state transitions) is handled correctly on the backend.
602-670: Review POST /api/v1/addons/{addonId}/publish Endpoint
The publish endpoint is consistent with the other operations, with clearly defined responses. There are no issues noted here.
671-671: Verify /api/v1/apps Endpoint Inclusion
The diff ends with definition of/api/v1/apps(line 671). Given the earlier removal of the “Apps” tag, please verify that including this endpoint remains intentional and aligns with the intended API grouping.
9592-9643: Review AddonCreate Schema
TheAddonCreateschema is well-defined with the required fields for addon creation. It aligns with the design of the other operations.
9654-9717: Review Paginated and Update Schemas
The definitions forAddonPaginatedResponseandAddonReplaceUpdateare consistent with standard patterns and look solid.
9717-9724: Review AddonStatus Schema
TheAddonStatusschema clearly defines the allowed status values (draft,active,archived). No issues found.
9732-9734: Review Address Schema Fragment
The diff ends with a partialAddressschema (lines 9732–9734). If this is intentional and will be extended in a follow-up change, please confirm; otherwise, complete its definition to avoid ambiguity.
chrisgacsal
force-pushed
the
feat/addons-api
branch
from
71fa2ca to
bf62c9a
Compare
chrisgacsal
added
release-note/ignore
chrisgacsal
force-pushed
the
feat/addons-api
branch
2 times, most recently
from
9bc99b6 to
19997da
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (10)
openmeter/server/router/addon.go (2)
15-19: Minor grammatical issue in comment.There's a small grammatical issue in the comment.
- // Create a addon + // Create an addon
39-43: Minor grammatical issue in comment.There's a small grammatical issue in the comment.
- // Update a addon + // Update an addonapi/spec/src/productcatalog/routes.tsp (3)
221-230: Minor grammatical issue in summary.There's a small grammatical issue in the summary.
- @summary("Create a addon") + @summary("Create an addon")
232-241: Minor grammatical issue in summary.There's a small grammatical issue in the summary.
- @summary("Update a addon") + @summary("Update an addon")
264-274: Delete operation has documentation inconsistency.The documentation for the delete operation refers to "addon" in one place and "a addon" in another place.
/** * Soft delete addon by id. * - * Once a addon is deleted it cannot be undeleted. + * Once an addon is deleted it cannot be undeleted. */api/openapi.yaml (5)
39-227: Endpoints for Listing and Creating Addons: Ensure Consistent Documentation and Filtering Parameters
The GET and POST endpoints for/api/v1/addonsare comprehensive. The filtering parameters (e.g.,includeDeleted,id,key,keyVersion,status, andcurrency), as well as the pagination and order-by references, are well defined. Please verify that the regex patterns and examples (such as for ULID) precisely reflect your business requirements and that any intended examples (like query usage) are clear to API consumers.
379-460: Addon Retrieval Endpoint: Check Duplicate x-go-type and Inclusion of Latest Version
The GET endpoint for/api/v1/addons/{addonId}includes a usefulincludeLatestquery parameter, allowing clients to request the most recent version. Notice that thex-go-typeattribute is specified twice for theaddonIdparameter; if this redundancy is not required by your code generation or validation tools, consider consolidating it.
9717-9732: AddonStatus and AddonType Enums: Descriptive and Correct
TheAddonStatusenum (draft,active,archived) and theAddonTypeenum (single,multiple) are well defined. Note a minor typo in theAddonTypedescription—consider changing "more then once" to "more than once" for clarity.
9732-9734: Address Schema: Verify Completeness
TheAddressschema appears to be in an incomplete state, with only the type and start of the properties defined. If this schema is to be used, please ensure its full definition is provided or add a TODO note for follow-up development.
16466-16472: Plan Order By Options: Consistency in Naming Convention
The update to the order-by options for plans introducescreated_at(and implicitlyupdated_at), which differ in naming style compared to camelCase used elsewhere (e.g.,createdAt). Please confirm that the naming convention for sorting parameters is consistent across your API specification.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
api/client/go/client.gen.go(2 hunks)api/client/javascript/src/client/schemas.ts(1 hunks)api/openapi.cloud.yaml(1 hunks)api/openapi.yaml(4 hunks)api/spec/src/productcatalog/addon.tsp(1 hunks)api/spec/src/productcatalog/main.tsp(1 hunks)api/spec/src/productcatalog/plan.tsp(1 hunks)api/spec/src/productcatalog/routes.tsp(1 hunks)openmeter/server/router/addon.go(1 hunks)openmeter/server/router/router.go(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- openmeter/server/router/router.go
🚧 Files skipped from review as they are similar to previous changes (2)
- api/spec/src/productcatalog/main.tsp
- api/spec/src/productcatalog/addon.tsp
🧰 Additional context used
🧬 Code Definitions (1)
openmeter/server/router/addon.go (1)
openmeter/server/router/router.go (1)
Router(171-191)
🔇 Additional comments (29)
api/client/javascript/src/client/schemas.ts (1)
6361-6361: Good correction for consistency.The change from
create_attocreated_atfixes a typo and brings this field naming in line with standard timestamp field naming patterns. This consistency improvement aligns with the changes made in other files mentioned in the summary (openapi.yaml, plan.tsp, and client.gen.go).api/openapi.cloud.yaml (1)
15473-15479: Consistent Naming Convention UpdateThe renaming of
create_attocreated_atin the list of order-by options for plans greatly improves clarity and consistency. This change aligns with similar updates across related API specification files, ensuring that both the backend and client libraries maintain a uniform schema.Action points:
- Verification: Double-check that all references to this field in both backend and client implementations (e.g., in
client.gen.go,schemas.ts, andplan.tsp) have been updated accordingly.- Documentation: Update any accompanying documentation or inline comments to reflect this corrected naming.
api/spec/src/productcatalog/plan.tsp (1)
255-255: Good fix for consistency in field naming.The change from
"create_at"to"created_at"corrects the string value for thecreatedAtfield in thePlanOrderByenum. This ensures consistent naming conventions across the API, which is particularly important when extending it with new features like Addons.api/client/go/client.gen.go (2)
461-467: Consistent naming convention updateThe constant name has been updated from
CreateAttoCreatedAtto use the standard past participle form, which is more aligned with typical API field naming conventions. This change improves API consistency.
32080-32495: Updates to the embedded swagger specificationThe encoded swagger specification has been updated, likely to reflect the same naming convention change from "create_at" to "created_at" and possibly to include the new Addon support mentioned in the PR objectives.
Since this is generated code, the changes look appropriate based on the upstream API specification changes.
openmeter/server/router/addon.go (6)
1-8: Package declaration and imports look good.The imports are correctly defined and the package name follows the project's convention.
9-13: Method skeleton for ListAddons looks good.The comment matches the corresponding endpoint definition with the correct HTTP method (GET) and path (/api/v1/addons).
21-25: Method skeleton for NextAddon looks good.The comment and method signature correctly match the endpoint definition.
27-31: Method skeleton for DeleteAddon looks good.The comment and method signature correctly match the endpoint definition.
33-37: Method skeleton for GetAddon looks good.The comment and method signature correctly match the endpoint definition.
45-55: Method skeletons for ArchiveAddon and PublishAddon look good.The comments and method signatures correctly match the endpoint definitions.
api/spec/src/productcatalog/routes.tsp (5)
163-165: Route and tag definitions look good.The route path and tag definition are appropriate for the addon resource.
166-219: List operation is well-defined.The list operation includes appropriate query parameters for filtering, pagination, and ordering. The documentation is clear and provides useful examples.
243-262: Get operation is well-defined.The get operation includes appropriate path and query parameters with clear documentation.
276-296: Publish and Archive operations look good.Both operations are well-defined with appropriate path parameters and return types.
298-313: Next operation is well-defined.The next operation includes appropriate path parameters and clear documentation about constraints.
api/openapi.yaml (13)
228-302: New Draft Addon Endpoint: Validate Parameter and Behavior
The endpoint/api/v1/addons/{addonIdOrKey}/nextis clearly documented and specifies that an error should be returned if there is already a draft or if the provided identifier does not reference the latest published version. Please double-check that the backend logic aligns with these conditions.
303-378: Addon Update Endpoint: Validate Schema and Parameter Consistency
The PUT endpoint for/api/v1/addons/{addonId}correctly uses a ULID-based pattern for theaddonIdparameter and references theAddonReplaceUpdateschema in its request body. Ensure that the schema’s constraints are fully enforced on the backend and that similar endpoints (for retrieval or deletion) use a consistent pattern.
461-527: Soft Delete Addon Endpoint: Clear and Concise Deletion Semantics
The DELETE endpoint for/api/v1/addons/{addonId}is defined with clear instructions for a soft delete, including an appropriate 204 response and detailed error responses. The description clearly states that deletion is irreversible, so no changes are suggested here.
528-597: Addon Archive Endpoint: Verify Response Schema and Error Codes
The archive endpoint (/api/v1/addons/{addonId}/archive) is documented to archive an addon version. The response definitions and error handling appear robust. Please confirm that the backend archival process strictly adheres to these specifications.
598-667: Addon Publish Endpoint: Validate Request Handling and Consistency
The publish endpoint (/api/v1/addons/{addonId}/publish) is consistent with the other endpoints in terms of request parameters, response codes, and error handling. Ensure that the implementation aligns with these definitions and that any business logic related to publishing is thoroughly tested.
667-669: Apps Endpoint: Confirm Backward Compatibility
The inclusion of the/api/v1/appsGET endpoint appears to support legacy or extended functionality. Please ensure that its behavior remains consistent with the rest of the API and that it does not conflict with the new addons endpoints.
8826-8842: Order By Parameters: Review Definition
The parametersAddonOrderByOrdering.orderandAddonOrderByOrdering.orderByare well defined, referencing appropriate schemas for sort order and order-by field respectively. Their definitions complement the filtering and sorting capabilities.
8843-8845: Incomplete Parameter Definition: BillingProfileCustomerOverrideOrderByOrdering.order
The parameterBillingProfileCustomerOverrideOrderByOrdering.orderis missing a schema and additional metadata (such as description or default value). Please complete its definition or remove it if it is not required. This comment reiterates earlier feedback on the same issue.
9474-9592: Addon Schema: Structure and Validation
The newAddonschema is comprehensive—it defines required properties such asid,name,createdAt,updatedAt,key,version,type,currency,status, andrateCardswith appropriate types, patterns, and read-only indicators. Verify that computed fields (e.g.,status) are correctly derived on the backend.
9592-9644: AddonCreate Schema: Ensure Required Fields and Constraints
TheAddonCreateschema mandates key fields likename,key,type,currency, andrateCards, and defines useful constraints (minLength, maxLength, pattern). Double-check that these restrictions align with your business rules and data validation logic.
9644-9654: AddonOrderBy Schema: Consistent Enumeration
TheAddonOrderByschema provides a clear enum for ordering options (id,key,type,version,created_at,updated_at). Ensure these options match the fields in your data store and are consistently used throughout your API endpoints.
9655-9680: AddonPaginatedResponse Schema: Paginated Response Definition
TheAddonPaginatedResponseschema clearly defines the structure of paginated results, includingtotalCount,page,pageSize, and the array ofitems. Confirm that the pagination logic in your implementation returns these fields accurately.
9680-9717: AddonReplaceUpdate Schema: Validate Update Model Integrity
TheAddonReplaceUpdateschema is designed to handle updates with required fields such asname,type, andrateCards. Ensure this schema is in sync with the fields allowed to be updated in your system and that any optional fields are handled gracefully.
chrisgacsal
force-pushed
the
feat/addons-api
branch
from
19997da to
a6e35f3
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
api/openapi.yaml (1)
8749-8771:⚠️ Potential issueIncomplete Parameter Definition Detected
The parameterBillingProfileCustomerOverrideOrderByOrdering.order(lines 8769–8771) is incomplete—it lacks a schema, description, and additional metadata. This mirrors previous feedback. Please complete the definition or remove the parameter if it’s not needed.
🧹 Nitpick comments (10)
openmeter/server/router/addon.go (2)
15-16: Consider correcting the article in the commentThe comment "Create a addon" would read better as "Create an addon" to match English grammar rules for words starting with vowels.
- // Create a addon + // Create an addon
33-37: Consider correcting the article in the commentThe comment "Update a addon" would read better as "Update an addon" to match English grammar rules for words starting with vowels.
- // Update a addon + // Update an addonapi/spec/src/productcatalog/routes.tsp (3)
226-227: Maintain consistent article usage for summariesThe create operation uses "Create an addon" while other operations use just "addon" without the article. Consider standardizing this across all operations.
- @summary("Create an addon") + @summary("Create addon")Alternatively, add articles to all other operations for consistency.
279-280: Consider using "an" before "addon" in documentationThe documentation uses "a addon version" which would read better as "an addon version" to match English grammar rules.
- * Publish a addon version. + * Publish an addon version.
290-291: Consider using "an" before "addon" in documentationThe documentation uses "a addon version" which would read better as "an addon version" to match English grammar rules.
- * Archive a addon version. + * Archive an addon version.api/openapi.yaml (5)
36-38: Verify and Clarify Tag Removal
The removal of the “Apps” tag (lines 36–37) raises the question of whether this tag is now obsolete or simply relocated. Please confirm that this change aligns with the updated API domain focus on addons.
167-225: Verify POST /api/v1/addons Endpoint Details
The POST endpoint is defined with a required request body using theAddonCreateschema. Consider adding examples or further annotations in the schema if possible to aid API consumers in providing correctly structured input.
9400-9517: Examine “Addon” Schema Consistency and Grammar
TheAddonschema is well‑structured, with required fields, clear patterns (e.g., ULID forid), and descriptive examples. A couple of points for consideration:
- The property names (e.g.,
createdAt,updatedAt) use camelCase while related ordering enums later use snake_case (e.g.,created_at); verify this naming choice for consistency.- The description ("Addon provide a template for extending Plans") contains a minor grammatical error; consider changing to "Addon provides a template for extending Plans."
9570-9580: Validate “AddonOrderBy” Enum Naming
TheAddonOrderByenum lists order-by fields includingcreated_atandupdated_at. Please ensure that this snake_case naming is intentional and documented (especially as the schema itself uses camelCase for similar fields).
16392-16398: Review Plan Order By Options Consistency
The updated order-by options for plans now includecreated_atandupdated_at(lines 16392–16398). Verify that the use of snake_case here is consistent with your overall API design, especially in contrast with camelCase used elsewhere.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
api/client/go/client.gen.go(2 hunks)api/client/javascript/src/client/schemas.ts(1 hunks)api/openapi.cloud.yaml(1 hunks)api/openapi.yaml(4 hunks)api/spec/src/productcatalog/addon.tsp(1 hunks)api/spec/src/productcatalog/main.tsp(1 hunks)api/spec/src/productcatalog/plan.tsp(1 hunks)api/spec/src/productcatalog/routes.tsp(1 hunks)openmeter/server/router/addon.go(1 hunks)openmeter/server/router/router.go(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- api/openapi.cloud.yaml
🚧 Files skipped from review as they are similar to previous changes (6)
- api/spec/src/productcatalog/plan.tsp
- api/spec/src/productcatalog/main.tsp
- openmeter/server/router/router.go
- api/client/go/client.gen.go
- api/spec/src/productcatalog/addon.tsp
- api/client/javascript/src/client/schemas.ts
🧰 Additional context used
🧬 Code Definitions (1)
openmeter/server/router/addon.go (1)
openmeter/server/router/router.go (1)
Router(171-191)
🔇 Additional comments (17)
openmeter/server/router/addon.go (1)
11-49: Approve REST API stubs for addon operationsThe implementation stubs for the addon REST API endpoints are correctly structured with appropriate HTTP methods, path parameters, and status codes. These stubs will allow the API to be registered and return "not implemented" until the full implementation is provided in a follow-up PR.
api/spec/src/productcatalog/routes.tsp (5)
236-237: Explicit route annotations added for path parametersGood job adding the explicit route annotations for path parameters. This makes the API definition more clear and ensures the path parameters are correctly processed.
247-249: Route annotation for get method prevents default routing behaviorThe get method includes an explicit route annotation that may be redundant if the TypeSpec framework automatically handles the path parameter. However, since you've used this pattern consistently across all methods, this approach is acceptable.
163-299: Approve Addons interface definitionThe Addons interface is well-structured and follows the same patterns as the Plans interface, making it consistent with the rest of the API. The CRUD operations, filtering capabilities, and versioning operations (publish, archive) are comprehensive and will provide a solid foundation for addon management.
217-219:Details
✅ Verification successful
Ensure AddonOrderBy enum is defined
The query ordering parameter uses
AddonOrderByenum type, which should be defined in the addon types definition file.
🏁 Script executed:
#!/bin/bash # Check if AddonOrderBy enum is defined grep -r "enum AddonOrderBy" api/spec/Length of output: 97
AddonOrderBy enum definition verified
The enum
AddonOrderByis defined inapi/spec/src/productcatalog/addon.tspas required, so no changes are needed here.
163-299:Details
❓ Verification inconclusive
Consider adding a "next" method for addons
The
Plansinterface includes anextmethod for creating a new draft version from a plan, but this functionality appears to be missing from theAddonsinterface. If addons should have similar versioning capabilities as plans, consider adding a corresponding method.
🏁 Script executed:
#!/bin/bash # Check if there's a reason for not including a "next" method in the Addons interface cat api/spec/src/productcatalog/addon.tsp | grep -n "version\|draft"Length of output: 281
API Versioning Consistency: Consider Implementing a "next" Method for Addons
In the current API specification, the
addonsdefinitions (as seen inapi/spec/src/productcatalog/addon.tsp) include versioning details—such asdraftandversionproperties—but no dedicatednextmethod exists. Since thePlansinterface provides anextmethod to create a new draft version, please review whether introducing a similar endpoint in theAddonsinterface would help maintain consistency across your product catalog versioning.api/openapi.yaml (11)
39-115: Review GET /api/v1/addons Endpoint Definition
The new GET endpoint for addons is very comprehensive. The query parameters (e.g.,includeDeleted,id,key,keyVersion,status, andcurrency) are well‑documented and include appropriate types, patterns, and descriptions. Please verify that the deep object style for thekeyVersionparameter is correctly supported in your API framework.
229-256: Review GET /api/v1/addons/{addonIdOrKey} Endpoint
This endpoint correctly accepts a path parameter that can be either an ID or key, and the addition of the optionalincludeLatestquery parameter is a good enhancement. Ensure that backend handling clearly differentiates the two lookup modes.
313-383: Examine PUT /api/v1/addons/{addonId} Endpoint
The update endpoint uses theAddonReplaceUpdateschema for the request body, which is consistent with the creation schema but tailored for update scenarios. Verify that server‐side validation properly enforces only the allowed changes per your business logic.
389-457: Confirm DELETE (Soft Delete) Implementation
The soft delete endpoint is well‑documented. The description clearly warns that deleted addons cannot be undeleted. Ensure that the downstream logic appropriately handles resource state after deletion.
457-527: Review Archive Endpoint Details
The archive endpoint (POST /api/v1/addons/{addonId}/archive) returns a 200 status with the archived addon. Confirm that the archiving business rules (e.g., version locking) are implemented as expected in the backend.
528-597: Review Publish Endpoint & Legacy /apps Endpoint Note
The publish endpoint (POST /api/v1/addons/{addonId}/publish) is defined with consistent response handling. Also, note that the snippet continues with an/api/v1/appsendpoint (line 597). Please verify that the coexistence of apps-related endpoints remains intentional given the addon focus of this PR.
9518-9570: Review “AddonCreate” Schema
TheAddonCreateschema clearly specifies the required fields (name,key,type,currency, andrateCards) and provides proper constraints. This looks good and aligns with standard resource creation models.
9580-9606: Review “AddonPaginatedResponse” Schema
The paginated response is straightforward and includes all essential pagination details (totalCount, page, pageSize, and items). This implementation is clear and should function as intended.
9606-9643: Review “AddonReplaceUpdate” Schema
This schema properly outlines the fields necessary for updating an addon while reusing common definitions (e.g.,AddonType). It appears consistent with the creation model.
9643-9658: Examine Enumeration Schemas for Addon Status and Type
TheAddonStatusandAddonTypeenums are concise, listing valid states and types. They meet the specification requirements well.
9659-9660: Clarify “Address” Schema Fragment
The snippet for theAddressschema (lines 9659–9660) appears incomplete. Please confirm whether this is an intentional placeholder for future work or if the full definition should be included.
chrisgacsal
force-pushed
the
feat/addons-api
branch
from
a6e35f3 to
bbf24c0
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
api/openapi.yaml (1)
8769-8771: 🛠️ Refactor suggestionIncomplete Parameter Definition: BillingProfileCustomerOverrideOrderByOrdering.order
The parameter defined on lines 8769–8771 only includesnameandinattributes without a schema, description, or additional metadata. This issue was noted in previous reviews. Please either complete the parameter’s definition or remove it if it is not needed.
🧹 Nitpick comments (13)
.spectral.yaml (1)
8-9: Consider making these rule exclusions temporary.These rules (
path-paramsandoas3-unused-component) are disabled, presumably to facilitate the initial development of addon functionality. Consider adding a TODO comment to re-enable them once the addon implementation is complete, as they help ensure API quality.# Disabled due to TypeSpec conversion oas3-valid-schema-example: off no-$ref-siblings: off - path-params: off - oas3-unused-component: off + # TODO: Temporarily disabled for addon development - re-enable after implementation is complete + path-params: off + oas3-unused-component: offopenmeter/server/router/addon.go (2)
15-17: Grammar fix: "a" should be "an" before "addon".Correct the article usage in the comment for better readability.
- // Create a addon + // Create an addon // (POST /api/v1/addons)
33-35: Grammar fix: "a" should be "an" before "addon".Correct the article usage in the comment for better readability.
- // Update a addon + // Update an addon // (PUT /api/v1/addons/{addonId})api/spec/src/productcatalog/addon.tsp (3)
66-71: Copy-paste error in effectiveTo documentation.The documentation refers to "plan" instead of "addon".
/** - * The date and time when the plan is no longer effective. When not specified, the plan is effective indefinitely. + * The date and time when the addon is no longer effective. When not specified, the addon is effective indefinitely. */
73-83: Copy-paste error in status documentation.The documentation refers to "plan" instead of "addon".
/** - * The status of the plan. + * The status of the addon. * Computed based on the effective start and end dates: * - draft = no effectiveFrom * - active = effectiveFrom <= now < effectiveTo * - archived = effectiveTo <= now * - scheduled = now < effectiveFrom < effectiveTo */
60-64: Copy-paste error in effectiveFrom documentation.The documentation refers to "plan" instead of "addon".
/** - * The date and time when the addon becomes effective. When not specified, the plan is a draft. + * The date and time when the addon becomes effective. When not specified, the addon is a draft. */api/spec/src/productcatalog/routes.tsp (3)
279-287: Grammar fix: "a" should be "an" before "addon".Correct the article usage in the documentation for better readability.
/** - * Publish a addon version. + * Publish an addon version. */
290-298: Grammar fix: "a" should be "an" before "addon".Correct the article usage in the documentation for better readability.
/** - * Archive a addon version. + * Archive an addon version. */
163-299: Consider adding 'next' method for consistency with Plans interface.The
Plansinterface has anextmethod for creating a new draft version, but theAddonsinterface doesn't have an equivalent. Consider adding this method for feature parity and consistency.You might want to add a method similar to:
/** * Create a new draft version from addon. * It returns error if there is already an addon in draft or addonId does not reference the latest published version. */ @post @route("/{addonIdOrKey}/next") @operationId("nextAddon") @summary("New draft addon") next( @extension("x-go-type", "string") @path addonIdOrKey: ULIDOrKey, ): { @statusCode _: 201; @body body: Addon; } | OpenMeter.NotFoundError | OpenMeter.CommonErrors;api/openapi.yaml (4)
79-89: Review KeyVersion Parameter Schema
ThekeyVersionparameter is defined as an object whose additional properties are arrays of integers. Please double-check that this structure meets your filtering requirements and that the backend processing is equipped to handle it.
230-256: Enhance Clarity of Path Parameter Pattern for Addon Retrieval
The regex pattern used on line 242 for theaddonIdOrKeypath parameter merges two alternative formats. Consider using explicit grouping (e.g., parentheses) to clearly delineate the two supported formats. This can help avoid potential misinterpretations by users or downstream systems.
9400-9459: Solid Addon Schema Definition with Minor Grammatical Note
TheAddonschema is well defined with required fields, validations, and examples. One minor suggestion: in the description (line 9517), “Addon provide a template for extending Plans” could be revised to “An addon provides a template for extending plans” for improved grammar and clarity.
9570-9578: Refine AddonInstanceType Description
TheAddonInstanceTypeenum definition is clear; however, the description can be improved for clarity and correctness. For example, consider the following revision:- - Single instance addons can be added to subscription only once while addons with multiple type can be added more then once. + - Single instance addons can be added to a subscription only once, while addons of type "multiple" can be added more than once.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.spectral.yaml(1 hunks)api/client/go/client.gen.go(2 hunks)api/client/javascript/src/client/schemas.ts(1 hunks)api/openapi.cloud.yaml(1 hunks)api/openapi.yaml(4 hunks)api/spec/src/productcatalog/addon.tsp(1 hunks)api/spec/src/productcatalog/main.tsp(1 hunks)api/spec/src/productcatalog/plan.tsp(1 hunks)api/spec/src/productcatalog/routes.tsp(1 hunks)openmeter/server/router/addon.go(1 hunks)openmeter/server/router/router.go(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- api/openapi.cloud.yaml
🚧 Files skipped from review as they are similar to previous changes (5)
- api/spec/src/productcatalog/main.tsp
- openmeter/server/router/router.go
- api/spec/src/productcatalog/plan.tsp
- api/client/javascript/src/client/schemas.ts
- api/client/go/client.gen.go
🧰 Additional context used
🧬 Code Definitions (1)
openmeter/server/router/addon.go (1)
openmeter/server/router/router.go (1)
Router(171-191)
🔇 Additional comments (17)
openmeter/server/router/addon.go (4)
27-31: Route comment doesn't match parameter name.The route comment shows
/api/v1/addons/{addonId}but the method signature usesaddonIdOrKeyas the parameter name.// Get addon - // (GET /api/v1/addons/{addonId}) + // (GET /api/v1/addons/{addonIdOrKey}) func (a *Router) GetAddon(w http.ResponseWriter, r *http.Request, addonIdOrKey string, params api.GetAddonParams) {
1-49: Missing addonHandler in Router struct.The
Routermethods have been added for addon operations, but the corresponding handler field may be missing in theRouterstruct definition.I see from past review comments that you mentioned the addonHandler field will be introduced in a follow-up PR. That's a reasonable approach for this incremental implementation.
17-19:Details
❌ Incorrect review comment
Request body parameter missing.
The
CreateAddonmethod doesn't include a parameter to receive the request body, unlike typical REST API patterns where creation operations require request data. Consider adding a request parameter.
🏁 Script executed:
#!/bin/bash # Check how other create methods are implemented grep -n "func.*Create.*" openmeter/server/router/* | grep -v "addon.go"Length of output: 2328
Clarification: The current method signature for CreateAddon is in line with our established REST API pattern.
- In our codebase, create endpoints (e.g., CreateCustomer, CreateBillingProfile, etc.) follow the standard signature of
(w http.ResponseWriter, r *http.Request), with the request body being accessed viar.Bodyrather than as an extra parameter.- Since other similar methods do not explicitly add a body parameter, the absence of one in CreateAddon is intentional. When the endpoint is fully implemented, any required body processing should occur by reading from
r.Body.Likely an incorrect or invalid review comment.
35-37:Details
❌ Incorrect review comment
Request body parameter missing.
The
UpdateAddonmethod doesn't include a parameter to receive the request body, unlike typical REST API patterns where update operations require request data. Consider adding a request parameter.
🏁 Script executed:
#!/bin/bash # Check how other update methods are implemented grep -n "func.*Update.*" openmeter/server/router/* | grep -v "addon.go"Length of output: 1885
Clarification on Request Body Handling
The
UpdateAddonmethod is intentionally designed to follow the established pattern used in other update endpoints (e.g.,UpdateApp,UpdateCustomer, etc.). In these routes, rather than passing the request body as an explicit parameter, the incoming data is expected to be extracted fromr.Bodywithin the function. Therefore, no additional request body parameter is needed.Likely an incorrect or invalid review comment.
api/spec/src/productcatalog/addon.tsp (1)
102-102: Fixed typo from past review.Good correction of the
createdAtproperty to map to "created_at" instead of "create_at" as mentioned in a previous review.api/spec/src/productcatalog/routes.tsp (3)
226-227: Consistent with suggested fixes in addon.go.Good usage of "an addon" in the summary, which is grammatically correct.
247-247: Route path redundancy fixed.The explicit
@route("/{addonIdOrKey}")is needed here since the parameter name differs from the default path segment name. The changes in response to the past review comment are correct.
270-270: Route path redundancy fixed.The explicit
@route("/{addonId}")is needed here to ensure the correct path parameter is used. The changes in response to the past review comment are correct.api/openapi.yaml (9)
36-38: Clarify Endpoint Inclusion: Apps vs. Addons
The tag definition on lines 36–37 for "Apps" and the endpoint/api/v1/apps(lines 597–599) appear alongside the newly introduced Addons endpoints. Since this PR is focused on basic support for Addons, please confirm whether the Apps endpoint should remain in this file or be handled in a separate change.Also applies to: 597-599
39-67: Solid GET /api/v1/addons Endpoint Definition
The GET/api/v1/addonsendpoint includes comprehensive query parameters such asincludeDeleted,id,key, andkeyVersion. The use of arrays for filtering on attributes likeidandkeyis clear, and the ULID pattern with example is well defined. Please verify that this design aligns with your intended filtering logic.
313-448: Consistent Definitions for Update, Delete, and Archive Endpoints
The PUT, DELETE, and archive operations for addons are comprehensively defined with detailed response handling and error scenarios. Ensure that these operations conform to the overall API error conventions and that the response schemas are correctly referenced across the specification.
527-595: Robust Publish Endpoint Specification
The publish addon endpoint is thoroughly specified with complete response codes and content definitions. Please verify that the schema references (e.g., for successful responses and errors) are current and consistent with the rest of the API.
9518-9569: Comprehensive AddonCreate Schema
TheAddonCreatemodel clearly enforces essential fields (such asname,key,instanceType,currency, andrateCards) for creating an addon. Verify that any optional metadata validations are handled appropriately in the backend.
9579-9614: Ordering and Pagination Schemas Look Good
TheAddonOrderByenum andAddonPaginatedResponseschema adhere to common API pagination and ordering patterns. No concerns noted here.
9614-9651: Consistent Update Model for Addons
TheAddonReplaceUpdateschema provides a clear model for update operations with required fields and proper validations. Please ensure that this matches the backend update logic for addons.
9651-9658: Clear Definition for AddonStatus
TheAddonStatusschema, with its enumerated values (draft,active, andarchived), is straightforward. Confirm that the backend correctly interprets these statuses based on the effective dates.
16392-16398: Review Ordering Options for Plans
The ordering options for plans are updated to include fields likecreated_atalongsideid,key,version, andupdated_at. Please verify that this ordering configuration is consistent with the intended sorting behavior for plan resources.
chrisgacsal
force-pushed
the
feat/addons-api
branch
from
bbf24c0 to
64178e4
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
openmeter/server/router/addon.go (1)
27-31:⚠️ Potential issueUpdate route comment to match method signature
The route comment specifies
/api/v1/addons/{addonId}but the method signature usesaddonIdOrKeyas the parameter name. This inconsistency could lead to confusion during implementation.// Get addon -// (GET /api/v1/addons/{addonId}) +// (GET /api/v1/addons/{addonIdOrKey}) func (a *Router) GetAddon(w http.ResponseWriter, r *http.Request, addonIdOrKey string, params api.GetAddonParams) {api/openapi.yaml (2)
8749-8771:⚠️ Potential issueIncomplete Parameter Definition for BillingProfileCustomerOverrideOrderByOrdering.order
The parameter definition currently specifies onlynameandinwithout providing a schema, description, or additional metadata. This issue was flagged previously. Please either complete the definition or remove the parameter if it is not needed.
9659-9660:⚠️ Potential issueIncomplete Address Schema Definition
TheAddressschema is incomplete—it only shows the type and the start of a properties section. Please either provide the full schema details or remove the incomplete definition if it is not required.
🧹 Nitpick comments (10)
openmeter/server/router/addon.go (1)
1-8: Consider adding error handling importsWhile the current implementation is simple, when you implement these handlers, you'll likely need error handling packages. Consider importing packages like "errors" or "fmt" for proper error handling in future implementations.
api/spec/src/productcatalog/routes.tsp (5)
226-226: Inconsistent article usage in summaryIn the create method summary, you use "Create an addon" while other methods use "Update addon", "Get addon", etc. without the article. Consider standardizing the summary text format across all methods.
@post @operationId("createAddon") - @summary("Create an addon") + @summary("Create addon")
280-280: Fix typo in commentThere's a minor grammatical issue in the comment for the publish method.
/** - * Publish a addon version. + * Publish an addon version. */
291-291: Fix typo in commentSimilar grammatical issue in the archive method comment.
/** - * Archive a addon version. + * Archive an addon version. */
267-270: Fix typo in commentThere's a minor issue in the delete method comment.
/** * Soft delete addon by id. * - * Once a addon is deleted it cannot be undeleted. + * Once an addon is deleted it cannot be undeleted. */
163-300:Details
❓ Verification inconclusive
Consider adding a next method for consistency with Plans
The Plans interface has a
nextmethod for creating new draft versions, but this functionality is missing from the Addons interface. If addons are expected to follow the same versioning model as plans, consider adding a similar method.
🏁 Script executed:
#!/bin/bash # Check if addons will have a versioning model similar to plans # Check if Addon model has version field echo "Checking if Addon has version field..." grep -n "version" api/spec/src/productcatalog/addon.tsp || echo "Version field not found in addon.tsp" # Check if AddonStatus has similar states to PlanStatus echo "Comparing status enums..." grep -n "enum AddonStatus" -A 10 api/spec/src/productcatalog/addon.tsp grep -n "enum PlanStatus" -A 10 api/spec/src/productcatalog/plan.tspLength of output: 968
Action Required: Align Versioning Endpoints for Consistency
The Addons interface already supports versioning with a defined version field (e.g., line 43 in the Addon model). However, unlike the Plans interface—which offers anextmethod for creating new draft versions—the Addons interface currently lacks this functionality. Please verify whether the Addons endpoint is intended to follow the same versioning model as Plans. If so, consider adding anextmethod to enable the creation of new draft addon versions. Also, note that while both models share similar status values (draft, active, archived), the Plans interface includes an additional "scheduled" state that might require further consideration.api/openapi.yaml (4)
36-597: Comprehensive Addon Endpoints Implementation
The new endpoints for managing addons (including listing, creating, retrieving, updating, deleting, archiving, and publishing) are well-structured with clear parameter definitions and response specifications. Consider adding more examples in the parameter descriptions—especially for complex query parameters likekeyVersionandstatus—to improve clarity for API consumers. Also, please verify that the inclusion of the/api/v1/appsendpoint in the same grouping is intentional and aligns with the overall API design.
9400-9518: Review Addon Schema Definition and Grammar
TheAddonschema is comprehensive and includes detailed constraints, examples, and read-only markers. However, the description ("Addon provide a template for extending Plans") has a grammatical error; consider changing it to "Addon provides a template for extending Plans" for improved clarity.
9571-9578: Grammar Note in AddonInstanceType
The description forAddonInstanceTypecontains a minor grammatical error ("more then once" should be "more than once").
16392-16398: Consistency in Order By Options Naming
The order by options for plans include snake_case fields (created_atandupdated_at), which may be inconsistent with the camelCase naming seen elsewhere in the API. Please verify that this naming convention is intentional and aligns with your overall API design guidelines.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.spectral.yaml(1 hunks)api/client/go/client.gen.go(2 hunks)api/client/javascript/src/client/schemas.ts(1 hunks)api/openapi.cloud.yaml(1 hunks)api/openapi.yaml(4 hunks)api/spec/src/productcatalog/addon.tsp(1 hunks)api/spec/src/productcatalog/main.tsp(1 hunks)api/spec/src/productcatalog/plan.tsp(1 hunks)api/spec/src/productcatalog/routes.tsp(1 hunks)openmeter/server/router/addon.go(1 hunks)openmeter/server/router/router.go(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- api/openapi.cloud.yaml
🚧 Files skipped from review as they are similar to previous changes (7)
- api/spec/src/productcatalog/main.tsp
- .spectral.yaml
- api/spec/src/productcatalog/plan.tsp
- openmeter/server/router/router.go
- api/spec/src/productcatalog/addon.tsp
- api/client/go/client.gen.go
- api/client/javascript/src/client/schemas.ts
🧰 Additional context used
🧬 Code Definitions (1)
openmeter/server/router/addon.go (1)
openmeter/server/router/router.go (1)
Router(171-191)
🔇 Additional comments (4)
openmeter/server/router/addon.go (1)
1-49:Details
✅ Verification successful
Implementation of stub handlers aligns with API spec
The addon endpoints are properly stubbed with
http.StatusNotImplementedresponses, which is appropriate for this initial PR. All required handlers are present and match the API specification inroutes.tsp.
🏁 Script executed:
#!/bin/bash # Verify that all addon routes defined in the OpenAPI spec have corresponding handler methods echo "Checking OpenAPI spec for addon routes..." grep -n "/api/v1/addons" api/openapi.yaml echo "Checking handlers in addon.go..." grep -n "func (a \*Router)" openmeter/server/router/addon.goLength of output: 1099
Stub endpoints fully align with the OpenAPI spec
The verification confirms that all addon endpoints in
openmeter/server/router/addon.goare correctly stubbed withhttp.StatusNotImplementedresponses. Specifically:
- ListAddons handles
GET /api/v1/addons- CreateAddon handles
POST /api/v1/addons- GetAddon handles
GET /api/v1/addons/{addonIdOrKey}- DeleteAddon, UpdateAddon handle
DELETE/PUT /api/v1/addons/{addonId}- ArchiveAddon handles
POST /api/v1/addons/{addonId}/archive- PublishAddon handles
POST /api/v1/addons/{addonId}/publishAll routes defined in the OpenAPI spec have corresponding handler methods, so the implementation is correct.
api/spec/src/productcatalog/routes.tsp (2)
163-300: Addon interface is well-structured following existing patternsThe Addons interface follows the same pattern as the Plans interface, maintaining consistency in the API design. The operations, parameters, and return types are appropriately defined for managing addons.
235-242: Use explicit route path for update methodYou've explicitly added the route path for the update method, which is good. This makes the API more consistent and easier to understand.
api/openapi.yaml (1)
9579-9587: Consistency Check for AddonOrderBy Enum
TheAddonOrderByenum includes the optiontype, yet theAddonschema defines the property asinstanceType. Please verify if this discrepancy is intentional or if the enum should be updated to useinstanceTypefor consistency with the schema definition.
chrisgacsal
force-pushed
the
feat/addons-api
branch
2 times, most recently
from
acc3306 to
b38930e
Compare
chrisgacsal
force-pushed
the
feat/addons-api
branch
from
b38930e to
4d0b8a8
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
api/openapi.yaml (2)
8756-8778:⚠️ Potential issueIncomplete Parameter Definition for BillingProfile Override
While the definitions forAddonOrderByOrdering.orderandAddonOrderByOrdering.orderByare complete, the parameterBillingProfileCustomerOverrideOrderByOrdering.orderis incomplete—it only includes thenameandinproperties without a schema, default, or description. This is similar to previous feedback on this parameter. Please complete its definition to ensure consistent behavior.
9404-9665: 🛠️ Refactor suggestionSchema Definitions & Computed Status Clarification
TheAddonschema and its related models (e.g.,AddonCreate,AddonInstanceType,AddonPaginatedResponse,AddonReplaceUpdate) are comprehensively defined. However, note the following:
- The computed logic for the
statusproperty specifies:
- draft: when no
effectiveFromis provided,- active: when
effectiveFrom <= now < effectiveTo, and- archived: when
effectiveTo <= now.
Previous guidance mentioned a possiblescheduledstatus for future effective addons. If the intent is to distinguish future effective addons (i.e. "scheduled"), then either update the computed logic (and the accompanying description) or expand theAddonStatusenum to includescheduled.- In addition, the
Addressschema appears at the end without a complete definition. Please confirm whether it is intentionally included in this specification or if it is leftover from earlier work.Likely an incorrect or invalid review comment.
🧹 Nitpick comments (7)
api/spec/src/productcatalog/addon.tsp (3)
20-23: Fix grammatical error in the documentation.There's a small grammatical error in the addon type documentation.
/** * The type of the addon. - * Single instance addons can be added to subscription only once while addons with multiple type can be added more then once. + * Single instance addons can be added to subscription only once while addons with multiple type can be added more than once. */
46-47: Improve wording in the property documentation.The documentation refers to "addons" (plural) while describing a property of a single addon instance.
/** - * The instanceType of the addons. Can be "single" or "multiple". + * The instanceType of the addon. Can be "single" or "multiple". */
73-82: Consider marking status as computed or read-only.The status property is described as computed based on other properties but lacks explicit indicators that it's not to be provided during creation.
/** * The status of the addon. * Computed based on the effective start and end dates: * - draft = no effectiveFrom * - active = effectiveFrom <= now < effectiveTo * - archived = effectiveTo <= now */ @summary("Status") @visibility(Lifecycle.Read) +@extension("x-computed", true) status: AddonStatus;api/spec/src/productcatalog/routes.tsp (3)
270-273: Fix grammar in documentation.There's a grammatical error in the comment for the delete method.
/** * Soft delete addon by id. * - * Once a addon is deleted it cannot be undeleted. + * Once an addon is deleted it cannot be undeleted. */
284-286: Fix grammar in documentation.There's a grammatical error in the comment for the publish method.
/** - * Publish a addon version. + * Publish an addon version. */
296-298: Fix grammar in documentation.There's a grammatical error in the comment for the archive method.
/** - * Archive a addon version. + * Archive an addon version. */api/openapi.yaml (1)
16397-16403: Plan Ordering Options Verification
The ordering options for plans are listed asid,key,version,created_at, andupdated_at. Ensure that these options are consistent with the naming conventions and that they fully reflect the fields available for ordering in the plans endpoint. A brief confirmation or update on this matter would help maintain consistency across the API.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.spectral.yaml(1 hunks)api/client/go/client.gen.go(2 hunks)api/client/javascript/src/client/schemas.ts(1 hunks)api/openapi.cloud.yaml(1 hunks)api/openapi.yaml(4 hunks)api/spec/src/productcatalog/addon.tsp(1 hunks)api/spec/src/productcatalog/main.tsp(1 hunks)api/spec/src/productcatalog/plan.tsp(1 hunks)api/spec/src/productcatalog/routes.tsp(1 hunks)openmeter/server/router/addon.go(1 hunks)openmeter/server/router/router.go(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- api/openapi.cloud.yaml
🚧 Files skipped from review as they are similar to previous changes (7)
- .spectral.yaml
- api/spec/src/productcatalog/main.tsp
- api/spec/src/productcatalog/plan.tsp
- openmeter/server/router/addon.go
- api/client/go/client.gen.go
- openmeter/server/router/router.go
- api/client/javascript/src/client/schemas.ts
🔇 Additional comments (4)
api/spec/src/productcatalog/addon.tsp (1)
91-102: Implementation aligns with previous feedback.The
AddonOrderByenum implementation correctly mapscreatedAtto"created_at", addressing feedback from a previous review.api/spec/src/productcatalog/routes.tsp (2)
163-220: Interface structure looks good.The Addons interface provides a comprehensive set of operations and follows patterns established in the Plans interface. The use of
@extension("x-internal", true)for all methods is a good practice for new API features that might evolve.
163-307: Consider adding a "next" method.The Plans interface has a
nextmethod for creating a new draft version from a plan, but there's no equivalent in the Addons interface. Consider whether a similar method would be useful for addons to maintain consistency between the two interfaces.Is there a reason why addons don't need a "next" method similar to plans?
api/openapi.yaml (1)
36-605: Comprehensive Addons Endpoints Definition
The new endpoints for managing addons (list, create, get, update, delete, archive, publish) are well defined with detailed query parameters, responses, and use of reusable components (e.g. pagination, ordering). Please verify that the use of thex-internal: trueflag is intended for all these operations and that the examples (e.g. patterns, descriptions) strictly align with your API design guidelines.
3 participants
Overview
Extend API spec with basic support for Addons. The spec in not finalized as there are missing parts which are required to be part of the spec in order to make Addons fully functional, expect follow-up changes.
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes