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

Fetch all transactions on member transactions page #142

Closed
wants to merge 7 commits into from

Conversation

amenconi
Copy link
Contributor

Enhancements included in this PR:

  • Added "getAllTransactions" function to Soonaverse Member API that will implement pagination to retrieve all transactions for a memberId from Build.5 API.
  • Member profile transactions page now will populate with full history of member transactions.
  • Member profile transactions page will also populate the "Export CSV" with all member transactions.

I've kept the legacy UX in place where desktop device mode will populate a paginated table with 50 transactions per page and mobile will implement infinite scroll to display transactions, they should now just have all transactions for the member listed.

Thank you.

…tions. Modified transaction page to use new function when exporting transaction to CSV/Excel.
…s and maintains desktop paginated table and mobile infitite scroll UX designs.
@amenconi amenconi added the enhancement New feature or request label Mar 14, 2024
@amenconi amenconi self-assigned this Mar 14, 2024
@amenconi amenconi linked an issue Mar 14, 2024 that may be closed by this pull request
Copy link

cloudflare-workers-and-pages bot commented Mar 14, 2024

Deploying prod-soonaverse-pr-previews with  Cloudflare Pages  Cloudflare Pages

Latest commit: 2099057
Status: ✅  Deploy successful!
Preview URL: https://8e5ed5f4.app-cqo.pages.dev
Branch Preview URL: https://amenconi-get-all-member-tran.app-cqo.pages.dev

View logs

@emmap3-do
Copy link
Contributor

@amenconi I confirm it is working, all the data is there now!
Although I think this should be improved, either in this sprint or in a future release.

My thoughts:

When you first visit the Transactions page, it says "No data" and it takes a lot of time to load (and I'm not a whale, I just have 43 pages of records).

Options to improve this could be: add a spinner gif or something indicating page is loading. Or, better yet, add a filter to this page where the user can filter by periods of time.

It can be a date range filter (date from - date to) or a drop down list with fixed time frames, like: Last 6 months, Last year, All time transactions. I think a date from - to would be best as it is more versatile to the user needs.

Also, having an improvement like that would reduce API/database hits, as the default filter range will bring fewer results, only increased manually by those who really need it.

@amenconi
Copy link
Contributor Author

amenconi commented Mar 14, 2024

Totally makes sense, shouldn't be too hard to implement.

I generated a couple hundred transactions on my testnet account in debug and it all loaded almost instantly but didn't think about how many transactions some power users may have racked up over the years and no reason to hit the API that many times every time they pull that page up.

Think it makes sense to keep the CSV export to all transactions or worth putting a filter for that as well? My gut says it should be OK to just export all each time but honestly I'm not sure how this is being used by the users mostly so that might be unnecessary load if most users would opt to only export a subset as well.

Thanks.

@emmap3-do
Copy link
Contributor

Hm, I think it would make sense if the exported CSV file contains the exact same data that the user filtered to see in the screen.

@amenconi
Copy link
Contributor Author

Ok latest push contains:

  • Ensure DataService has had time to populate member$ that on initial page load it doesn't try to fetch transactions with an undefined memberId input.
  • Initial page load fetches 100 transactions by default.
  • Desktop view has radio select for options: 100, 200, 500, 1000, ALL. Selecting any of these will fetch that number of transactions order by date starting with the most recent. This selection will also apply to export to CSV function.

I explored using a date range to filter transactions but on initial glance it didn't seem that Build.5 API supported date inputs as filters to fetch a subset of transactions. If this is a highly sought after change versus the quantity of transactions select filter currently implemented, we can fetch all transactions and filter from there to make it work but didn't want to go that route since it would be less efficient and a bit more complex to build out but can definitely swap or add that if needed.

Thank you.

@adamunchained
Copy link
Contributor

Great work guys,

In my case I've over 6000+ transactions so the full list can take some time. I personally think we should remove the pagination and just do the infinite scroll on the desktop with this approach. It'll make more sense. I think it's little confusing to have the radio buttons at the top as well as paging. People might paginate to the end and they not necessary realize there might be more transactions. This can actually might happen even with infinite scroll.

I would maybe rephrase this and say something along the lines of "Select number of recent transactions to view/export:"

Just curious, there wasn't an option to expand the current pagination so when they get to the end you'll try to retrieve more. For export, it'll always do ALL. If not with pagination, I'm sure the infinite scroll supports server call to get more content right? I feel this approach would be more intuitive.

Lastly, could you try to use same loading that we use on other pages? You should be able to use ng-zorro skeleton: https://ng.ant.design/components/skeleton/en#ng-content

see example:

Untitled.mov

@amenconi
Copy link
Contributor Author

Thanks Adam. Makes sense to keep to infinite scroll for both device views to me. As you say, infinite scroll is efficient as it always loads one "page" of transactions (backend "page" of 100) which is the minimal request we can do from the API (I think) and then incrementally grabs additional pages as needed when scrolled one fetch at a time.

Current infinite scroll logic should allow scrolling until there are no more transactions left to fetch from the API. Plus the infinite scroll logic is already there for mobile so should be easy to adapt the same for the desktop view.

With infinite scroll in place for mobile and desktop, would it make sense to keep the transaction count select in place but only apply it to the export? I would then change the text to something more appropriate and move it next to the Export CSV button if so.

Also I did mean to call out the loading icon as a placeholder, thanks for the ng-zorro skeleton link and will make sure that is in place on my next push.

@adamunchained
Copy link
Contributor

Thanks Adam. Makes sense to keep to infinite scroll for both device views to me. As you say, infinite scroll is efficient as it always loads one "page" of transactions (backend "page" of 100) which is the minimal request we can do from the API (I think) and then incrementally grabs additional pages as needed when scrolled one fetch at a time.

Current infinite scroll logic should allow scrolling until there are no more transactions left to fetch from the API. Plus the infinite scroll logic is already there for mobile so should be easy to adapt the same for the desktop view.

With infinite scroll in place for mobile and desktop, would it make sense to keep the transaction count select in place but only apply it to the export? I would then change the text to something more appropriate and move it next to the Export CSV button if so.

Also I did mean to call out the loading icon as a placeholder, thanks for the ng-zorro skeleton link and will make sure that is in place on my next push.

❤️

I would simplify it for the user and just export everything. Eventually there should be an option to select date range instead. This is a filter they will care for. This should work differently though, user should hit export and options should pop-up. This way it's clear what those options are for. At the moment, it's not very clear what those options are for.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

Export CSV doesn't show full list; pagination is not happening
3 participants