[PLA-2033] Changes for v2.0.0 on marketplace

[leonardocustodio]

PR Type

Enhancement, Tests

---

Description

• Introduced a new listing type OFFER in the ListingType enum and updated related logic.
• Refactored CreateListingMutation to use listingData instead of auctionData, supporting multiple listing types.
• Added new GraphQL input types ListingDataInputType and OfferParamsInputType to handle diverse listing data.
• Updated database schema with new fields offer_expiration and counter_offer_count for offers.
• Enhanced tests to cover new listing types and parameters.

---

Changes walkthrough 📝

	Relevant files
Enhancement	13 files MarketplaceListingFactory.php Update factory for new listing types and fields                    database/factories/MarketplaceListingFactory.php Renamed start_block and end_block to auction_start_block and auction_end_block. Added new fields offer_expiration and counter_offer_count. +4/-2      2024_10_11_143012_new_listing_type_to_marketplace_listings_table.php Add migration for new listing types and fields                      database/migrations/2024_10_11_143012_new_listing_type_to_marketplace_listings_table.php Added migration to rename columns and add new fields for offers. Introduced offer_expiration and counter_offer_count columns. +32/-0    ListingType.php Add OFFER case to ListingType enum                                              src/Enums/ListingType.php Added a new case OFFER to the ListingType enum. +2/-0      CreateListingMutation.php Refactor CreateListingMutation for multiple listing types src/GraphQL/Mutations/CreateListingMutation.php Replaced auctionData with listingData to support multiple listing types. Updated validation rules to accommodate new listing types. +60/-47  ListingDataInputType.php Add ListingDataInputType for diverse listing inputs            src/GraphQL/Types/Input/ListingDataInputType.php Introduced ListingDataInputType for handling various listing data inputs. Defined fields for type, auctionParams, and offerParams. +41/-0    OfferParamsInputType.php Introduce OfferParamsInputType for offer parameters            src/GraphQL/Types/Input/OfferParamsInputType.php Created OfferParamsInputType to handle offer-specific parameters. Defined expiration field for offers. +33/-0    OfferDataType.php Add OfferDataType for GraphQL offer representation              src/GraphQL/Types/OfferDataType.php Added OfferDataType to represent offer data in GraphQL. Included fields for type and expiration. +37/-0    OfferStateType.php Add OfferStateType for managing offer states                          src/GraphQL/Types/OfferStateType.php Introduced OfferStateType for managing offer states in GraphQL. Included counterOfferCount field. +37/-0    ListingDataUnion.php Extend ListingDataUnion with OfferData                                      src/GraphQL/Unions/ListingDataUnion.php Added OfferData to the ListingDataUnion. +2/-0      ListingStateUnion.php Extend ListingStateUnion with OfferState                                  src/GraphQL/Unions/ListingStateUnion.php Added OfferState to the ListingStateUnion. +2/-0      MarketplaceServiceProvider.php Register migration for new listing types                                  src/MarketplaceServiceProvider.php Registered new migration for listing types in the service provider. +1/-0      ListingDataParams.php Add ListingDataParams for encapsulating listing data          src/Models/Substrate/ListingDataParams.php Created ListingDataParams to encapsulate listing data parameters. Supported different listing types with specific parameters. +33/-0    OfferDataParams.php Add OfferDataParams for offer-specific data                            src/Models/Substrate/OfferDataParams.php Introduced OfferDataParams for handling offer-specific data. +21/-0
Tests	2 files CreateListingTest.php Update CreateListing tests for new listing types                  tests/Feature/GraphQL/Mutations/CreateListingTest.php Updated tests to use listingData instead of auctionData. Added tests for new listing types and parameters. +49/-33  CreateListing.graphql Update CreateListing mutation to use listingData                  tests/Feature/GraphQL/Resources/CreateListing.graphql Replaced auctionData with listingData in the mutation. +2/-2

---

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Code Redundancy The CreateListingMutation class contains redundant code in the resolve method where the TransactionSerializer::encode method is called. Consider refactoring to remove duplication and improve maintainability. Hardcoded Logic The factory uses hardcoded conditional logic to determine auction_start_block, auction_end_block, offer_expiration, and counter_offer_count. This could be refactored to a method to improve code clarity and reusability. Missing Validation The ListingDataInputType lacks validation for the nested input types auctionParams and offerParams. Ensure that these inputs are validated to prevent potential issues with malformed data.

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Possible issue	Ensure that the auction block range is logically sequential and non-overlapping Ensure that the range of auction_start_block and auction_end_block does not overlap or contradict, possibly by adding a validation to check that auction_start_block is less than auction_end_block. database/factories/MarketplaceListingFactory.php [38-39] -'auction_start_block' => $state == ListingType::AUCTION->name ? fake()->numberBetween(1, 100) : null, -'auction_end_block' => $state == ListingType::AUCTION->name ? fake()->numberBetween(100, 200) : null, +'auction_start_block' => $state == ListingType::AUCTION->name ? $start = fake()->numberBetween(1, 100) : null, +'auction_end_block' => $state == ListingType::AUCTION->name ? fake()->numberBetween($start + 1, 200) : null, Suggestion importance[1-10]: 8 Why: The suggestion addresses a potential logical error by ensuring that auction_start_block is always less than auction_end_block, which is crucial for maintaining data integrity in auction listings.	8
	Ensure that max_token_count and max_token_supply are consistent and logical Validate the consistency of max_token_count and max_token_supply to ensure they align with business logic, as their values are generated dynamically. tests/Feature/GraphQL/Traits/CreateCollectionData.php [46-47] -'max_token_count' => fake()->numberBetween(1), -'max_token_supply' => (string) fake()->numberBetween(1), +$maxTokenCount = fake()->numberBetween(1); +$maxTokenSupply = (string) $maxTokenCount; // Assuming max supply should not exceed count +'max_token_count' => $maxTokenCount, +'max_token_supply' => $maxTokenSupply, Suggestion importance[1-10]: 6 Why: The suggestion to align max_token_count and max_token_supply is logical and can prevent potential inconsistencies. However, the assumption that max_token_supply should not exceed max_token_count may not always be valid without further context.	6
