Part of #1823450: [Meta] Convert core listings to Views

Converting the admin/comments page to a view with operations would keep it more inline with the rest of core listing pages.

This also helps solve problems for #1978904: Convert comment_admin() to a Controller and #1946348: Convert all of confirm_form() in comment.admin.inc to the new form interface which currently needs special handling for confirm delete forms.

Remaining tasks

Files: 
CommentFileSizeAuthor
#85 80-1986606.patch1.34 MBadnen
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Drupal installation failed.
[ View ]
#84 80-1986606.patch1.34 MBadnen
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Drupal installation failed.
[ View ]
#84 80-1986606.patch1.34 MBadnen
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Drupal installation failed.
[ View ]
#79 comments_administration_view-1986606-79.patch58.88 KBsnig
FAILED: [[SimpleTest]]: [MySQL] 63,564 pass(es), 59 fail(s), and 1 exception(s).
[ View ]
#76 interdiff.txt751 bytesdawehner
#76 vdc-1986606.patch59.09 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 59,346 pass(es).
[ View ]
#45 1986606-comments_admin_views-45.patch91.11 KBpcambra
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1986606-comments_admin_views-45.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Comments

Assigned:Unassigned» pcambra
Issue tags:+drupalconportland

Status:Active» Needs work
StatusFileSize
new28.9 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]

Here's a very initial take, listing has been exported to views and added some bulk operations.

