-
Notifications
You must be signed in to change notification settings - Fork 66
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
[WIP] pdf conversion job #951
Conversation
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.
LGTM a few small comments
RSpec.describe DocumentUploads::PdfConversionJob do | ||
describe '#perform' do | ||
before(:all) do | ||
@file_path = Rails.root.join('spec/fixtures/files/pdf_conversion.jpg') |
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 think it's supposed to be
@file_path = Rails.root.join('spec', 'fixtures', 'files', 'pdf_conversion.jpg')
in the unlikely even that this spec is run on an OS where /
is not the default separator
before(:all) do | ||
@file_path = Rails.root.join('spec/fixtures/files/pdf_conversion.jpg') | ||
@file_path_converted = Rails.root.join('spec/fixtures/files/pdf_conversion.jpg.pdf') | ||
MiniMagick::Tool::Convert.new do |convert| |
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.
This is creating the .jpg right? Can this be abstracted into a helper for readability?
I suspect we're going to need |
@markolson cc @jcmeloni-usds @J-Quag are word docs a requirement for the first iteration of document uploads? Devops spidey-sense understandably being triggered by allowing files with macros. We'll have a virus scan that should block those files but it's safer to do only lossless conversion, as with image to PDF, than word to PDF which requires Office or OSS clone (LibreOffice) to reliably convert. |
So it is an interesting question. In EVSS we actually can’t do anything with Word, Excel, PPT, etc due apparently to licensing issues.
If there is a macro concern, let’s make sure we don’t have any vulnerabilities there. If Word is a problem, it hasn’t been something we’ve allowed, but would like to add it if its not a risk.
From: Alastair Dawson [mailto:notifications@github.com]
Sent: Thursday, May 18, 2017 3:49 PM
To: department-of-veterans-affairs/vets-api
Cc: Quagliaroli, Joshua, VBAVACO; Mention
Subject: [EXTERNAL] Re: [department-of-veterans-affairs/vets-api] [WIP] pdf conversion job (#951)
@markolson<https://github.com/markolson> cc @jcmeloni-usds<https://github.com/jcmeloni-usds> @J-Quag<https://github.com/j-quag> are word docs a requirement for the first iteration of document uploads? Devops spidey-sense understandably being triggered by allowing files with macros. We'll have a virus scan that should block those files but it's safer to do only lossless conversion, as with image to PDF, than word to PDF which requires Office or OSS clone (LibreOffice) to reliably convert.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#951 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/APJNWT3kzqhpWMNvmFGQzT-Nv10GLHBsks5r7KC7gaJpZM4NLi0S>.
|
HCA backend allowable types look like Word, PDF, RTF and JPG. Not sure what EDU supports, but we can probably just say TXT/RTF, Image and PDF and maybe have a link to print-to-pdf instructions for Word? |
New PR for this (as workflow task) against master instead of topic branch |
* [#951] create new route for Virtual Agent JWT proof of concept [original issue 951](https://github.com/department-of-veterans-affairs/va-virtual-agent/issues/951) Co-authored-by: Maurice Okumu <maurice.okumu@thoughtworks.com>
department-of-veterans-affairs/va-virtual-agent#951 1. Removed code associated with the POC. 2. Removed a no-longer-needed feature flag. Co-authored-by: Raina Huerta <raina.huerta@thoughtworks.com>
* [#951] create new route for Virtual Agent JWT proof of concept [original issue 951](https://github.com/department-of-veterans-affairs/va-virtual-agent/issues/951) Co-authored-by: Maurice Okumu <maurice.okumu@thoughtworks.com>
No description provided.