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

Fix: sidebar links to queue if there are queued txs #2357

Merged
merged 4 commits into from
Aug 22, 2023
Merged

Conversation

katspaugh
Copy link
Member

What it solves

Resolves #2352

How this PR fixes it

The /transactions route will now redirect to either /transactions/history or /transactions/queue if the queue is not empty.

@katspaugh katspaugh requested a review from schmanu August 7, 2023 12:09
@github-actions
Copy link

github-actions bot commented Aug 7, 2023

Branch preview

✅ Deploy successful!

https://queue_redirect--walletweb.review-wallet-web.5afe.dev

@katspaugh katspaugh changed the title Fix: sidebar goes to queue if there are queued txs Fix: sidebar links to queue if there are queued txs Aug 7, 2023
@github-actions
Copy link

github-actions bot commented Aug 7, 2023

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type Occurrences Fixable
Errors 0 0
Warnings 0 0
Ignored 0 N/A
  • Result: ✅ success
  • Annotations: 0 total

Report generated by eslint-plus-action

@usame-algan
Copy link
Member

Have we considered how we can measure the success of this change? I think it could be confusing if the same button has 2 different behaviours without the user knowing i.e. expecting to go to the history but ending up in the queue and vice versa especially if both pages look very similar it could slow the user down more.

@katspaugh
Copy link
Member Author

Valid point, @usame-algan.
@kirkkonen what do you think?

Copy link
Member

@schmanu schmanu left a comment

Choose a reason for hiding this comment

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

Should we add a small unit test, that checks that this rerouting will trigger?
Otherwise it's one of the small features that might get lost in a refactor.

Other than that this looks good and I'll already approve :)

@github-actions
Copy link

github-actions bot commented Aug 17, 2023

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type Occurrences Fixable
Errors 0 0
Warnings 0 0
Ignored 0 N/A
  • Result: ✅ success
  • Annotations: 0 total

Report generated by eslint-plus-action

@francovenica
Copy link
Contributor

The main feature works fine, the redirection to queue when there is a tx there works fine, to the history tab otherwise.

There is an issue with GA tho. Now visiting the tx history or queue triggers the /transaction/history or /queue event, but also there is an event for just "/transaction".
In Stg env only the /transaction/history or /queue shows:

transaction tracking

@katspaugh
Copy link
Member Author

@francovenica I've made the sidebar link itself change to either queue or history, so there won't be a /transaction event anymore.

@francovenica
Copy link
Contributor

Yeah, only 1 call for /history or /queue.

Looks good to me.

@katspaugh katspaugh merged commit 00765ee into dev Aug 22, 2023
7 checks passed
@katspaugh katspaugh deleted the queue-redirect branch August 22, 2023 13:08
@github-actions github-actions bot locked and limited conversation to collaborators Aug 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Transactions link: open History if Queue is empty
5 participants