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

fix: Separate properties and attributes in NFTInput #829

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

yukia3e
Copy link
Contributor

@yukia3e yukia3e commented Jan 10, 2025

Changes

  • Linear: https://linear.app/thirdweb/issue/INF-171/
  • In the existing metadata transformation logic, properties and attributes were treated interchangeably (properties: metadata.properties || metadata.attributes). However, these fields serve different purposes and should be handled separately to avoid potential data conflicts and maintain clarity.
  • Updated the logic to explicitly set properties and attributes as independent fields in the NFT metadata transformation.

How this PR will be tested

  • Open the dashboard and click X. Result: A modal should appear.
  • Call the /foo/bar API. Result: Returns 200 with "baz" in the response body.

Output

(Example: Screenshot/GIF for UI changes, cURL output for API changes)


PR-Codex overview

This PR focuses on updating the handling of properties and attributes in the mint-to.ts files for both erc721 and erc1155 contracts. The changes ensure that properties and attributes are set to undefined if they are not present in the metadata.

Detailed summary

  • Changed properties assignment from metadata.properties || metadata.attributes to metadata.properties ?? undefined.
  • Added attributes assignment with metadata.attributes ?? undefined in both erc721 and erc1155 files.

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

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.

1 participant