error in schema field description in node_access table

I'm going to change it.

I have changed description to: "The realm in which the user must possess the grant ID. Each node access from module can define one or more realms."

Hi guys, how are you?

Thank you very much @abarrio.

Works Good.

+RTBC

Good Work and Good Week.

Regards

+++ b/core/modules/node/node.install
@@ -74,7 +74,7 @@ function node_schema() {
+        'description' => 'The realm in which the user must possess the grant ID. Each node access from module can define one or more realms.',

I think this should be Node access modules can define one or more realms.

Note this text is exactly the same in Drupal 7 and Drupal 6...

In fact it's probably better to have something like: Modules can define one or more realms in hook_node_grants(). otherwise someone has to work out what a node access module is.

Enclosed is a patch changing the wording as outlined in #8.

Wouldn't it be more clear to say
"Modules can define one or more realms by implementing hook_node_grants()."?
Or it's already clear enough?

@valthebald IMHO #11 is certainly an improvement

Change the text to the suggestion on the #11

Looks good for me.

Thanks people.

+RTBC

It is better explained than other patchs for me.

Thanks @faline!!

I've updated the issue credit. I credited myself and @valthebald for making suggestions that altered the final patch. I credited @joachim for discovering and creating the issue. @RenatoG thank you reviewing the patch. Providing a screenshot of the wording was not necessary to review the issue. The actual wording was what needed review.

I think we should add an update function to update the table comment on existing sites too. Given the fact that this description is not API I don;'t think we need an update path test BUT it is important that people can compare the actual schema to a module's computed schema and this patch breaks that at the moment.

Patch with update function.

Added the proper comments to functions.

Thanks for working on the update function.

1. +++ b/core/modules/node/node.install
   @@ -155,6 +155,8 @@ function node_uninstall() {
    /**
   + * Implements hook_update_N().
   + *
   
   @@ -177,6 +179,8 @@ function node_update_8001() {
    /**
   + * Implements hook_update_N().
   + *
   
   @@ -193,6 +197,8 @@ function node_update_8002() {
    /**
   + * Implements hook_update_N().
   + *
   
   @@ -222,6 +228,8 @@ function node_update_8003() {
    /**
   + * Implements hook_update_N().
   + *
   
   @@ -236,6 +244,8 @@ function node_update_8300() {
    /**
   + * Implements hook_update_N().
   + *
   
   @@ -248,6 +258,8 @@ function node_update_8301() {
    /**
   + * Implements hook_update_N().
   + *
   

   @th_tushar that change is not in scope for this patch. See https://www.drupal.org/core/scope for guidelines and examples for Drupal core issue scope.
   And not only is this not in scope it is actually wrong because the update system uses the first line of the method documentation to tell a user what updates are being done.

2. +++ b/core/modules/node/node.install
   @@ -246,3 +246,16 @@ function node_update_8301() {
   +/**
   + * Change {node_access}.realm description.
   + */
   

   As the is used to tell users what's happening we should probably not include {node_access} like this.

   How about
   Fix realm column description on the node_access table

3. +++ b/core/modules/node/node.install
   @@ -246,3 +246,16 @@ function node_update_8301() {
   +  Database::getConnection()->schema()->changeField('node_access', 'realm', 'realm', [
   +    'description' => 'The realm in which the user must possess the grant ID. Modules can define one or more realms by implementing hook_node_grants().',
   +    'type' => 'varchar_ascii',
   +    'length' => 255,
   +    'not null' => TRUE,
   +    'default' => '',
   +  ]);
   

   Instead of settings the entire schema here we should get the current schema for the column and just update the description. This means that if anyone has done anything interesting we won't unnecessarily break stuff. You'll need to use drupal_get_module_schema('node', 'node_access'); to get the current schema.

4. +++ b/core/modules/node/node.install
   @@ -246,3 +246,16 @@ function node_update_8301() {
   +}
   \ No newline at end of file
   

   Missing new line at end of file.

working on that

changing the last patch according #21

Tested the patch and looks good. Marking this as RTBC.

Committed 1058cb9 and pushed to 8.4.x. Thanks!

@th_tushar thank you for submitting a patch I explained in #21.1 the changes you made were out-of-scope and incorrect. If you had instead reviewed the worked of @c.nish2k3 in #18 and asked whether or not this was necessary and confirmed the patch worked and reviewed the docs it was adding you would be credited on this issue.

#17 explains other crediting decisions.

I've tested the update function myself - work great.