1046 load from api orderstable #611

henrypalacios · 2021-08-26T18:16:45Z

Closes gnosis/cowswap#1046

Load orders from a wallet address.

🗒️ Summary:

Get the API to fetch data.
Refresh orders every 30s on the background.
Filter by time ago to save on calls.
Orders Loader

To Test

github-actions · 2021-08-26T18:46:44Z

🔭 Explorer: Explorer test app
🔒 Safe Swap App: Trade test app (add to your safe apps)
📈 Trade: Trade test app
📚 Storybook: Component stories

henrypalacios · 2021-08-26T21:05:31Z

I would like to clarify some doubts @alfetopito:

I have not been able to get the /getOrders endpoint to work, or at least how I think it works. I have done a few orders so I would expect them to show as filled but I always get []. I have tried these base_url with this query_string:
- https://protocol-rinkeby.dev.gnosisdev.com/api/v1/orders 👈 The one using by explorer app.
- https://protocol-rinkeby.gnosis.io/api/v1/orders
- https://protocol-mainnet.dev.gnosisdev.com/api/v1/orders 👈 The one from swagger
Should I use operatorApi/getOrders for this purpose? if so, I guess I should add default parameters here, e.g. what date (timestamp) should I use?
Since useNetworkId() could return null, should it not render if it is null?

alfetopito · 2021-08-26T22:22:22Z

I would like to clarify some doubts @alfetopito:
I have not been able to get the /getOrders endpoint to work, or at least how I think it works. I have done a few orders so I would expect them to show as filled but I always get []. I have tried these base_url with this query_string:
* https://protocol-rinkeby.dev.gnosisdev.com/api/v1/orders point_left The one using by **explorer app**.
* https://protocol-rinkeby.gnosis.io/api/v1/orders
* https://protocol-mainnet.dev.gnosisdev.com/api/v1/orders point_left The one from **swagger**

How did you place the orders?

I believe your issue is that you are placing orders against PROD backend endpoint and trying to fetch them against STAGING (dev) backend endpoint.
They are different databases, so you won't see orders placed in one in the other.

There are settings in both CowSwap and the Explorer to synchronize the environments used.
A brief summary:

PROD backend:
1. PROD CowSwap & PROD Explorer
2. STAGING CowSwap & STAGING Explorer
STAGING backend:
1. LOCAL, DEV and BARN CowSwap & LOCAL and DEV Explorer

There are a few ways to solve this:

Run cowswap locally, or from a PR or from dev endpoint (cowswap.dev.gnosisdev.com/) and place orders using it
Place orders using prod version as you already did and start the Explorer locally with env vars pointing to PROD backend:

$ OPERATOR_URL_STAGING_MAINNET=https://protocol-mainnet.gnosis.io/api OPERATOR_URL_STAGING_RINKEBY=https://protocol-rinkeby.gnosis.io/api OPERATOR_URL_STAGING_XDAI=https://protocol-xdai.gnosis.io/api yarn start:explorer

Should I use operatorApi/getOrders for this purpose? if so, I guess I should add default parameters here, e.g. what date (timestamp) should I use?

Yep. Although the filters implemented might not all be relevant for our frontend filters https://protocol-rinkeby.dev.gnosisdev.com/api/#/default/get_api_v1_orders
Probably only includeFullyExecuted=false to exclude filled orders and includeInvalidated=false to exclude cancelled orders.
Keep in mind these filters where not added with frontend in mind, we need to work on aligning them to our requirements.

Since useNetworkId() could return null, should it not render if it is null?

Correct. When network is null there's no data to show anyway.

henrypalacios · 2021-08-31T15:58:56Z

@alfetopito The problem I had with the API is that I need to extend the back date.

Also when when I was testing in Swagger-Rinkeby this was using was mainnet.

I would like to know, when you mentioned:

Probably only includeFullyExecuted=false to exclude filled orders and includeInvalidated=false to exclude cancelled orders.

