286 unlock 4 implement key loyalty card contract

[sebpalluel]

Type

enhancement, other

---

Description

• Refactored NftCollection to EventPassCollection for clarity.
• Implemented PackCollection for deploying and managing packs.
• Introduced LoyaltyCardCollection for loyalty card contract deployment and metadata management.
• Updated GraphQL types and queries to support new webhook fields and nullable tokenId.
• Enhanced error handling across event pass, pack, and loyalty card deployment.

---

Changes walkthrough

	Relevant files
Enhancement	index.ts Update GraphQL Types for Nullable Token IDs and Webhook Info libs/gql/admin/types/src/generated/index.ts Updated GraphQL types to handle nullable tokenId fields. Adjusted types for webhook information to include signing keys. +224/-28 index.ts Update GraphQL Queries with Webhook Fields and Rename Documents libs/gql/admin/api/src/generated/index.ts Modified GraphQL queries to include new webhook fields. Adjusted document names for clarity and consistency. +387/-30 index.ts Refactor Event Pass Collection Deployment and Error Handling libs/nft/thirdweb-organizer-event-pass/src/lib/index.ts Refactored class name from NftCollection to EventPassCollection. Implemented error handling improvements and code refactoring. Added new methods for deploying event pass collections. +48/-326 index.ts Implement Pack Collection Deployment and Management            libs/nft/thirdweb-organizer-pack/src/lib/index.ts Introduced PackCollection class for pack deployment and management. Added methods for deploying packs and associating them with event passes. +268/-0  index.ts Implement Loyalty Card Collection Deployment and Metadata Management libs/nft/thirdweb-organizer-loyalty-card/src/lib/index.ts Introduced LoyaltyCardCollection class for loyalty card deployment. Added methods for deploying loyalty card contracts and managing metadata. +115/-0
Tests	index.integration.test.ts Update Integration Tests for Event Pass Collection              libs/nft/thirdweb-organizer-event-pass/src/lib/index.integration.test.ts Updated tests to reflect class name change to EventPassCollection. Added new test cases for event pass collection deployment. +49/-36  index.spec.ts Update Unit Tests for Event Pass Collection Refactoring    libs/nft/thirdweb-organizer-event-pass/src/lib/index.spec.ts Updated unit tests for EventPassCollection class. Added tests for new deployment methods and error handling. +78/-66

---

✨ PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
back-office	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Apr 22, 2024 1:45pm
marketplace	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Apr 22, 2024 1:45pm
unlock	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Apr 22, 2024 1:45pm

[codiumai-pr-agent-free]

PR Description updated to latest commit (1ef0fdb)

[codiumai-pr-agent-free]

PR Review

⏱️ Estimated effort to review [1-5]	4, because the PR introduces a significant amount of new functionality, including enhancements and refactoring of existing code. The changes span across multiple files and include both backend and frontend modifications. The complexity of the changes, especially those related to blockchain interactions and the deployment of smart contracts, requires a thorough review to ensure correctness, security, and performance. Additionally, the introduction of new tests and the modification of existing ones indicate the need for careful evaluation of the test coverage and effectiveness.
🧪 Relevant tests	Yes
🔍 Possible issues	Possible Bug: The error handling in the deployEventPassCollection and deployAPack methods might not cover all failure scenarios, potentially leading to unhandled exceptions.
	Performance Concern: The getSelectedNftsFromPack method involves multiple asynchronous operations within a loop, which could lead to performance issues if not properly managed.
	Security Concern: The handling of secrets and private keys in the deployment process must be reviewed to ensure that sensitive information is not exposed or mishandled.
🔒 Security concerns	Sensitive information exposure: The PR involves operations related to smart contract deployment and interaction with blockchain networks. It is crucial to ensure that private keys, secrets, and other sensitive information are securely handled and not exposed in logs, error messages, or through improper API responses.

---

✨ Review tool usage guide:

---

Overview:
The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

• When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:

/review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...

• With a configuration file, use the following template:

[pr_reviewer]
some_config1=...
some_config2=...

