Skip to content
This repository has been archived by the owner on Aug 9, 2022. It is now read-only.

feat(storage): validates CID hash and prepends /ipfs/ #487

Merged
merged 6 commits into from
Nov 16, 2020

Conversation

jurajpiar
Copy link
Member

What the tin says.

Resolves #468

@jurajpiar jurajpiar requested a review from a team November 13, 2020 18:53
@jurajpiar
Copy link
Member Author

jurajpiar commented Nov 13, 2020

@rsksmart/rif-marketplace
Ok, so... the CID lib makes our test unhappy

FAIL  src/App.test.tsx
  ● Test suite failed to run

    TypeError: TextDecoder is not a constructor

    > 1 | import CID from 'cids'
        | ^
      2 | 
      3 | export const isEmpty = (
      4 |   text: string | unknown,
...

There is an open issue at jest about this here. The multibase lib have informed react-scripts about it (presumably).
The "fix" as stated there is basically to upgrade react-script to v4.
Another patch I found is to create custom test environment.

Which one is it going to be?

@julianmrodri
Copy link
Contributor

We can comment the test and resume work later on this once we upgrade to react 4 after release. But I dont think we should do that now. Also please consider that we also need to add the prefix /ipfs/ to the hash using UPLOAD SERVICE. Currently it is not working as Pinner rejects anything without that prefix.

Copy link
Contributor

@julianmrodri julianmrodri left a comment

Choose a reason for hiding this comment

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

  • Comment failing test for now until we upgrade to React 4 (we know we have to do some analysis for this specially around tasegir).
  • Add /ipfs/ also to hash created by Upload File Service

Copy link
Contributor

@itofarina itofarina left a comment

Choose a reason for hiding this comment

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

LG! I agree with @julianmrodri about re-taking the failing test once we move to react-scripts v4

src/components/organisms/storage/buy/PinningCard.tsx Outdated Show resolved Hide resolved
julianmrodri
julianmrodri previously approved these changes Nov 16, 2020
Copy link
Contributor

@julianmrodri julianmrodri left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@julianmrodri julianmrodri left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@jurajpiar jurajpiar merged commit 0be5d32 into master Nov 16, 2020
@jurajpiar jurajpiar deleted the feature/cidCheck branch November 16, 2020 14:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Validate IPFS Hash to Pin
3 participants