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

Making asset unavailable for purchase if the size is zero #595

Closed
wants to merge 1 commit into from

Conversation

jamiehewitt15
Copy link
Member

Closes:

Changes proposed in this PR:

  • Checking if the file size is 0
  • Making the asset unavailable if the file size is zero

@vercel
Copy link

vercel bot commented May 13, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/oceanprotocol/market/6iU2eN1MapMFARM7eq1tN8i9XJmD
✅ Preview: https://market-git-issue-581-file-length-check-oceanprotocol.vercel.app

@codeclimate
Copy link

codeclimate bot commented May 13, 2021

Code Climate has analyzed commit 35f3c46 and detected 0 issues on this pull request.

View more on Code Climate.

@@ -167,7 +167,11 @@ export default function Consume({
</div>
<div className={styles.pricewrapper}>
<Price price={price} conversion />
{!isInPurgatory && <PurchaseButton />}
{parseInt(file?.contentLength) === 0 ? (
Copy link
Contributor

@kremalicious kremalicious May 18, 2021

Choose a reason for hiding this comment

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

file here refers to the file metadata object within the DDO, which is only set upon publish, retrieved when file is added in publish form. So this check here would only kick in if file has been published with contentLength: 0 which is very unlikely

Copy link
Member

@bogdanfazakas bogdanfazakas May 24, 2021

Choose a reason for hiding this comment

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

I think we can use this provider POST method (https://docs.oceanprotocol.com/references/provider/#apiv1servicesfileinfo) to get the file Content-Type and Content-Length using the asset did

@kremalicious
Copy link
Contributor

kremalicious commented May 18, 2021

Feels like this is generally a better direction for solving #452, which could make #463 obsolete. If we can figure out file availability automatically by hitting the respective provider endpoint (instead of basing logic on fixed file metadata) then we can remove button as is done in this PR. If we have this there would not be much reasons to have publisher pay gas fees to manually disable/enable the button.

Likewise, this PR could be combined with #479

@kremalicious
Copy link
Contributor

As outlined in #479 we can't rely on content-length being correct all the time as it looks like. Considering this, informing user instead of disabling action in #479 seems like a better direction so closing this

@kremalicious kremalicious deleted the issue-581-file-length-check branch September 24, 2021 10:36
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.

3 participants