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-1707] update transaction status #91

Merged
merged 3 commits into from
Apr 15, 2024

Conversation

zlayine
Copy link
Contributor

@zlayine zlayine commented Apr 12, 2024

Type

enhancement, bug_fix


Description

  • Introduced enhancements to UI alignment and clarity in button texts across ConfirmModal and SettingsResetPassword components.
  • Significantly enhanced the Transactions page by adding dropdown menus for transaction actions and modals for retrying and canceling transactions, including the necessary API integration and feedback mechanisms.

Changes walkthrough

Relevant files
Enhancement
ConfirmModal.vue
Improved Button Alignment in Confirm Modal                             

resources/js/components/ConfirmModal.vue

  • Adjusted button alignment to justify-end for better UI consistency.
  • +1/-1     
    SettingsResetPassword.vue
    Clarified Password Reset Button Text                                         

    resources/js/components/pages/SettingsResetPassword.vue

  • Changed button text from "Reset password" to "Change password" for
    clarity.
  • +2/-2     
    Transactions.vue
    Added Transaction Management Features                                       

    resources/js/components/pages/Transactions.vue

  • Added items-center class to align items vertically in the center.
  • Introduced DropdownMenu component for transaction actions.
  • Implemented ConfirmModal for retry and cancel transaction actions.
  • Added functionality to retry and cancel transactions, including API
    calls and success/error handling.
  • +116/-3 

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

    @zlayine zlayine self-assigned this Apr 12, 2024
    @github-actions github-actions bot added enhancement New feature or request bug_fix labels Apr 12, 2024
    Copy link

    PR Description updated to latest commit (259622a)

    Copy link

    PR Review

    ⏱️ Estimated effort to review [1-5]

    3, because the PR involves UI changes, API integration, and state management which require careful review to ensure functionality and consistency across components. The addition of new components and modification of existing ones necessitate a thorough understanding of the Vue.js framework and the project's architecture.

    🧪 Relevant tests

    No

    🔍 Possible issues

    Possible Bug: The retry and cancel transaction functions do not check for the transaction's current state before performing actions. This could lead to unintended behavior if a transaction's state changes between the user action and the API call.

    Performance Concern: The openModalSlide function could be optimized to avoid repetitive checks for modal types (retry and cancel). This could improve the function's efficiency and readability.

    🔒 Security concerns

    No

    Code feedback:
    relevant fileresources/js/components/pages/Transactions.vue
    suggestion      

    Consider adding state checks in retryTransaction and cancelTransaction functions to ensure that only transactions in appropriate states are retried or canceled. This prevents unnecessary API calls and potential errors. [important]

    relevant lineconst retryTransaction = async () => {

    relevant fileresources/js/components/pages/Transactions.vue
    suggestion      

    Refactor the openModalSlide function to reduce redundancy and improve code readability by handling modal type checks more efficiently. For example, use a switch-case or a mapping object to map component names to actions. [medium]

    relevant lineconst openModalSlide = (componentName: string, transaction) => {


    ✨ 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=...
    
    [pr_reviewer]
    some_config1=...
    some_config2=...
    

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

    Copy link

    PR Code Suggestions

    CategorySuggestions                                                                                                                                                       
    Maintainability
    Improve variable naming for clarity.

    Consider using a more descriptive variable name for res in both retryTransaction and
    cancelTransaction functions to improve code readability. For example, response could be a
    better name that makes the code clearer.

    resources/js/components/pages/Transactions.vue [445-477]

    -const res = await TransactionApi.retryTransactions(
    +const response = await TransactionApi.retryTransactions(
     ...
    -const res = await TransactionApi.updateTransaction(
    +const response = await TransactionApi.updateTransaction(
     
    Reduce code duplication by extracting error handling.

    Extract the error handling logic in retryTransaction and cancelTransaction into a separate
    function to reduce code duplication and improve maintainability.

    resources/js/components/pages/Transactions.vue [460-495]

    -if (snackbarErrors(e)) return;
    -snackbar.error({
    +handleError(e, 'Retry transactions');
     ...
    -if (snackbarErrors(e)) return;
    -snackbar.error({
    +handleError(e, 'Cancel transactions');
     
    Enhancement
    Improve string handling with template literals.

    Use template literals for dynamic strings in snackbar.success and snackbar.error calls to
    make the code more readable and maintainable.

    resources/js/components/pages/Transactions.vue [454-495]

    -title: `Retry transactions`,
    +title: `${action} transactions`,
     ...
    -title: `Cancel transactions`,
    +title: `${action} transactions`,
     
    Best practice
    Use constants for action keys to avoid magic strings.

    For the actions array, consider defining an enum or constant object for action keys
    ('details', 'retry', 'cancel') to avoid magic strings and facilitate changes or additions
    in the future.

    resources/js/components/pages/Transactions.vue [281-297]

    +const ActionKeys = {
    +    DETAILS: 'details',
    +    RETRY: 'retry',
    +    CANCEL: 'cancel',
    +};
     const actions = [
         {
    -        key: 'details',
    +        key: ActionKeys.DETAILS,
             name: 'Details',
             component: 'DetailsTransactionSlideover',
         },
         {
    -        key: 'retry',
    +        key: ActionKeys.RETRY,
             name: 'Retry',
             component: 'RetryModal',
         },
         {
    -        key: 'cancel',
    +        key: ActionKeys.CANCEL,
             name: 'Cancel',
             component: 'CancelModal',
         },
     ];
     
    Ensure isLoading is always reset by using a finally block.

    Instead of manually toggling isLoading in retryTransaction and cancelTransaction, use a
    finally block to ensure it's always reset, reducing the risk of leaving the UI in a
    loading state accidentally.

    resources/js/components/pages/Transactions.vue [466-467]

    -isLoading.value = false;
    -...
    -isLoading.value = false;
    +try {
    +    ...
    +} catch (e) {
    +    ...
    +} finally {
    +    isLoading.value = false;
    +}
     

    ✨ 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=...
    
    [pr_code_suggestions]
    some_config1=...
    some_config2=...
    

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

    @zlayine zlayine merged commit 04743b0 into master Apr 15, 2024
    3 checks passed
    @zlayine zlayine deleted the feature/pla-1707/update-transaction-status branch April 15, 2024 07:02
    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