| Comment | File | Size | Author |
|---|---|---|---|
| #17 | interdiff.txt | 3.21 KB | benjy |
| #17 | 2843869-17.patch | 10.17 KB | benjy |
| #15 | interdiff.txt | 2 KB | benjy |
| #15 | 2843869-15.patch | 10.02 KB | benjy |
| #12 | 2843869_10.patch | 8.08 KB | sandeepguntaka |
| Comment | File | Size | Author |
|---|---|---|---|
| #17 | interdiff.txt | 3.21 KB | benjy |
| #17 | 2843869-17.patch | 10.17 KB | benjy |
| #15 | interdiff.txt | 2 KB | benjy |
| #15 | 2843869-15.patch | 10.02 KB | benjy |
| #12 | 2843869_10.patch | 8.08 KB | sandeepguntaka |
Comments
Comment #2
benjy commentedHere's a first pass.
Comment #3
bojanz commentedI was confused by the method name before I saw what's inside. Especially since it mentions a print engine but the variable used is $renderer. Maybe prepareRenderer() would make more sense?
Comment #4
benjy commentedThanks for the review, method name updated.
Comment #5
bojanz commentedLooks great now!
Comment #6
benjy commentedSince these are unmanaged files and might never get cleaned up, i've gone with a replace action. I'm presuming the most common use case would let the renderer generate the filename anyway.
Comment #7
sam152 commentedLooking pretty good.
is putting them in public:// by default safe? If someone misses the param of it's a falsey path, it writes them to public where entity access can't be checked?
What does $force_download mean in the context of saving to disk?
Comment #8
benjy commentedI did wonder about generating the files into public myself, I justified it with the fact that the API isn't used anywhere in this module so whatever actually triggers the generate of the file could handle securing it if required? I'll add a note to the comment block.
Good catch, i'll remove that parameter.
Comment #9
benjy commentedUpdated comments and remove parameter.
One other thing I've realised is that you can't have the filename generated for you if you want to use private files as you have to pass the full $uri. It might be worth having $file_name and $scheme parameters for slightly better flexibility.
Comment #10
sandeepguntaka commentedWhen i tried to save a pdf to public files dir i was getting a empty pdf. May be the data is not being rendered. I tried to call render() over the output print object. And there was a error in prepareRenderer() which i ve corrected. Correct me if i'm wrong.
Comment #12
sandeepguntaka commentedComment #13
sandeepguntaka commentedComment #15
benjy commentedThis is wrong, we can't ever rely on anything within the print object itself.
What you're describing is a bug with the Dompdf implementation that was introduced with the getBlob() method, Dompdf can only support rendering once, so lets keep track of that. Fix attached.
Comment #17
benjy commentedChanged to scheme and filename.
Comment #18
benjy commentedCommitted, thanks for all the reviews.
Comment #20
ibuildit commentedDivine timing Benjy, just when I needed it. Thanks a lot all contributers!
Comment #21
bleeuwen commentedHow to call this API directly
I do not want to tweak the module, I build an extension of EntityPrintController and made here the call to savePrintable. I only get it working via a route because of the abundance of parameters. Is there a way to call savePrintable directly, without routing??
Comment #22
benjy commentedThere are only 2 required parameters, the print engine you want to use and the array of entities. You can see a working code sample here which shows how you might attach the entity to an email: https://www.drupal.org/node/2831952#comment-11871535
Comment #23
bleeuwen commentedThanks Benyi
to make a PDF of a node I can do the following now:
$print_engine = \Drupal::service('plugin.manager.entity_print.print_engine')->createSelectedInstance('pdf');
$print_builder = \Drupal::service('entity_print.print_builder');
$nodes = \Drupal::entityTypeManager()->getStorage('node')->load($node_id);
$uri = $print_builder->savePrintable([$nodes], $print_engine);
Comment #24
frmcclurg commentedAwesome! Perfect timing. This is exactly what we needed.