Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 UTC on 18 March 2024, to get $100 off your ticket.
#1839516: Introduce entity operation providers hasn't happened yet, but that doesn't mean we can't have VBO support for our existing operation APIs.
Comment | File | Size | Author |
---|---|---|---|
#54 | empty_content.png | 15.21 KB | andypost |
#49 | Content__Content____Site-Install.png | 95.27 KB | alexpott |
#46 | node-1895160-46.patch | 45.25 KB | tim.plunkett |
#18 | content-view.png | 30.89 KB | irinaz |
#14 | before.png | 11.04 KB | dawehner |
Comments
Comment #1
tim.plunkettAll of the feedback I've been getting for #1894972: Provide a bulk form for user operations is valid here too, and the code is almost identical. I might as well wait until that one is polished.
Comment #2
klonos#1894972: Provide a bulk form for user operations was closed as a duplicate of #1851086: Replace admin/people with a View
Comment #3
klonos...and there's #1806334: Replace the node listing at /node with a view
Comment #4
tim.plunkettThis has nothing to do with that, its for /admin/content
Comment #5
tim.plunkettResurrecting this.
This will fail LocaleContentTest, we don't have any filter handlers for node.translation
Comment #6
tim.plunkettRetitling.
Comment #8
tim.plunkettOkay, this should be done now.
Comment #9
tim.plunkettMissed a line.
Comment #10
tim.plunkettTagging.
Comment #11
dawehner... This constructs an object ...
Can we use \Drupal::moduleHandler()?
I guess name it $row would clear it a bit better.
Afaik it's about ->changed, but it feels a bug of the NodeStorageController that you can't set it manually at all.
should be status 1
Comment #12
tim.plunkettModuleHandler cannot be injected right now, that's cropped up in several issues, I guess I should open an issue about that.
Still, we can use Drupal::moduleHandler(), for easier conversion later.
About $node->changed, this is weird:
In #722688: Allow programmatic setting of node->changed, this was fixed by wrapping it in a conditional.
Then #322759: Allow hook_node_presave() to alter other node fields undid this, with the reasoning that someone could implement hook_node_pre_save() to override this behavior.
That's exactly the approach I've taken here, except using a controller and not a hook.
Comment #13
tim.plunkettSee #1982058: ModuleHandler cannot be injected into forms
Comment #14
dawehnerCommented on that issue :)
Posting some screenshots, to make people more comfortable with the patch.
Before:
After:
I guess we maybe need some follow up to improve the visual clutter on this page.
After without views:
Comment #15
tim.plunkettAdjusted the parent tab to be a default as well.
Comment #16
klonosIn the "after" screenshot there are three "apply" buttons. I realize that the two are duplicate of the same thing, but still should we do something about it?
The button in the filter differs in style from the other two plus it is inline with the status/type filter drop-down menus while the other one wraps under the "With selection". It would perhaps be better to have that one have a label like "filter" instead of "apply".
Also, the "With selection" label and the "do something with selected content" feels redundant. How about we change the label to something like "Action:" or reduce the drop-down entries to be only verbs like "unpublish", "delete" and so on instead?
Comment #17
irinaz CreditAttribution: irinaz commentedI applied the patch to a standard Drupal 8 install and I cannot see filters. What do I need to do activate views?
Thanks, Irina
Comment #18
irinaz CreditAttribution: irinaz commentedIt would be great to have an option to filter by title also
I am adding a yml file with the view code
Comment #19
tim.plunkettYou would have to reinstall while the patch was applied to see it.
The current D7 page has no title filter, and this issue is a straight port.
We can discuss improvements to this view and all admin views in a follow-up once the basic port is done.
Comment #20
tim.plunkettOkay, I changed the top button to say "Filter".
Comment #21
tim.plunkett#20: admin-content-1895160-20.patch queued for re-testing.
Comment #22
dawehnerTwo small nitpicks. This is looking great.
Let's store the storage controller. The manager will be container aware or sort of container aware, so let's avoid where it's possible.
Should we use $this->container->get('state') instead?
Can we typehint if we change the parameter?
Comment #23
dawehnerBefore I forget: my manually testing showed that #1116326: Support admin overlay in exposed forms does also exist on this issue.
Comment #24
xjmNW for #22. About to test manually. :) simplytest.me choked for some reason.
Comment #25
xjmThe no-view fallback seems to work great. I didn't test it with generated content yet. The view works great too. :) Code review later.
Comment #26
xjmYeah, I noticed #1116326: Support admin overlay in exposed forms while I was recording my videos.
Comment #27
irinaz CreditAttribution: irinaz commentedTim, thanks, got your point. I will continue testing and hope that title filter will make it into D8 :)
Thanks, Irina
Comment #28
tim.plunkettThis addresses #22, and also contains some further clean-ups based on how other code has gone in D8 recently.
Comment #30
tim.plunkettEntityBCDecorator--
Comment #31
tim.plunkettThanks to Berdir for guidance on the fix.
Comment #32
ParisLiakos CreditAttribution: ParisLiakos commentedhow about changing
Status: yes/no
to
Published: yes/no
wondering if we should use RedirectResponse here
Comment #33
tim.plunkett_form does not allow returning other responses yet.
Also I might be mistaken, but I think the current listing does Status, so that's what we're doing.
Comment #34
ParisLiakos CreditAttribution: ParisLiakos commentedyes it does status, but like:
Status: published/not published
Status: yes/no makes no sense to me
besides that patch seems ready to fly!
Comment #35
tim.plunkettSee #1998192: Allow the boolean labels of exposed filters to be configurable
Comment #36
pcambraIs there any reason to create our own views form for the bulk operation instead of using the Action module? I think we could just add the "Action: Bulk update" field and configure the actions in the view.
Edit: dawehner pointed me to #1846172: Replace the actions API so I guess that's going to change a lot how we face the actions here.
Comment #37
tim.plunkett#31: node-1895160-31.patch queued for re-testing.
Comment #39
tim.plunkettRerolled. We have an issue with overriding existing callbacks with views right now, I will open a new issue.
Comment #41
tim.plunkettThis contains #2011006: Default local tasks provided by Views are broken, but I want to see if that's the only failure.
Rerolled on top of the actions commit.
Comment #42
tim.plunkettComment #44
tim.plunkettSome stragglers from the merge.
Comment #45
xjmComment #46
tim.plunkettThis no longer has any blockers.
No changes, just a reroll.
Comment #47
msonnabaum CreditAttribution: msonnabaum commentedLooks quite good to me.
Comment #48
ParisLiakos CreditAttribution: ParisLiakos commentedi agree
#1998192: Allow the boolean labels of exposed filters to be configurable should not block this issue
Comment #49
alexpottSomethings not quite right here... some missing handlers... both with standard and minimal + views + views_ui
Also if you goto to
admin/structure/views
and then click on the path for the content view you get a page not found.Comment #50
dawehnerThese two missing handlers are the translation link and the langcode filter one. As written in #1966424: Change notice: Allow Views handlers to be optional we needed some kind
of way to tell views that a certain handler is optional (so it should not fail if it does not exist). We don't see a better way to extend an existing view depending whether a certain module is enabled or not.
Beside from the motivation I think we should give a better message on the view page (if not even remove the link and just display some text) if it's optional and the link does not exist, though it seems to be sort of out of scope of this issue?
Comment #51
tim.plunkettOpened #2016953: Indicate when an optional handler is missing, that it is not "broken" and #2016939: Views listing page links are broken for default task page displays
Those are pre-existing and standalone issues.
Comment #52
alexpottCommitted 32697f2 and pushed to 8.x. Thanks!
Let's get stuff done... those are pre-existing issues.
Comment #53
dawehnerYeaaaaaaaaaaaaaaah!!!!!!
Comment #54
andypostThere's no border at left side of the table, needs follow-up
Comment #55
dawehnerOpened a follow up: #2017323: admin/content does not have a border anymore
Comment #56
PolFYI: I just tested D8 with simplytest.me, standard profile installation and the view is not working because of #49.
Comment #57
xjm@Pol, could you give a more specific explanation for "not working"?
Comment #58
PolI just tested a few minutes ago on simplytest.me, when I try to access to /admin/content/node, it says:
Comment #59
tim.plunkettThat is by design.
See #2016939: Views listing page links are broken for default task page displays for fixing the link
See #1995620: [policy, no patch] Document how to handle routes for MENU_DEFAULT_LOCAL_TASK for discussion of why it's okay for that to 404.
Comment #60
xjmWe definitely need to put the removal of
admin/content/node
in the change notice since people keep asking about it.Comment #60.0
xjm.
Comment #61
damiankloip CreditAttribution: damiankloip commentedI have opened #2022127: Change status filter on admin/content to 'published'/'unpublished' now that #1998192: Allow the boolean labels of exposed filters to be configurable is in.
Comment #62
klonosI tried disabling views_ui.module first and then views.module in order to test the fallback and got this while on the modules page:
Symfony\Component\Routing\Exception\RouteNotFoundException: Route "view.content.page_1" does not exist. in Drupal\Core\Routing\RouteProvider->getRouteByName() (line 126 of /home/sc98e1faf7be689e/www/core/lib/Drupal/Core/Routing/RouteProvider.php).
This error pops up when trying to get to /admin/modules. The page shows up fine in overlay (/#overlay=admin/modules).
Different issue?
Comment #63
amateescu CreditAttribution: amateescu commentedYep, that would be #2021779: Decouple shortcuts from menu links.
Comment #64
klonosThanx for the link to the issue Andrei!
So, has anybody actually tested the fallback to the non-views no bulk operations page or should we revisit this once that other issue is resolved?
Comment #65
amateescu CreditAttribution: amateescu commentedYou can easily test it by disabling the shortcut module :)
Comment #66
klonosRight. I would have never guessed that since I had absolutely no idea that shortcut is used for that view.
Anyways, I also want to say that in #2020167: Add a name and email search field to the admin/people view we added a field to search on user name and e-mail. Should we do the same here and search title and body? Did anybody file an issue for that?
Comment #67
tim.plunkettSee #2001922: Improve admin/content view
Comment #68
klonosThanx Tim.
Comment #69
ibullockgoing to write the API change notification.
Edit: Wrong chrome tab, wrong issue. :S
Comment #70
ibullockComment #71
pcambraAdded followup: #2029569: Convert node_admin_nodes to a new-style Controller
Comment #72
Wim LeersThe problem in #62 still has not yet been fixed over at #2021779: Decouple shortcuts from menu links.
I found the above through another regression: #2043403: admin/content View does not have "new"/"update" indicators.
Finally: this was committed on June 11, today is July 16, still no change notice.
Comment #73
claudiu.cristeaBulk delete broken: #2050321: Content bulk delete broken.
Comment #74
webwarrior CreditAttribution: webwarrior commentedProposed change notification:
Summary:
Various admin pages and core pages, as listed below, that lists entities was converted to use Views. As a fallback, if Views is not installed/enabled, a non-view will be used, but will not have any bulk operations available.
This change notification incorporates the following issues, as requested:
#1895160: Convert admin/content to a View, keep a non-views fallback with no bulk operations
#1851086: Replace admin/people with a View
#1806334: Replace the node listing at /node with a view
Pages changed:
admin/content
admin/content/node
was removed.In
core/modules/node/lib/Drupal/node/Plugin/views/field/NodeBulkForm.php
, class NodeBulkForm added.In
core/modules/node/node.admin.inc
removed functions:node_filters
node_build_filter_query
node_filter_form
node_filter_form_submit
node_admin_content
node_admin_nodes
node_admin_nodes_submit
node_multiple_delete_confirm
node_multiple_delete_confirm_submit
Functions with changed parameters:
node_mass_update
Before:
function node_mass_update($nodes, $updates, $langcode = NULL)
After:
function node_mass_update(array $nodes, array $updates, $langcode = NULL, $load = FALSE)
admin/people
admin/people/list
API changes:
In
core/modules/user/user.admin.inc
:router
admin/people/people
removedremoved functions:
user_admin
user_filter_form
user_filter_form_submit
user_admin_account_submit
user_admin_account_validate
user_admin_account
function changed to remove bulk operations./node
listing to a ViewAPI changes
In
core/modules/node/lib/Drupal/node/Plugin/views/area/ListingEmpty.php
, class ListingEmpty added.Router
/node
removed fromcore/modules/node/node.module
Removed functions:
node_page_default
Comment #75
jibranLet's add #1823450: [Meta] Convert core listings to Views in related issues. Change notice looks good.
Comment #76
alexpottPatch no longer applies
Comment #77
jibranIt was RTBC for change notice patch was already committed in #52. Sorry for the confusion. @webwarrior please go ahead an create a change notice and suggestion in #75 to it.
Comment #78
webwarrior CreditAttribution: webwarrior commentedSorry, I'm a bit confused.
What do you mean "Patch no longer applies" in #76? Do I need to change something in notice at #74?
Or can I go ahead and create a change notice?
Comment #79
tim.plunkett@webwarrior please go ahead and create the change notice.
Comment #80
xjmHaha @webwarrior, you are a victim of @alexpott's new patch-checking automation which missed the mark in this case. Disregard #76. I'll review the change notice in a bit. :)
Comment #81
xjmComment #82
catch#2083415: [META] Write up all outstanding change notices before release.
Comment #83
webwarrior CreditAttribution: webwarrior commentedChange notification added. Should we remove the tag?
https://drupal.org/node/2084727
Comment #84
star-szrYes, thanks for working on the change notification @webwarrior!
Comment #85.0
(not verified) CreditAttribution: commentedAdding a Related issues section.
Comment #86
xjmComment #87
klonosComment #88
klonos...added #2020393: Convert "Recent content" block to a View as a child instead of related issue.