Problem/Motivation

Steps to reproduce:

- setup a view with a path like node/%node/foo
- use a conditional argument
- add validation on node type to the conditioanl argument
- add a local task entry for the view via the views UI

Expected result: the local task doesn't show up for invalid node types
Actual result: it does

Proposed resolution

Somehow translate the argument validation to access checking - we'd want to keep the behaviour of not loading the view though, and might not be possible to do this generically - i.e. we might have to create access checks 1-1 with validator plugins?

Remaining tasks

User interface changes

API changes

Data model changes

Comments

catch created an issue. See original summary.

dawehner’s picture

Yeah I think argument validators should be able to allow something similar to what \Drupal\views\Plugin\views\access\AccessPluginBase is doing.

larowlan’s picture

Yeah, I just debugged a similar issue on a project, we ended up going with a route subscriber to add additional access checks. This would be a great addition.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

sukanya.ramakrishnan’s picture

This would be a very important addition for us too. Looking fwd to this !

tetranz’s picture

Assigned: Unassigned » tetranz

I'm taking a stab at this.

tetranz’s picture

Status: Active » Needs review
FileSize
5.49 KB

I think this is close to what we need. Unless I'm missing something, a generic solution fell into place quite nicely once I figured out how.

A few notes and things that might require some discussion or confirmation.
I made it only apply when the fail validation behavior is set to "page not found" or "access denied". If someone really wants to display one of the others, i.e., all results a "no results" message or a summary then the menu should probably still appear.

This applies to page and feed displays because Drupal\views\Plugin\views\display\PathPluginBase::getRoute was a good place to put this. If feeds is considered inappropriate then I can move some code to Drupal\views\Plugin\views\display\Page although I'd have to refactor a little to make $argument_map available.

I'm still a little uncertain about what AccessResult is the correct one in the various situations although I think I have it correct mostly by studying what happens on a view with an access restriction. AccessResult::neutral() makes the menu link disappear.

I think AccessResult::allowed() is the appropriate result when we're trying to access the view itself although intuitively AccessResult::neutral() would seem more likely. AccessResult::allowed() appears to work correctly. If the view has a failed access restriction and a validation, it still fails with access denied as intended despite my code returning a AccessResult::allowed();

I will look into writing some tests. I think I need to import a new view configuration to test this or at least add a display to one of the existing test views.

Status: Needs review » Needs work

The last submitted patch, 8: views-args-access-2778345-8.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

tetranz’s picture

tetranz’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 10: views-args-access-2778345-10.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

tetranz’s picture

Status: Needs work » Needs review
FileSize
5.68 KB
1.07 KB
tetranz’s picture

tetranz’s picture

Assigned: tetranz » Unassigned

I was feeling my way a little with the test. I'm not sure if it's in the most appropriate place or should / could do something different.