I'm not sure what query strings to will be used to display orders in OrdersTable by default, because as I imagined it was to show the past orders including the executed.
(I used default values for show data like a oneYearAgo).
I had to go looking for the token contracts for the orders, How about using a context here in OrdersTableWidget to hold the multiple ERC20 contracts used in OrdersTable as TradesTable?

alfetopito · 2021-08-31T21:33:45Z

@alfetopito The problem I had with the API is that I need to extend the back date.

Well, I was sure I used this before, but it seems I mistook /orders with /trades endpoint.
Yeah, minValidTo must be set in the past to show older orders.
By default to start with all trades we could set this to 0.
Subsequent calls we perform the same query with minValidTo set to 10 min ago or something like that, then update the existing orders.

Also, set all boolean fields to true such as https://protocol-rinkeby.dev.gnosisdev.com/api/v1/orders?owner=0x5B0ABE214aB7875562ADeE331dEfF0Fe1912FE42&includeFullyExecuted=true&includeInvalidated=true&includeInsufficientBalance=true&includePresignaturePending=true&includeUnsupportedTokens=true&minValidTo=0

We want to show everything on the explorer.

Also when when I was testing in Swagger-Rinkeby this was using was mainnet.

You can switch on top which endpoint you are calling. You can access all of them from the same page. I know, weird.

I would like to know, when you mentioned:

Probably only includeFullyExecuted=false to exclude filled orders and includeInvalidated=false to exclude cancelled orders.

I'm not sure what query strings to will be used to display orders in OrdersTable by default, because as I imagined it was to show the past orders including the executed.

