-
-
Notifications
You must be signed in to change notification settings - Fork 364
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
Refactor: useTransaction
for drop mint
#9908
Conversation
SUCCESS @Jarsen136 PR for issue #9750 which is assigned to you. Please wait for review and don't hesitate to grab another issue in the meantime! |
✅ Deploy Preview for koda-canary ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
||
const args = item.nfts.map((allocatedNft, index) => | ||
api.tx.nfts.mint(item.collectionId, allocatedNft.id, accountId.value, { | ||
ownedItem: (item.availableSerialNumbers || [])[index] || null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(item.availableSerialNumbers || [])[index]
could this error if index
is out of bounds?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No error. If out of bounds, it returns undefined and falls back to null
finally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code looks clean and straightforward
@prury please verify that minting in all drops types still works
This is happening in production iirc from time to time. |
please check: #9908 (comment) |
@Jarsen136 is this still thing? |
NO. It means prury has tested and has not found a new bug/issue related to this PR. The issues mentioned above existed before. I think it's ready to merge. |
Let me provide a better explanation: i could not test this PR because most test drops aren't working properly for me. But the problem is not coming from this PR, is from canary |
This PR focuses on refactoring and unifying the minting process for both
We already have this kind of issue. #9765
Let's raise issues for these bugs as they are from canary. |
Code Climate has analyzed commit e44074c and detected 0 issues on this pull request. View more on Code Climate. |
Quality Gate passedIssues Measures |
I'm aware of this and I'm not blaming you or this PR, i just want to make clear that while i can test a
Feel free to merge the PR, I'm just saying I'm unable to test PR because of external problems
done ✔️ |
Thanks! |
😍 Perfect, I’ve sent the payout 🪅 Let’s grab another issue and get rewarded! |
Thank you for your contribution to the Koda - Generative Art Marketplace.
👇 __ Let's make a quick check before the contribution.
PR Type
Needs QA check
Context
useTransaction
in Drops #9750use
useTransaction
to unify the minting process betweenpaid
andholderOf
drop.Did your issue had any of the "$" label on it?
Screenshot 📸