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

Show progress query #23

Merged
merged 9 commits into from
Oct 27, 2022
Merged

Show progress query #23

merged 9 commits into from
Oct 27, 2022

Conversation

LudovicoGranata
Copy link
Collaborator

closes #11 .
Added visual feedback on the query execution.

This also means that when the query is initiated the application must keep track of these status changes and keep track of the query ID. In practise this means that after I send the query and if I reload the page I must be redirected to the same page with my query waiting to be satisfied or rejected.

After some trial, I was not able to implement this part. It's quite difficult to keep track of the current state of the query execution process if the page is reloaded (e.g what if the page is reloaded when the frontend is waiting for the query to being bought? Do we buy it another time? what if the MetaMask user is changed?...).

@relu91
Copy link
Member

relu91 commented Sep 30, 2022

After some trial, I was not able to implement this part. It's quite difficult to keep track of the current state of the query execution process if the page is reloaded (e.g what if the page is reloaded when the frontend is waiting for the query to being bought? Do we buy it another time? what if the MetaMask user is changed?...).

Use a best-effort approach and record only the confirmed transactions, then we can refine it. In practice, every time you send a transaction keep track of its hashID, if you get back a confirmation (or the result) move to the next one, etc.

Copy link
Member

@relu91 relu91 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 I also tried it locally, and I would still give it a try to implement the state tracking as explained above. Also, I left a question below.

src/app/pages/query-page/query-page.component.html Outdated Show resolved Hide resolved
Copy link
Member

@relu91 relu91 left a comment

Choose a reason for hiding this comment

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

Wait for merging for this:

Use a best-effort approach and record only the confirmed transactions, then we can refine it. In practice, every time you send a transaction keep track of its hashID, if you get back a confirmation (or the result) move to the next one, etc.

@iosonopersia
Copy link
Collaborator

That's it, this is the best I could achieve without changing how the SDK works too.

I was able to implement only one checkpoint but the logic can be extended in the future with more of them, maybe.

Once the requestID is obtained the first time, a snapshot of the query execution is saved in the localStorage.
If the user reloads the page in the middle of the query execution, a dialog box is shown that asks the user whether to dismiss the previous operation or to resume it.

When #22 will be merged, it will become possible to integrate a slightly more complex logic that could handle also swapping the wallets without reloading the page. What's needed is to change the data structure saved in the cache from an object to a "dictionary" of objects:

{
  "0x<USER_1>": { <QUERY_STATE_FOR_USER_1 },
  "0x<USER_2>": { <QUERY_STATE_FOR_USER_2 },
  ...
}

The page will check if, for the currently selected user, the cached query state (if not existing it must be initialized) tells that a query is still unfinished, and act accordingly.

@iosonopersia iosonopersia force-pushed the showProgressQuery branch 2 times, most recently from 30470a8 to 627d02f Compare October 3, 2022 15:15
Copy link
Member

@relu91 relu91 left a comment

Choose a reason for hiding this comment

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

Just tried locally. I found an issue, step to reproduce:

  • issue a query
  • wait till stage 3 Executing query ( do all the transactions requested)
  • reload the page
  • the DApp correctly asks for resuming the query
  • it starts from stage 2 and ask for doing the transactions again.

@iosonopersia
Copy link
Collaborator

I added the handling of "user changed" events, which works exactly as explained in a previous comment: #23 (comment).

  • If the wallet is changed during the execution of a query, the only thing that can be done is reloading the page (if the old wallet is selected afterwards, the user will be able to resume from the last saved checkpoint):
  • otherwise, the QueryPage component acknowledges the newly-selected wallet and looks into the cache to see whether a checkpoint had already been saved for such wallet (and, in that case, it will ask the user for resuming the query execution process).

Just tried locally. I found an issue, step to reproduce:
[...]

Due to some limits of the actual interface of the Desmo class in the SDK, it's not possible from outside the SDK to know at which point of execution we are. The only thing we can know at the moment is whether the requestID has already been obtained or not. This is why we have only one checkpoint after the "RequestID phase".

Actually, I now added another checkpoint that could be useful. It saves the query built by the user before retrieving the requestID. So, if the user builds a query and closes the page/changes user BEFORE accepting the "requestID transaction", he/she will be able to resume from that same transaction without having to build again the same query (which can take a while...).

The current code can be easily extended with (an)other checkpoint(s), but this would require changing the SDK interface for the Desmo class (refer to vaimee/desmo-sdk#12 (comment)).

@relu91
Copy link
Member

relu91 commented Oct 27, 2022

Just tried locally. I found an issue, step to reproduce:

  • issue a query
  • wait till stage 3 Executing query ( do all the transactions requested)
  • reload the page
  • the DApp correctly asks for resuming the query
  • it starts from stage 2 and ask for doing the transactions again.

Did the same thing and the problem remains; I'll merge this PR and open an issue to keep track of that bug.

@relu91 relu91 merged commit e603996 into main Oct 27, 2022
@iosonopersia iosonopersia deleted the showProgressQuery branch October 27, 2022 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Give visual feedback about query execution phases to the user
3 participants