Possible bug	Validate that endBlock is always greater than startBlock to maintain auction integrity Add validation to ensure that the endBlock is always greater than the startBlock in auctionParams to prevent logical errors in auction timing. tests/Feature/GraphQL/Traits/CreateListingParameters.php [29-30] -'startBlock' => fake()->numberBetween(1011, 5000), -'endBlock' => fake()->numberBetween(5001, 10000), +$startBlock = fake()->numberBetween(1011, 5000); +$endBlock = fake()->numberBetween($startBlock + 1, 10000); // Ensuring endBlock is always greater +'startBlock' => $startBlock, +'endBlock' => $endBlock, Suggestion importance[1-10]: 8 Why: Ensuring that endBlock is always greater than startBlock is crucial for maintaining auction integrity. This suggestion addresses a potential logical error and enhances the reliability of the auction parameters.	8
	Add error handling around dynamic listing creation to manage potential runtime exceptions Ensure that the createListing method handles potential exceptions or errors, especially when dealing with dynamic input for the number of listings. tests/Feature/GraphQL/Queries/GetListingsTest.php [19] -$listings = $this->createListing(fake()->numberBetween(1, 100)); +try { + $listings = $this->createListing(fake()->numberBetween(1, 100)); +} catch (Exception $e) { + // Handle exception +} Suggestion importance[1-10]: 7 Why: Adding error handling around the createListing method is a good practice to manage potential runtime exceptions, especially when dealing with dynamic input. This suggestion improves the robustness of the test.	7
Enhancement	Add validation rules to ensure listingData is always provided and correctly formatted Validate that listingData is provided and correctly structured to prevent runtime errors or malformed data submissions. src/GraphQL/Mutations/CreateListingMutation.php [91-93] 'listingData' => [ 'type' => GraphQL::type('ListingDataInput'), 'description' => __('enjin-platform-marketplace::input_type.listing_data.description'), + 'rules' => ['required', 'array'], ], Suggestion importance[1-10]: 7 Why: Adding validation rules for listingData enhances data integrity and prevents runtime errors by ensuring that the data is always present and correctly structured.	7
	Ensure the expiration field in offer parameters is a valid future timestamp Add validation rules to the expiration field to ensure it represents a future timestamp and adheres to expected ranges. src/GraphQL/Types/Input/OfferParamsInputType.php [27-29] 'expiration' => [ 'type' => GraphQL::type('Int'), 'description' => __('enjin-platform-marketplace::type.auction_data.field.startBlock'), + 'rules' => ['required', 'integer', 'min:' . time()], ], Suggestion importance[1-10]: 6 Why: Adding validation to ensure expiration is a future timestamp helps prevent logical errors and ensures that the offer parameters are meaningful and valid. However, the description for the expiration field seems incorrect, which slightly reduces the score.	6
Best practice	Enhance data integrity by adding validation rules to the auction parameters Include specific field validations within the AuctionParamsInputType to ensure all necessary data is provided and valid. src/GraphQL/Types/Input/AuctionParamsInputType.php [24-25] public function fields(): array +{ + return [ + 'startBlock' => [ + 'type' => GraphQL::type('Int!'), + 'rules' => ['required', 'integer', 'min:1'], + 'description' => __('enjin-platform-marketplace::type.auction_data.field.startBlock'), + ], + 'endBlock' => [ + 'type' => GraphQL::type('Int!'), + 'rules' => ['required', 'integer', 'min:1', 'gt:startBlock'], + 'description' => __('enjin-platform-marketplace::type.auction_data.field.endBlock'), + ], + ]; +} Suggestion importance[1-10]: 6 Why: The suggestion improves data validation by ensuring that startBlock and endBlock are valid integers and that endBlock is greater than startBlock, which is a good practice for maintaining data consistency.	6
	Validate the encoded data format and length for consistency with expected blockchain formats Ensure that the encoded data string length and format match expected blockchain transaction formats, particularly when new fields are added. tests/Unit/EncodingTest.php [43] -"0x{$callIndex}4277010004427701000404041074657374019101a10f00", +// Validate the length and format of the encoded data +$expectedFormat = "0x{$callIndex}4277010004427701000404041074657374019101a10f00"; +$this->assertEquals($expectedFormat, $data); Suggestion importance[1-10]: 5 Why: While validating the encoded data format is a good practice, the suggestion is somewhat redundant as the existing code already asserts the expected format. The improvement is marginal in this context.	5

[v16Studios]

Suggested change

	use Enjin\Platform\Marketplace\Models\Laravel\MarketplaceListing;
	use Enjin\Platform\Marketplace\Models\MarketplaceListing;

Can we use the Model Resolver version here?

[leonardocustodio]

Doesn't work, will give a type error. Would have to use just Model but that takes type hint away

[v16Studios]

That's fine, we use Model elsewhere for this reason too so should try to keep it consistent 👍🏻

stale