Composable-transactionMint #5276

daiagi · 2023-03-15T11:27:53Z

Thank you for your contribution to the KodaDot NFT gallery.

👇 _ Let's make a quick check before the contribution.

PR Type

Bugfix
Feature
Refactoring

Context

Closes composable transaction transactionMint #5034
Requires deployment <>

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 </bsx/collection>
tested by minting on snek and RMRK, with and without listing for sell
I've tested PR on mobile
I've written unit tests 🧪
I've found edge cases

Had issue bounty label?

Fill up your KSM address: Payout

Community participation

Are you at KodaDot Discord?

Screenshot 📸

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

…mposable-transactionMint

kodabot · 2023-03-15T11:27:58Z

WARNING @daiagi PR for issue #5034 which isn't assigned to you. Please be warned that this PR may get rejected if there's another assignee for issue #5034

netlify · 2023-03-15T11:28:02Z

✅ Deploy Preview for koda-nuxt ready!

Name	Link
🔨 Latest commit	`2740e9f`
🔍 Latest deploy log	https://app.netlify.com/sites/koda-nuxt/deploys/64187d73b99a7b00084abbe7
😎 Deploy Preview	https://deploy-preview-5276--koda-nuxt.netlify.app/
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

composables/transaction/transactionMintToken.ts

daiagi · 2023-03-15T11:29:04Z

this PR covers minting Token composable
Minting collection will follow in another PR

composables/transaction/transactionMintToken.ts

vikiival

^

vikiival · 2023-03-15T12:22:58Z

Minting collection will follow in another PR

I would rather do this first

vikiival · 2023-03-15T13:17:30Z

Yeah sorry, closing that was a bit harsh. Sorry for that I just overreacted

I just rather do the collections first, it is much easier and 80% of pinning logic can be extracted away.

The reasoning is this is hard to review. Contains a lot of dupictated code.

daiagi · 2023-03-15T13:23:49Z

Yeah, it was challenging to write as well

I'll do the collection minting, sure

But as for this one, how about I split the files to make it easier read?

And there are actually no duplications
In fact I've removed duplicate code

composables/transaction/transactionMintToken.ts

vikiival · 2023-03-15T13:43:38Z

But as for this one, how about I split the files to make it easier read?

Yes please decompose pinning from the file

I'll do the collection minting, sure

Yeah I think it's easier to do.

Sorry one more time for closing that 🙈

and thanks @roiLeo for reopening that

composables/transaction/transactionMintToken.ts

composables/transaction/MintToken/transactionMinRmrk.ts

composables/transaction/MintToken/constructMeta.ts

composables/transaction/MintToken/transactionMintBasilisk.ts

roiLeo · 2023-03-15T13:47:14Z

Hey! I don't want to interfere but I've just tested on snek (/snek/gallery/331660682-9) and for me it works very well.
It's already a good start and it allows to get the logic out of the components (better readability). And that is what I asked when I've opened #5034.
In any case it will be useful when we'll rewrite minting experence (and maybe massmint).
Nice work, keep it up! 😉

composables/transaction/MintToken/transactionMinRmrk.ts

daiagi · 2023-03-15T13:48:40Z

Hey! I don't want to interfere but I've just tested on snek (/snek/gallery/331660682-9) and for me it works very well. It's already a good start and it allows to get the logic out of the components (better readability). And that is what I asked when I've open #5034. In any case it will be useful when we'll rewrite minting experence (and maybe massmint). Nice work, keep it up! wink

Thanks @roiLeo
Appreciate the positive feedback 😄

vikiival · 2023-03-15T13:50:37Z

Wondering why are you refactoring SimpleMint instead of rmrk/CreateToken.vue ?

vikiival · 2023-03-15T13:51:04Z

But so far really good job 🤗

…mposable-transactionMint

…i/nft-gallery into composable-transactionMint

composables/transaction/mintCollection/transactionMintCollectionBasilisk.ts

composables/transaction/mintToken/transactionMintRmrk.ts

composables/transaction/mintCollection/transactionMintCollectionRmrk.ts

composables/transaction/mintToken/transactionMintBasilisk.ts

