In the View for the My Edits tab, the title link is set to " Link this field to its content revision." This setting turns the link into the format:
node/NID/revision/VID
But if there is only one revision on the node, this page returns Access Denied due to the logic in _node_revision_access():
if ($is_current_revision && (db_query('SELECT COUNT(vid) FROM {node_revision} WHERE nid = :nid', array(':nid' => $node->nid))->fetchField() == 1 || $op == 'update' || $op == 'delete')) {
IMO, this is a Views bug, but we should simply work around it and link to the proper Node view page, as expected.
Why don't we just use the non-revisioned titles here?
| Comment | File | Size | Author |
|---|---|---|---|
| #21 | views_current_vid_link-1862014-d8.patch | 2.29 KB | quicksketch |
| #20 | views_current_vid_link.patch | 2.19 KB | quicksketch |
| #14 | vdc-1862014-14.patch | 917 bytes | tim.plunkett |
| #9 | views-1862014-9.patch | 860 bytes | tim.plunkett |
| #8 | views-1862014-8.patch | 866 bytes | tim.plunkett |
Comments
Comment #1
agentrickardAnd a patch.
Comment #3
bbinkovitz commentedKen, can you reroll this patch to not change the name of the view, as that may break other things? Or would you like me to try doing this?
Comment #4
agentrickardHm. I thought it was safer to change the view to a new file. If you can re-roll that way, please do.
I'd also like to get @stevector's thoughts about why this View uses the node_revision base table.
Comment #5
tim.plunkettThis looks like it might be a bug in Views in D7 and D8?
Compare http://drupalcode.org/project/views.git/blob/refs/heads/7.x-3.x:/modules... and http://drupalcode.org/project/views.git/blob/refs/heads/7.x-3.x:/modules...
Not sure why the first handler even exists? I must be missing something.
Comment #6
tim.plunkettAlso, rerolling the patch so the diff means something would be great.
Comment #7
tim.plunkettI think http://drupalcode.org/project/drupal.git/blob/HEAD:/core/modules/node/li... might have the same problem, checking with the master :)
Comment #8
tim.plunkettWell, let's try this.
Comment #9
tim.plunkettWhoops, I meant this
Comment #10
agentrickardThat seems to fix the issue.
Comment #12
agentrickardComment #13
dawehnerAwesome, let's port that to d8.
Comment #14
tim.plunkettStill needs tests.
Comment #15
agentrickardSorry. This doesn't actually work. The logic is too simple:
This is an insufficient check, because if you create node 1, it works. Then revision node 1 to have vid 2.
Then create node 2, it gets vid 3 and the link breaks again.
The way core works this in D7 is to literally check the COUNT of rows in the {node_revision} table. Without patching core, I'm not sure we have any choice but to do that.
Comment #16
stevectorYeah, this logic is not sufficient. The real question is "is this vid which is coming from {node_revision} also in {node}"
I've got a patch in the State Machine queue that is grappling with the same issue:
#1775540: Additional fields joins break with the latest version of Views
That handler is asking "is this vid which is coming from {node_revision} also in {node} and does it have a status of 1" The patch includes a test, which is now passing thanks to ugly additional queries that directly check the node table. This is needed there because these broke with a recent Views update.
Tim or dawehner, do know what could be done in the handler or elsewhere to get this additional field piece working again? If so, that could be the fix here as well as in the State Machine patch.
Comment #17
agentrickard@stevector
Even that condition can cause a break in expected functionality. The vid may be in both tables and the link would still take you to an access denied, because the access check is for COUNT of rows in {node_revision}.
Comment #18
stevectoragentrickard, I think I wasn't clear in my last comment. If the vid for the given result is in both tables, then the link should go to node/1. If the vid for this result is not in both tables then there must be more than one vid (or something else has gone seriously wrong) and this vid is not the one in the node table so the link should go to node/%/revision/%/view.
Comment #19
agentrickardYes.
Comment #20
quicksketchHere's a D7 patch for Views. I'll work on porting to D8.
Comment #21
quicksketchD8 port. I have to say: the D8 version of Views is refreshingly similar to the D7 module.
Comment #22
danmuzyka commentedAssigning.
Comment #23
danmuzyka commentedRan out of time during code sprint. Assigning back to anonymous.
Comment #24
dawehnerI don't see how this can cover all the cases, see issue summary.
Comment #24.0
dawehnerClarifies.
Comment #25
xjm(Merging "node system" and "node.module" components for 8.x; disregard.)
Comment #34
mstrelan commentedThis may not be needed if #3226487: Always allow access to revisions based on access/permissions, not on the count of revisions is committed.
Comment #35
acbramley commentedThis actually didn't need the fix above anymore due to how the link is rendered
If there is only 1 revision, it will be the default revision and therefore the link will be the canonical url. Otherwise it will link to the revision page.
If we are talking about a different handler, it will be fixed by the issue above.
Please re-open if I've made a mistake :)