Comments

benjy created an issue. See original summary.

benjy’s picture

Status: Active » Needs review
StatusFileSize
new7.84 KB

Here's a first pass.

bojanz’s picture

+  /**
+   * Configure the print engine with the passed entities.
+   *
+   * @param array $entities
+   *   An array of entities.
+   * @param \Drupal\entity_print\Plugin\PrintEngineInterface $print_engine
+   *   The print engine.
+   * @param bool $use_default_css
+   *   TRUE if we want the default CSS included.
+   *
+   * @return \Drupal\entity_print\Renderer\RendererInterface
+   *   A print renderer.
+   */
+  protected function configurePrintEngine(array $entities, PrintEngineInterface $print_engine, $use_default_css) {

I 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?

benjy’s picture

StatusFileSize
new7.83 KB
new1.79 KB

Thanks for the review, method name updated.

bojanz’s picture

Looks great now!

benjy’s picture

StatusFileSize
new7.83 KB
new440 bytes

Since 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.

sam152’s picture

Looking pretty good.

  1. +++ b/src/PrintBuilder.php
    @@ -89,4 +74,56 @@ class PrintBuilder implements PrintBuilderInterface {
    +    if (!$uri) {
    +      $uri = file_default_scheme() . '://' . $renderer->getFilename($entities) . '.' . $print_engine->getExportType()->getFileExtension();
    +    }
    

    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?

  2. +++ b/src/PrintBuilderInterface.php
    @@ -38,7 +38,27 @@ interface PrintBuilderInterface {
    +   * @param bool $force_download
    +   *   (optional) TRUE to try and force the document download.
    ...
    +  public function savePrintable(array $entities, PrintEngineInterface $print_engine, $uri, $force_download = FALSE, $use_default_css = TRUE);
    

    What does $force_download mean in the context of saving to disk?

benjy’s picture

I 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.

benjy’s picture

StatusFileSize
new8.02 KB
new2.22 KB

Updated 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.

sandeepguntaka’s picture

StatusFileSize
new2.4 KB

When 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.

Status: Needs review » Needs work

The last submitted patch, 10: 2843869_10.patch, failed testing.

sandeepguntaka’s picture

StatusFileSize
new8.08 KB
sandeepguntaka’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 12: 2843869_10.patch, failed testing.

benjy’s picture

Status: Needs work » Needs review
StatusFileSize
new10.02 KB
new2 KB
+++ b/src/PrintBuilder.php
@@ -89,4 +74,56 @@ class PrintBuilder implements PrintBuilderInterface {
+    $print_engine->getPrintObject()->render();

This 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.

Status: Needs review » Needs work

The last submitted patch, 15: 2843869-15.patch, failed testing.

benjy’s picture

Status: Needs work » Needs review
StatusFileSize
new10.17 KB
new3.21 KB

Changed to scheme and filename.

benjy’s picture

Status: Needs review » Fixed

Committed, thanks for all the reviews.

  • benjy committed 3458bcf on 8.x-2.x
    Issue #2843869 by benjy, sandeepreddyg, bojanz, Sam152: Provide an API...
ibuildit’s picture

Divine timing Benjy, just when I needed it. Thanks a lot all contributers!

bleeuwen’s picture

How 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??

benjy’s picture

There 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

bleeuwen’s picture

Thanks 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);

frmcclurg’s picture

Awesome! Perfect timing. This is exactly what we needed.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.