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.

Sign up for GitHub

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

Jump to bottom

[ABW-2589] Transaction History #1035

Merged
merged 143 commits into from
Mar 15, 2024
Merged

[ABW-2589] Transaction History #1035

merged 143 commits into from
Mar 15, 2024

Conversation

kugel3
Copy link
Contributor

@kugel3 kugel3 commented Feb 27, 2024 •
edited
Loading

Jira ticket: ABW-2589

Description

  • Transaction history, in all its glory

Notes

Also completely refactors handling of resources, especially with amounts, and displaying those in the app, and a lot of other things

How to test

Select an account, then tap History
It supports:

  • Scrolling down to see older transactions
  • Scrolling up past the top to see later, and the repeat this (insertion of transactions is not smooth)
  • Tapping a month label to load that month
  • Setting up and modifying filters (Filters sheet)
  • Removing filters right from History

This pre-alpha contains code that mocks a very active account when on mainnet. On stokenet it will use your actual account.

Video

paste link here

PR submission checklist

Tx.History.-.Screen.Recording.2024-03-13.at.05.55.54.mov
  • I have tested account to account transfer flow and have confirmed that it works

@kugel3 kugel3 marked this pull request as draft February 27, 2024 11:18
@kugel3 kugel3 force-pushed the ABW-2589_Transaction-history branch 2 times, most recently from 98fa446 to b1c0531 Compare March 1, 2024 12:03
@kugel3 kugel3 changed the title Abw 2589 transaction history [ABW-2589] Transaction History Mar 7, 2024
@kugel3 kugel3 force-pushed the ABW-2589_Transaction-history branch 7 times, most recently from a0bf9fc to ad47e04 Compare March 12, 2024 22:15
@kugel3 kugel3 marked this pull request as ready for review March 13, 2024 04:32
GhenadieVP
GhenadieVP reviewed Mar 13, 2024
View reviewed changes
Copy link
Contributor

@GhenadieVP GhenadieVP left a comment

Choose a reason for hiding this comment

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

Humongous amount of work 👏!
Looks very good, I have mostly some comments about some expected functionalities.

RadixWallet/Clients/TransactionClient/Models/TransactionFee.swift Show resolved Hide resolved
RadixWallet/Clients/TransactionHistoryClient/TransactionHistoryClient+Live.swift Outdated

let dateformatter1 = ISO8601DateFormatter()
dateformatter1.formatOptions.insert(.withFractionalSeconds)
let dateformatter2 = ISO8601DateFormatter()
Copy link
Contributor

Choose a reason for hiding this comment

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

Could be fallbackDateFormatter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should discuss this one, it seems that confirmedAt is basically always present, so we could start with that. And it's strange to think that the same date formatter can't handle the case where the milliseconds happen to be 0. We shouldn't need 2.

RadixWallet/Features/AccountHistory/TransactionHistory+Reducer.swift Show resolved Hide resolved
RadixWallet/Features/AccountHistory/TransactionHistory+View.swift Show resolved Hide resolved
RadixWallet/Features/AccountHistory/TransactionHistory+View.swift Outdated Show resolved Hide resolved
...tures/AssetsFeature/Components/HelperViews/ResourceBalance/ResourceBalanceView+Helpers.swift Show resolved Hide resolved
CyonAlexRDX
CyonAlexRDX approved these changes Mar 13, 2024
View reviewed changes
Copy link
Contributor

@CyonAlexRDX CyonAlexRDX left a comment

Choose a reason for hiding this comment

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

I could not meaningfully review this I think, too big and I'm too out of context, but: BRAVO! Great job, looks great, and big and important feature! ⭐️ 👏

@kugel3 kugel3 force-pushed the ABW-2589_Transaction-history branch 3 times, most recently from 6095ef8 to 2e9efcc Compare March 15, 2024 12:03
@kugel3 kugel3 force-pushed the ABW-2589_Transaction-history branch from 2e9efcc to 5e4e672 Compare March 15, 2024 12:04
@GhenadieVP GhenadieVP merged commit 3dea7a9 into main Mar 15, 2024
6 checks passed
@GhenadieVP GhenadieVP deleted the ABW-2589_Transaction-history branch March 15, 2024 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jakub-rdx jakub-rdx jakub-rdx left review comments

@CyonAlexRDX CyonAlexRDX CyonAlexRDX approved these changes

@GhenadieVP GhenadieVP GhenadieVP approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@kugel3 @CyonAlexRDX @GhenadieVP @jakub-rdx