In #2267453: Views plugins do not store additional dependencies we added the ability for views plugins to expose their dependencies to the configuration system. This task will add dependencies from argument_validator and argument_default plugins by creating and configuring the plugins in ArgumentPluginBase::calculateDependencies() and returning the dependencies.

Files: 
CommentFileSizeAuthor
#13 2368767-13.patch18.9 KBWim Leers
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 84,851 pass(es). View

Comments

Wim Leers’s picture

Status: Postponed » Active
Issue tags: +VDC

We should've unpostponed this a long time ago.

Should this be critical?

xjm’s picture

Priority: Major » Critical
Issue tags: +D8 upgrade path, +Configuration system

Yeah there are probably config dependencies in there. :)

dawehner’s picture

Status: Active » Needs review
FileSize
4.05 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 84,536 pass(es), 1 fail(s), and 1 exception(s). View
4.05 KB

Well, I have seen major issues which have been way more critical, anyway.

Wim Leers’s picture

Issue tags: +Needs tests

Patch looks sane. Needs tests?

Status: Needs review » Needs work

The last submitted patch, 3: 2368767-3.patch, failed testing.

alexpott’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests
FileSize
14.99 KB
18.15 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 84,580 pass(es). View

Added test coverage.

I looked into adding content entity dependencies for when you provide a fixed argument for something like a UID or NID. Since it is perfectly possible to put non existent IDs here and they are not converted to UUIDs I think that is beyond the scope here and might even be undesirable.

dawehner’s picture

I looked into adding content entity dependencies for when you provide a fixed argument for something like a UID or NID. Since it is perfectly possible to put non existent IDs here and they are not converted to UUIDs I think that is beyond the scope here and might even be undesirable.

Yeah, the problem is that you don't even know what people might do with the number entered there. It could be that they use it for >comparisons, in which not validating the number as dependency would be the thing you want

xjm’s picture

Issue summary: View changes

Sounds like validation fun time in a child issue of #2392823: [meta] Much Views UI input is not validated.

For the specific entity dependencies, I do think that's worth a followup since the 80% usecase probably is referencing a specific entity, and the default argument could be a config or content entity depending on the plugin. It might wait on the entity referencing widget bit we talked about as a followup for #2341357: Views entity area config is not deployable and missing dependencies.

dawehner’s picture

FileSize
18.3 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 84,825 pass(es), 1 fail(s), and 1 exception(s). View
2.13 KB

Thank you alex for writing a test!

Let's expand the test coverage to argument validator plugins as well.

dawehner’s picture

FileSize
19.39 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 84,831 pass(es). View
1.09 KB

Forgot one new file.

The last submitted patch, 9: 2368767-9.patch, failed testing.

jibran’s picture

Status: Needs review » Reviewed & tested by the community

This is beautiful and elegant imo so RTBC.

+++ b/core/modules/views/tests/modules/views_test_data/config/schema/views_test_data.views.schema.yml
@@ -95,3 +95,10 @@ views.display_extender.display_extender_test:
+  type: ignore
...
+  type: ignore

I didn't know about that.

Wim Leers’s picture

FileSize
18.9 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 84,851 pass(es). View
929 bytes
+++ b/core/modules/views/src/Plugin/views/argument_validator/Entity.php
@@ -7,11 +7,13 @@
+use Drupal\Component\Plugin\Exception\PluginNotFoundException;
...
+use SebastianBergmann\Exporter\Exception;

Neither of these are used. Removed them.

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.0.x, thanks!

  • catch committed 5be46c3 on 8.0.x
    Issue #2368767 by dawehner, Wim Leers, alexpott: Implement...

Status: Fixed » Closed (fixed)

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