Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

urvashi_vora created an issue. See original summary.

avpaderno’s picture

Priority: Normal » Minor
Status: Needs review » Needs work
 /**
  * @file
+ * Module book_access.
  */

The usual module description is Hook implementations for the [module name] module. where [module name] is replaced by the module name shown in the .info.yml file.

-  array_combine($node->getOwnerId() == 0 ? BookAccess::defaultGrants() : \Drupal::config('book_access.settings')->get("book_access_default_author_access"), $node->getOwnerId() == 0 ? BookAccess::defaultGrants() : \Drupal::config('book_access.settings')->get("book_access_default_author_access"))
+  array_combine($node->getOwnerId() == 0 ?
+  BookAccess::defaultGrants() :
+  \Drupal::config('book_access.settings')->
+  get("book_access_default_author_access"),
+  $node->getOwnerId() == 0 ?
+  BookAccess::defaultGrants() :
+  \Drupal::config('book_access.settings')->
+  get("book_access_default_author_access"))
   );

Code lines are not required to be shorter than 81 characters. Line length and wrapping, part of the Drupal coding standards, says:

  • Lines containing longer function names, function/class definitions, variable declarations, etc are allowed to exceed 80 characters.
  • Control structure conditions may exceed 80 characters, if they are simple to read and understand.

(Emphasis is mine.)

-  // $roleDefaults = variable_get( "book_access_default_role_{$rid}_access", array());
+  // $roleDefaults = variable_get(
+  "book_access_default_role_{$rid}_access", array());

That change is not correct for the same reason given for the previous change. Furthermore the "book_access_default_role_{$rid}_access", array()); part must be commented out too.

+  /* if (isset($node->book['bid']) && $node->book['bid']) {
+  // Check for existing permissions
+  //    $rowCount = \Drupal::database()->
+  // select('book_access_author', 'book_access_author')
+  //      ->condition( 'nid', $node->book['bid'], '=')
+  //      ->condition( 'uid', $node->getOwnerId())
+  //      ->countQuery()
+  //      ->execute()
+  //      ->fetchField();
+  if ($rowCount == 0) {
+  // @FIXME
+  // Could not extract the default value because it is either indeterminate, or
+  // not scalar. You'll need to provide a default value in
+  // config/install/book_access.settings.yml and
+  // config/schema/book_access.schema.yml.
+  // @FIXME
+  // Could not extract the default value because it is either indeterminate, or
+  // not scalar. You'll need to provide a default value in
+  // config/install/book_access.settings.yml and
+  // config/schema/book_access.schema.yml.
+  // BookAccess::setAuthorGrants(
+  //        $node->book['bid'],
+  //        $node->getOwnerId(),
+  //        array_combine($node->getOwnerId() == 0 ?
+  // BookAccess::defaultGrants() :
+  // \Drupal::config('book_access.settings')->
+  // get("book_access_default_author_access"),
+  // $node->getOwnerId() == 0 ? BookAccess::defaultGrants() :
+  // \Drupal::config('book_access.settings')->
+  // get("book_access_default_author_access"))
+  //      );

Commented out code must still be indented and formatted as per coding standards.

+  $roles = user_roles();
+  $rids = array_keys($roles);
+  $roleResultSet = \Drupal::database()->
+  // select('book_access_role', 'book_access_role')

Why is the call to select() commented out?

+  $roleResultSet = \Drupal::database()->
+  // select('book_access_role', 'book_access_role')
+  ->condition('nid', $node->book['bid'], '=')
+  ->condition('rid', $rids, 'IN')
+  ->fields('book_access_role', ['rid'])
+  ->distinct()
+  ->execute();

Those lines are not correctly indented.

+  // @FIXME
+  // // @FIXME
+  // // The correct configuration object could not be determined.
+  // You'll need to
+  // // rewrite this call manually.

Avoiding lines exceed 80 characters does not mean avoiding they exceed 20 characters.

+  // BookAccess::addRoleGrants( $node->book['bid'],
+  // array($rid), $roleDefaults);.

Periods are not appended to commented out code.

+ *   Form.
  * @param \Drupal\Core\Form\FormStateInterface $form_state
+ *   Form state.

The descriptions are each missing a definite article.

-  $items['node/%node/outline']['access callback'] = '_book_access_outline_access';
+  $items['node/%node/outline']['access callback'] =
+  'bookAccessOutlineAccess';
   }
-  if ($user = \Drupal::entityTypeManager()->getStorage('user')->load($row->gid)) {
+  if ($user = \Drupal::entityTypeManager()->
+  getStorage('user')->load($row->gid)) {

Code lines are not required to be shorter than 81 characters. Line length and wrapping, part of the Drupal coding standards, says:

  • Lines containing longer function names, function/class definitions, variable declarations, etc are allowed to exceed 80 characters.
  • Control structure conditions may exceed 80 characters, if they are simple to read and understand.

(Emphasis is mine.)

+ * The class BookAccess.
  */
