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

fallback nft meta is null #2749

Merged
merged 7 commits into from
Apr 12, 2022
Merged

fallback nft meta is null #2749

merged 7 commits into from
Apr 12, 2022

Conversation

Jarsen136
Copy link
Contributor

@Jarsen136 Jarsen136 commented Apr 2, 2022

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

PR type

  • Bugfix
  • Feature
  • Refactoring

What's new?

the error is caused by the incrooect nft meta, which id is null.
image

the nft: /rmrk/gallery/10869444-c6017764e7a1d03b5e-MOOSENFT-MAGGIE_MOOSE_29-0000000000000029

expected nft meta is an object with id and image.
image

Would be cool if we can filter the events in squid.

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 tested it at </rmrk/collection/26902bc2f7c20c546a-1FVG7>
  • I've tested PR on mobile and everything seems works
  • I found edge cases
  • I've written some unit tests 🧪

Had issue bounty label?

  • Fill up your KSM address: Payout

Community participation

Screenshot

  • My fix has changed something on UI; a screenshot is best to understand changes for others.

@netlify
Copy link

netlify bot commented Apr 2, 2022

Deploy Preview for koda-nuxt failed.

Name Link
🔨 Latest commit 839c83b
🔍 Latest deploy log https://app.netlify.com/sites/koda-nuxt/deploys/62554a7806c0270008e7f412

@yangwao yangwao requested a review from vikiival April 4, 2022 15:11
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.

fallback

fallback is when you ask one girl for a date but in case you ask another girl just in case the first one would say no

The implementation filters out nfts which does not have meta (which is super strange :)) But the correct fallback is to take metadata field and fetch it on the client

@Jarsen136 Jarsen136 requested a review from a team as a code owner April 5, 2022 12:30
@Jarsen136
Copy link
Contributor Author

I mock the empty meta and then it displays all the images and works well without any error.

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.

Extend queries/rmrk/subsquid/lastNftListByEvent.graphql

  1. extend query with metadata
query lastNftListByEvent($limit: Int!, $event: Interaction!, $and: [EventWhereInput!]) {
  events(
    limit: $limit
    where: { interaction_eq: $event, AND: $and, nft: {burned_eq: false} }
    orderBy: timestamp_DESC
  ) {
    meta
    timestamp
    nft {
      id
      name
      issuer
      currentOwner
      metadata
      meta {
        id
        image
      }
    }
  }
}
  1. fetch metadata
  2. update cache
  3. fetch image via cloudflare image

@yangwao
Copy link
Member

yangwao commented Apr 8, 2022

lol probably accidentally found that moment lol and seems working

image

image

@Jarsen136
Copy link
Contributor Author

lol probably accidentally found that moment lol and seems working

image

image

yup, you caught the same error again 😄

@Jarsen136
Copy link
Contributor Author

Jarsen136 commented Apr 9, 2022

Extend queries/rmrk/subsquid/lastNftListByEvent.graphql

  1. extend query with metadata
query lastNftListByEvent($limit: Int!, $event: Interaction!, $and: [EventWhereInput!]) {
  events(
    limit: $limit
    where: { interaction_eq: $event, AND: $and, nft: {burned_eq: false} }
    orderBy: timestamp_DESC
  ) {
    meta
    timestamp
    nft {
      id
      name
      issuer
      currentOwner
      metadata
      meta {
        id
        image
      }
    }
  }
}
  1. fetch metadata
  2. update cache
  3. fetch image via cloudflare image

Now it works for me but I'm not sure if the code meets your expectation.
And I still don't know how I try to fetch meta.image if 'meta' is null. 😕

components/rmrk/Gallery/LatestSales.vue Show resolved Hide resolved
components/rmrk/Gallery/NewestList.vue Outdated Show resolved Hide resolved
@Jarsen136 Jarsen136 requested a review from vikiival April 9, 2022 10:25
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.

👀

@yangwao
Copy link
Member

yangwao commented Apr 12, 2022

Are conversations resolved?

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.

YOLO MODE ON.

@vikiival
Copy link
Member

pay 99 usd

@yangwao
Copy link
Member

yangwao commented Apr 12, 2022

😍 Perfect, I’ve sent the payout
💵 $99 @ 156.21 USD/KSM ~ 0.634 $KSM
🧗 Caiv9TbPz68q5dC8EcHu5xKYPRnremimGzqmEejDFNpWWLG
🔗 0x9584cedba95dc54eb2dec18c4c0e176c21feb8182f809f9a566acc51f0d67bb2

🪅 Let’s grab another issue and get rewarded!
🪄 github.com/kodadot/nft-gallery/issues

@yangwao yangwao added the paid pull-request has been paid label Apr 12, 2022
@vikiival vikiival merged commit c76d1e6 into kodadot:main Apr 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
paid pull-request has been paid
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Seems newest list from landing is gone
3 participants