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

Attempt to switch Prince PDF call from axios to fetch #2018

Merged
merged 5 commits into from
Dec 19, 2023

Conversation

benmartin-coforma
Copy link
Contributor

Description

We recently discovered an issue in production where users are unable to print the export. That issue may come down to an incompatibility between the libraries we were using to sign the request. This PR trades those libraries for more basic, hopefully longer-lasting ones.

Note that we DO NOT use this API endpoint to print individual measures from their own pages; those print buttons use the browser's built-in print functionality. We may, at some point, wish to unify our printing approach; it feels odd to use Prince for some things and the browser for others.

Related ticket(s)

CMDCT-3150


How to test

  1. Log in and find yourself on the page for FFY 2023
  2. In the table of coresets, open one of the context menus under "Core Set Actions"
  3. Choose Export
  4. Click the orange "PRINT PDF" button at the top of the page

Important updates

n/a


Author checklist

  • I have performed a self-review of my code
  • I have added thorough tests, if necessary
  • [ ] I have updated relevant documentation, if necessary

Copy link

codeclimate bot commented Dec 19, 2023

Code Climate has analyzed commit 25ccb0b and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 0.0% (90% is the threshold).

This pull request will bring the total coverage in the repository to 73.1% (-0.3% change).

View more on Code Climate.

@BearHanded BearHanded marked this pull request as ready for review December 19, 2023 18:41
Copy link
Contributor

@BearHanded BearHanded left a comment

Choose a reason for hiding this comment

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

Confirmed the update works in the deployed branch. I removed the logging and extra packages Ben pointed out

Copy link
Contributor

@gmrabian gmrabian left a comment

Choose a reason for hiding this comment

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

also confirmed

@BearHanded BearHanded merged commit 56235fb into master Dec 19, 2023
106 of 107 checks passed
@BearHanded BearHanded deleted the fetch-prince-3150 branch December 19, 2023 19:16
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