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

use GetTransactions method for fetching tickets #567

Merged
merged 27 commits into from Sep 15, 2021

Conversation

oshorefueled
Copy link
Contributor

@oshorefueled oshorefueled commented Aug 19, 2021

This PR replaces the use of getTicket method with GetTransactions to fetch wallet tickets. It uses different transaction filters to fetch tickets with different status.
Fix #546
Fix #580
Fix #570
Fix #474
Close #458

The ticket activity page and section on the overview page are commented out. The code implemented looks like a duplicate of the ticket list page and having it as part of the ticket module is redundant. I suggest that page be reviewed before it is added back as an accessible feature.

Copy link
Contributor

@beansgum beansgum left a comment

Choose a reason for hiding this comment

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

  • The tickets page freezes the app a bit before opening.
  • Some of the tooltips added by Ticket hover #488 aren't showing

ui/load/ticket.go Outdated Show resolved Hide resolved
ui/page/tickets/overview.go Outdated Show resolved Hide resolved
ui/load/ticket.go Outdated Show resolved Hide resolved
ui/load/ticket.go Outdated Show resolved Hide resolved
ui/load/ticket.go Outdated Show resolved Hide resolved
ui/load/ticket.go Outdated Show resolved Hide resolved
overview.Voted += ov.Voted
}

wl.MultiWallet.GetLowestBlockTimestamp()
Copy link
Contributor

@beansgum beansgum Aug 20, 2021

Choose a reason for hiding this comment

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

What does this do?

}
}

