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

Construct table summary data only when transactions and deposits summary data has been loaded. #1788

Merged
merged 4 commits into from
May 20, 2021

Conversation

naman03malhotra
Copy link
Contributor

@naman03malhotra naman03malhotra commented May 12, 2021

Fixes #1639

Presently we render the number of transactions and deposits in the summary section of the respective tables. While the data is being fetched from the server the count is rendered as undefined.

isSummaryLoading is set to false initially which leads to construction of an array with empty/falsy values.

Changes proposed in this Pull Request

Testing instructions

  • switch to fix/1639-transaction-deposit-count
  • goto deposits page, the table summary should appear after the data has been loaded.
  • goto transactions page, the table summary should appear after the data has been loaded.

  • Added changelog entry (or does not apply)
  • Covered with tests (or have a good reason not to test in description ☝️)
  • Tested on mobile (or does not apply)

@naman03malhotra naman03malhotra self-assigned this May 12, 2021
@naman03malhotra naman03malhotra force-pushed the fix/1639-transaction-deposit-count branch from 1a94f93 to 6c152b6 Compare May 13, 2021 14:53
@naman03malhotra naman03malhotra changed the title Pass summary data as falsy if transactions and deposits summary data is yet to be loaded. Construct summary data only when transaction and deposit summary data has been loaded. May 13, 2021
@naman03malhotra naman03malhotra changed the title Construct summary data only when transaction and deposit summary data has been loaded. Construct table summary data only when transactions and deposits summary data has been loaded. May 13, 2021
@naman03malhotra naman03malhotra force-pushed the fix/1639-transaction-deposit-count branch from 301c711 to 1d6fd97 Compare May 13, 2021 15:04
@naman03malhotra naman03malhotra requested a review from kalessil May 13, 2021 15:05
@naman03malhotra naman03malhotra force-pushed the fix/1639-transaction-deposit-count branch 2 times, most recently from e5defe7 to fd9c53c Compare May 17, 2021 19:56
@naman03malhotra naman03malhotra marked this pull request as ready for review May 17, 2021 20:02
@naman03malhotra naman03malhotra requested a review from a team May 17, 2021 20:08
Copy link
Contributor

@kalessil kalessil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a few comments, which are rather for better defining the code intention.

@naman03malhotra naman03malhotra force-pushed the fix/1639-transaction-deposit-count branch from 05dba30 to 4836e89 Compare May 19, 2021 08:53
@naman03malhotra naman03malhotra force-pushed the fix/1639-transaction-deposit-count branch 2 times, most recently from 5078ef8 to fb260af Compare May 19, 2021 15:56
Copy link
Contributor

@kalessil kalessil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, :shipit: !

- if transaction summary and depost summary data is loaded then only contruct summary array
- added tests for the table summary changes
- for both desposts and transactions list
- snapshot update
- restored use of isSummaryLoading for both deposits and transations page
- added comment for better clarification for initializing summary as undefined
@naman03malhotra naman03malhotra force-pushed the fix/1639-transaction-deposit-count branch from fb260af to 518797e Compare May 20, 2021 08:18
@naman03malhotra naman03malhotra merged commit 85cf06f into develop May 20, 2021
@naman03malhotra naman03malhotra deleted the fix/1639-transaction-deposit-count branch May 20, 2021 08:45
ismaeldcom pushed a commit that referenced this pull request May 27, 2021
… summary data has been loaded. (#1788)

- if transaction summary and deposit summary data is loaded then only construct the summary array.
- added tests for the table summary changes for both deposits and transactions list.
- snapshot update.
- added changelog entry.
- added a comment for better clarification for initializing summary as undefined.
ismaeldcom pushed a commit that referenced this pull request May 27, 2021
… summary data has been loaded. (#1788)

- if transaction summary and deposit summary data is loaded then only construct the summary array.
- added tests for the table summary changes for both deposits and transactions list.
- snapshot update.
- added changelog entry.
- added a comment for better clarification for initializing summary as undefined.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Transactions and deposits counts on the table summary are rendered as "undefined" while the data is loading.
2 participants