Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Part of meta #1853522: [META] (Re)introduce Views data integration for core modules. From the 8.x-3.x branch of Views.
Comment | File | Size | Author |
---|---|---|---|
#39 | interdiff.txt | 2.89 KB | dawehner |
#39 | vdc-1853526-38.patch | 10.99 KB | dawehner |
#37 | 1853526-37.patch | 10.92 KB | jibran |
#37 | interdiff.txt | 6.55 KB | jibran |
#34 | 1853526-34.patch | 10.67 KB | damiankloip |
Comments
Comment #2
dawehnerSome small cleanup.
Comment #3
dawehnerActually wrote a test and realized how much was actually broken, like it never actually appeared on some use-cases.
Comment #4
aspilicious CreditAttribution: aspilicious commentedTested this, works, code looks great, tests looks complete, comments look great.
Awesome!
Comment #5
dawehnerLeft the UUID in the test yml file.
Comment #6
catchIs this really the only way to check the access? It's an extra query for every row when you enable this. I realise contextual module does it as well but it's a horrible pattern. The only thing I can think of though is doing what contact module does in its own callback.
Comment #8
dawehnerWe could rewrite _contact_personal_tab_access to accept multiple accounts, though I'm really wondering whether it's worth to do so,
as N function calls on N rows is nothing compared to all other things going on like entity API.
Would you be okay with directly calling the access method of contact? We didn't used that in D7 because people might want to replace the access logic and so could do this in one place via hook_menu_alter.
Comment #9
catchI think directly calling the access method is probably OK. It's not that much different to calling node_access() really.
If you think that's too fragile, just adding a comment explaining why we have to use menu_get_item() there might be enough as well.
Comment #10
dawehnerTo be honest, this is such an edge case that to require overriding the views handler is fine for me.
Comment #12
dawehner#10: drupal-1853526-10.patch queued for re-testing.
Comment #14
dawehner#10: drupal-1853526-10.patch queued for re-testing.
Comment #16
dawehner#10: drupal-1853526-10.patch queued for re-testing.
Comment #17
dawehner#10: drupal-1853526-10.patch queued for re-testing.
Comment #18
dawehner#10: drupal-1853526-10.patch queued for re-testing.
Comment #20
dawehner#10: drupal-1853526-10.patch queued for re-testing.
Comment #22
dawehnerMoved some tiny bits around and fixed things here and there. The interdiff thought would be hard to read as files have been moved.
Comment #23
andypost#22: drupal-1853526-22.patch queued for re-testing.
Comment #25
dawehnerThis should fix it.
Comment #27
dawehnerNow!
Comment #28
andypostAssertLink is not sufficient here?
Comment #29
dawehnerYeah we could use it here, though it feels always safer for me, to check the full output here, as this adds additional kind of test coverage.
Feel free to convince me of the opposite, then I'm happy to rerole.
Comment #30
xjmI think it's fine as-is.
Comment #31
xjm#27: drupal-1853526-27.patch queued for re-testing.
Comment #32
webchick#27: drupal-1853526-27.patch queued for re-testing.
Comment #33
tstoecklerThat should be label, I think.
Comment #34
damiankloip CreditAttribution: damiankloip commentedGood catch, we def need to change this since the label patch landed :)
Comment #35
dawehner#34: 1853526-34.patch queued for re-testing.
Comment #37
jibranReroll. Unable to figure out
Uncaught PHP Exception Symfony\Component\DependencyInjection\Exception\InvalidArgumentException: "The service definition "plugin.manager.views.access" does not exist." at D:\xampp\htdocs\d8\core\vendor\symfony\dependency-injection\Symfony\Component\DependencyInjection\ContainerBuilder.php line 875
Comment #39
dawehnerI shouted so many bad words while fixing those.
Comment #40
andypostGreat!
Should be converted in #1938390: Convert contact_site_page and contact_person_page to a new-style Controller
Comment #41
alexpottCommitted 1d3e064 and pushed to 8.x. Thanks!