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-1975] Add custom queue #71

Merged
merged 2 commits into from
Nov 17, 2024
Merged

Conversation

enjinabner
Copy link
Contributor

@enjinabner enjinabner commented Nov 5, 2024

PR Type

enhancement


Description

  • Added a new configuration option for setting a dedicated queue in config/enjin-platform-fuel-tanks.php.
  • Introduced HasCustomQueue trait to manage custom queue settings across multiple event classes.
  • Applied HasCustomQueue trait to various event classes to utilize the custom queue configuration.

Changes walkthrough 📝

Relevant files
Configuration changes
1 files
enjin-platform-fuel-tanks.php
Add configuration for custom queue                                             

config/enjin-platform-fuel-tanks.php

  • Added configuration for a dedicated queue.
  • Introduced a new configuration key queue.
  • +9/-0     
    Enhancement
    10 files
    AccountAdded.php
    Use custom queue for AccountAdded event                                   

    src/Events/Substrate/FuelTanks/AccountAdded.php

    • Added HasCustomQueue trait to use custom queue.
    +3/-0     
    AccountRemoved.php
    Use custom queue for AccountRemoved event                               

    src/Events/Substrate/FuelTanks/AccountRemoved.php

    • Added HasCustomQueue trait to use custom queue.
    +3/-0     
    CallDispatched.php
    Use custom queue for CallDispatched event                               

    src/Events/Substrate/FuelTanks/CallDispatched.php

    • Added HasCustomQueue trait to use custom queue.
    +3/-0     
    FreezeStateMutated.php
    Use custom queue for FreezeStateMutated event                       

    src/Events/Substrate/FuelTanks/FreezeStateMutated.php

    • Added HasCustomQueue trait to use custom queue.
    +3/-0     
    FuelTankCreated.php
    Use custom queue for FuelTankCreated event                             

    src/Events/Substrate/FuelTanks/FuelTankCreated.php

    • Added HasCustomQueue trait to use custom queue.
    +3/-0     
    FuelTankDestroyed.php
    Use custom queue for FuelTankDestroyed event                         

    src/Events/Substrate/FuelTanks/FuelTankDestroyed.php

    • Added HasCustomQueue trait to use custom queue.
    +3/-0     
    FuelTankMutated.php
    Use custom queue for FuelTankMutated event                             

    src/Events/Substrate/FuelTanks/FuelTankMutated.php

    • Added HasCustomQueue trait to use custom queue.
    +3/-0     
    RuleSetInserted.php
    Use custom queue for RuleSetInserted event                             

    src/Events/Substrate/FuelTanks/RuleSetInserted.php

    • Added HasCustomQueue trait to use custom queue.
    +3/-0     
    RuleSetRemoved.php
    Use custom queue for RuleSetRemoved event                               

    src/Events/Substrate/FuelTanks/RuleSetRemoved.php

    • Added HasCustomQueue trait to use custom queue.
    +3/-0     
    HasCustomQueue.php
    Introduce HasCustomQueue trait for custom queue management

    src/Traits/HasCustomQueue.php

  • Introduced HasCustomQueue trait.
  • Provides method to set custom queue.
  • +11/-0   

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

    Copy link

    github-actions bot commented Nov 5, 2024

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

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

    Missing Documentation
    The HasCustomQueue trait lacks comprehensive documentation on its methods and usage which could lead to confusion or incorrect implementation in future uses.

    Copy link

    github-actions bot commented Nov 5, 2024

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Ensure the environment variable is defined to prevent runtime issues

    Ensure that the environment variable 'PLATFORM_FUEL_TANKS_QUEUE' is properly set up
    in your environment configurations to avoid runtime errors.

    config/enjin-platform-fuel-tanks.php [12]

    -'queue' => env('PLATFORM_FUEL_TANKS_QUEUE', 'default'),
    +'queue' => env('PLATFORM_FUEL_TANKS_QUEUE', 'default'),  // Ensure this environment variable is defined
    Suggestion importance[1-10]: 1

    Why: The suggestion to ensure the environment variable is defined is valid but does not directly impact the functionality or correctness of the code as the default value 'default' is already provided.

    1
    Check for conflicts with the HasCustomQueue trait in event classes

    Ensure that the HasCustomQueue trait does not conflict with any existing methods or
    properties in the PlatformBroadcastEvent class or its subclasses.

    src/Events/Substrate/FuelTanks/AccountAdded.php [14]

    -use HasCustomQueue;
    +use HasCustomQueue;  // Check for method/property conflicts in PlatformBroadcastEvent
    Suggestion importance[1-10]: 1

    Why: The suggestion to check for method or property conflicts is a general good practice but does not address a specific problem in the PR, as no evidence of such conflicts is provided.

    1
    Possible bug
    Check for the availability and correct implementation of config and onQueue

    Verify that the config function and the onQueue method are available and correctly
    implemented in the application context where the trait HasCustomQueue is used.

    src/Traits/HasCustomQueue.php [9]

    -$this->onQueue(config('enjin-platform-fuel-tanks.queue'));
    +$this->onQueue(config('enjin-platform-fuel-tanks.queue'));  // Verify availability of config and onQueue
    Suggestion importance[1-10]: 1

    Why: The suggestion to verify the availability of config and onQueue methods is a precautionary check and does not directly address any specific issue or improvement in the code.

    1
    Best practice
    Ensure parameters are correctly managed with the HasCustomQueue trait

    Confirm that all necessary parameters ($event, $transaction, $extra, $fuelTank) are
    handled correctly within the constructor when using the HasCustomQueue trait.

    src/Events/Substrate/FuelTanks/FuelTankCreated.php [19]

    -public function __construct(FuelTankCreatedPolkadart $event, ?Model $transaction = null, ?array $extra = null, ?Model $fuelTank = null)
    +public function __construct(FuelTankCreatedPolkadart $event, ?Model $transaction = null, ?array $extra = null, ?Model $fuelTank = null)  // Confirm parameter handling with HasCustomQueue
    Suggestion importance[1-10]: 1

    Why: The suggestion to confirm parameter handling is a general best practice but does not provide a specific improvement or correction to the existing implementation in the PR.

    1

    @enjinabner enjinabner marked this pull request as ready for review November 6, 2024 06:55
    @enjinabner
    Copy link
    Contributor Author

    This requires this PR to pass tests enjin/platform-core#285

    @enjinabner enjinabner merged commit aac45c3 into master Nov 17, 2024
    7 checks passed
    @enjinabner enjinabner deleted the feature/pla-1975/add-custom-queue branch November 17, 2024 23:56
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Development

    Successfully merging this pull request may close these issues.

    2 participants