I've answered this on the top above. Set all flags to true and start with minValidTo=0

   (I used default values for show data like a [oneYearAgo](https://github.com/gnosis/gp-ui/blob/1046-load-from-api-orderstable/src/api/operator/operatorApi.ts#L200)).
I had to go looking for the token contracts for the orders, How about using a context here in OrdersTableWidget to hold the multiple ERC20 contracts used in OrdersTable as TradesTable?

Yes, that makes a lot of sense. It'll be very handy as well for @stackseeder since trades have even less context and need the respective orders in order to build the full row of data.

The tokens are already stored on global state (once loaded). Orders and trades are not because it only makes sense to do so if viewing the user details page so your idea of a context is great for the user details page.

src/api/operator/operatorApi.ts

elena-zh · 2021-09-01T11:18:17Z

Hey @henrypalacios , @alfetopito ,
I might be a bit lost, but is seems to me that we do not have 'Search', 'Filtering' and 'Pagination' components for a user's orders/trades table

I have an endless table for my Rinkeby account with lines overlapping the table header, that is why I raised this issue
https://pr611--gpui.review.gnosisdev.com/rinkeby/address/0xFF714b8b0e2700303eC912BD40496C3997ceEa2b/

elena-zh · 2021-09-01T11:33:18Z

I know, that this PR is WIP, but I would like to add a suggestion

to add a default sort order to tables (created desc or so)
to display see/buy amounts better or shorter..
to app a loader when order details are loaded
https://drive.google.com/file/d/1gYDU8PymA690htXpTuf0SQg6Wz4BZLXU/view
to open an order link in a new tab
Can we move the \/ Copied to a new row?
Will we add a sorting feature to columns?

alfetopito · 2021-09-01T20:39:35Z

Hey @henrypalacios , @alfetopito ,
I might be a bit lost, but is seems to me that we do not have 'Search', 'Filtering' and 'Pagination' components for a user's orders/trades table

We agreed to not have search, filtering and pagination on the first version.

I have an endless table for my Rinkeby account with lines overlapping the table header, that is why I raised this issue
https://pr611--gpui.review.gnosisdev.com/rinkeby/address/0xFF714b8b0e2700303eC912BD40496C3997ceEa2b/

Hmm, I do not see any orders when I open this link :/

alfetopito · 2021-09-01T20:45:45Z

I know, that this PR is WIP, but I would like to add a suggestion

to add a default sort order to tables (created desc or so)

Orders are supposed to come directly from the API sorted by creation date descending.
For now, they are not. I'll have a sync with backend team tomorrow to discuss this.

to display see/buy amounts better or shorter..

Agree this should be improved. But I think we can do it all at once, similar to what's been done on CowSwap.
It'll involve updating how Order details are displayed as well so better to do in a separated task.

to app a loader when order details are loaded
https://drive.google.com/file/d/1gYDU8PymA690htXpTuf0SQg6Wz4BZLXU/view

👍

to open an order link in a new tab

👍

5. Can we move the `\/ Copied` to a new row?
   ![image](https://user-images.githubusercontent.com/70885163/131675051-da094998-59cc-4301-8ae0-1632bbf51885.png)

Not a big deal to me right now. Maybe we could replace the text with 'copied' depending on the screen size.
But again, also another task.

6. Will we add a sorting feature to columns?

Yes, but not on the first version.

elena-zh · 2021-09-02T08:55:20Z

Hmm, I do not see any orders when I open this link :/

I was not able to open this page yesterday, today I see orders again. So here is the video: https://drive.google.com/file/d/1VpDeAdbZHNziW-xyqKNVEGiTwgL37jP2/view

Btw, looks a bit weird when order list flashing when a user's order info is updated.

elena-zh · 2021-09-02T10:02:40Z

I have created separate tasks for cases:

move the \/ Copied to a new row User orders list: move the \/ Copied to a new row #630
enhance amounts displaying User orders list: display amounts better #629

henrypalacios · 2021-09-02T12:23:34Z

@elena-zh

I was not able to open this page yesterday

you're right, I had problems with this, I thought it was my changes, but then I realized that something had changed in the API.

Btw, looks a bit weird when order list flashing when a user's order info is updated.

I'm working on it 😅 , I just wanted to make sure the table is updated.

And thank you for your other remarks.

henrypalacios · 2021-09-07T23:38:37Z

src/apps/explorer/const.ts

@@ -2,6 +2,8 @@ import { AnalyticsDimension } from 'types'

 /** Explorer app constants */
 export const ORDER_QUERY_INTERVAL = 10000 // in ms
+export const ORDERS_QUERY_INTERVAL = 10000 // in ms


We had agreed on 30min but I have set it to 10min to test.

alongoni · 2021-09-09T14:09:57Z

Looks great!
We need to add the Title: "User details"

alfetopito

Working quite nicely!

Left a few refactoring suggestions.

And, this isn't part of the logic but I missed when reviewing the table design.
On full screen the columns don't fill the whole table

src/apps/explorer/components/OrdersTableWidget/context/OrdersTableContext.tsx

alfetopito · 2021-09-09T20:56:46Z

src/apps/explorer/components/OrdersTableWidget/useGetOrders.tsx

+      try {
+        const ordersFetched = await getOrders({ networkId: network, owner, minValidTo: minTimeHistoryTimeStamp })
+        const newErc20Addresses = ordersFetched.reduce((accumulator: string[], element) => {
+          const updateAccumulator = (tokenAddress: string): void => {


What about using a Set (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set) for this?

const newErc20Address = new Set<string>() ordersFetched.forEach(order => { newErc20Address.add(order.buyToken) newErc20Address.add(order.sellToken) })

I love the Set, it would be much more readable however but I went for the reduce for performance.
Do you think it makes up for it or should I go for the Set? https://jsben.ch/ognIg

Wow, no idea it had such a performance impact. Guess they make mose sense in languages where Sets are first class citizens.

I prefer sets for readability but maybe we should stick with reducer due to performance as you suggest.

@anxolin @W3stside FYI

src/apps/explorer/components/OrdersTableWidget/useGetOrders.tsx

elena-zh · 2021-09-10T13:08:13Z

Hey @henrypalacios , great pr!
A new tiny issue from me: ann empty orders table is jumping when updating each 30 secs
https://drive.google.com/file/d/14c41B_FzYnQHCu4zEk6XoI9Z-tFeqB3M/view

Btw, can we change 'Canceled' status to 'Cancelled' to match Cowswap app?
I know that it is a known pending issue under the #612 task, but may be it is an easy fix?

Also, can we add validation to URL on address format? However, I'm not sure about it..

elena-zh · 2021-09-20T13:37:29Z

Hey @henrypalacios , changes you implemented look great!
But still orders table is jumping each 30 sec when it is empty:
https://drive.google.com/file/d/1bTWypM3bHfc4OHVbfUrrraXgxiL5JvQe/view
Also, it looks a bit weird when hovered state is applied to an empty table

Thanks!

alfetopito

Love it! Working great!

test/utils/operator/orderFormatAmount.test.ts

henrypalacios · 2021-09-20T19:28:37Z

@elena-zh hover has been removed in the row when there are not orders.

@alongoni has suggested using the same size of the table when reloading so it doesn't look strange.

@alfetopito I have set the interval back to 30sec. I also think something is going wrong in the evaluation of the test coverage.

alongoni

Looks great!

alfetopito · 2021-09-20T20:44:04Z

I force merged it.
The failure is due to an outage on Coveralls https://status.coveralls.io/ unrelated to your changes

elena-zh · 2021-09-21T13:11:40Z

Hey @henrypalacios , great!

Just curious: why do I always see these 404 errors in the console?

In addition, please, don't kill me.. But still I see blinking table when it is updating each 30 seconds (see from 19 sec )
https://drive.google.com/file/d/1DFNOWTSpBvpr8hJgzFAuiIOH9OvkFxO0/view

A bit strange to see this scroll in the empty table. It is also displayed when a table has orders, but it is more logical to show it there, but not in the empty table..

henrypalacios force-pushed the 1046-load-from-api-orderstable branch from 2da3f7c to 164bfc2 Compare August 26, 2021 18:41

henrypalacios added the Protofire task to the protofire team label Aug 31, 2021

alfetopito reviewed Aug 31, 2021

View reviewed changes

src/api/operator/operatorApi.ts Outdated Show resolved Hide resolved

This was referenced Sep 2, 2021

User orders list: display amounts better #629

Closed

User orders list: move the \/ Copied to a new row #630

Open

henrypalacios marked this pull request as ready for review September 7, 2021 23:35

henrypalacios commented Sep 7, 2021

View reviewed changes

Fix branch conflicts

ea58455

henrypalacios force-pushed the 1046-load-from-api-orderstable branch from 74fb547 to ea58455 Compare September 9, 2021 18:40

alfetopito reviewed Sep 9, 2021

View reviewed changes

Add test to comply with the coverage

0b53823

henrypalacios added 4 commits September 10, 2021 17:10

Fix space rows in table

3e36ca8

Add owner Address validation, title

2f61d15

Remove OrdersTableState and apply style to owner address

c848c44

Suggestions in the container of the pages

66acfc9

henrypalacios requested review from alfetopito, ramirotw and alongoni September 20, 2021 15:55

fix hover styles and empty container height

bad76f6

alfetopito approved these changes Sep 20, 2021

View reviewed changes

test/utils/operator/orderFormatAmount.test.ts Outdated Show resolved Hide resolved

henrypalacios added 2 commits September 20, 2021 16:10

Remove unnecessary arrow function in the test

649e7e6

Set the order interval to 30sec

cfdd5b3

alongoni added 2 commits September 20, 2021 17:06

fix overflow on order tables

36e7048

padding on h1 - user details

eb5d63f

alongoni approved these changes Sep 20, 2021

View reviewed changes

alfetopito merged commit 1381c0a into develop Sep 20, 2021

alfetopito deleted the 1046-load-from-api-orderstable branch September 20, 2021 20:43

henrypalacios mentioned this pull request Sep 21, 2021

New api orderstable #661

Merged

This was referenced Sep 22, 2021

Orders table: Horizontal scrollbar appears in a reduced screen #683

Closed

Order table: empty table is blinking each 30 sec when reloading #684

Closed

alfetopito mentioned this pull request Oct 13, 2021

Release/2.3.0 #761

Merged

anxolin mentioned this pull request Apr 3, 2022

User orders list: move the \/ Copied to a new row cowprotocol/explorer#12

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

1046 load from api orderstable #611

1046 load from api orderstable #611

henrypalacios commented Aug 26, 2021 •

edited

Loading

github-actions bot commented Aug 26, 2021

henrypalacios commented Aug 26, 2021 •

edited

Loading

alfetopito commented Aug 26, 2021

henrypalacios commented Aug 31, 2021 •

edited

Loading

alfetopito commented Aug 31, 2021

I would like to know, when you mentioned:

elena-zh commented Sep 1, 2021 •

edited

Loading

elena-zh commented Sep 1, 2021 •

edited

Loading

alfetopito commented Sep 1, 2021

alfetopito commented Sep 1, 2021

elena-zh commented Sep 2, 2021

elena-zh commented Sep 2, 2021 •

edited

Loading

henrypalacios commented Sep 2, 2021

henrypalacios Sep 7, 2021

alongoni commented Sep 9, 2021 •

edited

Loading

alfetopito left a comment

alfetopito Sep 9, 2021

henrypalacios Sep 9, 2021 •

edited

Loading

alfetopito Sep 20, 2021

elena-zh commented Sep 10, 2021 •

edited

Loading

elena-zh commented Sep 20, 2021

alfetopito left a comment

henrypalacios commented Sep 20, 2021 •

edited

Loading

alongoni left a comment

alfetopito commented Sep 20, 2021

elena-zh commented Sep 21, 2021

1046 load from api orderstable #611

1046 load from api orderstable #611

Conversation

henrypalacios commented Aug 26, 2021 • edited Loading

To Test

github-actions bot commented Aug 26, 2021

henrypalacios commented Aug 26, 2021 • edited Loading

alfetopito commented Aug 26, 2021

henrypalacios commented Aug 31, 2021 • edited Loading

I would like to know, when you mentioned:

alfetopito commented Aug 31, 2021

I would like to know, when you mentioned:

elena-zh commented Sep 1, 2021 • edited Loading

elena-zh commented Sep 1, 2021 • edited Loading

alfetopito commented Sep 1, 2021

alfetopito commented Sep 1, 2021

elena-zh commented Sep 2, 2021

elena-zh commented Sep 2, 2021 • edited Loading

henrypalacios commented Sep 2, 2021

henrypalacios Sep 7, 2021

Choose a reason for hiding this comment

alongoni commented Sep 9, 2021 • edited Loading

alfetopito left a comment

Choose a reason for hiding this comment

alfetopito Sep 9, 2021

Choose a reason for hiding this comment

henrypalacios Sep 9, 2021 • edited Loading

Choose a reason for hiding this comment

alfetopito Sep 20, 2021

Choose a reason for hiding this comment

elena-zh commented Sep 10, 2021 • edited Loading

elena-zh commented Sep 20, 2021

alfetopito left a comment

Choose a reason for hiding this comment

henrypalacios commented Sep 20, 2021 • edited Loading

alongoni left a comment

Choose a reason for hiding this comment

alfetopito commented Sep 20, 2021

elena-zh commented Sep 21, 2021

henrypalacios commented Aug 26, 2021 •

edited

Loading

henrypalacios commented Aug 26, 2021 •

edited

Loading

henrypalacios commented Aug 31, 2021 •

edited

Loading

elena-zh commented Sep 1, 2021 •

edited

Loading

elena-zh commented Sep 1, 2021 •

edited

Loading

elena-zh commented Sep 2, 2021 •

edited

Loading

alongoni commented Sep 9, 2021 •

edited

Loading

henrypalacios Sep 9, 2021 •

edited

Loading

elena-zh commented Sep 10, 2021 •

edited

Loading

henrypalacios commented Sep 20, 2021 •

edited

Loading