See the review usage page for a comprehensive guide on using this tool.

[codiumai-pr-agent-free]

PR Code Suggestions

Category	Suggestions
Enhancement	Improve error message clarity in the CollectionDeploymentError constructor. Consider using a more descriptive error message that includes the type of event pass when deployment fails. This will help in debugging and understanding the context of the error more clearly. libs/nft/thirdweb-organizer-event-pass/src/lib/index.ts [51] -super(`Error deploying an event pass: ${error.message}`); +super(`Error deploying an event pass of type ${type}: ${error.message}`);
	Add detailed error logging in the catch block of deployEventPassCollection. For better error handling, consider logging the error details when catching exceptions in the deployEventPassCollection method. This can aid in debugging by providing more context about the failure. libs/nft/thirdweb-organizer-event-pass/src/lib/index.ts [110] -} else console.error(error); +} else { + console.error('Failed to deploy event pass collection:', error); +}
	Use a string enum for the currency field in passPricing for better readability. For the currency field in passPricing, consider using a string enum instead of Types.Currency_Enum. This would make the code more self-contained and readable, especially if the currency codes follow standard ISO currency codes. libs/gql/user/types/src/generated/index.ts [148] -passPricing?: { __typename?: 'passPricing', amount: number, currency: Types.Currency_Enum } | null +passPricing?: { __typename?: 'passPricing', amount: number, currency: 'USD' | 'EUR' | 'GBP' } | null