-
 final class BookAccess extends EntityAccessControlHandler implements EntityHandlerInterface

Class descriptions must not start with The class or Class nor repeat the class name.

+  /**
+   * Creates an NodeViewController object.
+   *
+   * @param \Drupal\Core\Session\AccountInterface $current_user
+   *   The current user.
+   */
+  public function __construct(
+    AccountInterface $current_user = NULL
+  ) {

The description for a constructor must start with Constructs a new followed by the class name (including its namespace), and end with object.
The class name is also wrong, since that class is BookAccess.
Method declarations must be written in a single line.

-    drupal_write_record('book_access_author', $row, $bool ? array('nid', 'uid') : array());
+    drupal_write_record('book_access_author', $row, $bool ? ['nid', 'uid'] : []);

I am not sure it makes sense to make that change, since drupal_write_record() is not implemented in Drupal 9.5.

urvashi_vora’s picture

StatusFileSize
new44.4 KB
new6.82 KB

Hi @apaderno, I have made the required changes as per your feedback.

Please review.

Thanks

urvashi_vora’s picture

Status: Needs work » Needs review
a.aaronjake’s picture

Status: Needs review » Needs work

Hi @urvashi_vora,

Applied your patch not-so successfully, might be the reason errors are still thrown. Please see below.

book_access git:(1.0.x) curl https://www.drupal.org/files/issues/2023-05-19/coding-standard-fixes-3.patch | patch -p1
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 45468  100 45468    0     0  94912      0 --:--:-- --:--:-- --:--:-- 96740
patching file book_access.api.php
patching file book_access.module
patching file src/Access/BookAccess.php
patching file src/Routing/RouteSubscriber.php
patching file tests/src/Kernel/BookAccessDefaultsTest.php
Hunk #1 succeeded at 98 (offset 6 lines).
Hunk #2 succeeded at 114 (offset 7 lines).
➜  book_access git:(1.0.x) ✗ cd ..
➜  contrib git:(main) ✗ phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig book_access

FILE: ...mo-site/drupal-orgissue/web/modules/contrib/book_access/book_access.module
--------------------------------------------------------------------------------
FOUND 1 ERROR AND 5 WARNINGS AFFECTING 6 LINES
--------------------------------------------------------------------------------
   9 | ERROR   | [x] Use statements should be sorted alphabetically. The first
     |         |     wrong one is Drupal\Core\Session\AccountInterface.
  58 | WARNING | [ ] Line exceeds 80 characters; contains 85 characters
 109 | WARNING | [ ] Line exceeds 80 characters; contains 221 characters
 275 | WARNING | [ ] Line exceeds 80 characters; contains 91 characters
 375 | WARNING | [ ] Line exceeds 80 characters; contains 82 characters
 376 | WARNING | [ ] Line exceeds 80 characters; contains 84 characters
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------


FILE: .../drupal-orgissue/web/modules/contrib/book_access/src/Access/BookAccess.php
--------------------------------------------------------------------------------
FOUND 8 ERRORS AFFECTING 8 LINES
--------------------------------------------------------------------------------
   6 | ERROR | [x] Use statements should be sorted alphabetically. The first
     |       |     wrong one is Drupal\Core\Entity\EntityAccessControlHandler.
  40 | ERROR | [x] Multi-line function declarations must have a trailing comma
     |       |     after the last parameter
 440 | ERROR | [x] The first parameter of a multi-line function declaration
     |       |     must be on the line after the opening bracket
 441 | ERROR | [x] Multi-line function declarations must have a trailing comma
     |       |     after the last parameter
 730 | ERROR | [x] The first parameter of a multi-line function declaration
     |       |     must be on the line after the opening bracket
 732 | ERROR | [x] Multi-line function declarations must have a trailing comma
     |       |     after the last parameter
 764 | ERROR | [x] The first parameter of a multi-line function declaration
     |       |     must be on the line after the opening bracket
 765 | ERROR | [x] Multi-line function declarations must have a trailing comma
     |       |     after the last parameter
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 8 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------


FILE: ...e/drupal-orgissue/web/modules/contrib/book_access/src/BookAccessHelper.php
--------------------------------------------------------------------------------
FOUND 4 ERRORS AFFECTING 2 LINES
--------------------------------------------------------------------------------
 43 | ERROR | [x] The first parameter of a multi-line function declaration must
    |       |     be on the line after the opening bracket
 44 | ERROR | [x] Multi-line function declaration not indented correctly;
    |       |     expected 4 spaces but found 30
 44 | ERROR | [x] Multi-line function declarations must have a trailing comma
    |       |     after the last parameter
 44 | ERROR | [x] The closing parenthesis of a multi-line function declaration
    |       |     must be on a new line
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 4 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------

Time: 503ms; Memory: 14MB

Kindly check.

Thanks,
Jake

atul_ghate made their first commit to this issue’s fork.

avpaderno’s picture

Issue summary: View changes
atul_ghate’s picture

Status: Needs work » Needs review

joseph.olstad’s picture

joseph.olstad’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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