func filterToStatus(txFilter int32) string {
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like dcrlibwallet tx filters can be referenced directly instead of converting to identical constants.

@Sirmorrison Sirmorrison added this to To do in godcr board Aug 20, 2021
@Sirmorrison Sirmorrison moved this from To do to Review in progress in godcr board Aug 20, 2021
@Sirmorrison Sirmorrison moved this from Review in progress to In Progress in godcr board Aug 23, 2021
@beansgum beansgum self-assigned this Sep 12, 2021
@beansgum beansgum force-pushed the fetch_tickets branch 2 times, most recently from 2b19b50 to 633497b Compare September 13, 2021 08:27
ui/page/tickets/list_page.go Show resolved Hide resolved
ui/page/tickets/purchase_modal.go Outdated Show resolved Hide resolved
ui/page/tickets/utils.go Outdated Show resolved Hide resolved
ui/page/tickets/utils.go Show resolved Hide resolved
ui/page/tickets/utils.go Show resolved Hide resolved
ui/page/tickets/utils.go Outdated Show resolved Hide resolved
ui/page/tickets/utils.go Show resolved Hide resolved
ui/values/dimensions.go Show resolved Hide resolved
@@ -291,7 +288,7 @@ func (win *Window) Loop(w *app.Window, shutdown chan int) {
break
}

win.updateStates(e.Resp)
// win.updateStates(e.Resp)
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove if no longer needed.

godcr board automation moved this from In Progress to Review in progress Sep 14, 2021
dreacot
dreacot previously approved these changes Sep 14, 2021
@dreacot dreacot dismissed their stale review September 14, 2021 23:48

found a bug

@dreacot
Copy link
Collaborator

dreacot commented Sep 14, 2021

Even after selecting a wallet with enough funds, the Review Purchase button is disabled
Screenshot 2021-09-15 at 12 49 02 AM

Copy link
Contributor

@Sirmorrison Sirmorrison left a comment

Choose a reason for hiding this comment

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

  • Any specific reason the ticket activity page was removed?

  • When you have only one ticket, the ticket is center aligned rather than being aligned to the top left corner of the screen.

image

  • if I click the purchase button without entering my spending password (N:B i needed a password for my wallet), it says Attempting to Purchase ticket (1) but no tickets are purchased.

I understand that we should be able to allow for an empty password but since I currently need a password and non was provided, which also did not allow the ticket to be purchase (i think so), I should get an error message of some sort stating that the ticket purchase was not successful.

image

  • Button color is not app default blue color.
    image

Copy link
Contributor

@Sirmorrison Sirmorrison left a comment

Choose a reason for hiding this comment

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

After successfully purchasing as ticket, the staking record still shows 0 unmined tickets instead of 1.
image

image

Waiting a while, i see that i now have one unmined ticket as a live ticket, and one staking record however, in the view all ticket page, the ticket is shown as immature.

overview page

image

All tickets page
image

SO to narrow down the issues, the ticket overview page is only refreshed when i Navigate to another Page and then back to the ticket page.

I believe at this point, the OnResume() is called which updates the page.

Copy link
Contributor

@Sirmorrison Sirmorrison left a comment

Choose a reason for hiding this comment

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

Pr looks good with functionality.

Some observations.

  • On the all tickets page, the wallet name is not displayed on the ticket card.
  • When i hover on the wallet name from the ticket overview page, there is no tooltip pop up. This was earlier implemented inPR Ticket hover #488.

Ticket overview page.
image

all ticket page
image

This is the mock up
image

- remove unused methods
- remove multiwallet methods that can be called directly from pages since
  pages already have multiwallet in the Load struct.
- add ticket methods for fetch different ticket types in the load package
oshorefueled and others added 20 commits September 15, 2021 19:11
- display correct tickets reward value
- add transactionItem to wrap ticket transaction and tooltips
- Apply shadow to more list items and back up seed bottom pane
- use GridLayout for tickets grid view to fix scrolling lag issues
- show ticket maturity progress and display correct value on time label
- use correct progress bar and track colors
- add TxStatus struct
- sort ticket transactions and prioritize spent tickets
- add tooltip messages for all ticket statuses and format block time
  for voted and revoked tickets
- fix bug where overview page shows the same details for all tickets
- add ticket status and confirmations property to transactionItem struct
- add shadow to tooltip and remove border stroke
- Add second progress bar layout that fixes a border radius bug
- Add shared data to transactionItem
- Refresh window after loading data to remove lag in ui update
- Use correct format for ticket maturity time
godcr board automation moved this from Review in progress to Approved Sep 15, 2021
@Sirmorrison Sirmorrison merged commit b68db77 into planetdecred:master Sep 15, 2021
godcr board automation moved this from Approved to Done Sep 15, 2021
song50119 pushed a commit to song50119/godcr that referenced this pull request Apr 24, 2022
* clean up wallet/commands.go

- remove unused methods
- remove multiwallet methods that can be called directly from pages since
  pages already have multiwallet in the Load struct.

* add ticket type and methods to load package

* clean up ticket list page

- add ticket methods for fetch different ticket types in the load package

* resolve inactive button bug on ticket purchase modal

* remove global ticket implementation

* use dcrlibwallet staking PR

* cleanup fetch ticket code

* delete ticket file from load package

* move tooltip initialization to onResume

* Cleanup tickets overview page

- display correct tickets reward value

* Fix only tickets purchase tx showing as 'All' in tickets list page

- add transactionItem to wrap ticket transaction and tooltips

* Add shadow to LinearLayout

- Apply shadow to more list items and back up seed bottom pane

* Add GridLayout

- use GridLayout for tickets grid view to fix scrolling lag issues

* tickets: fix ticket cards layouts

- show ticket maturity progress and display correct value on time label
- use correct progress bar and track colors
- add TxStatus struct
- sort ticket transactions and prioritize spent tickets

* Fix ticket status tooltip

- add tooltip messages for all ticket statuses and format block time
  for voted and revoked tickets
- fix bug where overview page shows the same details for all tickets
- add ticket status and confirmations property to transactionItem struct
- add shadow to tooltip and remove border stroke

* Fix content displayed in ticket age, days to vote and maturity duration tooltips

* Use transactionItem to populate tickets list values

- Add second progress bar layout that fixes a border radius bug
- Add shared data to transactionItem
- Refresh window after loading data to remove lag in ui update
- Use correct format for ticket maturity time

* Fix popups and complete tx details not showing on tx overview page

* Remove tickets activity page and unused util functions

* Cleanup ticket purchase modal

* Show error if selected ticket purchase account has insufficient balance

* Process ticket purchase in review modal

* Refresh tickets overview data after successful ticket purchase

* Add tooltip to wallet name

* Use dcrlibwallet master(31878a6)

Co-authored-by: Olanrewaju Collins <ocollins444@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
godcr board
  
Done
4 participants