Best practice	Use template literals for safer string conversion of chainId. To ensure the chainId is always a string as expected by your application logic, consider using template literals or the String() function for conversion instead of calling toString() directly on chainIdNumber. This approach is more robust and can handle null or undefined values without throwing an error. libs/nft/thirdweb-organizer-event-pass/src/lib/index.ts [71] -const chainId = chainIdNumber.toString(); +const chainId = `${chainIdNumber}`;
	Use a custom error class for missing eventPassDelayedRevealed. Instead of using a generic Error for missing eventPassDelayedRevealed, define a custom error class that extends Error. This allows for more specific error handling and makes the codebase more consistent with other custom errors like CollectionDeploymentError. libs/nft/thirdweb-organizer-event-pass/src/lib/index.ts [354] -throw new Error('Missing eventPassDelayedRevealed'); +throw new MissingEventPassDelayedRevealedError();
	Validate encryptedIntegritySecret before decryption to prevent errors. Consider validating the encryptedIntegritySecret before attempting to decrypt it. This validation can prevent potential errors or unexpected behavior when the secret is undefined or not in the expected format. libs/integrations/external-api-handlers/src/lib/shopify.spec.ts [57] +if (!encryptedIntegritySecret) { + throw new Error('encryptedIntegritySecret is required'); +} (decryptSecret as jest.Mock).mockReturnValue('decryptedSecret');
	Use a date library for robust and readable time comparisons. To ensure the robustness of your timestamp validation, consider using a library like moment.js or date-fns to handle time comparisons. This can make the code more readable and prevent potential bugs related to time calculations. libs/integrations/external-api-handlers/src/lib/shopify.spec.ts [38] -timestamp: (Date.now() - 10).toString(), // recent timestamp +timestamp: moment().subtract(10, 'seconds').toString(), // Using moment.js for time manipulation
	Add detailed error logging for asynchronous operations to aid in debugging. When handling errors, especially in asynchronous operations like mintLoyaltyCardWithPassword, it's important to provide detailed error logging. This can help in diagnosing issues faster and more accurately in production environments. libs/integrations/external-api-handlers/src/lib/shopify.spec.ts [297-298] mockLoyaltyCardSdk.mintWithPassword.mockRejectedValue( new Error('Unexpected error'), -); +).catch(error => console.error('Error minting loyalty card:', error));
	Replace any type with a more specific type for tokenId in EventPassNftFieldsFragment. Consider explicitly typing the tokenId field in EventPassNftFieldsFragment and other related fragments to improve type safety and clarity. Using any can lead to potential bugs and makes the code harder to understand. If the token ID is a numeric value, you might use number or string if it's alphanumeric. libs/gql/user/types/src/generated/index.ts [150] -export type EventPassNftFieldsFragment = { __typename?: 'eventPassNft', tokenId?: any | null, eventId: string, eventPassId: string, organizerId: string, isRevealed: boolean, currentOwnerAddress?: string | null }; +export type EventPassNftFieldsFragment = { __typename?: 'eventPassNft', tokenId?: string | null, eventId: string, eventPassId: string, organizerId: string, isRevealed: boolean, currentOwnerAddress?: string | null };
	Use specific types for dateStart and dateEnd fields instead of any. For the dateStart and dateEnd fields within eventDateLocation, consider using a more specific type than any. If these fields represent dates, using Date or a string type formatted as a date would enhance type safety and code readability. libs/gql/user/types/src/generated/index.ts [148] -eventDateLocation?: { __typename?: 'EventDateLocation', dateStart: any, dateEnd: any, locationAddress: { __typename?: 'LocationAddress', city: string, country: string, placeId?: string | null, postalCode: string, state?: string | null, street?: string | null, venue?: string | null, coordinates: { __typename?: 'Location', latitude: number, longitude: number } } } | null +eventDateLocation?: { __typename?: 'EventDateLocation', dateStart: string, dateEnd: string, locationAddress: { __typename?: 'LocationAddress', city: string, country: string, placeId?: string | null, postalCode: string, state?: string | null, street?: string | null, venue?: string | null, coordinates: { __typename?: 'Location', latitude: number, longitude: number } } } | null
	Ensure consistent handling of the isRevealed field across the application. The isRevealed field is present in multiple fragments with a boolean type. Ensure that the default value or handling of this field is consistent across the application, especially in cases where the data might be undefined or null. libs/gql/user/types/src/generated/index.ts [150] -isRevealed: boolean +isRevealed: boolean // Ensure consistent handling of this field across the application
	Add assertions for mock function calls to ensure they are used correctly. To ensure the robustness of your tests, consider adding assertions for the mock functions' calls and parameters. This ensures that your mocks are being used as expected within your functions. libs/nft/thirdweb-organizer-event-pass/src/lib/index.integration.test.ts [17-24] +const mockCreateWebhooksForEvent = jest.fn().mockResolvedValue({ + activityWebhook: { + id: 'mocked_webhook_id', + signingKey: 'mocked_signing_key', + }, +}); jest.mock('@features/back-office/events-api', () => ({ ...jest.requireActual('@features/back-office/events-api'), - createWebhooksForEvent: jest.fn().mockResolvedValue({ - activityWebhook: { - id: 'mocked_webhook_id', - signingKey: 'mocked_signing_key', - }, - }), + createWebhooksForEvent: mockCreateWebhooksForEvent, })); +// Later in your tests +expect(mockCreateWebhooksForEvent).toHaveBeenCalledWith(expectedArguments);
	Use specific types instead of any for better type safety. Instead of using any for the type of mockContract, consider defining a more specific interface or type. This will improve type safety and make the code more maintainable by providing clearer expectations of the object's structure. libs/nft/thirdweb-organizer-event-pass/src/lib/index.integration.test.ts [373] -let mockContract: any; +interface MockContract { + // Define the expected properties and methods here +} +let mockContract: MockContract;
	Add custom error messages to test assertions for clearer debugging. For better error handling and debugging, consider adding custom error messages to the expect().toThrow() assertions in your tests. This will help identify which test case fails more quickly if the expected error is not thrown. libs/nft/thirdweb-organizer-event-pass/src/lib/index.integration.test.ts [297-313] await expect( eventPassCollection.deployEventPassCollection( event.eventPasses[0], { eventId: event.id, organizerId: eventPassOrganizerId, contractType: EventPassNftContractType_Enum.Normal, }, ), ).rejects.toThrow( new CollectionDeploymentError( new Error( 'Error deploying a normal collection : Error deploying a drop contract : Error in deployBuiltInContract', ), ), -); +).withErrorMessage('Expected CollectionDeploymentError was not thrown for normal collection deployment');
