Issue #2359213: Clean up and comment code.

src/Component/Utility/FillPdf.php

@@ -8,6 +8,7 @@ use Drupal\views\Views;
 use \Symfony\Component\Process\Process;
 
 class FillPdf {
+
   public static function checkPdftkPath($pdftk_path = '') {
     // An empty value means we should leave it to the PATH.
     if (empty($pdftk_path)) {
@@ -39,7 +40,7 @@ class FillPdf {
 
     $view = Views::getView($name);
     if (!$view || !$view->access($display_id)) {
-      return;
+      return NULL;
     }
 
     return $view->preview($display_id, $args);

src/Controller/HandlePdfController.php

@@ -18,6 +18,7 @@ use Drupal\fillpdf\FillPdfContextManagerInterface;
 use Drupal\fillpdf\FillPdfFormInterface;
 use Drupal\fillpdf\FillPdfLinkManipulatorInterface;
 use Symfony\Component\DependencyInjection\ContainerInterface;
+use Symfony\Component\HttpFoundation\RedirectResponse;
 use Symfony\Component\HttpFoundation\RequestStack;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpFoundation\ResponseHeaderBag;
@@ -83,9 +84,11 @@ class HandlePdfController extends ControllerBase {
     // @todo: Emit event (or call alter hook?) before populating PDF. fillpdf_merge_fields_alter -> should be renamed to fillpdf_populate_fields_alter
 
     // TODO: Extract the PDF parameters, and act accordingly - this is pretty much just calling FillPdfBackendPluginInterface::populateWithFieldData and then reading the FillPdfForm options to determine whether to serve as a file download or fill in the field data.
+    /** @var FillPdfFormInterface $fillpdf_form */
     $fillpdf_form = FillPdfForm::load($context['fid']);
     if (!$fillpdf_form) {
       drupal_set_message($this->t('FillPDF Form (fid) not found in the system. Please check the value in your FillPDF Link.'), 'error');
+      return new RedirectResponse('/');
     }
 
     $fields = FillPdfFormField::loadMultiple(
@@ -103,7 +106,7 @@ class HandlePdfController extends ControllerBase {
     ];
 
     $mapped_fields = &$field_mapping['fields'];
-    $image_data = &$field_mapping['images'];
+//    $image_data = &$field_mapping['images'];
     foreach ($fields as $field) {
       $pdf_key = $field->pdf_key->value;
       if ($context['sample']) {
@@ -148,7 +151,10 @@ class HandlePdfController extends ControllerBase {
 
     // @todo: When Rules integration ported, emit an event or whatever.
 
-    // TODO: Take the appropriate action on the PDF.
+    // Determine the appropriate action for the PDF.
+
+
+    // TODO: figure out what to do about $token_objects. Should I make buildObjects manually re-run everything or just use the final entities passed of each type? Maybe just the latter, since that is what I do in
     return $this->handlePopulatedPdf($fillpdf_form, $populated_pdf, []);
   }
 
@@ -157,36 +163,49 @@ class HandlePdfController extends ControllerBase {
    *
    * Figure out what to do with the PDF and do it.
    *
-   * @param \Drupal\fillpdf\Controller\An|\Drupal\fillpdf\FillPdfFormInterface $fillpdf_form An object containing the loaded record from {fillpdf_forms}
-   * .
-   * @param $pdf_data A string containing the content of the merged PDF.
-   * @param array|\Drupal\fillpdf\Controller\An $token_objects An array of objects to be used in replacing tokens.
-   * Here, specifically, it's for generating the filename of the handled PDF.
-   * @param \Drupal\fillpdf\Controller\One|string $action One of the following keywords: default, download, save,
-   * redirect. These correspond to performing the configured action (from
-   * admin/structure/fillpdf/%), sending the PDF to the user's browser, saving it
-   * to a file, and saving it to a file and then redirecting the user's browser to
-   * the saved file.
-   * @param array|\Drupal\fillpdf\Controller\If $options If set, this function will always end the request by
-   * sending the filled PDF to the user's browser.
+   * @param FillPdfFormInterface $fillpdf_form
+   *   An object containing the loaded record from {fillpdf_forms}.
+   *
+   * @param string $pdf_data
+   *   A string containing the content of the merged PDF.
+   *
+   * @param array $token_objects
+   *   An array of objects to be used in replacing tokens.
+   *   Here, specifically, it's for generating the filename of the handled PDF.
+   *
+   * @param string $action
+   *   One of the following keywords: default, download, save,
+   *   redirect. These correspond to performing the configured action (from
+   *   admin/structure/fillpdf/%), sending the PDF to the user's browser, saving it
+   *   to a file, and saving it to a file and then redirecting the user's browser to
+   *   the saved file.
+   * @todo ^ Make this a plugin too
+   *
+   * @param array $options
+   *   If set, this function will always end the request by
+   *   sending the filled PDF to the user's browser.
+   *
+   * @return NULL|Response
    */
-  protected function handlePopulatedPdf(FillPdfFormInterface $fillpdf_form, $pdf_data, array $token_objects, $action = 'download', array $options = []) {
+  protected function handlePopulatedPdf(FillPdfFormInterface $fillpdf_form, $pdf_data, array $token_objects, $action, array $options = []) {
     // TODO: Convert rest of this function.
     $force_download = FALSE;
     if (!empty($option['force_download'])) {
       $force_download = TRUE;
     }
 
-    if (in_array($action, [
-        'default',
-        'download',
-        'save',
-        'redirect'
-      ]) === FALSE
-    ) {
+    $valid_actions = [
+      'default',
+      'download',
+      'save',
+      'redirect'
+    ];
+    if (!in_array($action, $valid_actions)) {
       // Do nothing if the function is called with an invalid action.
-      return;
+      // TODO: Add an assertion here?
+      return NULL;
     }
+
     // Generate the filename of downloaded PDF from title of the PDF set in
     // admin/structure/fillpdf/%fid
     $output_name = $this->buildFilename($fillpdf_form->title->value, $token_objects);
@@ -216,6 +235,7 @@ class HandlePdfController extends ControllerBase {
       case 'redirect':
         $redirect_to_file = $fillpdf_form->destination_redirect;
       case 'save':
+        // TODO: Port this to use its own function that in itself will return a file, period. Then handle the redirect logic in the controller instead of as part of the save-to-file method. Also base it off the new code from Drupal 7
         fillpdf_save_to_file($fillpdf_form, $pdf_data, $token_objects, $output_name, !$options, $redirect_to_file);
       // FillPDF classic!
       case 'download':

src/Entity/FillPdfForm.php

@@ -5,11 +5,15 @@
  */
 
 namespace Drupal\fillpdf\Entity;
+
+use Drupal\Core\Annotation\Translation;
+use Drupal\Core\Entity\Annotation\ContentEntityType;
 use Drupal\Core\Entity\ContentEntityBase;
 use Drupal\Core\Entity\EntityTypeInterface;
 use Drupal\Core\Field\BaseFieldDefinition;
 use Drupal\Core\Url;
 use Drupal\fillpdf\FillPdfFormInterface;
+use Drupal\fillpdf\Service\FillPdfAdminFormHelper;
 
 /**
  * Defines the entity for managing uploaded FillPDF forms.
@@ -41,7 +45,7 @@ class FillPdfForm extends ContentEntityBase implements FillPdfFormInterface {
    * {@inheritdoc}
    */
   public static function baseFieldDefinitions(EntityTypeInterface $entity_type) {
-    $fields = array();
+    $fields = [];
 
     $fields['fid'] = BaseFieldDefinition::create('integer')
       ->setLabel(t('FillPDF Form ID'))
@@ -64,19 +68,19 @@ class FillPdfForm extends ContentEntityBase implements FillPdfFormInterface {
     // @todo: what is wrong with the below?
     $fields['admin_title'] = BaseFieldDefinition::create('string')
       ->setLabel(t('Administrative description'))
-      ->setDescription(t('Enter the name of the form here, and it will be shown on the <a href="@overview_url">form overview page</a>. It has no effect on functionality, but it can help you identify which form configuration you want to edit.', array('@overview_url' => $overview_url)))
-      ->setDisplayOptions('form', array(
+      ->setDescription(t('Enter the name of the form here, and it will be shown on the <a href="@overview_url">form overview page</a>. It has no effect on functionality, but it can help you identify which form configuration you want to edit.', ['@overview_url' => $overview_url]))
+      ->setDisplayOptions('form', [
         'type' => 'string',
         'weight' => 0,
-      ));
+      ]);
 
     $fields['title'] = BaseFieldDefinition::create('string')
       ->setLabel(t('Filename pattern'))
       ->setDescription(t('Enter a title for this mapping configuration. This will be used for deciding the filename of your PDF. <strong>This field supports tokens.</strong>'))
-      ->setDisplayOptions('form', array(
+      ->setDisplayOptions('form', [
         'type' => 'string',
         'weight' => 10,
-        ));
+      ]);
 
     // @todo: Validate this with a custom constraint or whatever
     $fields['default_entity_type'] = BaseFieldDefinition::create('string')
@@ -87,10 +91,10 @@ class FillPdfForm extends ContentEntityBase implements FillPdfFormInterface {
     $fields['default_entity_id'] = BaseFieldDefinition::create('integer')
       ->setLabel(t('Default entity ID'))
       ->setDescription(t('The default entity ID to be filled from this FillPDF Form.'))
-      ->setDisplayOptions('form', array(
+      ->setDisplayOptions('form', [
         'type' => 'string',
         'weight' => 15,
-      ));
+      ]);
 
     // @todo: set display options on this
     $fields['destination_path'] = BaseFieldDefinition::create('string')
@@ -131,13 +135,13 @@ class FillPdfForm extends ContentEntityBase implements FillPdfFormInterface {
     $fields['destination_redirect'] = BaseFieldDefinition::create('boolean')
       ->setLabel(t('Redirect browser directly to saved PDF'))
       ->setDescription(t("<strong>This setting is applicable only if <em>Where to save generated PDFs</em> is set.</strong> Instead of redirecting your visitors to the front page, it will redirect them directly to the PDF. However, if you pass Drupal's <em>destination</em> query string parameter, that will override this setting."))
-      ->setDisplayOptions('form', array(
+      ->setDisplayOptions('form', [
         'type' => 'boolean_checkbox',
         'weight' => 30,
-        'settings' => array(
+        'settings' => [
           'display_label' => TRUE,
-        ),
-      ));
+        ],
+      ]);
 
     return $fields;
   }

src/FillPdfAccessController.php

@@ -8,8 +8,10 @@ namespace Drupal\fillpdf;
 
 use Drupal\Core\Access\AccessResult;
 use Drupal\Core\DependencyInjection\ContainerInjectionInterface;
+use Drupal\Core\Entity\EntityInterface;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 use Symfony\Component\HttpFoundation\RequestStack;
+use Drupal\Core\Session\AccountInterface;
 
 class FillPdfAccessController implements ContainerInjectionInterface {
 

src/FillPdfBackendPluginInterface.php

@@ -18,7 +18,7 @@ interface FillPdfBackendPluginInterface {
    *
    * @param FillPdfFormInterface $fillpdf_form The PDF whose fields
    *   are going to be parsed.
-   * @return array An array of FillPdfField entities containing metadata about
+   * @return array An array of FillPdfFormField entities containing metadata about
    *   the fields in the PDF. These can be iterated over and saved by the
    *   caller.
    */

src/Form/FillPdfFormDeleteForm.php

 <?php
 /**
-  * @file
-  * Contains \Drupal\fillpdf\Form\FillPdfFormDeleteForm.
-  */
+ * @file
+ * Contains \Drupal\fillpdf\Form\FillPdfFormDeleteForm.
+ */
 
 namespace Drupal\fillpdf\Form;
 

src/Form/FillPdfFormForm.php

@@ -46,7 +46,6 @@ class FillPdfFormForm extends ContentEntityForm {
       '#type' => 'details',
       '#title' => $this->t('Tokens'),
       '#weight' => 11,
-
       'token_tree' => $this->adminFormHelper->getAdminTokenForm(),
     );
 
@@ -81,14 +80,12 @@ class FillPdfFormForm extends ContentEntityForm {
       '#type' => 'fieldset',
       '#title' => $this->t('PDF form information'),
       '#weight' => $form['default_entity_id']['#weight'] + 1,
-
       'submitted_pdf' => array(
         '#type' => 'item',
         '#title' => t('Uploaded PDF'),
         '#description' => $file_entity->getFileUri(),
         '#weight' => $pdf_info_weight++,
       ),
-
       // @todo: make work
       'upload_pdf' => array(
         '#type' => 'file',
@@ -96,7 +93,6 @@ class FillPdfFormForm extends ContentEntityForm {
         '#description' => 'Update the PDF template used by this form',
         '#weight' => $pdf_info_weight++,
       ),
-
       'sample_populate' => array(
         '#type' => 'item',
         '#title' => 'Sample PDF',
@@ -108,7 +104,6 @@ class FillPdfFormForm extends ContentEntityForm {
           $this->t('If you have set a custom path on this PDF, the sample will be saved there silently.'),
         '#weight' => $pdf_info_weight++,
       ),
-
       'form_id' => array(
         '#type' => 'item',
         '#title' => 'Form Info',
@@ -123,8 +118,10 @@ class FillPdfFormForm extends ContentEntityForm {
         '#title' => 'Fill PDF from default node',
         '#description' => $this->l($this->t('Download this PDF filled with data from the default entity (@entity_type:@entity).',
             array(
-              '@entity_type', $entity->get('default_entity_type')->first()->value,
-              '@entity' => $entity->get('default_entity_id')->first()->value)
+              '@entity_type',
+              $entity->get('default_entity_type')->first()->value,
+              '@entity' => $entity->get('default_entity_id')->first()->value
+            )
           ),
             $this->linkManipulator->generateLink(array('fid' => $fid))) . '<br />' .
           $this->t('If you have set a custom path on this PDF, the sample will be saved there silently.'),
@@ -136,14 +133,16 @@ class FillPdfFormForm extends ContentEntityForm {
       '#type' => 'details',
       '#title' => $this->t('Additional settings'),
       '#weight' => $form['pdf_info']['#weight'] + 1,
-      '#open' => $entity->get('destination_path')->first()->value || $entity->get('destination_redirect')->first()->value,
+      '#open' => $entity->get('destination_path')
+          ->first()->value || $entity->get('destination_redirect')
+          ->first()->value,
     );
 
     $form['destination_path']['#group'] = 'additional_settings';
     $form['scheme']['#group'] = 'additional_settings';
     $form['destination_redirect']['#group'] = 'additional_settings';
 
-    $form['fillpdf_fields'] = FillPdf::embedView('fillpdf_form_fields',
+    $form['fillpdf_fields']['fields'] = FillPdf::embedView('fillpdf_form_fields',
       'block_1',
       $entity->id());
 

src/Form/FillPdfOverviewForm.php

@@ -5,12 +5,10 @@
  */
 namespace Drupal\fillpdf\Form;
 
-use Drupal\Component\Plugin\Exception\PluginNotFoundException;
 use Drupal\Core\Entity\Query\QueryFactory;
 use Drupal\Core\Extension\ModuleHandlerInterface;
 use Drupal\Core\File\FileSystemInterface;
 use Drupal\Core\Form\FormStateInterface;
-use Drupal\Component\Utility\String;
 use Drupal\Core\Session\AccountInterface;
 use Drupal\Core\Url;
 use Drupal\file\Entity\File;
@@ -27,7 +25,7 @@ class FillPdfOverviewForm extends FillPdfAdminFormBase {
   /**
    * The backend manager (finds the filling plugin the user selected).
    *
-   * @var \Drupal\fillpdf\FillPdfBackendManager
+   * @var FillPdfBackendManager
    */
   protected $backendManager;
 
@@ -104,20 +102,20 @@ class FillPdfOverviewForm extends FillPdfAdminFormBase {
           '#description' => 'Upload a PDF template to create a new form',
         ];
 
-        $form['submit'] = array(
+        $form['submit'] = [
           '#type' => 'submit',
           '#value' => $this->t('Upload'),
           '#weight' => 15,
-        );
+        ];
       }
       else {
-        drupal_set_message($this->t('You must install the <a href="@xmlrpc">contributed XML-RPC module</a> in order to use FillPDF Service as your PDF-filling method.', array('@xmlrpc' => Url::fromUri('https://drupal.org/project/xmlrpc')->toString())), 'error');
+        drupal_set_message($this->t('You must install the <a href="@xmlrpc">contributed XML-RPC module</a> in order to use FillPDF Service as your PDF-filling method.', ['@xmlrpc' => Url::fromUri('https://drupal.org/project/xmlrpc')->toString()]), 'error');
       }
     }
     else {
-      $form['message'] = array(
-        '#markup' => '<p>' . $this->t('Before you can upload PDF files, you must !link.', array('!link' => $this->l($this->t('configure FillPDF'), Url::fromRoute('fillpdf.settings')))) . '</p>',
-      );
+      $form['message'] = [
+        '#markup' => '<p>' . $this->t('Before you can upload PDF files, you must !link.', ['!link' => $this->l($this->t('configure FillPDF'), Url::fromRoute('fillpdf.settings'))]) . '</p>',
+      ];
       drupal_set_message($this->t('FillPDF is not configured.'), 'error');
     }
     return $form;
@@ -153,7 +151,7 @@ class FillPdfOverviewForm extends FillPdfAdminFormBase {
 
           $errors = file_validate_extensions($new_file, 'pdf');
 
-          if (!empty($errors)) {
+          if (count($errors)) {
             $form_state->setErrorByName('upload_pdf', $this->t('Only PDF files are supported, and they must end in .pdf.'));
           }
           else {
@@ -216,10 +214,10 @@ class FillPdfOverviewForm extends FillPdfAdminFormBase {
       $fillpdf_form_field->save();
     }
 
-    if (empty($fields)) {
-      drupal_set_message($this->t("No fields detected in PDF. Are you sure it contains editable fields?"), 'warning');
+    if (count($fields) === 0) {
+      drupal_set_message($this->t('No fields detected in PDF. Are you sure it contains editable fields?'), 'warning');
     }
 
-    $form_state->setRedirect('entity.fillpdf_form.edit_form', array('fillpdf_form' => $fillpdf_form->id()));
+    $form_state->setRedirect('entity.fillpdf_form.edit_form', ['fillpdf_form' => $fid]);
   }
 }

src/Form/FillpdfSettingsForm.php

@@ -52,8 +52,8 @@ class FillPdfSettingsForm extends ConfigFormBase {
 
     // Assemble service options. Warning messages will be added next as needed.
     $options = array(
-      'pdftk' => $this->t('Use locally-installed pdftk: You will need a VPS or a dedicated server so you can install pdftk: (!see_documentation).', array('!see_documentation' => $this->l($this->t('see documentation'),  Url::fromUri('http://drupal.org/documentation/modules/fillpdf')))),
-      'local' => $this->t('Use locally-installed PHP/JavaBridge: You will need a VPS or dedicated server so you can deploy PHP/JavaBridge on Apache Tomcat: (!see_documentation).', array('!see_documentation' => $this->l($this->t('see documentation'),  Url::fromUri('http://drupal.org/documentation/modules/fillpdf')))),
+      'pdftk' => $this->t('Use locally-installed pdftk: You will need a VPS or a dedicated server so you can install pdftk: (!see_documentation).', array('!see_documentation' => $this->l($this->t('see documentation'), Url::fromUri('http://drupal.org/documentation/modules/fillpdf')))),
+      'local' => $this->t('Use locally-installed PHP/JavaBridge: You will need a VPS or dedicated server so you can deploy PHP/JavaBridge on Apache Tomcat: (!see_documentation).', array('!see_documentation' => $this->l($this->t('see documentation'), Url::fromUri('http://drupal.org/documentation/modules/fillpdf')))),
       'fillpdf_service' => $this->t('Use FillPDF Service: Sign up for <a href="https://fillpdf-service.com">FillPDF Service</a>.'),
     );
 
@@ -114,10 +114,10 @@ class FillPdfSettingsForm extends ConfigFormBase {
    * {@inheritdoc}
    */
   public function validateForm(array &$form, FormStateInterface $form_state) {
-    if ($form_state->getValue('fillpdf_pdftk_path')) {
-      $status = FillPdf::checkPdftkPath($form_state->getValue('fillpdf_pdftk_path'));
+    if ($form_state->getValue('pdftk_path')) {
+      $status = FillPdf::checkPdftkPath($form_state->getValue('pdftk_path'));
       if ($status === FALSE) {
-        $form_state->setErrorByName('fillpdf_pdftk_path', $this->t('The path you have entered for
+        $form_state->setErrorByName('pdftk_path', $this->t('The path you have entered for
       <em>pdftk</em> is invalid. Please enter a valid path.'));
       }
     }

src/Service/FillPdfAdminFormHelper.php

 <?php
 /**
-  * @file
-  * Contains \Drupal\fillpdf\Service\FillPdfAdminFormHelper.
-  */
+ * @file
+ * Contains \Drupal\fillpdf\Service\FillPdfAdminFormHelper.
+ */
 
 namespace Drupal\fillpdf\Service;