-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
entitlements read from wingback #4804
Conversation
WalkthroughThis pull request introduces comprehensive changes to the Wingback licensing and subscription management system. The modifications span multiple files, enhancing type definitions, error handling, and configuration management. Key changes include adding a new log context, updating feature detail types, refining customer creation processes, improving entitlement retrieval, and introducing new exception handling mechanisms. The changes aim to provide more robust and flexible licensing functionality with improved type safety and error management. Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)src/core/error-handling/unhandled.exception.filter.ts (1)Pattern Context Files (Do Not Review):
Guidelines:
🔇 Additional comments (3)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
🔭 Outside diff range comments (1)
src/platform/licensing/wingback-subscription/licensing.wingback.subscription.resolver.mutations.ts (1)
Implement environment checks and consider relocating test endpoint
The review comment's claim about a TODO marker is incorrect. However, the suggestions about environment checks and relocating the test endpoint are valid:
- Add environment check to prevent production usage, following established patterns in the codebase
- Consider moving this endpoint to a dedicated test/development module (e.g.,
src/platform/testing/
)Example implementation remains valid as suggested in the review.
🔗 Analysis chain
Line range hint
29-52
: Address TODO and consider environment restrictions.The method is marked with a TODO to be moved. Additionally, as this is a test customer creation endpoint:
- Consider moving this to a dedicated test/development module
- Add environment checks to prevent execution in production
Example implementation:
@UseGuards(GraphqlGuard) @Mutation(() => String, { description: 'Create a test customer on wingback.', }) public async adminWingbackCreateTestCustomer( @CurrentUser() agentInfo: AgentInfo ) { if (process.env.NODE_ENV === 'production') { throw new Error('Test customer creation is not allowed in production'); } // ... rest of the implementation }🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash # Check for TODO in the file echo "=== Checking for TODO in the file ===" rg -A 2 -B 2 "TODO" "src/platform/licensing/wingback-subscription/licensing.wingback.subscription.resolver.mutations.ts" echo -e "\n=== Looking for similar test endpoints ===" rg "test.*customer" -A 2 -B 2 echo -e "\n=== Checking for environment checks pattern ===" rg "process\.env\.NODE_ENV.*production" -A 2 -B 2Length of output: 4928
🧹 Nitpick comments (10)
src/services/external/wingback/types/wingback.type.feature.ts (4)
23-25
: Consider using an enum for 'pricing_strategy'.Using an enum for 'pricing_strategy' can improve type safety and prevent typos.
You could define an enum like:
export enum PricingStrategy { FLAT = 'flat', // other strategies... }And update the type:
- pricing_strategy: 'flat'; + pricing_strategy: PricingStrategy.FLAT;
26-36
: Consider using enums for 'pricing_strategy' and 'over_usage_charge_strategy'.Utilizing enums enhances type safety and reduces the risk of typos.
Define enums for both fields:
export enum PricingStrategy { USAGE = 'usage', // other strategies... } export enum OverUsageChargeStrategy { ALL_USAGE_OVERCHARGE = 'all_usage_overcharge', EXTRA_USAGE_OVERCHARGE = 'extra_usage_overcharge', }And update the type:
- pricing_strategy: 'usage'; + pricing_strategy: PricingStrategy.USAGE; - over_usage_charge_strategy: 'all_usage_overcharge' | 'extra_usage_overcharge'; + over_usage_charge_strategy: OverUsageChargeStrategy;
37-39
: Consider using an enum for 'pricing_strategy'.Using an enum can improve consistency across different feature detail types.
40-42
: Consider using an enum for 'pricing_strategy'.Defining an enum for 'pricing_strategy' ensures consistent usage throughout the codebase.
src/services/external/wingback/wingback.manager.ts (1)
117-118
: Ensure 'subscription' is defined before destructuring.If 'entitlements' is empty, 'subscription' will be undefined. The subsequent checks handle this, but consider adding a comment for clarity.
src/services/external/wingback/types/wingback.type.create.customer.ts (1)
37-38
: Improve type safety of metadata fieldUsing
Record<string, any>
reduces type safety. Consider defining a specific interface for the metadata structure if the possible keys and value types are known.interface CustomerMetadata { // Define specific fields and their types [key: string]: string | number | boolean; // Or other specific types }src/services/external/wingback/types/entitlement-details/wingback.feature.detail.per.unit.ts (2)
9-18
: Consider using numeric types for unit countsUsing string types for numeric values (
contracted_unit_count
,used_unit_count
,minimum_units
,maximum_units
) could lead to unnecessary type conversions and potential errors. Consider using number type if these values are always numeric.
17-18
: Fix copy-paste error in documentationThe comment "Minimum number of units that is possible to purchase" is incorrect for the
maximum_units
field.- /** Minimum number of units that is possible to purchase */ + /** Maximum number of units that is possible to purchase */ maximum_units: string | null;src/common/enums/logging.context.ts (1)
79-79
: LGTM! Consider adding JSDoc comment.The addition of the WINGBACK context follows the established naming convention. Consider adding a JSDoc comment to document its specific usage scope.
+ /** Logging context for Wingback subscription and licensing related operations */ WINGBACK = 'wingback',
src/platform/licensing/wingback-subscription/licensing.wingback.subscription.resolver.mutations.ts (1)
41-41
: Consider adding validation for email format.The email construction should include validation to ensure it follows a valid email format, even for test data.
- invoice: `main${randomUUID()}@alkem.io`, + invoice: `test-${randomUUID().slice(0, 8)}@alkem.io`,
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/common/enums/logging.context.ts
(1 hunks)src/platform/licensing/wingback-subscription/licensing.wingback.subscription.resolver.mutations.ts
(1 hunks)src/platform/licensing/wingback-subscription/licensing.wingback.subscription.service.ts
(2 hunks)src/platform/licensing/wingback-subscription/wingback.constants.ts
(1 hunks)src/services/external/wingback/types/entitlement-details/wingback.feature.detail.per.unit.ts
(1 hunks)src/services/external/wingback/types/wingback.type.create.customer.ts
(2 hunks)src/services/external/wingback/types/wingback.type.feature.ts
(1 hunks)src/services/external/wingback/util/to.licensing.granted.entitlements.ts
(1 hunks)src/services/external/wingback/wingback.manager.ts
(3 hunks)src/services/infrastructure/license-entitlement-usage/license.entitlement.usage.service.ts
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
src/services/infrastructure/license-entitlement-usage/license.entitlement.usage.service.ts (1)
Pattern src/**/*.{ts,js}
: Review the TypeScript/JavaScript code for NestJS best practices, dependency injection, module structure, and potential bugs.
Context Files (Do Not Review):
docs/Design.md
- Design overview of the projectdocs/Pagination.md
- Pagination design overviewdocs/Developing.md
- Development setup overviewdocs/graphql-typeorm-usage.md
- overview of GraphQL and TypeORM usage and how they are used together with NestJS in the projectdocs/database-definitions.md
- guidelines for creating TypeORM entity defnitionssrc/core/error-handling/graphql.exception.filter.ts
- GraphQL error handlingsrc/core/error-handling/http.exception.filter.ts
- HTTP error handlingsrc/core/error-handling/rest.error.response.ts
- REST error responsesrc/core/error-handling/unhandled.exception.filter.ts
- Global exception handler
Guidelines:
- Our project uses global exception handlers (
UnhandledExceptionFilter
), so avoid suggesting additionaltry/catch
blocks unless handling specific cases. - Use NestJS latest documentation from
https://docs.nestjs.com/
for reference on NestJS best practices. - Use TypeORM latest documentation from
https://typeorm.io/
for reference on TypeORM best practices. - Refer to the design overview in the context files for better understanding.
src/common/enums/logging.context.ts (1)
Pattern src/**/*.{ts,js}
: Review the TypeScript/JavaScript code for NestJS best practices, dependency injection, module structure, and potential bugs.
Context Files (Do Not Review):
docs/Design.md
- Design overview of the projectdocs/Pagination.md
- Pagination design overviewdocs/Developing.md
- Development setup overviewdocs/graphql-typeorm-usage.md
- overview of GraphQL and TypeORM usage and how they are used together with NestJS in the projectdocs/database-definitions.md
- guidelines for creating TypeORM entity defnitionssrc/core/error-handling/graphql.exception.filter.ts
- GraphQL error handlingsrc/core/error-handling/http.exception.filter.ts
- HTTP error handlingsrc/core/error-handling/rest.error.response.ts
- REST error responsesrc/core/error-handling/unhandled.exception.filter.ts
- Global exception handler
Guidelines:
- Our project uses global exception handlers (
UnhandledExceptionFilter
), so avoid suggesting additionaltry/catch
blocks unless handling specific cases. - Use NestJS latest documentation from
https://docs.nestjs.com/
for reference on NestJS best practices. - Use TypeORM latest documentation from
https://typeorm.io/
for reference on TypeORM best practices. - Refer to the design overview in the context files for better understanding.
src/platform/licensing/wingback-subscription/licensing.wingback.subscription.resolver.mutations.ts (1)
Pattern src/**/*.{ts,js}
: Review the TypeScript/JavaScript code for NestJS best practices, dependency injection, module structure, and potential bugs.
Context Files (Do Not Review):
docs/Design.md
- Design overview of the projectdocs/Pagination.md
- Pagination design overviewdocs/Developing.md
- Development setup overviewdocs/graphql-typeorm-usage.md
- overview of GraphQL and TypeORM usage and how they are used together with NestJS in the projectdocs/database-definitions.md
- guidelines for creating TypeORM entity defnitionssrc/core/error-handling/graphql.exception.filter.ts
- GraphQL error handlingsrc/core/error-handling/http.exception.filter.ts
- HTTP error handlingsrc/core/error-handling/rest.error.response.ts
- REST error responsesrc/core/error-handling/unhandled.exception.filter.ts
- Global exception handler
Guidelines:
- Our project uses global exception handlers (
UnhandledExceptionFilter
), so avoid suggesting additionaltry/catch
blocks unless handling specific cases. - Use NestJS latest documentation from
https://docs.nestjs.com/
for reference on NestJS best practices. - Use TypeORM latest documentation from
https://typeorm.io/
for reference on TypeORM best practices. - Refer to the design overview in the context files for better understanding.
src/platform/licensing/wingback-subscription/wingback.constants.ts (1)
Pattern src/**/*.{ts,js}
: Review the TypeScript/JavaScript code for NestJS best practices, dependency injection, module structure, and potential bugs.
Context Files (Do Not Review):
docs/Design.md
- Design overview of the projectdocs/Pagination.md
- Pagination design overviewdocs/Developing.md
- Development setup overviewdocs/graphql-typeorm-usage.md
- overview of GraphQL and TypeORM usage and how they are used together with NestJS in the projectdocs/database-definitions.md
- guidelines for creating TypeORM entity defnitionssrc/core/error-handling/graphql.exception.filter.ts
- GraphQL error handlingsrc/core/error-handling/http.exception.filter.ts
- HTTP error handlingsrc/core/error-handling/rest.error.response.ts
- REST error responsesrc/core/error-handling/unhandled.exception.filter.ts
- Global exception handler
Guidelines:
- Our project uses global exception handlers (
UnhandledExceptionFilter
), so avoid suggesting additionaltry/catch
blocks unless handling specific cases. - Use NestJS latest documentation from
https://docs.nestjs.com/
for reference on NestJS best practices. - Use TypeORM latest documentation from
https://typeorm.io/
for reference on TypeORM best practices. - Refer to the design overview in the context files for better understanding.
src/services/external/wingback/types/wingback.type.create.customer.ts (1)
Pattern src/**/*.{ts,js}
: Review the TypeScript/JavaScript code for NestJS best practices, dependency injection, module structure, and potential bugs.
Context Files (Do Not Review):
docs/Design.md
- Design overview of the projectdocs/Pagination.md
- Pagination design overviewdocs/Developing.md
- Development setup overviewdocs/graphql-typeorm-usage.md
- overview of GraphQL and TypeORM usage and how they are used together with NestJS in the projectdocs/database-definitions.md
- guidelines for creating TypeORM entity defnitionssrc/core/error-handling/graphql.exception.filter.ts
- GraphQL error handlingsrc/core/error-handling/http.exception.filter.ts
- HTTP error handlingsrc/core/error-handling/rest.error.response.ts
- REST error responsesrc/core/error-handling/unhandled.exception.filter.ts
- Global exception handler
Guidelines:
- Our project uses global exception handlers (
UnhandledExceptionFilter
), so avoid suggesting additionaltry/catch
blocks unless handling specific cases. - Use NestJS latest documentation from
https://docs.nestjs.com/
for reference on NestJS best practices. - Use TypeORM latest documentation from
https://typeorm.io/
for reference on TypeORM best practices. - Refer to the design overview in the context files for better understanding.
src/platform/licensing/wingback-subscription/licensing.wingback.subscription.service.ts (1)
Pattern src/**/*.{ts,js}
: Review the TypeScript/JavaScript code for NestJS best practices, dependency injection, module structure, and potential bugs.
Context Files (Do Not Review):
docs/Design.md
- Design overview of the projectdocs/Pagination.md
- Pagination design overviewdocs/Developing.md
- Development setup overviewdocs/graphql-typeorm-usage.md
- overview of GraphQL and TypeORM usage and how they are used together with NestJS in the projectdocs/database-definitions.md
- guidelines for creating TypeORM entity defnitionssrc/core/error-handling/graphql.exception.filter.ts
- GraphQL error handlingsrc/core/error-handling/http.exception.filter.ts
- HTTP error handlingsrc/core/error-handling/rest.error.response.ts
- REST error responsesrc/core/error-handling/unhandled.exception.filter.ts
- Global exception handler
Guidelines:
- Our project uses global exception handlers (
UnhandledExceptionFilter
), so avoid suggesting additionaltry/catch
blocks unless handling specific cases. - Use NestJS latest documentation from
https://docs.nestjs.com/
for reference on NestJS best practices. - Use TypeORM latest documentation from
https://typeorm.io/
for reference on TypeORM best practices. - Refer to the design overview in the context files for better understanding.
src/services/external/wingback/util/to.licensing.granted.entitlements.ts (1)
Pattern src/**/*.{ts,js}
: Review the TypeScript/JavaScript code for NestJS best practices, dependency injection, module structure, and potential bugs.
Context Files (Do Not Review):
docs/Design.md
- Design overview of the projectdocs/Pagination.md
- Pagination design overviewdocs/Developing.md
- Development setup overviewdocs/graphql-typeorm-usage.md
- overview of GraphQL and TypeORM usage and how they are used together with NestJS in the projectdocs/database-definitions.md
- guidelines for creating TypeORM entity defnitionssrc/core/error-handling/graphql.exception.filter.ts
- GraphQL error handlingsrc/core/error-handling/http.exception.filter.ts
- HTTP error handlingsrc/core/error-handling/rest.error.response.ts
- REST error responsesrc/core/error-handling/unhandled.exception.filter.ts
- Global exception handler
Guidelines:
- Our project uses global exception handlers (
UnhandledExceptionFilter
), so avoid suggesting additionaltry/catch
blocks unless handling specific cases. - Use NestJS latest documentation from
https://docs.nestjs.com/
for reference on NestJS best practices. - Use TypeORM latest documentation from
https://typeorm.io/
for reference on TypeORM best practices. - Refer to the design overview in the context files for better understanding.
src/services/external/wingback/wingback.manager.ts (1)
Pattern src/**/*.{ts,js}
: Review the TypeScript/JavaScript code for NestJS best practices, dependency injection, module structure, and potential bugs.
Context Files (Do Not Review):
docs/Design.md
- Design overview of the projectdocs/Pagination.md
- Pagination design overviewdocs/Developing.md
- Development setup overviewdocs/graphql-typeorm-usage.md
- overview of GraphQL and TypeORM usage and how they are used together with NestJS in the projectdocs/database-definitions.md
- guidelines for creating TypeORM entity defnitionssrc/core/error-handling/graphql.exception.filter.ts
- GraphQL error handlingsrc/core/error-handling/http.exception.filter.ts
- HTTP error handlingsrc/core/error-handling/rest.error.response.ts
- REST error responsesrc/core/error-handling/unhandled.exception.filter.ts
- Global exception handler
Guidelines:
- Our project uses global exception handlers (
UnhandledExceptionFilter
), so avoid suggesting additionaltry/catch
blocks unless handling specific cases. - Use NestJS latest documentation from
https://docs.nestjs.com/
for reference on NestJS best practices. - Use TypeORM latest documentation from
https://typeorm.io/
for reference on TypeORM best practices. - Refer to the design overview in the context files for better understanding.
src/services/external/wingback/types/entitlement-details/wingback.feature.detail.per.unit.ts (1)
Pattern src/**/*.{ts,js}
: Review the TypeScript/JavaScript code for NestJS best practices, dependency injection, module structure, and potential bugs.
Context Files (Do Not Review):
docs/Design.md
- Design overview of the projectdocs/Pagination.md
- Pagination design overviewdocs/Developing.md
- Development setup overviewdocs/graphql-typeorm-usage.md
- overview of GraphQL and TypeORM usage and how they are used together with NestJS in the projectdocs/database-definitions.md
- guidelines for creating TypeORM entity defnitionssrc/core/error-handling/graphql.exception.filter.ts
- GraphQL error handlingsrc/core/error-handling/http.exception.filter.ts
- HTTP error handlingsrc/core/error-handling/rest.error.response.ts
- REST error responsesrc/core/error-handling/unhandled.exception.filter.ts
- Global exception handler
Guidelines:
- Our project uses global exception handlers (
UnhandledExceptionFilter
), so avoid suggesting additionaltry/catch
blocks unless handling specific cases. - Use NestJS latest documentation from
https://docs.nestjs.com/
for reference on NestJS best practices. - Use TypeORM latest documentation from
https://typeorm.io/
for reference on TypeORM best practices. - Refer to the design overview in the context files for better understanding.
src/services/external/wingback/types/wingback.type.feature.ts (1)
Pattern src/**/*.{ts,js}
: Review the TypeScript/JavaScript code for NestJS best practices, dependency injection, module structure, and potential bugs.
Context Files (Do Not Review):
docs/Design.md
- Design overview of the projectdocs/Pagination.md
- Pagination design overviewdocs/Developing.md
- Development setup overviewdocs/graphql-typeorm-usage.md
- overview of GraphQL and TypeORM usage and how they are used together with NestJS in the projectdocs/database-definitions.md
- guidelines for creating TypeORM entity defnitionssrc/core/error-handling/graphql.exception.filter.ts
- GraphQL error handlingsrc/core/error-handling/http.exception.filter.ts
- HTTP error handlingsrc/core/error-handling/rest.error.response.ts
- REST error responsesrc/core/error-handling/unhandled.exception.filter.ts
- Global exception handler
Guidelines:
- Our project uses global exception handlers (
UnhandledExceptionFilter
), so avoid suggesting additionaltry/catch
blocks unless handling specific cases. - Use NestJS latest documentation from
https://docs.nestjs.com/
for reference on NestJS best practices. - Use TypeORM latest documentation from
https://typeorm.io/
for reference on TypeORM best practices. - Refer to the design overview in the context files for better understanding.
🔇 Additional comments (11)
src/services/external/wingback/types/wingback.type.feature.ts (5)
1-2
: Import statement is correct.The imported module path is valid and necessary for the type definitions.
5-5
: Property 'slug' documentation is clear and precise.The comment appropriately describes the purpose of the 'slug' property.
8-8
: Enhanced type safety for 'entitlement_details'.Changing 'entitlement_details' to use the union type
WingbackFeatureDetails
improves type safety and allows for better extensibility.
11-14
: Good use of generics in 'WingbackTypedFeature'.Introducing the generic type
WingbackTypedFeature<TDetails>
enhances flexibility by allowing specific detail types for features.
16-22
: Union type 'WingbackFeatureDetails' is well-defined.The union type accommodates multiple pricing strategies, enhancing scalability and maintainability.
src/platform/licensing/wingback-subscription/licensing.wingback.subscription.service.ts (3)
4-17
: Imports are correctly organized and necessary.The added imports are appropriate for the functionality implemented.
40-43
: Refactored 'getEntitlements' method enhances clarity.Returning the result of
toLicensingGrantedEntitlements
directly simplifies the method.
50-85
: Efficient mapping of Wingback features to licensing entitlements.The
toLicensingGrantedEntitlements
method effectively filters and maps features, handling unsupported slugs appropriately.src/services/external/wingback/wingback.manager.ts (1)
10-12
: Imports for exceptions and enums are appropriate.Including
BaseException
and relevant enums enhances error handling.src/services/external/wingback/util/to.licensing.granted.entitlements.ts (1)
6-22
: Remove duplicate enum and mappingThis enum and mapping are duplicated from
wingback.constants.ts
. Import them from there instead.src/services/infrastructure/license-entitlement-usage/license.entitlement.usage.service.ts (1)
Line range hint
82-89
: Fix or remove commented code in getAccountSpacesTypeCount.The commented code indicates a non-working path. Either fix the implementation or remove the commented code and create a ticket to track this issue.
Would you like me to help implement the space filtering logic or create a GitHub issue to track this task?
src/services/external/wingback/util/to.licensing.granted.entitlements.ts
Outdated
Show resolved
Hide resolved
src/services/external/wingback/util/to.licensing.granted.entitlements.ts
Outdated
Show resolved
Hide resolved
src/services/infrastructure/license-entitlement-usage/license.entitlement.usage.service.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/services/external/wingback/wingback.manager.ts (2)
41-50
:⚠️ Potential issueUse BaseException for consistent error handling.
Replace the generic Error with BaseException to align with the project's error handling practices.
Apply this diff:
- throw new Error('Wingback is not enabled'); + throw new BaseException( + 'Wingback is not enabled', + LogContext.WINGBACK, + AlkemioErrorStatus.NOT_IMPLEMENTED + );
116-145
:⚠️ Potential issueAdd null check for subscription.plan.
The function accesses
subscription.plan.features
without verifying thatplan
exists.Apply this diff:
if (!subscription) { throw new BaseException( 'No subscriptions found for customer', LogContext.WINGBACK, AlkemioErrorStatus.ENTITY_NOT_FOUND, { customerId } ); } + if (!subscription.plan) { + throw new BaseException( + 'Subscription plan is undefined', + LogContext.WINGBACK, + AlkemioErrorStatus.ENTITY_NOT_FOUND, + { customerId } + ); + } return subscription.plan.features;
🧹 Nitpick comments (11)
src/services/external/wingback/types/wingback.type.feature.ts (4)
3-8
: Add JSDoc comments for all fields inWingbackFeature
type.While
slug
has a helpful comment, other fields likename
,original_feature_id
, andentitlement_details
would benefit from similar documentation for better maintainability.export type WingbackFeature = { + /** The display name of the feature */ name: string; /** a feature short name (nameID) */ slug: string; + /** The original identifier of the feature in the external system */ original_feature_id: string; + /** The details of the feature's entitlement configuration */ entitlement_details: WingbackFeatureDetails; };
11-14
: Add JSDoc documentation for the generic type.The
WingbackTypedFeature
generic type would benefit from documentation explaining its purpose and usage.+/** + * A type-safe version of WingbackFeature that allows specifying the exact type of entitlement details. + * @template TDetails The specific type of entitlement details, must extend WingbackFeatureDetails + */ export type WingbackTypedFeature<TDetails extends WingbackFeatureDetails> = WingbackFeature & { entitlement_details: TDetails; };
16-21
: Add JSDoc documentation for the feature details union type.The
WingbackFeatureDetails
union type would benefit from documentation explaining the available pricing strategies.+/** + * Represents all possible feature detail types based on different pricing strategies: + * - flat: Simple flat-rate pricing + * - per_unit: Per-unit based pricing + * - usage: Usage-based pricing with overcharge strategies + * - unit_tiered: Tiered pricing based on units + * - usage_tiered: Tiered pricing based on usage + */ export type WingbackFeatureDetails = | WingbackFeatureDetailFlat | WingbackFeatureDetailPerUnit | WingbackFeatureDetailUsage | WingbackFeatureDetailUnitTiered | WingbackFeatureDetailUsageTiered;
37-42
: Add documentation for tiered pricing strategies.The
WingbackFeatureDetailUnitTiered
andWingbackFeatureDetailUsageTiered
types would benefit from documentation explaining their pricing models.+/** + * Represents a feature with tiered pricing based on units. + * The price varies based on predefined unit tiers. + */ export type WingbackFeatureDetailUnitTiered = { pricing_strategy: 'unit_tiered'; }; +/** + * Represents a feature with tiered pricing based on usage. + * The price varies based on predefined usage tiers. + */ export type WingbackFeatureDetailUsageTiered = { pricing_strategy: 'usage_tiered'; };src/domain/space/account/account.service.license.ts (1)
95-109
: Robust error handling implementation.The error handling implementation follows NestJS best practices with proper initialization and logging. Consider enhancing the error message to include the error type for better debugging:
- `Skipping Wingback entitlements for account ${account.id} since it returned with an error: ${e}`, + `Skipping Wingback entitlements for account ${account.id} due to ${e.name}: ${e.message}`,src/common/exceptions/internal/retry.exception.ts (1)
5-9
: Remove unnecessary constructorThe constructor can be removed as it only forwards parameters to the parent class without additional logic.
-export class RetryException extends BaseExceptionInternal { - constructor(error: string, context: LogContext, details?: ExceptionDetails) { - super(error, context, details); - } -} +export class RetryException extends BaseExceptionInternal {}🧰 Tools
🪛 Biome (1.9.4)
[error] 6-8: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
src/common/exceptions/internal/timeout.exception.ts (1)
5-9
: Remove unnecessary constructorThe constructor can be removed as it only forwards parameters to the parent class without additional logic.
-export class TimeoutException extends BaseExceptionInternal { - constructor(error: string, context: LogContext, details?: ExceptionDetails) { - super(error, context, details); - } -} +export class TimeoutException extends BaseExceptionInternal {}🧰 Tools
🪛 Biome (1.9.4)
[error] 6-8: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
src/common/exceptions/internal/base.exception.internal.ts (1)
5-17
: Remove redundant exceptionName propertyThe
exceptionName
property is redundant as the class name is already set in thename
property through the constructor.export class BaseExceptionInternal extends Error { - private readonly exceptionName = this.constructor.name; constructor( public readonly message: string, public readonly context: LogContext, public readonly details?: ExceptionDetails, public readonly errorId: string = randomUUID() ) { super(message); this.name = this.constructor.name; } }
src/types/alkemio.config.ts (1)
41-42
: Add type constraints and documentation for configuration propertiesThe new wingback configuration properties would benefit from:
- JSDoc documentation explaining their purpose and expected values
- Type constraints to ensure positive numbers
wingback: { enabled: boolean; key: string; endpoint: string; - retries: number; - timeout: number; + /** Number of retry attempts for wingback operations. Must be positive. */ + retries: Positive<number>; + /** Timeout in milliseconds for wingback operations. Must be positive. */ + timeout: Positive<number>; };Consider creating a utility type for positive numbers:
type Positive<T extends number> = number extends T ? never : `${T}` extends `-${number}` ? never : T;src/platform/licensing/wingback-subscription/licensing.wingback.subscription.service.ts (1)
56-96
: LGTM! Well-structured feature mapping with robust error handling.The implementation effectively handles feature mapping with proper type guards, graceful handling of unknown features, and appropriate logging.
Consider adding debug logging for successfully mapped features to aid in troubleshooting.
Add debug logging for successfully mapped features:
return { type: licenseEntitlementType, limit: Number(entitlement_details.contracted_unit_count), }; + this.logger.debug?.( + `Successfully mapped Wingback feature "${slug}" to "${licenseEntitlementType}"`, + LogContext.WINGBACK + );src/core/error-handling/unhandled.exception.filter.ts (1)
52-61
: Use ConfigService to access environment variablesAccessing environment variables directly via
process.env
is discouraged in NestJS applications. Consider injectingConfigService
from@nestjs/config
to manage configuration variables, aligning with NestJS best practices and supporting flexible configuration management.Apply this diff to inject
ConfigService
and use it:import { WINSTON_MODULE_NEST_PROVIDER } from 'nest-winston'; +import { ConfigService } from '@nestjs/config'; import { ContextTypeWithGraphQL } from '@src/types/context.type'; import { GraphQLError } from 'graphql'; import { AlkemioErrorStatus } from '@common/enums'; @Catch(Error) export class UnhandledExceptionFilter implements ExceptionFilter { constructor( @Inject(WINSTON_MODULE_NEST_PROVIDER) private readonly logger: LoggerService, + private readonly configService: ConfigService, ) {} catch(exception: Error, host: ArgumentsHost) { // ... } else if (contextType === 'graphql') { - if (process.env.NODE_ENV === 'production') { + if (this.configService.get('NODE_ENV') === 'production') { // return a new error with only the message and the id // that way we are not exposing any internal information return new GraphQLError(exception.message, { extensions: { errorId: secondParam.errorId, code: AlkemioErrorStatus.UNSPECIFIED, }, }); } // if not in PROD, return everything return new GraphQLError(exception.message, { extensions: { - ...exception, - message: undefined, // do not repeat the message + errorId: secondParam.errorId, + name: exception.name, + stack: exception.stack, + code: AlkemioErrorStatus.UNSPECIFIED, }, }); } // ...
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
alkemio.yml
(1 hunks)src/common/exceptions/internal/base.exception.internal.ts
(1 hunks)src/common/exceptions/internal/index.ts
(1 hunks)src/common/exceptions/internal/retry.exception.ts
(1 hunks)src/common/exceptions/internal/timeout.exception.ts
(1 hunks)src/core/error-handling/unhandled.exception.filter.ts
(2 hunks)src/domain/space/account/account.service.license.ts
(2 hunks)src/platform/licensing/wingback-subscription/licensing.wingback.subscription.resolver.mutations.ts
(4 hunks)src/platform/licensing/wingback-subscription/licensing.wingback.subscription.service.ts
(2 hunks)src/services/external/wingback/types/wingback.type.feature.ts
(1 hunks)src/services/external/wingback/wingback.manager.ts
(4 hunks)src/services/infrastructure/license-entitlement-usage/license.entitlement.usage.service.ts
(2 hunks)src/types/alkemio.config.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/common/exceptions/internal/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/services/infrastructure/license-entitlement-usage/license.entitlement.usage.service.ts
- src/platform/licensing/wingback-subscription/licensing.wingback.subscription.resolver.mutations.ts
🧰 Additional context used
📓 Path-based instructions (9)
src/common/exceptions/internal/timeout.exception.ts (1)
Pattern src/**/*.{ts,js}
: Review the TypeScript/JavaScript code for NestJS best practices, dependency injection, module structure, and potential bugs.
Context Files (Do Not Review):
docs/Design.md
- Design overview of the projectdocs/Pagination.md
- Pagination design overviewdocs/Developing.md
- Development setup overviewdocs/graphql-typeorm-usage.md
- overview of GraphQL and TypeORM usage and how they are used together with NestJS in the projectdocs/database-definitions.md
- guidelines for creating TypeORM entity defnitionssrc/core/error-handling/graphql.exception.filter.ts
- GraphQL error handlingsrc/core/error-handling/http.exception.filter.ts
- HTTP error handlingsrc/core/error-handling/rest.error.response.ts
- REST error responsesrc/core/error-handling/unhandled.exception.filter.ts
- Global exception handler
Guidelines:
- Our project uses global exception handlers (
UnhandledExceptionFilter
), so avoid suggesting additionaltry/catch
blocks unless handling specific cases. - Use NestJS latest documentation from
https://docs.nestjs.com/
for reference on NestJS best practices. - Use TypeORM latest documentation from
https://typeorm.io/
for reference on TypeORM best practices. - Refer to the design overview in the context files for better understanding.
src/common/exceptions/internal/base.exception.internal.ts (1)
Pattern src/**/*.{ts,js}
: Review the TypeScript/JavaScript code for NestJS best practices, dependency injection, module structure, and potential bugs.
Context Files (Do Not Review):
docs/Design.md
- Design overview of the projectdocs/Pagination.md
- Pagination design overviewdocs/Developing.md
- Development setup overviewdocs/graphql-typeorm-usage.md
- overview of GraphQL and TypeORM usage and how they are used together with NestJS in the projectdocs/database-definitions.md
- guidelines for creating TypeORM entity defnitionssrc/core/error-handling/graphql.exception.filter.ts
- GraphQL error handlingsrc/core/error-handling/http.exception.filter.ts
- HTTP error handlingsrc/core/error-handling/rest.error.response.ts
- REST error responsesrc/core/error-handling/unhandled.exception.filter.ts
- Global exception handler
Guidelines:
- Our project uses global exception handlers (
UnhandledExceptionFilter
), so avoid suggesting additionaltry/catch
blocks unless handling specific cases. - Use NestJS latest documentation from
https://docs.nestjs.com/
for reference on NestJS best practices. - Use TypeORM latest documentation from
https://typeorm.io/
for reference on TypeORM best practices. - Refer to the design overview in the context files for better understanding.
src/common/exceptions/internal/retry.exception.ts (1)
Pattern src/**/*.{ts,js}
: Review the TypeScript/JavaScript code for NestJS best practices, dependency injection, module structure, and potential bugs.
Context Files (Do Not Review):
docs/Design.md
- Design overview of the projectdocs/Pagination.md
- Pagination design overviewdocs/Developing.md
- Development setup overviewdocs/graphql-typeorm-usage.md
- overview of GraphQL and TypeORM usage and how they are used together with NestJS in the projectdocs/database-definitions.md
- guidelines for creating TypeORM entity defnitionssrc/core/error-handling/graphql.exception.filter.ts
- GraphQL error handlingsrc/core/error-handling/http.exception.filter.ts
- HTTP error handlingsrc/core/error-handling/rest.error.response.ts
- REST error responsesrc/core/error-handling/unhandled.exception.filter.ts
- Global exception handler
Guidelines:
- Our project uses global exception handlers (
UnhandledExceptionFilter
), so avoid suggesting additionaltry/catch
blocks unless handling specific cases. - Use NestJS latest documentation from
https://docs.nestjs.com/
for reference on NestJS best practices. - Use TypeORM latest documentation from
https://typeorm.io/
for reference on TypeORM best practices. - Refer to the design overview in the context files for better understanding.
src/types/alkemio.config.ts (1)
Pattern src/**/*.{ts,js}
: Review the TypeScript/JavaScript code for NestJS best practices, dependency injection, module structure, and potential bugs.
Context Files (Do Not Review):
docs/Design.md
- Design overview of the projectdocs/Pagination.md
- Pagination design overviewdocs/Developing.md
- Development setup overviewdocs/graphql-typeorm-usage.md
- overview of GraphQL and TypeORM usage and how they are used together with NestJS in the projectdocs/database-definitions.md
- guidelines for creating TypeORM entity defnitionssrc/core/error-handling/graphql.exception.filter.ts
- GraphQL error handlingsrc/core/error-handling/http.exception.filter.ts
- HTTP error handlingsrc/core/error-handling/rest.error.response.ts
- REST error responsesrc/core/error-handling/unhandled.exception.filter.ts
- Global exception handler
Guidelines:
- Our project uses global exception handlers (
UnhandledExceptionFilter
), so avoid suggesting additionaltry/catch
blocks unless handling specific cases. - Use NestJS latest documentation from
https://docs.nestjs.com/
for reference on NestJS best practices. - Use TypeORM latest documentation from
https://typeorm.io/
for reference on TypeORM best practices. - Refer to the design overview in the context files for better understanding.
src/domain/space/account/account.service.license.ts (1)
Pattern src/**/*.{ts,js}
: Review the TypeScript/JavaScript code for NestJS best practices, dependency injection, module structure, and potential bugs.
Context Files (Do Not Review):
docs/Design.md
- Design overview of the projectdocs/Pagination.md
- Pagination design overviewdocs/Developing.md
- Development setup overviewdocs/graphql-typeorm-usage.md
- overview of GraphQL and TypeORM usage and how they are used together with NestJS in the projectdocs/database-definitions.md
- guidelines for creating TypeORM entity defnitionssrc/core/error-handling/graphql.exception.filter.ts
- GraphQL error handlingsrc/core/error-handling/http.exception.filter.ts
- HTTP error handlingsrc/core/error-handling/rest.error.response.ts
- REST error responsesrc/core/error-handling/unhandled.exception.filter.ts
- Global exception handler
Guidelines:
- Our project uses global exception handlers (
UnhandledExceptionFilter
), so avoid suggesting additionaltry/catch
blocks unless handling specific cases. - Use NestJS latest documentation from
https://docs.nestjs.com/
for reference on NestJS best practices. - Use TypeORM latest documentation from
https://typeorm.io/
for reference on TypeORM best practices. - Refer to the design overview in the context files for better understanding.
src/services/external/wingback/types/wingback.type.feature.ts (1)
Pattern src/**/*.{ts,js}
: Review the TypeScript/JavaScript code for NestJS best practices, dependency injection, module structure, and potential bugs.
Context Files (Do Not Review):
docs/Design.md
- Design overview of the projectdocs/Pagination.md
- Pagination design overviewdocs/Developing.md
- Development setup overviewdocs/graphql-typeorm-usage.md
- overview of GraphQL and TypeORM usage and how they are used together with NestJS in the projectdocs/database-definitions.md
- guidelines for creating TypeORM entity defnitionssrc/core/error-handling/graphql.exception.filter.ts
- GraphQL error handlingsrc/core/error-handling/http.exception.filter.ts
- HTTP error handlingsrc/core/error-handling/rest.error.response.ts
- REST error responsesrc/core/error-handling/unhandled.exception.filter.ts
- Global exception handler
Guidelines:
- Our project uses global exception handlers (
UnhandledExceptionFilter
), so avoid suggesting additionaltry/catch
blocks unless handling specific cases. - Use NestJS latest documentation from
https://docs.nestjs.com/
for reference on NestJS best practices. - Use TypeORM latest documentation from
https://typeorm.io/
for reference on TypeORM best practices. - Refer to the design overview in the context files for better understanding.
src/core/error-handling/unhandled.exception.filter.ts (1)
Pattern src/**/*.{ts,js}
: Review the TypeScript/JavaScript code for NestJS best practices, dependency injection, module structure, and potential bugs.
Context Files (Do Not Review):
docs/Design.md
- Design overview of the projectdocs/Pagination.md
- Pagination design overviewdocs/Developing.md
- Development setup overviewdocs/graphql-typeorm-usage.md
- overview of GraphQL and TypeORM usage and how they are used together with NestJS in the projectdocs/database-definitions.md
- guidelines for creating TypeORM entity defnitionssrc/core/error-handling/graphql.exception.filter.ts
- GraphQL error handlingsrc/core/error-handling/http.exception.filter.ts
- HTTP error handlingsrc/core/error-handling/rest.error.response.ts
- REST error responsesrc/core/error-handling/unhandled.exception.filter.ts
- Global exception handler
Guidelines:
- Our project uses global exception handlers (
UnhandledExceptionFilter
), so avoid suggesting additionaltry/catch
blocks unless handling specific cases. - Use NestJS latest documentation from
https://docs.nestjs.com/
for reference on NestJS best practices. - Use TypeORM latest documentation from
https://typeorm.io/
for reference on TypeORM best practices. - Refer to the design overview in the context files for better understanding.
src/platform/licensing/wingback-subscription/licensing.wingback.subscription.service.ts (1)
Pattern src/**/*.{ts,js}
: Review the TypeScript/JavaScript code for NestJS best practices, dependency injection, module structure, and potential bugs.
Context Files (Do Not Review):
docs/Design.md
- Design overview of the projectdocs/Pagination.md
- Pagination design overviewdocs/Developing.md
- Development setup overviewdocs/graphql-typeorm-usage.md
- overview of GraphQL and TypeORM usage and how they are used together with NestJS in the projectdocs/database-definitions.md
- guidelines for creating TypeORM entity defnitionssrc/core/error-handling/graphql.exception.filter.ts
- GraphQL error handlingsrc/core/error-handling/http.exception.filter.ts
- HTTP error handlingsrc/core/error-handling/rest.error.response.ts
- REST error responsesrc/core/error-handling/unhandled.exception.filter.ts
- Global exception handler
Guidelines:
- Our project uses global exception handlers (
UnhandledExceptionFilter
), so avoid suggesting additionaltry/catch
blocks unless handling specific cases. - Use NestJS latest documentation from
https://docs.nestjs.com/
for reference on NestJS best practices. - Use TypeORM latest documentation from
https://typeorm.io/
for reference on TypeORM best practices. - Refer to the design overview in the context files for better understanding.
src/services/external/wingback/wingback.manager.ts (1)
Pattern src/**/*.{ts,js}
: Review the TypeScript/JavaScript code for NestJS best practices, dependency injection, module structure, and potential bugs.
Context Files (Do Not Review):
docs/Design.md
- Design overview of the projectdocs/Pagination.md
- Pagination design overviewdocs/Developing.md
- Development setup overviewdocs/graphql-typeorm-usage.md
- overview of GraphQL and TypeORM usage and how they are used together with NestJS in the projectdocs/database-definitions.md
- guidelines for creating TypeORM entity defnitionssrc/core/error-handling/graphql.exception.filter.ts
- GraphQL error handlingsrc/core/error-handling/http.exception.filter.ts
- HTTP error handlingsrc/core/error-handling/rest.error.response.ts
- REST error responsesrc/core/error-handling/unhandled.exception.filter.ts
- Global exception handler
Guidelines:
- Our project uses global exception handlers (
UnhandledExceptionFilter
), so avoid suggesting additionaltry/catch
blocks unless handling specific cases. - Use NestJS latest documentation from
https://docs.nestjs.com/
for reference on NestJS best practices. - Use TypeORM latest documentation from
https://typeorm.io/
for reference on TypeORM best practices. - Refer to the design overview in the context files for better understanding.
🪛 Biome (1.9.4)
src/common/exceptions/internal/timeout.exception.ts
[error] 6-8: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
src/common/exceptions/internal/retry.exception.ts
[error] 6-8: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Travis CI - Pull Request
🔇 Additional comments (5)
src/domain/space/account/account.service.license.ts (2)
17-17
: LGTM! Import added correctly.The new import for
LicensingGrantedEntitlement
is properly placed and used in the type declaration.
Line range hint
110-122
: Verify the business impact of skipped entitlements.When Wingback entitlements are skipped due to an error, the license entitlements remain in their default state. Please verify if this fallback behavior aligns with the business requirements.
Let's analyze the potential impact:
✅ Verification successful
Default entitlement state provides secure fallback behavior
The current implementation follows the principle of least privilege by defaulting to disabled entitlements (enabled=false) with zero limits when Wingback processing fails. This is a secure fallback behavior that prevents unauthorized access rather than potentially granting excessive permissions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for default values of license entitlements and their usage in the codebase # Find default values for license entitlements rg -A 5 "class.*implements.*ILicenseEntitlement" # Find places where license entitlements are checked rg "entitlement\.(enabled|limit)"Length of output: 2564
src/platform/licensing/wingback-subscription/licensing.wingback.subscription.service.ts (1)
37-54
: LGTM! Clean refactoring of entitlements processing.The function is well-structured with clear separation of concerns, good error handling through the wingbackManager, and appropriate logging.
src/services/external/wingback/wingback.manager.ts (1)
177-256
: LGTM! Robust HTTP request handling with retries and timeouts.The implementation includes:
- Clear error messages with context
- Proper timeout handling
- Retry mechanism with exponential backoff
- Comprehensive error logging
alkemio.yml (1)
93-94
: LGTM! Reasonable defaults for retries and timeout.The configuration parameters:
- Retry count of 3 provides good balance between reliability and responsiveness
- 30-second timeout is appropriate for external API calls
* entitlements read from wingback * code readability improvements * better exception handling * reverted homeserver.yaml * rabbit comments * rabbit comments
externalSubscriptionID
.Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores