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

Various changes #17

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

Various changes #17

wants to merge 17 commits into from

Conversation

MGotink
Copy link

@MGotink MGotink commented Mar 25, 2014

I've been working on various changes for the gem:

  • Allow parameters in a URL
  • Don't set the full (server side) PDF path in the session
  • Lots of refactoring especially in the Middleware class
  • Various smaller changes

Martin

eriktiemens and others added 17 commits March 25, 2014 13:59
If a URL with parameters (like http://example.org/test.pdf?x=42) is requested generating the PDF will fail because the .pdf extension is not stripped from the URL.

The regular expression for replacing the .pdf extension now correctly handles this case.
Only the path of the URL is used to create a unique output file name, but different URL parameters might result in different contents so a different file name should be used.

The whole URL (including the parameters) will now be used to generate a unique file name.
The complete path to the PDF being rendered is being set in the session, making this path visible to the public.

Now only the PDF filename is being set in the session.
@benrudolph
Copy link

👍

@cpursley
Copy link

Is there a status update on this and merging other potential PRs? Thanks!

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.

None yet

4 participants