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

last event support passion list #102

Merged
merged 2 commits into from
Jun 28, 2022
Merged

Conversation

Jarsen136
Copy link
Contributor

Thank you for your contribution to the KodaDot Indexer.
👇 _ Let's make a quick check before the contribution.

PR type

  • Bugfix
  • Feature
  • Refactoring

What's new?

Before submitting Pull Request, please make sure:

  • My contribution builds clean without any errors or warnings
  • I've merged recent default branch -- main and I've no conflicts
  • I've tried to respect high code quality standards
  • I've didn't break any original functionality
  • I've posted a screenshot of demonstrated change in this PR

Optional

  • I've run the indexer and it hasn't failed
  • I've changed schema and performed a migration
  • I found edge cases

Screenshot

  • My contribution is a resolver so I have pic from playground

image

image

@vikiival vikiival changed the title last event supprot passion list last event support passion list Jun 22, 2022
Copy link
Member

@vikiival vikiival left a comment

Choose a reason for hiding this comment

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

3.6

@vikiival
Copy link
Member

Thinking if we shouldnt do it otherwise ?
Have you tested that on people that have more items ?

@Jarsen136
Copy link
Contributor Author

Thinking if we shouldnt do it otherwise ?

It's what we have done with subquery in graphql. Or is there any better solution?
image

Have you tested that on people that have more items ?

How about 10 address. It still works well and fast.
image

@vikiival
Copy link
Member

Or is there any better solution?

maybe have argument passion: Boolean
and fetch passion that data directly in resolver

How about 10 address. It still works well and fast.

And If you provide like 300 of them wouldn't the query request fail?

@Jarsen136
Copy link
Contributor Author

Or is there any better solution?

maybe have argument passion: Boolean and fetch passion that data directly in resolver

I'm not sure if it would be better, because the resolver need to request passion list through accountId, and then request lastEvent. It seems costs more time 👀
Please correct me if I understand wrong.

And If you provide like 300 of them wouldn't the query request fail?

In this case, I guess the query would fail : )

@vikiival
Copy link
Member

Please correct me if I understand wrong.

Yeah you are right, but I just want to avoid case when person with a lot of buys breaks the query

@Jarsen136
Copy link
Contributor Author

Yeah you are right, but I just want to avoid case when person with a lot of buys breaks the query

It makes sense.

@Jarsen136
Copy link
Contributor Author

Jarsen136 commented Jun 25, 2022

updated!

image

image


image

@vikiival
Copy link
Member

I gave also merging rights to @kodadot/autocode so @petersopko can review and merge too

@Jarsen136
Copy link
Contributor Author

Hey there, pls take a look at this PR if you have time. ❤️ @petersopko

@petersopko
Copy link
Contributor

Hey there, pls take a look at this PR if you have time. ❤️ @petersopko

took a look, tested it out, lgtm

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.

3 participants