Skip to content

Fix issue of trying to create new PDFs in preexisting directory. - #2

Open
dustin-lennon wants to merge 26 commits into
bypasslane:masterfrom
dustin-lennon:master
Open

Fix issue of trying to create new PDFs in preexisting directory.#2
dustin-lennon wants to merge 26 commits into
bypasslane:masterfrom
dustin-lennon:master

Conversation

@dustin-lennon

Copy link
Copy Markdown

PDFs would fail to be created due to save directory already existing and current code cannot do a make directory with a folder of the same name. Pull request checks if directory exists and if it does not creates the directory and does the save to the directory. If the directory does exist, saves the PDF to the directory.

@txssseal

Copy link
Copy Markdown
Contributor

@deejayres @sdiachenko @B-3PO Some additonal pr's from people using this library. May help, please see if this will cause any breaking changes.

@deejayres

Copy link
Copy Markdown

@absalas

Comment thread lib/pdfGenerator.js Outdated

console.log('Files: ', files);
console.log('PDFtk Options: ', pdftkOptions);
console.log('Function test: ', convertObjString(pdftkOptions));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can we get rid of the console logs?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

done

Comment thread lib/saveFile.js
console.error(err)
return;
}
// Check if directory being written to already exists

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👏

Comment thread lib/pdfGenerator.js
var pexec = Promise.promisify(exec);

module.exports = function generatePDF(options, templates, fileName, pdftkPath) {
module.exports = function generatePDF(options, templates, fileName, pdftkPath, pdftkOptions) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@absalas in some of your changes you will need to take into account his new changes

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

specifically passing in the whole options object

@deejayres

Copy link
Copy Markdown

Heyo, we're in the middle of the process of deploying features that depend on this, and are trying to minimize the moving parts until we can flex on this in prod. Honestly had no idea this was an open repo. We can keep the PR open, but it might be a while before we can approve.

snyk-test and others added 4 commits July 4, 2019 02:16
The following vulnerabilities are fixed with a Snyk patch:
- https://snyk.io/vuln/SNYK-JS-LODASH-450202
…bf4579e17f5b8

[Snyk] Fix for 1 vulnerable dependencies
The following vulnerabilities are fixed with a Snyk patch:
- https://snyk.io/vuln/SNYK-JS-LODASH-450202
…e2ec803451aab

[Snyk] Fix for 1 vulnerable dependencies
@aybhalala

Copy link
Copy Markdown

Any ETA when will this be merged?

The following vulnerabilities are fixed with an upgrade:
- https://snyk.io/vuln/SNYK-JS-UGLIFYJS-1727251
The following vulnerabilities are fixed with an upgrade:
- https://snyk.io/vuln/SNYK-JS-HTMLPDF-467248
The following vulnerabilities are fixed with an upgrade:
- https://snyk.io/vuln/SNYK-JS-EJS-6689533
The following vulnerabilities are fixed with an upgrade:
- https://snyk.io/vuln/SNYK-JS-PUGCODEGEN-7086056
…933ca6dd945830

[Snyk] Security upgrade pug from 2.0.4 to 3.0.0
…f9a1e42bae1eea

[Snyk] Security upgrade ejs from 2.7.4 to 3.1.10
…eab14c0080e9fe

[Snyk] Security upgrade html-pdf from 2.2.0 to 3.0.1
…584e1f5e733198

[Snyk] Security upgrade pug from 2.0.4 to 3.0.0
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.

5 participants