composables/transaction/mintToken/transactionMintRmrk.ts

components/rmrk/Create/CreateCollection.vue

composables/transaction/mintToken/transactionMintRmrk.ts

composables/transaction/mintToken/transactionMintBasilisk.ts

composables/transaction/mintCollection/transactionMintCollectionRmrk.ts

daiagi · 2023-03-18T06:06:25Z

tested again by minting collection and NFT on BSX and RMRK

/bsx/collection/651189115
/rmrk/collection/669DB118F2897C6014-PLN

vikiival · 2023-03-19T13:41:10Z

Can please @JustLuuuu or @helloitsdamsky test?

daiagi · 2023-03-20T09:09:00Z

/reviewpad summarize

JustLuuuu · 2023-03-20T09:09:12Z

Can please @JustLuuuu or @helloitsdamsky test?

If Im using the correct Deploy link for testing, it's still not working for me:

I have created a new wallet especially to test this while I was writing an article about minting on Basilisk. This account has exactly 0 bsx tokens. Only KSM (bridged). I believe even if you set KSM as a fee, it still somehow requires bsx. Test it with a wallet where you have 0 bsx.

When I tried to test it with a wallet where I had some bsx tokens. It worked.

reviewpad · 2023-03-20T09:09:41Z

This pull request includes various changes across multiple files. Changes involve new functions, types, and modifications to existing functions related to minting NFTs and collections. The execMintToken, execMintBasilisk, execMintCollection, execMintCollectionRmrk, useNewCollectionId, constructMeta, and useTransaction functions were added or modified. Various files such as CreateToken.vue, transactionMintCollection.ts, utils.ts, and notification.ts were also updated. Other changes include updates to import statements and regular expressions.

daiagi · 2023-03-20T09:12:36Z

I have created a new wallet especially to test this while I was writing an article about minting on Basilisk. This account has exactly 0 bsx tokens. Only KSM (bridged). I believe even if you set KSM as a fee, it still somehow requires bsx. Test it with a wallet where you have 0 bsx.

When I tried to test it with a wallet where I had some bsx tokens. It worked.

strongly suspect it has to do with this:

Not possible to mint with only bridged KSM #5284

daiagi · 2023-03-20T11:30:53Z

I had a chat with @JustLuuuu who confirmed minting on Kusama works
I have also created PR #5305 to fix the minting with bridged KSM on Basilisk issue

vikiival · 2023-03-20T10:26:35Z

components/bsx/Create/CreateCollection.vue

@@ -43,33 +43,22 @@ import {
  getMetadataDeposit,
  getclassDeposit,
 } from '@/components/unique/apiConstants'
-import { getRandomValues, hasEnoughToken } from '@/components/unique/utils'
-import { uploadDirect } from '@/utils/directUpload'
+import { hasEnoughToken } from '@/components/unique/utils'


this will fail on Basilisk 🐍

ref:

Not possible to mint with only bridged KSM #5284

didn't touch that either, it is marked because of changed imports

it is never called..
see the comment out line below

// await this.checkBalanceBeforeTx() showNotification( this.$t('mint.creatingCollection', { name: this.base.name }), notificationTypes.info ) this.status = 'loader.ipfs' transaction({

vikiival · 2023-03-20T10:28:56Z

components/bsx/Create/CreateToken.vue

-  offsetAttribute,
-  secondaryFileVisible,
-} from '@/components/rmrk/Create/mintUtils'
+import { offsetAttribute } from '@/utils/mintUtils'


you cannot currently use offset attribute in BSX

oh ok, this is not something I've changed in this PR
It's marked because I've moved mintUtils

remove it?

components/bsx/Create/CreateToken.vue

components/rmrk/Create/CreateToken.vue

vikiival · 2023-03-20T10:44:12Z

components/rmrk/Create/CreateToken.vue