Maintainability	Extract external URL generation into a separate function. To improve code maintainability and readability, consider extracting the logic for generating external_url into a separate function. This will make it easier to modify the URL structure in the future and keeps the code DRY. libs/nft/thirdweb-organizer-event-pass/src/lib/index.ts [125] -external_url: `${EVENT_PASS_BASE_URL}${organizerId}/event/${eventId}/eventPass/${eventPassId}/${i}`, +external_url: generateExternalUrl(organizerId, eventId, eventPassId, i),
	Use dependency injection for the LoyaltyCardNftWrapper to improve test flexibility. Instead of directly mocking LoyaltyCardNftWrapper with a jest mock, consider using dependency injection to pass a mock or a real instance. This approach enhances the test's flexibility and maintainability by decoupling the test setup from the implementation details. libs/integrations/external-api-handlers/src/lib/shopify.spec.ts [12-16] -jest.mock('@nft/loyalty-card', () => ({ - LoyaltyCardNftWrapper: jest.fn().mockImplementation(() => ({ - mintWithPassword: jest.fn(), - })), -})); +// In the constructor of ShopifyWebhookAndApiHandler or as a method parameter +constructor(private loyaltyCardNftWrapper: LoyaltyCardNftWrapper = new LoyaltyCardNftWrapper()) {} +// In tests +const mockLoyaltyCardNftWrapper = { mintWithPassword: jest.fn() }; +const handler = new ShopifyWebhookAndApiHandler(mockLoyaltyCardNftWrapper);
	Abstract common setup logic into a separate function or beforeEach block for better readability. To improve the readability and maintainability of the test cases, consider abstracting the common setup logic into a separate function or before each block. This includes mocking return values and creating common objects like mockRequest. libs/integrations/external-api-handlers/src/lib/shopify.spec.ts [50-55] -(getSecretApiKey as jest.Mock).mockResolvedValue({ - type: ApiKeyType_Enum.Shopify, - encryptedIntegritySecret: 'encryptedSecret', - allowlist: 'https://example.myshopify.com', - organizerId: 'org123', +beforeEach(() => { + (getSecretApiKey as jest.Mock).mockResolvedValue({ + type: ApiKeyType_Enum.Shopify, + encryptedIntegritySecret: 'encryptedSecret', + allowlist: 'https://example.myshopify.com', + organizerId: 'org123', + }); + // Other common setup logic });
	Rename the passPricing type to PassPricing for consistency. The passPricing field's type is passPricing, which seems inconsistent with the naming convention used for other types (e.g., EventPass, Asset). Consider renaming passPricing to PassPricing to maintain consistency and improve readability. libs/gql/user/types/src/generated/index.ts [148] -passPricing?: { __typename?: 'passPricing', amount: number, currency: Types.Currency_Enum } | null +passPricing?: { __typename?: 'PassPricing', amount: number, currency: Types.Currency_Enum } | null
	Improve variable naming for better readability. Consider using a more descriptive variable name instead of mockContract to improve code readability. For instance, if this mock contract is simulating a specific type of contract, naming it accordingly can make the tests easier to understand. libs/nft/thirdweb-organizer-event-pass/src/lib/index.integration.test.ts [373] -let mockContract: any; +let mockEventPassContract: any;
	Extract mock setup to a separate function for clarity. To improve the test's clarity and maintainability, consider extracting the setup of mocked values and instances (like mockedThirdwebSDKInstance and mockContract) into a separate function or before each block. This helps in reusing setup code and keeping individual tests focused and concise. libs/nft/thirdweb-organizer-event-pass/src/lib/index.integration.test.ts [372-373] let mockedThirdwebSDKInstance: ThirdwebSDK; let mockContract: any; +beforeEach(() => { + // Setup mockedThirdwebSDKInstance and mockContract here +});

---

✨ Improve tool usage guide:

---

Overview:
The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.

• When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:

/improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...

• With a configuration file, use the following template:

[pr_code_suggestions]
some_config1=...
some_config2=...

See the improve usage page for a comprehensive guide on using this tool.