🇺🇸United States @lokapujya

quietone → credited lokapujya → .

#326 no longer applies, neither does #342

It would be even more helpful, in general for any issue, if folks can say how they tested a patch rather than just saying "works for me".

The detailed examples in the issue summary become documentation. This issue might never get resolved, unless it's first agreed how it should work. Where is the discussion from #337?

We need a 321-326 interdiff.

So we need to go with:

Show me a list of nodes I can view. If there are any attached nodes I can view, show additional data from those nodes.

which is the #282 fix? The tests will probably need to be updated. Are there any scenarios where this doesn't work and someone would intend the view to work the like the #302 fix? But first, we need an issue summary update because that's not what the issue summary reads.

The problem with the patch in 41/42/45 is that if you cannot access the attached node, it will deny access to the main node. I think that behavior is wrong.

I don't think we should fix this. That sounds like a feature request.

What might be helpful is if we could publicize the test case in the issue descriptions in simplest form possible so that we agree on how it should work. This would eventually become part of the documentation.
For example:

Left      Right   Role    Access
Public    Public  normal  granted
Public    [NULL]  normal  granted
Public    Private normal  ??

Changing to Needs Work because The description in the comment of the test needs some re-working anyway. For example:

1. +++ b/core/modules/node/src/Tests/NodeAccessJoinTest.php
   @@ -0,0 +1,322 @@
   +   * - Create user with "access content" and "create article" permissions which
   +   *   will be author having access to some private articles but not others.
   

   be an author having

2. +++ b/core/modules/node/src/Tests/NodeAccessJoinTest.php
   @@ -0,0 +1,322 @@
   +   * - Create pages with articles as reference for any combination of article
   

   ??

3. +++ b/core/modules/node/src/Tests/NodeAccessJoinTest.php
   @@ -0,0 +1,322 @@
   +   *   he is not the author of.
   

   he => the user

From the duplicate issue, Credit should also be attributed to: benjifisher, TechNikh, Ericmaster

Also, it's interesting the last D8 patch (#148 maybe) in this issue did not use a view just a db_select(). But I think I like the view better because it provides more test coverage.

+++ b/core/modules/node/src/Tests/NodeAccessJoinTest.php
@@ -0,0 +1,179 @@
+    $this->regularUser = $this->drupalCreateUser(['access content']);

For example, It was used here within the new file. So we might as well use it in all places.

We could remove some of the unneeded settings in the view to make it simpler, but since that is what the configuration system exported, maybe we should just leave it that way?

+++ b/core/modules/node/tests/modules/node_test_views/test_views/views.view.test_node_access_join.yml
@@ -0,0 +1,264 @@
+      fields:
+        title:
+          id: title
+          table: node_field_data
+          field: title
+          entity_type: node
+          entity_field: title
+          alter:
+            alter_text: false
+            make_link: false
+            absolute: false
+            trim: false
+            word_boundary: false
+            ellipsis: false
+            strip_tags: false
+            html: false
+          hide_empty: false
+          empty_zero: false

so many settings...

+++ b/core/modules/node/src/Tests/NodeAccessJoinTest.php
@@ -0,0 +1,179 @@
+    $node = $this->drupalCreateNode($edit);

variable $node does not get used anywhere.

Also, since this is a new file and we are using the shorthand array style in some places, we might as well use it consistently.