TODO's

  • Add operations edit link (it's throwing an error).
  • Add count for unapproved comments on the top. "Unapproved comments (5)"
  • Tweak bulk operations form: unpublish & delete
  • Update tests
  • Expose some filters by default?

Status:Needs work» Needs review
StatusFileSize
new17.61 KB
new31.38 KB
FAILED: [[SimpleTest]]: [MySQL] 56,119 pass(es), 3 fail(s), and 0 exception(s).
[ View ]

Here's a new version of the patch, added a delete action and tests for it.

Also discussed with dawehner, we're delaying adding the number of comments to the "Unapproved comments" menu item as it could be a performance risk and actually there's no "right" way to do it apparently.

Status:Needs review» Needs work

The last submitted patch, 1986606-comments_admin_views-3.patch, failed testing.

Assigned:pcambra» Unassigned
Status:Needs work» Needs review
StatusFileSize
new2.26 KB
new31.48 KB
FAILED: [[SimpleTest]]: [MySQL] 55,332 pass(es), 2 fail(s), and 0 exception(s).
[ View ]

The tab menus are not working good enough, I'd say that there's a conflict with the parent menu so the tabs are displayed but the parent is always the original screen, we could embed the view in case views is enabled but there are also other options such as replacing the secondary tabs by a filter.

Also, actions module is not a views dependency, resulting into a Broken/Missing handler in the view by default which I don't think is a good thing to display, we might want to add a custom form just the way as admin/content and admin/people are doing for other reasons.

Status:Needs review» Needs work

The last submitted patch, 1986606-comments_admin_views-5.patch, failed testing.

#1846172: Replace the actions API will change how the actions stuff works.
The tab issues are caused by #2011006: Default local tasks provided by Views are broken

Assigned:Unassigned» pcambra
Issue tags:-drupalconportland+drupaldevdays

Resuming work in this

Status:Needs work» Needs review
StatusFileSize
new48.77 KB
new48.99 KB
FAILED: [[SimpleTest]]: [MySQL] 58,126 pass(es), 3 fail(s), and 0 exception(s).
[ View ]

Here's the view, I guess that we can work in the entity list controller in #1978904: Convert comment_admin() to a Controller and deal with the number of unread comments in a follow up.

Status:Needs review» Needs work

The last submitted patch, 1986606-comments_admin_views-9.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new5.3 KB
new53.2 KB
PASSED: [[SimpleTest]]: [MySQL] 58,127 pass(es).
[ View ]

StatusFileSize
new53.2 KB
PASSED: [[SimpleTest]]: [MySQL] 58,075 pass(es).
[ View ]
new934 bytes

A little leftover regarding excluded fields

Status:Needs review» Needs work
Issue tags:-VDC, -drupaldevdays

The last submitted patch, 1986606-comments_admin_views-12.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+VDC, +drupaldevdays

Issue tags:+Needs tests

I guess we need better testing support for the comment admin view, sort of NodeAdminTest, but not sure what makes sense to test here.

+++ b/core/modules/comment/config/views.view.comments.ymlundefined
@@ -0,0 +1,1454 @@
+      empty:
+        area:
+          id: area
+          table: views
+          field: area
+          relationship: none
+          group_type: group
+          admin_label: ''
+          empty: '1'
+          content: "<p>No comments available.</p>\n"
+          format: basic_html
+          tokenize: '0'

This should use the area_text_custom ('Unfiltered text') handler instead. Otherwise we are tied to a format, and if that is not available nothing will be displayed at all.

+++ b/core/modules/comment/config/views.view.comments.ymlundefined
@@ -0,0 +1,1454 @@
+uuid: df052bd2-a90f-4632-811b-277cf6ef38eb

At the moment I thought we were still removing UUIDs but not sure if that patch/issue to add UUIDs to default config got resolved yet?

+++ b/core/modules/comment/lib/Drupal/comment/Plugin/views/field/LinkDelete.phpundefined
@@ -19,16 +19,19 @@
+  function renderLink($comment, $values) {

public function, while we're there :)

+++ b/core/modules/comment/lib/Drupal/comment/Plugin/views/field/LinkEdit.phpundefined
@@ -37,10 +37,15 @@ public function buildOptionsForm(&$form, &$form_state) {
+  public function access() {
...
+  function renderLink($comment, $values) {
+++ b/core/modules/comment/lib/Drupal/comment/Plugin/views/field/LinkReply.phpundefined
@@ -19,17 +19,15 @@
+  function renderLink($comment, $values) {

Can we get a docblock for these (and public)? again, while we're there...

+++ b/core/modules/comment/lib/Drupal/comment/Tests/CommentActionsTest.phpundefined
@@ -76,4 +76,22 @@ function testCommentUnpublishByKeyword() {
+  function testCommentDeleteAction() {

public

+++ b/core/modules/comment/lib/Drupal/comment/Tests/CommentActionsTest.phpundefined
@@ -76,4 +76,22 @@ function testCommentUnpublishByKeyword() {
+    $action = entity_load('action', 'comment_delete_action');
...
+    $comment = comment_load($comment->id());

Maybe we should be trying to use the storage controller instead? Would be good to get a definitive answer on this, because I'm not actually sure myself.

+++ b/core/modules/views/lib/Drupal/views/Tests/DefaultViewsTest.phpundefined
@@ -105,10 +112,37 @@ protected function setUp() {
+   * Returns a new term with random properties in vocabulary $vid.

@return needed

From a conversation with @damiankloip the test we might need would be in the lines of FrontPageTest:

  • Make sure it's on the page we expect
  • Check if the empty text is there
  • Add comments and check if they're there
  • Maybe the comment delete link needs it's own test

"We shouldn;t need to test too much as view tests should cover a lot of these things"

Some additional feedback.

+++ b/core/modules/comment/comment.views.incundefined
@@ -247,15 +247,23 @@ function comment_views_data() {
-  $data['comment']['view_comment'] = array(
+  // Define some fields based upon views_handler_field_entity in the entity
+  // table so they can be re-used with other query backends.
+  // @see views_handler_field_entity
+
+  // Define the base group of this table. Fields that don't have a group defined
+  // will go into this field by default.

I don't think that this is longer true in Drupal 8 world and at least we should not assume it.

+++ b/core/modules/comment/lib/Drupal/comment/Plugin/views/field/Link.phpundefined
@@ -44,13 +44,12 @@ public function query() {}
+    return $this->renderLink($comment, $values);
...
-  function render_link($data, $values) {
+  function renderLink($comment, $values) {
+++ b/core/modules/comment/lib/Drupal/comment/Plugin/views/field/LinkApprove.phpundefined
@@ -23,17 +23,14 @@ public function access() {
+  function renderLink($comment, $values) {
+++ b/core/modules/comment/lib/Drupal/comment/Plugin/views/field/LinkDelete.phpundefined
@@ -19,16 +19,19 @@
+  function renderLink($comment, $values) {
+++ b/core/modules/comment/lib/Drupal/comment/Plugin/views/field/LinkEdit.phpundefined
@@ -37,10 +37,15 @@ public function buildOptionsForm(&$form, &$form_state) {
+  function renderLink($comment, $values) {
+    parent::renderLink($comment, $values);
+++ b/core/modules/comment/lib/Drupal/comment/Plugin/views/field/LinkReply.phpundefined
@@ -19,17 +19,15 @@
+  function renderLink($comment, $values) {

Then let's typehint this with CommentInterface

+++ b/core/modules/views/lib/Drupal/views/Tests/DefaultViewsTest.phpundefined
@@ -60,9 +67,9 @@ protected function setUp() {
+    $field_name = drupal_strtolower($this->randomName());

Let's better do Unicode::strtolower

StatusFileSize
new57.44 KB
PASSED: [[SimpleTest]]: [MySQL] 56,822 pass(es).
[ View ]
new10.42 KB

Thanks @dawehner & @damiankloip

This should use the area_text_custom ('Unfiltered text') handler instead. Otherwise we are tied to a format, and if that is not available nothing will be displayed at all.

Done.

At the moment I thought we were still removing UUIDs but not sure if that patch/issue to add UUIDs to default config got resolved yet?

As for discussed on IRC, uuids should be there.

while we're there :)

Fixed those as well :). Also "typehinted" with CommentInterface

Maybe we should be trying to use the storage controller instead? Would be good to get a definitive answer on this, because I'm not actually sure myself.

Replaced, let's see how it looks.

I don't think that this is longer true in Drupal 8 world and at least we should not assume it.

Grabbed that from the content one, is just the comment what is wrong or it's the actual thing?

Maybe the comment delete link needs it's own test

We've got a test for the action.

Attaching a new patch covering the above, still need to create a new test.

Issue tags:-Needs tests
StatusFileSize
new67.93 KB
PASSED: [[SimpleTest]]: [MySQL] 58,138 pass(es).
[ View ]
new40.22 KB

Added tests and fixed the optional/broken links

StatusFileSize
new66.71 KB
PASSED: [[SimpleTest]]: [MySQL] 58,337 pass(es).
[ View ]
new32.14 KB

Removed the views_entity_comment references from the data hooks and added a test for the empty text

After discussing with @dawehner, this and #1978904: Convert comment_admin() to a Controller needs to happen at the same time, so I'll include the fallback conversion of comment_admin() to a controller in this patch.

Status:Needs work» Needs review
StatusFileSize
new85.77 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1986606-comments_admin_views-24.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
new20.04 KB

Added support for multiple comment deletion using tempstorage as nodes do and menu page callback cleanup.

It's not converted to wscci yet, but that could go to a followup just as the node is.

Status:Needs review» Needs work

The last submitted patch, 1986606-comments_admin_views-24.patch, failed testing.

StatusFileSize
new85.67 KB
FAILED: [[SimpleTest]]: [MySQL] 58,187 pass(es), 46 fail(s), and 0 exception(s).
[ View ]

It had a conflict with the comment approve issue, here's a re-roll to HEAD

Status:Needs review» Needs work

The last submitted patch, 1986606-comments_admin_views-26.patch, failed testing.

Issue tags:+Needs screenshot

It would be great to have some screenshots of before and after in the summary.

+++ b/core/modules/comment/comment.admin.incundefined
@@ -279,7 +80,7 @@ function comment_confirm_delete_page(Comment $comment) {
/**
  * Form constructor for the confirmation form for comment deletion.
  *
- * @param Drupal\comment\Comment $comment
+ * @param Drupal\comment\CommentInterface $comment
  *   The comment that is about to be deleted.
  *

This change seems out of scope, especially because the actual code did not changed. Let's try to have the smallest change as possible.

+++ b/core/modules/comment/comment.admin.incundefined
@@ -315,3 +116,153 @@ function comment_confirm_delete_submit($form, &$form_state) {
+  $query = db_select('comment', 'c')
+    ->extend('Drupal\Core\Database\Query\PagerSelectExtender')
+    ->extend('Drupal\Core\Database\Query\TableSortExtender');
+  $query->join('node_field_data', 'n', 'n.nid = c.nid');
+  $query->addTag('node_access');
+  $result = $query
+    ->fields('c', array('cid', 'nid', 'subject', 'name', 'changed', 'status'))
+    ->limit(50)
+    ->orderByHeader($header)
+    ->execute();

So if we are build up a fallback we should do it properly and use EQ, and maybe even a fallback

StatusFileSize
new77.39 KB
new83.57 KB

Before:

comments-before.png

After:

comments-after.png

So, we're still missing the bulk operations part.

See core/modules/node/lib/Drupal/node/Plugin/views/field/NodeBulkForm.php and core/modules/node/node.views.inc line 230 for how comment should go about implementing a bulk form handler

This is with views module enabled, implementing the actions, it's the fallback what doesn't have the operations anymore.

Comments  drupal8.local 2013-06-29 10-10-27.png

Operations are there too, had a cache issue.

Summing up, with views and action modules enabled, we get the bulk operations in the comment screen which is a view. Without views enabled, we get a fallback that doesn't show the bulk operations, just lists comments with status.

Comments  drupal8.local 2013-06-29 10-16-46.png

Status:Needs work» Needs review
StatusFileSize
new86.12 KB
FAILED: [[SimpleTest]]: [MySQL] 56,574 pass(es), 46 fail(s), and 0 exception(s).
[ View ]

Here's a fix for the actions test that checks tempstore instead of deletion, also converted the query into an entity query as for #28, but then we're losing the ability to order by the node posted in as it's in a different table. Is there any way to overcome this or is ok to drop that for the fallback?

Regarding the views + action dependency, I think it's fine to require action module for having the VBO feature enabled in the view, otherwise, we'd need to provide a form and that one won't be configurable with the actions desired, I'm not sure why the content listing took this path.

This patch will still fail in the operations part. Let's see what we agree about the bulk operations before fixing that one.

Status:Needs review» Needs work

The last submitted patch, 1986606-comments_admin_views-34.patch, failed testing.

+++ b/core/modules/comment/comment.admin.incundefined
@@ -315,3 +116,135 @@ function comment_confirm_delete_submit($form, &$form_state) {
+function comment_admin_comments() {
+++ b/core/modules/comment/config/action.action.comment_delete_action.ymlundefined
--- /dev/null
+++ b/core/modules/comment/config/views.view.comments.ymlundefined

It is just amazing how big views config is.

+++ b/core/modules/comment/config/views.view.comments.ymlundefined
@@ -0,0 +1,1487 @@
diff --git a/core/modules/comment/lib/Drupal/comment/CommentStorageController.php b/core/modules/comment/lib/Drupal/comment/CommentStorageController.php
diff --git a/core/modules/comment/lib/Drupal/comment/CommentStorageController.php b/core/modules/comment/lib/Drupal/comment/CommentStorageController.php
index 4e6ca4c..0e0b5b2 100644
index 4e6ca4c..0e0b5b2 100644
--- a/core/modules/comment/lib/Drupal/comment/CommentStorageController.php
--- a/core/modules/comment/lib/Drupal/comment/CommentStorageController.php
+++ b/core/modules/comment/lib/Drupal/comment/CommentStorageController.phpundefined
+++ b/core/modules/comment/lib/Drupal/comment/CommentStorageController.phpundefined
+++ b/core/modules/comment/lib/Drupal/comment/CommentStorageController.phpundefined
@@ -232,4 +232,5 @@ public function getChildCids(array $comments) {
@@ -232,4 +232,5 @@ public function getChildCids(array $comments) {
       ->execute()
       ->fetchCol();
   }
+
}

Let's skip this part of the patch.

+++ b/core/modules/comment/lib/Drupal/comment/Plugin/views/field/LinkEdit.phpundefined
@@ -37,12 +38,19 @@ public function buildOptionsForm(&$form, &$form_state) {
+  public function access() {
+    // Needs permission to administer comments in general.
+    return user_access('administer comments');
+  }

As people can replace the access controller for comments, having an access to a link has to be checked via $comment->access all the time, so we should skip the basic access() method.

+++ b/core/modules/comment/lib/Drupal/comment/Tests/CommentActionsTest.phpundefined
@@ -76,4 +91,23 @@ function testCommentUnpublishByKeyword() {
+    $GLOBALS['user'] = $this->admin_user;

Is this really needed after drupalLogin?

+++ b/core/modules/comment/lib/Drupal/comment/Tests/CommentApprovalTest.phpundefined
@@ -11,6 +11,13 @@
class CommentApprovalTest extends CommentTestBase {
+  /**

Let's just make an empty line here.

Last patch #34 is no longer compatible with current 8.x
(warning: "Reversed (or previously applied) patch detected!")

Status:Needs work» Needs review
StatusFileSize
new85.38 KB
FAILED: [[SimpleTest]]: [MySQL] 56,384 pass(es), 45 fail(s), and 0 exception(s).
[ View ]
new3.3 KB

Addressing @dawehner comments in #36

It is just amazing how big views config is

Yes, as we have the published/unpublished thing, we need to keep several displays, we could agree in dropping the local subtabs in favor of an exposed filter for published / unpublished.

Let's skip this part of the patch.

Ok

so we should skip the basic access() method.

Dropped access method in all Link* views field plugins.

Is this really needed after drupalLogin?

Yes, as the action is executed with the user 2 (admin_user) but the tempstore is retrieved from uid 1 so that's the only way I had to access the right tempstore, it's not super nice but it's used normally in tests.

Let's just make an empty line here.

Done

As #37 mentions, re-rolled the patch against latest head.

Back to CNR, we still need to agree if action module dependency and fallback.

Status:Needs review» Needs work

The last submitted patch, 1986606-comments_admin_views-37.patch, failed testing.

Opened this followup I found while doing this #2031599: Replace collection by getCollectionName() in Tempstore

Action plugins are not dependent on the action module.

The action_bulk_form plugin used here is a generic handler, but node and user each subclass \Drupal\system\Plugin\views\field\BulkFormBase, and comment should too.

Look at UserBulkForm and NodeBulkForm; they're relatively simple classes.

Status:Needs work» Needs review
Issue tags:-Needs screenshot, -drupaldevdays
StatusFileSize
new85.66 KB
FAILED: [[SimpleTest]]: [MySQL] 57,143 pass(es), 46 fail(s), and 3 exception(s).
[ View ]
new7.73 KB

Rebased to latest head and also added a CommentBulkForm as for #41

+++ b/core/modules/comment/lib/Drupal/comment/Plugin/views/field/CommentBulkForm.phpundefined
@@ -0,0 +1,62 @@
+    if ($this->displayHandler->display['id'] == 'page_1') {
+      unset($this->actions['comment_publish_action']);
+    }
+    elseif ($this->displayHandler->display['id'] == 'page_2') {
+      unset($this->actions['comment_unpublish_action']);

This looks very brittle.

Status:Needs review» Needs work

The last submitted patch, 1986606-comments_admin_views-42.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new41.57 KB
new91.11 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1986606-comments_admin_views-45.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

So here's another round, the tests will still fail on the anonymous and maybe approval as I've found that any of the actions are displaying any confirmation message (i.e. "Deleted 1 comment"), but neither node actions do so I don't know if we should include them here or not.

This includes a reroll as many things have happened since #42.

As for an IRC conversation with @tim.plunkett and @dawehner, I've replaced the code in #43 by a options settings form for excluding/including the operations (i.e. not show the unpublish action for unpublished comments) but at this point the code is identical to the one in Drupal\action\Plugin\views\field\BulkForm, it might be good to move that one out of action to views as the action plugins are available even if the action module is not.
An alternative would be to make available the action form in views regardless the status of the action module.

Also fixed some updating issues related to #2011018: Reconcile entity forms and confirm forms.

Status:Needs review» Needs work

The last submitted patch, 1986606-comments_admin_views-45.patch, failed testing.

So, I am 300% in favor of converting this to a view, but I'm not sure we can get away with closing #1978904: Convert comment_admin() to a Controller as a duplicate, at least not yet. That, as part of the routing meta, is release-blocking; and Dries has stated several times that View conversions do not block release.

Maybe we can reopen the other at postponed, under the hope that this patch lands in time?

Also, this and #1907960: Helper issue for "Comment field" are going to seriously punch each other in the face. :) Might be worth considering waiting until that one is in (for either this or a rote form conversion), since that (or rather the main issue, sorry) is currently marked critical.

Status:Needs work» Postponed

Setting status as for ##4

I think it makes no sense to mark duplicate #1978904: Convert comment_admin() to a Controller
It much easy to convert and fallback is needed anyway

Assigned:pcambra» Unassigned
Status:Postponed» Active

I think this is not postponed anymore

Let's better wait until https://drupal.org/comment/7894541#comment-7894541 is in, given that it conflicts in a lot of places with this patch.

Views conversions are soooo novice issues!

Issue summary:View changes
Status:Active» Postponed

Parent issue:» #1978904: Convert comment_admin() to a Controller

Status:Postponed» Active

Status:Active» Needs review

Status:Needs review» Needs work

The last submitted patch, 45: 1986606-comments_admin_views-45.patch, failed testing.

Status:Needs work» Needs review

Reroll
Caution: Delete multiple action is not working. For this we should add comment list controller. View empty area is also not rendered properly I think we already have an issue for that.

StatusFileSize
new41.97 KB
FAILED: [[SimpleTest]]: [MySQL] 59,370 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Now with patch

Status:Needs review» Needs work

The last submitted patch, 62: d8-comment-view-1986606-61.patch, failed testing.

Status:Needs work» Needs review

62: d8-comment-view-1986606-61.patch queued for re-testing.

Status:Needs review» Needs work

The last submitted patch, 62: d8-comment-view-1986606-61.patch, failed testing.

  1. +++ b/core/modules/comment/comment.views.inc
    @@ -425,6 +425,14 @@ function comment_views_data() {
    +      'title' => t('Comment operations bulk form'),

    use 2 spaces for indent

  2. +++ b/core/modules/comment/config/system.action.comment_delete_action.yml
    @@ -0,0 +1,6 @@
    +id: comment_delete_action

    seems uuid still needed for shipped config

seems we really need list controller for fallback

seems we really need list controller for fallback

Nope. The only fallbacks needed are node and user, as decided by Dries. If they want a list, use a view.

Currently we don't have a comment.delete_multipe route in core which is required by comment delete action. Deletion of multiple comments are done using comment.admin route.
I am purposing here that if we have to add comment.delete_multipe route to fix comment delete action then why not follow the same pattern as node list controller because comment list controller is almost similar to node list controller unlike taxonomy terms listing page.

Just had a discussion with @tim.plunkett. We have to add comment.delete_multiple route to complete this patch and we can move the discussion of replacing/moving comment.admin_overview route to list controller to the follow up issue because it is related to comment module not views conversion.

Status:Needs work» Needs review
StatusFileSize
new55.76 KB
FAILED: [[SimpleTest]]: [MySQL] 59,294 pass(es), 15 fail(s), and 0 exception(s).
[ View ]
new15.01 KB

Fixed #66. Comment delete is working now. Re-factor some code for delete multiple.

Fun Fact: Can someone translate uuid in comment delete action?

The last submitted patch, 71: d8-comment-view-1986606-71.patch, failed testing.

StatusFileSize
new58.35 KB
FAILED: [[SimpleTest]]: [MySQL] 59,366 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
new2.59 KB

Fixed the comment tests. But I don't know how to fix DefaultViewsTest. It ready for review.

The last submitted patch, 73: d8-comment-view-1986606-73.patch, failed testing.

Status:Needs review» Needs work

Status:Needs work» Needs review
StatusFileSize
new59.09 KB
PASSED: [[SimpleTest]]: [MySQL] 59,346 pass(es).
[ View ]
new751 bytes

Thank you for working on that issue! The bulk form would profit from #2049573: Move most or all of the ActionBulkForm class to BulkFormBase though this is certainly no blocker

I had a look at the test and realized that it is 100% bullshit. We cannot just test all default views and expect that there is never any result ... I removed the part of the test which does not make sense
though kept the bits which do: checking which display is actually executed.

Should we use the "Show the empty text in the table" with this one too? That is enabled on people, content and files.

Re #76 We could just create few unpublished comments here and remove that part (or test only views' views) in a separate issue?

Status:Needs review» Needs work
Issue tags:+Needs reroll

Status:Needs work» Needs review
Issue tags:-Needs reroll
StatusFileSize
new58.88 KB
FAILED: [[SimpleTest]]: [MySQL] 63,564 pass(es), 59 fail(s), and 1 exception(s).
[ View ]

re-rolled

Status:Needs review» Needs work

The last submitted patch, 79: comments_administration_view-1986606-79.patch, failed testing.

This issue is a clear sign that we don't have 100% schema coverage.

Issue tags:+TUNIS_2014_MARCH

Will tackle this in Sprint 29 March.

Assigned:Unassigned» adnen
Status:Needs work» Active

StatusFileSize
new1.34 MB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Drupal installation failed.
[ View ]
new1.34 MB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Drupal installation failed.
[ View ]

Status:Active» Needs review
StatusFileSize
new1.34 MB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Drupal installation failed.
[ View ]

Status:Needs review» Needs work

The last submitted patch, 85: 80-1986606.patch, failed testing.

Status:Needs work» Needs review

85: 80-1986606.patch queued for re-testing.

Status:Needs review» Needs work

The last submitted patch, 85: 80-1986606.patch, failed testing.

The last submitted patch, 84: 80-1986606.patch, failed testing.

The last submitted patch, 84: 80-1986606.patch, failed testing.

Status:Needs work» Needs review

85: 80-1986606.patch queued for re-testing.

Status:Needs review» Needs work

The last submitted patch, 85: 80-1986606.patch, failed testing.

You've included what looks to be your entires /sites directory. Please don't do that :)

@adnen
Good to see that someone continues to work on that!
I have to dissapoint you, but your fill sites directory is still smaller as the original VDC patch :P #1805996: [META] Views in Drupal Core

Issue summary:View changes
Issue tags:+Needs issue summary update

:) https://drupal.org/contributor-tasks/create-patch has links to docs with examples and git information on creating patches that might help. And also ask anything in #drupal-contribute (https://drupal.org/irc).

Also see the git configuration instructions for how to ignore the files dir when creating patches.