Skip to content

Conversation

@cameroncaci
Copy link

@cameroncaci cameroncaci commented Nov 15, 2024

Important

Jest does not support testing anything with pdfjs-dist
Jest's jsdom environment relies on jest-canvas-mock, and even then it's limited due to the canvas dependency. Canvas's pre-built binary is only supported in the pre-release and I have not seen Jest support for it
Please see the associated milmove playwright test for coverage at https://github.com/transcom/mymove/pull/14218

Ticket

B-21230

Summary

Fixed webpack chunking to enable MyMove support. Previously, when MyMove would import this exact branch, it was not possible to reach our webpack chunks. Since we dynamically import pdfjs-dist, webpack bundlers make it an entirely separate chunk, which is good. But MyMove just needed a unique output path and a script to handle the /dist/ output. See more in the MyMove PR https://github.com/transcom/mymove/pull/14218

Fixed web worker support by enabling better pdfjs task and lifecycle handling. This allows MyMove to swap between more than 1 PDF doc per orders without anything breaking.

Added rotation support for the PDF driver.

Technical Details

How to test

Note

The MyMove yarn portion will sound jank, and that's because it is and I don't know how to fix it. Here are the steps

Warning

Do not share the file or image of your locally CAC signed document. It contains PII

  1. Run brew install pkg-config cairo pango libpng jpeg giflib librsvg per https://github.com/Automattic/node-canvas/wiki/Installation:-Mac-OS-X
  2. Checkout this repo, run yarn install && export NODE_OPTIONS=--openssl-legacy-provider && yarn build
  3. Checkout this MyMove PR branch https://github.com/transcom/mymove/pull/14218
  4. Within MyMove, run rm -rf node_modules && yarn install && ./scripts/copy-react-file-viewer && ./scripts/rebuild-dependencies-without-binaries && make client_run
  5. Proceed with the next steps while you wait for the client to spin up.
  6. Plug in your CAC reader and CAC
  7. Download a PDF document with a CAC signature option
    a. One can be found here
  8. Sign the document with your CAC and save it
  9. Hopefully your client is spun up now. Login as a new customer
  10. Create a new orders and shipment, uploading your CAC signed form
  11. After submitting as the customer, login as SC to review
  12. As SC, navigate to your new move -> view/edit orders -> ensure you can see the CAC signature in the document
  13. As SC, click manage orders in the top right and upload a duplicate document of the CAC signature
  14. You should now have 2 documents you can view (Click the document icon in the top left to see the list)
  15. Switch between these two documents and ensure you can zoom in and out
  16. You should have been able to switch between documents without crashing
  17. Do the same but with rotating the document

Success!

test.pdf.screen.recording.mov

@cameroncaci cameroncaci self-assigned this Nov 18, 2024
@cameroncaci cameroncaci marked this pull request as ready for review November 18, 2024 20:14
Copy link

@danieljordan-caci danieljordan-caci left a comment

Choose a reason for hiding this comment

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

Niiiiiice! Able to view the CAC signed doc! LGTM!

Screenshot 2024-11-19 at 11 39 35 AM

Copy link

@traskowskycaci traskowskycaci left a comment

Choose a reason for hiding this comment

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

Worked great and I could see the signatures!

The only thing I wondered about was whether the orientation of the PDF should persist when swapping between multiple files (like rotating it one way, swapping to the other file, then swapping back to the rotated file but it's no longer rotated) - but that is such a nitpick I am leaving it up to you because I think it's fine :)

Looks good!

@cameroncaci
Copy link
Author

Worked great and I could see the signatures!

The only thing I wondered about was whether the orientation of the PDF should persist when swapping between multiple files (like rotating it one way, swapping to the other file, then swapping back to the rotated file but it's no longer rotated) - but that is such a nitpick I am leaving it up to you because I think it's fine :)

Looks good!

Oh no it absolutely should be a feature, it was missed when rotation saving was first added to milmove. I figured I'd at least restore some of the missing functionality while I was at it. Saving the persisting is def another enhancement BLI though

@traskowskycaci
Copy link

Worked great and I could see the signatures!
The only thing I wondered about was whether the orientation of the PDF should persist when swapping between multiple files (like rotating it one way, swapping to the other file, then swapping back to the rotated file but it's no longer rotated) - but that is such a nitpick I am leaving it up to you because I think it's fine :)
Looks good!

Oh no it absolutely should be a feature, it was missed when rotation saving was first added to milmove. I figured I'd at least restore some of the missing functionality while I was at it. Saving the persisting is def another enhancement BLI though

Agree that would be an enhancement!

@cameroncaci cameroncaci merged commit 7c3da43 into main Nov 20, 2024
1 check passed
@cameroncaci
Copy link
Author

Ref for future in case we return to this. This is where rotation and rotation saving was added to milmove https://github.com/transcom/mymove/pull/13572

@traskowskycaci
Copy link

Ref for future in case we return to this. This is where rotation and rotation saving was added to milmove transcom/mymove#13572

Would this be a possible enhancement to mention to Paul/Wendy as they plan upcoming features?

@cameroncaci
Copy link
Author

Ref for future in case we return to this. This is where rotation and rotation saving was added to milmove transcom/mymove#13572

Would this be a possible enhancement to mention to Paul/Wendy as they plan upcoming features?

Yes, good idea. I will mention this to them

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants