Per the Module documentation guidelines, a module should Provide help text in the Drupal UI. This module is missing an implementation of hook_help().
Documentation:
https://www.drupal.org/docs/develop/documenting-your-project/module-docu...
https://www.drupal.org/node/632280
Patch to follow, thanks!

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alonaoneill created an issue. See original summary.

alonaoneill’s picture

Assigned: alonaoneill » Unassigned
Status: Active » Needs review
FileSize
1.37 KB
stefanos.petrakis@gmail.com’s picture

Status: Needs review » Needs work

Thanks for this, here is a first review:

--- a/webform_translation_permissions.module
+++ b/webform_translation_permissions.module
@@ -1,5 +1,29 @@
 <?php
+use Drupal\Core\Routing\RouteMatchInterface;

Needs an newline between the opening php tag and the use statement.

--- a/webform_translation_permissions.module
+++ b/webform_translation_permissions.module
@@ -1,5 +1,29 @@
 function webform_translation_permissions_config_translation_info_alter(&$info) {
   $info['webform']['class'] = 'Drupal\webform_translation_permissions\ConfigTranslation\WebformMapper';
-}
\ No newline at end of file
+}

Needs an newline between previous function and upcoming doc comment

--- a/webform_translation_permissions.module
+++ b/webform_translation_permissions.module
@@ -1,5 +1,29 @@
+      $output .= '<p>' . t('Defines the following permissions to enable a user to translate a webforms configuration without granting them the "translate configuration" permission needlessly.') . '</p>';
+
+            // Add a link to the Drupal.org project.

webform's instead of webforms
Unnecessary extra newline
Wrong indentation for the inline comment.

--- a/webform_translation_permissions.module
+++ b/webform_translation_permissions.module
@@ -1,5 +1,29 @@
+      $output .= t('Visit the <a href=":project_link">Webform Translation Permissions project page</a> on Drupal.org for more information.',[
+        ':project_link' => 'https://www.drupal.org/project/webform_translation_permissions'
+        ]);

Missing whitespace between the , and the [ characters.
Wrong indentation for the closing brackets and parenthesis.

Well, nothing really serious here to correct. :-)
Cheers

JeroenT’s picture

Status: Needs work » Needs review
FileSize
1.49 KB
1.95 KB

Fixed the suggestions made in #3 and since we are updating the .module file, I added a file comment and a comment that we're implementing `hook_config_translation_info_alter`.

stefanos.petrakis@gmail.com’s picture

Status: Needs review » Needs work

Thanks for the update.

The basic test coverage that was added thanks to your work on #3098717: Provide basic test coverage allowed testing the patch,
the bot reported the following coding standard messages:

webform_translation_permissions.module

  • 33: Case breaking statements must be followed by a single blank line
  • 34: Code after the RETURN statement on line 33 cannot be executed
  • 34: Line indented incorrectly; expected 4 spaces, found 6

I would also prefer if the break statement was removed, that would make the code a tiny bit neater.
Also, I would expect to find the help hook implementation function as the first function in the file (cannot explain that further, it's really subjective).

Otherwise, tests are green. :-)

JeroenT’s picture

Fixed the coding standards, removed the break statemend and moved hook_help to the top of the module file.

JeroenT’s picture

Status: Needs work » Needs review
stefanos.petrakis@gmail.com’s picture

Status: Needs review » Reviewed & tested by the community

Perfect, thanks a bunch. Going to commit this in a couple of minutes.

  • stefanos.petrakis committed f55cebf on 8.x-1.x
    Issue #3069498 by JeroenT, alonaoneill, stefanos.petrakis: Hook Help is...
stefanos.petrakis@gmail.com’s picture

Status: Reviewed & tested by the community » Fixed

Commited and fixed. Thanks again to the both of you!

FYI: I intend to make a proper release out of the current dev branch, since it's minimally in a production-ready state (thanks to the tests).

stefanos.petrakis@gmail.com’s picture

@JeroenT: Missed #3099279: Roadmap for stable release, agree completely with the roadmap.

Status: Fixed » Closed (fixed)

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