+            setTimeout(
+              () =>
+                this.listForSale(
+                  createdNFTs.value,
+                  blockNumber.value as string
+                ),
+              300


Isn't this failing?

Because you can't see this in arrow fn

i was wondering the same, but no. it works

vikiival · 2023-03-20T11:56:32Z

composables/transaction/mintToken/transactionMintBasilisk.ts

+
+  const create = api.tx.nft.mint(collectionId, nextId, metadata)
+
+  const list = price


For some magical reason someone minted token with price 0

sorry i don't follow
are you saying i should check that price isn't 0?

resolved

const list = Boolean(Number(price)) ? [api.tx.marketplace.setPrice(collectionId, nextId, price)] : []

composables/transaction/mintToken/transactionMintBasilisk.ts

composables/useTransaction.ts

vikiival · 2023-03-20T12:02:01Z

More like suggestions otherwise LGTM

…i/nft-gallery into composable-transactionMint

composables/transaction/mintToken/transactionMintBasilisk.ts

vikiival · 2023-03-20T15:36:40Z

pay 100 usd

yangwao · 2023-03-20T15:36:53Z

😍 Perfect, I’ve sent the payout
💵 $100 @ 35.66 USD/KSM ~ 2.804 $KSM
🧗 EfmnRhHaQqfT3phm4cUCHCU3gFVDoSPR1U9WXzMRQBMqZ4L
🔗 0xc516fcda2389cb05b71352d71bcc3885de14deface541810ad216fbb1a4dc5ab

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

codeclimate · 2023-03-20T15:41:24Z

Code Climate has analyzed commit 2740e9f and detected 14 issues on this pull request.

Here's the issue category breakdown:

Category	Count
Complexity	6
Duplication	8

View more on Code Climate.

daiagi added 2 commits March 15, 2023 18:24

composable transactonMintToken

952df33

Merge branch 'main' of https://github.com/kodadot/nft-gallery into co…

ffd8174

…mposable-transactionMint

daiagi requested a review from a team as a code owner March 15, 2023 11:27

daiagi requested review from preschian and vikiival and removed request for a team March 15, 2023 11:27

daiagi requested a review from roiLeo March 15, 2023 11:28

codeclimate bot reviewed Mar 15, 2023

View reviewed changes

composables/transaction/transactionMintToken.ts Outdated Show resolved Hide resolved

composables/transaction/transactionMintToken.ts Outdated Show resolved Hide resolved

composables/transaction/transactionMintToken.ts Outdated Show resolved Hide resolved

cleanup, fix deepsource

6f75873

vikiival reviewed Mar 15, 2023

View reviewed changes

composables/transaction/transactionMintToken.ts Show resolved Hide resolved

vikiival requested changes Mar 15, 2023

View reviewed changes

vikiival closed this Mar 15, 2023

assign metdata to metadataRef (for retries)

133b5b4

roiLeo reopened this Mar 15, 2023

codeclimate bot reviewed Mar 15, 2023

View reviewed changes

composables/transaction/transactionMintToken.ts Outdated Show resolved Hide resolved

split files

eb60390

codeclimate bot reviewed Mar 15, 2023

View reviewed changes

vikiival reviewed Mar 15, 2023

View reviewed changes

composables/transaction/MintToken/transactionMinRmrk.ts Outdated Show resolved Hide resolved

fix file name typo

e2d4c99

daiagi added 3 commits March 18, 2023 11:51

Merge branch 'main' of https://github.com/kodadot/nft-gallery into co…

cbbdbaa

…mposable-transactionMint

translation fix

140a675

Merge branch 'composable-transactionMint' of https://github.com/daiag…

6ce2d8f

…i/nft-gallery into composable-transactionMint

codeclimate bot reviewed Mar 18, 2023

View reviewed changes

daiagi commented Mar 18, 2023

View reviewed changes

components/rmrk/Create/CreateCollection.vue Show resolved Hide resolved

remove a line

4108af3

codeclimate bot reviewed Mar 18, 2023

View reviewed changes

composables/transaction/mintToken/transactionMintRmrk.ts Show resolved Hide resolved

composables/transaction/mintToken/transactionMintBasilisk.ts Show resolved Hide resolved

composables/transaction/mintCollection/transactionMintCollectionRmrk.ts Show resolved Hide resolved

clear unused imports

1441cf5

kodadot deleted a comment from reviewpad bot Mar 20, 2023

Merge branch 'main' into composable-transactionMint

a46145c

vikiival reviewed Mar 20, 2023

View reviewed changes

daiagi added 4 commits March 20, 2023 19:28

as @vikiival suggested

0f9f391

Merge branch 'composable-transactionMint' of https://github.com/daiag…

8c5facd

…i/nft-gallery into composable-transactionMint

Basilisk - check price > 0 for listing

274cbce

fix typing issue

721201d

vikiival approved these changes Mar 20, 2023

View reviewed changes

composables/transaction/mintToken/transactionMintBasilisk.ts Outdated Show resolved Hide resolved

Update composables/transaction/mintToken/transactionMintBasilisk.ts

2740e9f

yangwao added the paid pull-request has been paid label Mar 20, 2023

vikiival merged commit d555927 into kodadot:main Mar 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Composable-transactionMint #5276

Composable-transactionMint #5276

daiagi commented Mar 15, 2023 •

edited

kodabot commented Mar 15, 2023

netlify bot commented Mar 15, 2023 •

edited

daiagi commented Mar 15, 2023

vikiival left a comment

vikiival commented Mar 15, 2023

vikiival commented Mar 15, 2023 •

edited

daiagi commented Mar 15, 2023

vikiival commented Mar 15, 2023 •

edited

roiLeo commented Mar 15, 2023 •

edited

daiagi commented Mar 15, 2023

vikiival commented Mar 15, 2023

vikiival commented Mar 15, 2023

daiagi commented Mar 18, 2023

vikiival commented Mar 19, 2023

daiagi commented Mar 20, 2023

JustLuuuu commented Mar 20, 2023

reviewpad bot commented Mar 20, 2023

daiagi commented Mar 20, 2023

daiagi commented Mar 20, 2023

vikiival Mar 20, 2023

daiagi Mar 20, 2023 •

edited

vikiival Mar 20, 2023

daiagi Mar 20, 2023

vikiival Mar 20, 2023

daiagi Mar 20, 2023

vikiival Mar 20, 2023

daiagi Mar 20, 2023

daiagi Mar 20, 2023

vikiival commented Mar 20, 2023

vikiival commented Mar 20, 2023

yangwao commented Mar 20, 2023

codeclimate bot commented Mar 20, 2023


		const create = api.tx.nft.mint(collectionId, nextId, metadata)

		const list = price

Composable-transactionMint #5276

Composable-transactionMint #5276

Conversation

daiagi commented Mar 15, 2023 • edited

PR Type

Context

Before submitting pull request, please make sure:

Optional

Had issue bounty label?

Community participation

Screenshot 📸

kodabot commented Mar 15, 2023

netlify bot commented Mar 15, 2023 • edited

✅ Deploy Preview for koda-nuxt ready!

daiagi commented Mar 15, 2023

vikiival left a comment

Choose a reason for hiding this comment

vikiival commented Mar 15, 2023

vikiival commented Mar 15, 2023 • edited

daiagi commented Mar 15, 2023

vikiival commented Mar 15, 2023 • edited

roiLeo commented Mar 15, 2023 • edited

daiagi commented Mar 15, 2023

vikiival commented Mar 15, 2023

vikiival commented Mar 15, 2023

daiagi commented Mar 18, 2023

vikiival commented Mar 19, 2023

daiagi commented Mar 20, 2023

JustLuuuu commented Mar 20, 2023

reviewpad bot commented Mar 20, 2023

daiagi commented Mar 20, 2023

daiagi commented Mar 20, 2023

Choose a reason for hiding this comment

daiagi Mar 20, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vikiival commented Mar 20, 2023

vikiival commented Mar 20, 2023

yangwao commented Mar 20, 2023

codeclimate bot commented Mar 20, 2023

daiagi commented Mar 15, 2023 •

edited

netlify bot commented Mar 15, 2023 •

edited

vikiival commented Mar 15, 2023 •

edited

vikiival commented Mar 15, 2023 •

edited

roiLeo commented Mar 15, 2023 •

edited

daiagi Mar 20, 2023 •

edited