search

onflow / nft-catalog

https://www.flow-nft-catalog.com/
The Unlicense
35 stars 11 forks source link

Add cadence 1.0 functionality. #149

Open aishairzay opened 3 months ago

aishairzay commented 3 months ago

The cadence 1.0 upgrades do NOT contain any changes for the UI to point to previewnet, all catalog addition requests, etc should be done with transactions for the time-being.

Also, the transaction generation portion of the catalog is not configured to work. This can be added in the future if it is a requested feature when all dependency contracts that are used by the tx generation are available with cadence 1.0 compatibility.

Also all JS testing is not currently being used for testing in favor of the new Cadence testing framework. It has not been deleted because there is an update coming to flow-js-testing which may allow the old testing to work in the future.

vercel[bot] commented 3 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
nft-catalog ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 8, 2024 8:44pm
aishairzay commented 3 months ago

Thanks @joshuahannan, latest push should address everything mentioned and some more. I also removed a lot of code for areas of the catalog that are not relevant anymore with flix around (transaction generation).

joshuahannan commented 2 months ago

@sisyphusSmiling I got the Cadence tests passing here and re-staged the contract on testnet so it would hopefully pass the next migration. I need help with all the JS stuff, so if you could take a look and try to get the tests passing at some point, that would be super helpful. There isn't a huge rush thought since we're already staged on testnet.

I do need to try to figure out where the previewnet private key information is to upgrade the contract there, so if you know anything about that, please let me know

sisyphusSmiling commented 2 months ago

No problem, @joshuahannan. I'll pick up JS tests later this week

sisyphusSmiling commented 1 month ago

@jribbink is there a version of flow-js-testing that's compatible with Cadence 1.0?

sisyphusSmiling commented 1 month ago

FYI I updated flow-js-testing to 0.6.0-stable-cadence.2 and flow-cadut to ^0.3.0-stable-cadence.1 plus some audit fixes. I wasn't able to get the js tests to pass and kept getting thrown: undefined on all the tests' beforeEach setup.

jribbink commented 1 month ago

FYI I updated flow-js-testing to 0.6.0-stable-cadence.2 and flow-cadut to ^0.3.0-stable-cadence.1 plus some audit fixes. I wasn't able to get the js tests to pass and kept getting thrown: undefined on all the tests' beforeEach setup.

I'm in the process of pushing through some more fixes for JS Testing now. It may solve the issues here.

joshuahannan commented 1 month ago

@sisyphusSmiling I addressed your comments and resolved the conflicts. Thanks for updating the JS stuff! There is no rush on getting them passing though. You can come back to it after you get back from your time off and hopefully the JS testing fixes are in

jribbink commented 1 month ago

@sisyphusSmiling @joshuahannan There's a PR here https://github.com/onflow/nft-catalog/pull/152 for updating Flow JS Testing. When I run it locally, it seems to work, but it's failing tests because of issues in scripts/tx, maybe this helps.

Example of Cadence scripts causing errors below:

Screenshot 2024-05-22 at 2 53 52 PM
joshuahannan commented 1 day ago

@jribbink I'm not sure what the errors in the CI mean. They don't look like Cadence errors. Mostly seeing:

Not sure what I need to change about the JS tests to fix those.

I also can't run the tests locally yet. Still trying to figure out how to install all the correct dependencies. When I run npm test, i see this: 

 FAIL  cadence/__test__/test/nftcatalog.test.js
  ● Test suite failed to run

    Cannot find module '@onflow/flow-js-testing' from 'cadence/__test__/test/nftcatalog.test.js'
jribbink commented 1 day ago

@joshuahannan

@jribbink I'm not sure what the errors in the CI mean. They don't look like Cadence errors. Mostly seeing:

  • TypeError: Cannot read properties of undefined (reading 'kill')
  • thrown: "Could not determine Flow CLI version, please make sure it is installed and available in your PATH"

Not sure what I need to change about the JS tests to fix those.

The CI issue on GitHub is coming from the fact that the install script changed for flow-c1 recently, it just needs to be updated to the new script. (I had made this PR before the script was changed a couple weeks ago)

(this line just needs to be updated https://github.com/onflow/nft-catalog/pull/149/files#diff-faff1af3d8ff408964a57b2e475f69a6b7c7b71c9978cccc8f471798caac2c88R20)

I also can't run the tests locally yet. Still trying to figure out how to install all the correct dependencies. When I run npm test, i see this:

 FAIL  cadence/__test__/test/nftcatalog.test.js
  ● Test suite failed to run

    Cannot find module '@onflow/flow-js-testing' from 'cadence/__test__/test/nftcatalog.test.js'

This issue just looks like you need to run npm install before trying to run the tests locally.

Re. TypeError: Cannot read properties of undefined (reading 'kill'), this will probably disappear when the other issues are solved, as it's most likely a symptom of the emulator not starting correctly/bad error reporting in Flow JS Testing