Use commerce_pos jQuery print library in receipt submodule

The labels issue actually added the library to commerce_pos so we just need to change the receipt module to use that one instead of its own.

+    $path = base_path() . '/libraries/jQuery.print';
+    if (!file_exists($path)) {

The required file is /libraries/jQuery.print/jQuery.print.js (see commerce_pos.libraries.yml) so we could check for that rather that just the jQuery.print directory.

+  if ($phase == 'install') {
+    $path = base_path() . '/libraries/jQuery.print';
+    if (!file_exists($path)) {
+      drupal_set_message(t('jQuery.print library is not installed.'), 'error');

hook_requirements()'s return value is used to indicate what's wrong, whether the requirement is installed, and a severity. That could be better than a message during install and could be used during the runtime phase too. See https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Extension...

I think this should also be added to the changes in the previous patch rather than a new patch that only adds the requirements hook.

The issues with trailing whitespace mentioned in #10 are still there. Git points them out when applying the patch:

$ git apply commerce_pos-provide_a_jquery_print_library-2924548-15.patch
commerce_pos-provide_a_jquery_print_library-2924548-15.patch:11: trailing whitespace.
This module depends on the `jQuery.print` plugin which should reside in your 
commerce_pos-provide_a_jquery_print_library-2924548-15.patch:26: trailing whitespace.
3. Add `"libraries/{$name}": ["type:drupal-library"]` 
commerce_pos-provide_a_jquery_print_library-2924548-15.patch:52: trailing whitespace.
    }   
commerce_pos-provide_a_jquery_print_library-2924548-15.patch:59: trailing whitespace.
2. `cd` into libraries folder and run 
warning: 4 lines add whitespace errors.

The hook_requirements implementation needs a bit of work.

+function commerce_pos_receipt_requirements($phase) {
+  $requirements = [];
+  if ($phase == 'install') {
+      $path = base_path() . '/libraries/jQuery.print/jQuery.print.js';
+    if (!file_exists($path)) {
+      $requirements['commerce_pos_receipt'] = [
+        'description' => t("jQuery.print library is not installed."),
+        'severity' => REQUIREMENT_ERROR,
+      ];
+    }
+  }
+
+  return $requirements;
+}

From https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Extension... we know that REQUIREMENT_ERROR prevents intall:

If a requirement has a severity of REQUIREMENT_ERROR, install.php will abort or at least the module will not install.

I don't think we should prevent install. Let's make it a warning so the user knows the next step, but isn't prevented from installing just because they didn't place the js library in its home yet.

There's also no check during the runtime phase yet. Let's add that so the Drupal status page reports whether the dependency is there or not.

The label module also uses this library so the hook_requirements should probably go in the main module, not the receipt one. I suggest looking at whether either submodule is installed and only warning at runtime if either actually exists.

The patch needs to address Alex's feedback about removing the documentation from the receipt module's README so it's not in two places.

Deepak, there's feedback that is still not addressed from #17 - there's no checking of requirements during the runtime phase so the dependency doesn't show on the status report page.

There's also no check during the runtime phase yet. Let's add that so the Drupal status page reports whether the dependency is there or not.

The label module also uses this library so the hook_requirements should probably go in the main module, not the receipt one. I suggest looking at whether either submodule is installed and only warning at runtime if either actually exists.

Here's a hook_requirements implementation from the Drupal 7 version of this module for an example of checking requirements during the runtime phase.

/**
 * Implements hook_requirements().
 */
function commerce_pos_payment_requirements($phase) {
  $requirements = array();
  $payments_available = commerce_pos_get_payment_options();
  if ($phase == 'runtime') {
    if ($payments_available) {
      $requirements['commerce_pos_payment'] = array(
        'title' => t('Commerce Point of Sale Payment'),
        'severity' => REQUIREMENT_OK,
        'value' => t('Payment Methods Configured'),
      );
    }
    else {
      $link = l(t('configure payment methods here.'), '/admin/commerce/config/custom-offline-payments');
      $requirements['commerce_pos_payment'] = array(
        'title' => t('Commerce Point of Sale Payment'),
        'severity' => REQUIREMENT_WARNING,
        'value' => t("No payment methods configured. Enable Commerce POS Payments for default payment methods or !link", array('!link' => $link)),
      );
    }
  }

  return $requirements;
}

That example brings to mind something else that could improve with this patch. As an error it says "jQuery.print library is not installed.". That doesn't tell the user what to do, it just says what is. In the D7 version's commerce_pos_payment_requirements it describes the state when everything is OK "Payment methods configured" but helps the user figure out what to do if something is wrong: "No payment methods configured. Enable Commerce POS Payments for default payment methods or !link".

I had in mind that we'd be checking for the receipt/labels modules in commerce_pos_requirements() rather than make another module but now it doesn't seem like a great idea. There's already a third module that uses the print library and it's probably no good to have to remember to update the requirements hook every time something else finds a use for the library.

I chatted with Deepak and we decided it'd be better to go with another module as a bridge for this as Alex suggested in #11.

• Make a print submodule (commerce_pos_print?).
• Put the readme/hook_requirements() in commerce_pos_print.
• Little update to receipt, label, reports submodules to use the other module.

The hook_help() implementation looks like it was taken right from commerce_pos_label; it calls a function from that module.

    // Main module help for the commerce_pos_label module.
    case 'help.page.commerce_pos_print':
      $output = '';
      $output .= '<h3>' . t('About') . '</h3>';
      $output .= '<p>' . t('Provides the ability to give print functionality for other sub-modules.') . '</p>';

      $label_formats = commerce_pos_label_get_label_formats();

      // Print out our currently defined label formats.

There's still references to commerce_pos/jQuery.print in the report and label module. They would have to use commerce_pos_print/jQuery.print now.

There shouldn't be any space between the two closing parentheses here.

    $path = base_path() . '/libraries/jQuery.print/jQuery.print.js';
    if (!file_exists($path) ) {
      $requirements['commerce_pos_print'] = [

I'm still not a fan of the description here because it doesn't tell the user what to do.

        'description' => t("jQuery.print library is not installed."),

How about something like "To print receipts, labels, and reports you must install the jQuery.print library."? Typically descriptions also link to somewhere but since we don't have documentation to link to and there's no way to install it from the UI we can assume people will look into the README at that point.

The patch makes all the sub-modules depend on the new print one. Do modules like commerce_pos_receipt actually require the print library? If not, they should recommend the print module, not depend on it. At this point I'm not sure if they already require it and making it optional could be a future enhancement or if we need to leave the print module as optional right now.