-
-
Notifications
You must be signed in to change notification settings - Fork 273
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
[docs] Add Stripe script example #2477
Conversation
An interesting issue I'm facing is that the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added just some comments but if none of them needs any action I can approve the PR.
console.error('Invoice already exists:', dest); | ||
return; | ||
} | ||
exec(`curl '${invoice.invoice_pdf}' -L -o '${dest}'`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An alternative to this, which works but does not give a lot of feedback, would be to create a custom component for the download button that uses an anchor tag with the download
attribute, so that the PDF is downloaded on the browser.
Not sure how easy/possible it would be but it might be an improvement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree, the real utility of this app would be if the invoices could be downloaded on the browser. The issue is how to allow bulk downloads in that case - I think I can modify the script to create a zipped file and create a browser download of that file if there are multiple inovices.
starting_after, | ||
}); | ||
|
||
return JSON.stringify(list.data); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's strange that we need to stringify this, it should just work.
Maybe the data returned by the function as an object is too big? Could we maybe try a lower limit or filter out some properties?
I don't have a Stripe token so just speculating, let me know if you want me to try it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It worked with just returning the list
instead of list.data
- not quite sure why but leaving this here to offer clarity in case of attempts of future resolution
examples/stripe-script/package.json
Outdated
"dependencies": { | ||
"@mui/toolpad": "^0.1.22", | ||
"child_process": "^1.0.2", | ||
"fs": "^0.0.1-security", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these not included with node? (fs
, path
, child_process
...)
Do we need to include them in package.json?
I've read and followed the contributing guide on how to create great pull requests.
I've updated the relevant documentation for any new or updated feature
Closes Add stripe app to Github examples #2422
Updated to download the
PDFs
to the client, and use a Stripe data provider to show a paginated list of invoices:stripe.mov