Proper offers pagination, account offers

[charlie-wasp]

Now offers have proper cursor-based pagination too 💪 Also, accounts now have connection to offers

I played a little with union types to make forward and backward motion related properties exclusive in PagingParams type. I used custom type guard for the first time. Tell me, if it's too much TS magic 🙂

---

This change is 

[nebolsin]

Reviewed 16 of 16 files at r1.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @charlie-wasp)

---

src/model/offer.ts, line 47 at r1 (raw file):

  public get paging_token() {
    return this.id;
  }

And do we need Offer model now that we have Offer entity?

---

src/orm/entities/offer.ts, line 39 at r1 (raw file):

  public get paging_token() {
    return this.id;

I think default ordering for top-level offers query is (price, id) so paging token should include price component in this case. We still want ordering by id only for account { offers } queries.

---

src/orm/repository/offer.ts, line 17 at r1 (raw file):

      .where("offers.selling = :selling", { selling })
      .andWhere("offers.buying = :buying", { buying })
      .getRawOne();

Ordering by price and selecting the first row would guarantee that we hit the index

---

src/schema/offers.ts, line 71 at r1 (raw file):

      seller: AccountID
      selling: AssetCode
      buying: AssetCode

I think it would be reasonable to make selling and buying mandatory for top-level offers query, and also remove seller parameter here since it could now be queried as account { offers }.

---

src/schema/resolvers/offer.ts, line 93 at r1 (raw file):

      }

      return makeConnection<Offer>(await paginate(qb, paging, "offers.id"));

To reiterate, I think seller is not necessary here, selling and buying are required and default sorting should be (price, id).

---

src/schema/resolvers/trades.ts, line 20 at r1 (raw file):

  const offers = await getRepository(Offer).findByIds(ids);

  return ids.map(id => offers.find(o => o.id === id) || null);

This likely won't work because quite often after a trade have already happened, the offers would no longer exist in the database. Probably for now the best solution is to just return the offer ids in trades, without links to the actual offers.

[charlie-wasp]

Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @charlie-wasp and @nebolsin)

---

src/model/offer.ts, line 47 at r1 (raw file):

Previously, nebolsin (Sergey Nebolsin) wrote…

And do we need Offer model now that we have Offer entity?

Definitely no! Simply forgot about it

[charlie-wasp]

Reviewable status: 12 of 22 files reviewed, 6 unresolved discussions (waiting on @charlie-wasp and @nebolsin)

---

src/orm/entities/offer.ts, line 39 at r1 (raw file):

Previously, nebolsin (Sergey Nebolsin) wrote…

I think default ordering for top-level offers query is (price, id) so paging token should include price component in this case. We still want ordering by id only for account { offers } queries.

Working

---

src/orm/repository/offer.ts, line 17 at r1 (raw file):

Previously, nebolsin (Sergey Nebolsin) wrote…

Ordering by price and selecting the first row would guarantee that we hit the index

Working

---

src/schema/offers.ts, line 71 at r1 (raw file):

Previously, nebolsin (Sergey Nebolsin) wrote…

I think it would be reasonable to make selling and buying mandatory for top-level offers query, and also remove seller parameter here since it could now be queried as account { offers }.

Done

---

src/schema/resolvers/offer.ts, line 93 at r1 (raw file):

Previously, nebolsin (Sergey Nebolsin) wrote…

To reiterate, I think seller is not necessary here, selling and buying are required and default sorting should be (price, id).

Working

---

src/schema/resolvers/trades.ts, line 20 at r1 (raw file):

Previously, nebolsin (Sergey Nebolsin) wrote…

This likely won't work because quite often after a trade have already happened, the offers would no longer exist in the database. Probably for now the best solution is to just return the offer ids in trades, without links to the actual offers.

Done

[charlie-wasp]

Reviewable status: 12 of 22 files reviewed, 6 unresolved discussions (waiting on @charlie-wasp and @nebolsin)

---

src/orm/repository/offer.ts, line 17 at r1 (raw file):

Previously, charlie-wasp (Timur Ramazanov) wrote…

Working

Hmm, I ran explain select min(price) as min_price from offers where sellingasset = 'AAAAAU1PQkkAAAAAPHEwK55l2uMBECcQxzsOUsAWg1YwXwD+ZUTDWZEbZWQ=' and buyingasset = 'AAAAAA==', and it seems that index is used already. It says -> Index Only Scan using bestofferindex on offers (cost=0.41..597.64 rows=546 width=8)

[charlie-wasp]

Reviewable status: 10 of 23 files reviewed, 6 unresolved discussions (waiting on @charlie-wasp and @nebolsin)

---

src/orm/entities/offer.ts, line 39 at r1 (raw file):

Previously, charlie-wasp (Timur Ramazanov) wrote…

Working

Done.

---

src/schema/resolvers/offer.ts, line 93 at r1 (raw file):

Previously, charlie-wasp (Timur Ramazanov) wrote…

Working

Done.

[nebolsin]

Reviewed 13 of 13 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @charlie-wasp)