Skip to content
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

[PLA-2041] Add ListingRemovedUnderMinimum event #68

Merged
merged 1 commit into from
Oct 16, 2024
Merged

Conversation

leonardocustodio
Copy link
Member

@leonardocustodio leonardocustodio commented Oct 15, 2024

PR Type

Enhancement


Description

  • Added a new event type LISTING_REMOVED_UNDER_MINIMUM to the MarketplaceEventType enum.
  • Implemented a new event class ListingRemovedUnderMinimum with broadcasting capabilities.
  • Created a processor class for handling ListingRemovedUnderMinimum events, including methods for running, logging, and broadcasting.
  • Corrected channel identifiers in the ListingCancelled event for consistency.

Changes walkthrough 📝

Relevant files
Enhancement
MarketplaceEventType.php
Add new event type for listing removal under minimum         

src/Enums/Substrate/MarketplaceEventType.php

  • Added a new event type LISTING_REMOVED_UNDER_MINIMUM.
  • Included the processor for the new event type.
  • +3/-0     
    ListingRemovedUnderMinimum.php
    Implement ListingRemovedUnderMinimum event class                 

    src/Events/Substrate/Marketplace/ListingRemovedUnderMinimum.php

  • Introduced a new event class for ListingRemovedUnderMinimum.
  • Set up broadcasting channels for the new event.
  • +32/-0   
    ListingRemovedUnderMinimum.php
    Add processor for ListingRemovedUnderMinimum event             

    src/Services/Processor/Substrate/Events/Implementations/Marketplace/ListingRemovedUnderMinimum.php

  • Created a processor class for handling ListingRemovedUnderMinimum
    events.
  • Implemented methods to run, log, and broadcast the event.
  • +74/-0   
    Bug fix
    ListingCancelled.php
    Fix channel identifiers in ListingCancelled event               

    src/Events/Substrate/Marketplace/ListingCancelled.php

  • Corrected channel identifiers to use consistent naming conventions.
  • +2/-2     

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

    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Missing Import
    The new event class ListingRemovedUnderMinimum is used but not imported in the enum file. This might cause a runtime error if the namespace resolution fails.

    Error Handling
    The error handling in the run method logs an error but does not halt execution or handle the error in a way that prevents further potentially invalid operations.

    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible bug
    Add null checks to prevent runtime exceptions when the seller wallet is not found

    Add error handling for potential null values when finding the seller wallet to
    prevent runtime exceptions.

    src/Services/Processor/Substrate/Events/Implementations/Marketplace/ListingRemovedUnderMinimum.php [29]

     $seller = Wallet::find($listing->seller_wallet_id);
    +if (!$seller) {
    +    throw new \Exception("Seller wallet not found.");
    +}
    Suggestion importance[1-10]: 9

    Why: This suggestion adds a necessary null check to handle cases where the seller wallet might not be found, preventing potential runtime exceptions and improving the robustness of the code.

    9

    @leonardocustodio leonardocustodio merged commit 9a9033f into master Oct 16, 2024
    5 of 7 checks passed
    @leonardocustodio leonardocustodio deleted the PLA-2041 branch October 16, 2024 12:24
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    Development

    Successfully merging this pull request may close these issues.

    3 participants