Skip to content

pdfshift adaptor documentation #698

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

hunterachieng
Copy link
Contributor

@hunterachieng hunterachieng commented Aug 14, 2025

Short Description

Add pdfShift documentation with examples.

Closes #679

AI Usage

Please disclose how you've used AI in this work (it's cool, we just want to
know!):

  • Code generation (copilot but not intellisense)
  • Learning or fact checking
  • Strategy / design
  • Optimisation / refactoring
  • Translation / spellchecking / doc gen
  • Other
  • I have not used AI

You can read more details in our
Responsible AI Policy

Copy link
Collaborator

@josephjclark josephjclark left a comment

Choose a reason for hiding this comment

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

The hardest bit of this, I think, is understanding how to use the PDF once it's been generated. What do you do with it?

I'm not sure you can pass it downstream to another step? Have we tried this? Because it may not serialise well, and that may cause problems either in the dataclip or in downstream state. And if so, that's stuff needs documenting.

// Post generated PDF link to WhatsApp
fn(state => {
const { data } = state;
const pdfData = JSON.parse(data);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hey this is weird - why are we JSON parsing this?

I think we need to update generatePDF to parse the JSON automatically. There's no sense in forcing users to do it.

Also, I think we need to better document what generatePDF returns. I assumed it was a PDF buffer of some kind

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It returns a PDF buffer or a base64. But if you want a url that is hosted on pdfShift's s3 bucket, then you add the filename: <filename> option which returns a JSON string :

 "data": "{\"success\":true,\"url\":\"https://pdfshift.s3.amazonaws.com/d/2/2025-08/fbe4debd68d841e4bf05eb04f0dd2693/trials.pdf\",\"filesize\":50023,\"duration\":2156,\"response\":{\"status-code\":200,\"content-length\":0,\"requests\":0,\"duration\":1985.0274124145508},\"executed\":\"2025-08-18T14:38:12.560874\",\"pdf_pages\":1}",

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, but my surprise and confusion here is a problem.

Where is this stuff documented? How is the user supposed to know?


request('POST', 'messages', state => ({
to: '254712345678',
body: `Please check on the PDF and download in 24hours. ${state.pdfData.url}`,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it true that the link won't work for 24 hours? Is this documented anywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is documented here. I should change to 48 hours

Signed-off-by: Hunter Achieng <[email protected]>
@josephjclark
Copy link
Collaborator

@hunterachieng before we can merge this I think we need to look at OpenFn/adaptors#1338

And these docs are surfacing a couple of behavioural things that I'm worried about, like this JSON.parse step

I think we need to patch the adaptor before we release these docs

@hunterachieng
Copy link
Contributor Author

@josephjclark Since the JSON.parse comes from an option, should I add an example for that in the adaptor fix with the option and document it?

@josephjclark
Copy link
Collaborator

The option being filename?

What I really mean is: if PDF shift returns us a JSON response, we should automatically parse that and return it to state.data. That should be magic within the adaptor, not an intervention in job code

@hunterachieng
Copy link
Contributor Author

Understood. Let me work on that ASAP.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

Add docs for PDF generator adaptor
2 participants