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

Adding Transport Override for PDF use case #98

Merged
merged 7 commits into from
Nov 8, 2020

Conversation

haislip
Copy link
Contributor

@haislip haislip commented Oct 6, 2020

#97

In order to save the PDF generated from the APIs properly, the transport passed into the popsicle API needs to be one of type "buffer" instead of "text" (which is the default). This changes keeps the default of using "text", but allows the calling function to pass in the override.

@abisalehalliprasan
Copy link
Collaborator

Thanks, @haislip for the contribution. This PR will go in our next release.

@abisalehalliprasan abisalehalliprasan merged commit 9eddd1e into intuit:develop Nov 8, 2020
@krishnal
Copy link

hi @abisalehalliprasan

Any idea when this will be published over npm? Seems like you merged it but i think it's not published yet.

@thesoftwarephilosopher
Copy link

The /pdf route is broken until this is pushed to NPM. Not that I'm part of the dev team, but I just reviewed and verified it and it looks good, and there is currently no non-patching workaround to this afaik.

@abisalehalliprasan
Copy link
Collaborator

@sdegutis : Just curious what is the transport that you passed to the makeApiCall method ?

Would it be similar to :

oauthClient
    .makeApiCall({ url: `${url}v3/company/${companyID}/invoice/${invoiceNumber}/pdf?minorversion=59` , transport: popsicle.createTransport({type: "buffer"})})

@thesoftwarephilosopher
Copy link

Yep

@abisalehalliprasan
Copy link
Collaborator

I get a server error. wondering if the transport parameter that I have passed is correct :

    response: Response {
      Url: [Url],
      rawHeaders: [Array],
      body: <Buffer 7b 22 46 61 75 6c 74 22 3a 7b 22 45 72 72 6f 72 22 3a 5b 7b 22 4d 65 73 73 61 67 65 22 3a 22 41 6e 20 61 70 70 6c 69 63 61 74 69 6f 6e 20 65 72 72 6f ... 184 more bytes>,
      status: 500,
      statusText: 'Server Error'
    },
    body: <Buffer 7b 22 46 61 75 6c 74 22 3a 7b 22 45 72 72 6f 72 22 3a 5b 7b 22 4d 65 73 73 61 67 65 22 3a 22 41 6e 20 61 70 70 6c 69 63 61 74 69 6f 6e 20 65 72 72 6f ... 184 more bytes>,
    json: { Fault: [Object], time: '2021-05-26T14:33:13.224-07:00' },
    intuit_tid: '1-60aebe99-2bd8211b0a8e83d913308d52'
  },
  originalMessage: 'Response has an Error',
  error: 'Server Error',
  error_description: 'Server Error',
  intuit_tid: '1-60aebe99-2bd8211b0a8e83d913308d52'

@abisalehalliprasan
Copy link
Collaborator

@sdegutis : I did not pass the Accept header also as application/pdf

.makeApiCall({ url: `${url}v3/company/${companyID}/invoice/2/pdf?minorversion=59` , headers:{'Content-Type': 'application/pdf','Accept':'application/pdf'}, transport: popsicle.createTransport({type: 'buffer'})})

This returns a binary BLOB for the pdf response. Thanks for your help. I am good here 👍

@abisalehalliprasan abisalehalliprasan mentioned this pull request May 26, 2021
abisalehalliprasan added a commit that referenced this pull request May 27, 2021
* Update CHANGELOG.md

Update CHANGELOG.md for all releases.

* Adding Transport Override for PDF use case (#98)

* Update CHANGELOG.md (#95)

Update CHANGELOG.md for all releases.

* adding transport

* updating package name and version

* updating to use npm username in package

* fixing behavior and version

* removing package.json changes

Co-authored-by: abisalehalliprasan <38014312+abisalehalliprasan@users.noreply.github.com>

* Test PDF transport changes sdk release 3.0.3

* Release candidate 4.0.0

* Release 4.0.0 README update

Co-authored-by: Greg Haislip <54045190+haislip@users.noreply.github.com>
Co-authored-by: abisalehalliprasan <abisalehalliprasan@intuitdepc017d.corp.intuit.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants