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

Improved Subgraph Events Sorting #2972

Merged
merged 20 commits into from
Dec 1, 2021
Merged

Conversation

rdig
Copy link
Member

@rdig rdig commented Nov 29, 2021

This PR introduces events sorting on the subgraph side, by adding a timestamp prop on all event entities on that are coming from the subgraph, so that we can pass query selection fields like orderBy and orderDirection

Besides the above, I've also refactored the ColonyEvents component (it actually started this) to properly have pagination (before this, we would just fetch the first 100 events, then split them up locally)

Note that we cannot do the same for the ColonyActions component due to the way we are combining 4 data streams into one, and not always symmetrical (this will become a problem once a colony surpasses 1000 combined actions)

Since sorting is now done directly, prior to the data reaching our data, I also removed the sortSubgraphEventByIndex util, as it was no longer required

Testing note

Since this is using an update version of the subgraph, make sure to run npm run provision prior to testing this

@rdig rdig requested a review from a team November 29, 2021 15:25
@rdig rdig self-assigned this Nov 29, 2021
Copy link
Member

@arrenv arrenv left a comment

Choose a reason for hiding this comment

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

This is a nice improvement to the SubGraph data and handling sorting.

I did notice that the 'Load more' button still appears on the last item when there are no more results.

event-sorting

Copy link
Member

@arrenv arrenv left a comment

Choose a reason for hiding this comment

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

Sorting looks to be working much better.

(I'm assuming the 5th Dec date is unrelated and a result of the dev environment?)

Previous

events-sorting-before

New

events-sorting-after

@rdig
Copy link
Member Author

rdig commented Nov 30, 2021

(I'm assuming the 5th Dec date is unrelated and a result of the dev environment?)

Two things about dates in the local dev environment to keep in mind:

  • account for them
  • but don't trust them

They can and are manipulated either by us directly or by various services we have running in the background

@rdig rdig force-pushed the refactor/better-event-subgraph-sorting branch from 13b56b9 to 924f6ea Compare November 30, 2021 11:24
Copy link
Contributor

@chinins chinins left a comment

Choose a reason for hiding this comment

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

Very good!

Copy link
Contributor

@ArmandoGraterol ArmandoGraterol left a comment

Choose a reason for hiding this comment

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

It's working as expected from what I could see! Well done

@rdig rdig force-pushed the refactor/better-event-subgraph-sorting branch from 924f6ea to 41db368 Compare December 1, 2021 14:50
@rdig rdig merged commit 33df6a3 into master Dec 1, 2021
@rdig rdig deleted the refactor/better-event-subgraph-sorting branch December 1, 2021 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants