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

feat: content_cid not id is used from nft data upload not uploads #1755

Merged
merged 6 commits into from
Apr 4, 2022

Conversation

dashcraft
Copy link
Contributor

We had been using ID but id is not accurate, content_cid is the id that relates nft uploads to uploads.

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Apr 3, 2022

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 5ce1404
Status: ✅  Deploy successful!
Preview URL: https://79c227f5.nft-storage-1at.pages.dev

View logs

@dashcraft dashcraft changed the title content_cid not id is used from nft data upload not uploads feat: content_cid not id is used from nft data upload not uploads Apr 3, 2022
@JeffLowe JeffLowe requested a review from cmunns April 4, 2022 13:25
packages/api/src/routes/nfts-upload.js Outdated Show resolved Hide resolved
@@ -125,20 +125,20 @@ export class DBClient {
updated_at: now,
})
.select(this.uploadQuery)
.eq('id', data.id)
.eq('content_cid', data.content_cid)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to update the upload via the CID the user knows it by (the source_cid). This is not necessarily our normalized v1 CID.

Also the upload listing does not list normalized CIDs so for anyone uploading or pinning v0 CIDs they'll not be able to update their upload (see here).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed this to source_id, @alanshaw does this handle the case of v0 CID's? Is there special precautions or exceptions to make for v0 I need to write?

packages/api/src/utils/db-client.js Outdated Show resolved Hide resolved
packages/api/src/routes/nfts-upload.js Show resolved Hide resolved
packages/api/src/routes/nfts-upload.js Outdated Show resolved Hide resolved
@dashcraft dashcraft requested a review from alanshaw April 4, 2022 16:05
@dashcraft dashcraft merged commit 21e8eb0 into main Apr 4, 2022
@dashcraft dashcraft deleted the feat/quick-update-fix branch April 4, 2022 22:27
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.

2 participants