From 8.x-3.x.

Files: 
CommentFileSizeAuthor
#77 drupal-1821844-77.patch35.63 KBolli
PASSED: [[SimpleTest]]: [MySQL] 58,083 pass(es).
[ View ]
#77 interdiff.txt1.59 KBolli
#73 1821844-73.patch35.89 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] 57,918 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#65 1821844-65.patch35.71 KBjibran
PASSED: [[SimpleTest]]: [MySQL] 57,405 pass(es).
[ View ]
#65 interdiff.txt10.64 KBjibran
#63 1821844-63.patch35.54 KBjibran
PASSED: [[SimpleTest]]: [MySQL] 57,290 pass(es).
[ View ]
#63 interdiff.txt1.67 KBjibran
#61 1821844-61.patch35.57 KBjibran
PASSED: [[SimpleTest]]: [MySQL] 55,233 pass(es).
[ View ]
#61 interdiff.txt12.25 KBjibran
#59 1821844-59.patch31.03 KBjibran
PASSED: [[SimpleTest]]: [MySQL] 55,518 pass(es).
[ View ]
#59 interdiff.txt13.84 KBjibran
#55 drupal-1821844-55.patch32.11 KBwamilton
FAILED: [[SimpleTest]]: [MySQL] 56,158 pass(es), 31 fail(s), and 150 exception(s).
[ View ]
#55 interdiff.txt2.31 KBwamilton
#52 drupal-1821844-52.patch30.72 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 53,994 pass(es).
[ View ]
#52 interdiff.txt714 bytesdawehner
#51 1821844-51.patch30.73 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 54,017 pass(es).
[ View ]
#51 interdiff.txt806 bytesdamiankloip
#48 1821844-48.patch31.01 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 54,056 pass(es).
[ View ]
#48 interdiff.txt1.93 KBdamiankloip
#41 drupal-1821844-41.patch31 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 54,106 pass(es).
[ View ]
#41 interdiff.txt3.35 KBdawehner
#37 drupal-1821844-37.patch31.18 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 54,013 pass(es).
[ View ]
#37 interdiff.txt1.71 KBdawehner
#34 drupal-1821844-34.patch31.13 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 53,714 pass(es), 0 fail(s), and 1 exception(s).
[ View ]
#34 interdiff.txt3.45 KBdawehner
#33 interdiff.txt981 bytesdawehner
#32 drupal-1821844-32.patch30.99 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 53,706 pass(es).
[ View ]
#30 interdiff.txt2.06 KBdawehner
#29 drupal-1821844-29.patch31.31 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 53,713 pass(es).
[ View ]
#27 drupal-1821844-26.patch31.38 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 53,699 pass(es).
[ View ]
#22 interdiff.txt1.35 KBdawehner
#22 drupal-1821844-22.patch22.11 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 51,771 pass(es), 380 fail(s), and 173 exception(s).
[ View ]
#20 drupal-1821844-20.patch22.1 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 49,098 pass(es).
[ View ]
#20 interdiff.txt5.25 KBdawehner
#17 drupal-1821844-17.patch22.44 KBParisLiakos
PASSED: [[SimpleTest]]: [MySQL] 49,122 pass(es).
[ View ]
#17 interdiff.txt608 bytesParisLiakos
#14 drupal-1821844-14.patch22.32 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 49,220 pass(es).
[ View ]
#12 drupal-1821844-12.patch22.09 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 50,502 pass(es), 2 fail(s), and 2 exception(s).
[ View ]
#10 drupal-1821844-10.patch22.84 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 48,358 pass(es).
[ View ]
#4 aggregator-1821844-4.patch23.72 KBxjm
PASSED: [[SimpleTest]]: [MySQL] 46,282 pass(es).
[ View ]
#4 interdiff.txt879 bytesxjm
#3 aggregator-1821844-3.patch23.65 KBxjm
FAILED: [[SimpleTest]]: [MySQL] 46,257 pass(es), 0 fail(s), and 267 exception(s).
[ View ]
#3 interdiff.txt832 bytesxjm
aggregator-views.patch22.84 KBxjm
PASSED: [[SimpleTest]]: [MySQL] 46,266 pass(es).
[ View ]

Comments

Component:views_ui.module» aggregator.module
Issue tags:+VDC

Status:Active» Needs review

HandlerAllTest still enables aggregator; need to double-check that its handlers are actually tested.

StatusFileSize
new832 bytes
new23.65 KB
FAILED: [[SimpleTest]]: [MySQL] 46,257 pass(es), 0 fail(s), and 267 exception(s).
[ View ]

This should be better for reviewing the test coverage.

StatusFileSize
new879 bytes
new23.72 KB
PASSED: [[SimpleTest]]: [MySQL] 46,282 pass(es).
[ View ]

That's broken. This isn't, and locally shows results for aggregator.

I'll spin the patch for debugging messages off into a separate issue.

FYI, #293318: Convert Aggregator feeds into entities converts feed items to EntityNG, doesn't use a property table yet, however. so this shouldn't conflict much...

Thanks @Berdir.

Before anyone goes crazy reviewing and complaining about the odd inline comments, this is a straight port from 8.x-3.x and isn't really ready to be reviewed yet.

Status:Needs review» Needs work

Making that more explicit. :)

Assigned:Unassigned» xjm

StatusFileSize
new22.84 KB
PASSED: [[SimpleTest]]: [MySQL] 48,358 pass(es).
[ View ]

Another issue which is blocked on #1783196: Get the views plugin manager from the DIC when possible (because you simple can't add a new view)

The diff in #4 got in somewhere else.

Status:Needs work» Needs review

Sending to the bot.

StatusFileSize
new22.09 KB
FAILED: [[SimpleTest]]: [MySQL] 50,502 pass(es), 2 fail(s), and 2 exception(s).
[ View ]

Rerolled, updated all the codestyles, fixed a lot of the code.

Status:Needs review» Needs work

The last submitted patch, drupal-1821844-12.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new22.32 KB
PASSED: [[SimpleTest]]: [MySQL] 49,220 pass(es).
[ View ]

Fix these errors: Init method has been wrong.

Assigned:xjm» Unassigned

Oops, didn't mean to leave this assigned to me.

#14: drupal-1821844-14.patch queued for re-testing.

StatusFileSize
new608 bytes
new22.44 KB
PASSED: [[SimpleTest]]: [MySQL] 49,122 pass(es).
[ View ]

i think something is missing, that could allow us to replace admin/config/services/aggregator in the future

That's a good idea, did you reviewed the patch?

Status:Needs review» Needs work

Nope, i didnt review it, i was just playing with views:P
I took a closer look now

+++ b/core/modules/aggregator/lib/Drupal/aggregator/Plugin/views/argument/Fid.phpundefined
@@ -0,0 +1,42 @@
+    $result = db_query("SELECT f.title FROM {aggregator_feed} f WHERE f.fid IN (:fids)", array(':fids' => $this->value));
+    $query = db_select('aggregator_feed', 'f');
+    $query->addField('f', 'title');
+    $query->condition('f.fid', $this->value);
+    $result = $query->execute();

I guess the first line needs to be removed..we could use entity_load_multiple but dunno, if it is worth it.

+++ b/core/modules/aggregator/lib/Drupal/aggregator/Plugin/views/argument/Iid.phpundefined
@@ -0,0 +1,42 @@
+    $placeholders = implode(', ', array_fill(0, sizeof($this->value), '%d'));

I dont see placeholders variable used after this..i guess one more leftover

+++ b/core/modules/aggregator/lib/Drupal/aggregator/Plugin/views/row/Rss.phpundefined
@@ -0,0 +1,112 @@
+    $query->addExpression('ai.link', 'feed_LINK');

i dont get why link is uppercase

Status:Needs work» Needs review
StatusFileSize
new5.25 KB
new22.1 KB
PASSED: [[SimpleTest]]: [MySQL] 49,098 pass(es).
[ View ]

I don't know about the feed_LINK part but this has been part of the aggregator integration since views2.

Status:Needs review» Needs work

All i can complain about is

+++ b/core/modules/aggregator/lib/Drupal/aggregator/Plugin/views/argument/Fid.phpundefined
@@ -0,0 +1,38 @@
+      $titles[] = check_plain($feed->title);

should be ->label()

+++ b/core/modules/aggregator/lib/Drupal/aggregator/Plugin/views/argument/Iid.phpundefined
@@ -0,0 +1,38 @@
+      $titles[] = check_plain($feed->title);

same

Status:Needs work» Needs review
StatusFileSize
new22.11 KB
FAILED: [[SimpleTest]]: [MySQL] 51,771 pass(es), 380 fail(s), and 173 exception(s).
[ View ]
new1.35 KB

There we go.

do we need tests here?

#22: drupal-1821844-22.patch queued for re-testing.

Assigned:Unassigned» dawehner

Working on writing tests and fixing some minor issues.

Status:Needs review» Needs work

The last submitted patch, drupal-1821844-22.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new31.38 KB
PASSED: [[SimpleTest]]: [MySQL] 53,699 pass(es).
[ View ]

Started to write some tests and fix some of the handlers.

Would be great to get #1956220: Allow to use xpath on drupal unit tests/unit tests so we can test the feed output, on the DUTB.

Status:Needs review» Needs work

+++ b/core/modules/aggregator/lib/Drupal/aggregator/ItemStorageController.phpundefined
@@ -25,7 +25,9 @@ public function create(array $values) {
     // Set an initial timestamp, this will be overwritten if known.
-    $entity->timestamp->value = REQUEST_TIME;
+    if (!isset($entity->timestamp->value)) {
+      $entity->timestamp->value = REQUEST_TIME;
+    }

Maybe remove the comment too?

+++ b/core/modules/aggregator/lib/Drupal/aggregator/Plugin/views/row/Rss.phpundefined
@@ -0,0 +1,113 @@
+    $query->addExpression('ai.link', 'feed_LINK');

Lets make this lowercase here, it is ugly as hell and i am pretty sure it wont break anything

+++ b/core/modules/aggregator/lib/Drupal/aggregator/Tests/Views/IntegrationTest.phpundefined
@@ -0,0 +1,131 @@
+    debug($view->preview());

oops

Status:Needs work» Needs review
StatusFileSize
new31.31 KB
PASSED: [[SimpleTest]]: [MySQL] 53,713 pass(es).
[ View ]

StatusFileSize
new2.06 KB

Thanks for the review, here is an interdiff.

Status:Needs review» Needs work

+++ b/core/modules/aggregator/lib/Drupal/aggregator/Plugin/views/row/Rss.phpundefined
@@ -0,0 +1,110 @@
+    $query = db_select('aggregator_item', 'ai');
+    $query->fields('ai');
+    $query->condition('iid', $iid);
+    $item = $query->execute()->fetchObject();

i see you removed the extra fields, so i guess if they are not needed, lets load the item using the entity storage?items are entities:)

+++ b/core/modules/aggregator/lib/Drupal/aggregator/Tests/Views/IntegrationTest.phpundefined
@@ -0,0 +1,131 @@
+  /**
+   * Tests basic aggregator_item view.
+   */
+  public function testAggregatorFeedView() {
+
+  }

Fill it or remove it:) also wrong docblock

Status:Needs work» Needs review
StatusFileSize
new30.99 KB
PASSED: [[SimpleTest]]: [MySQL] 53,706 pass(es).
[ View ]

There we go.

StatusFileSize
new981 bytes

/me sighs about myself.

StatusFileSize
new3.45 KB
new31.13 KB
FAILED: [[SimpleTest]]: [MySQL] 53,714 pass(es), 0 fail(s), and 1 exception(s).
[ View ]

So this time I converted the rss.php to load the entity.

Status:Needs review» Needs work

The last submitted patch, drupal-1821844-34.patch, failed testing.

sorry, i just noticed them

+++ b/core/modules/aggregator/lib/Drupal/aggregator/Plugin/views/row/Rss.phpundefined
@@ -0,0 +1,111 @@
+        'attributes' => array('isPermaLink' => 'false')

missing comma

+++ b/core/modules/aggregator/lib/Drupal/aggregator/Plugin/views/row/Rss.phpundefined
@@ -0,0 +1,111 @@
+      'row' => $item

comma as well

Status:Needs work» Needs review
StatusFileSize
new1.71 KB
new31.18 KB
PASSED: [[SimpleTest]]: [MySQL] 54,013 pass(es).
[ View ]

Thanks for the review. Fixed the exception as well.

Oh wow, I would have not exception that this patch is so big.

Status:Needs review» Reviewed & tested by the community

this looks awesome now:) thanks
RTBC!

#37: drupal-1821844-37.patch queued for re-testing.

Status:Reviewed & tested by the community» Needs work

+++ b/core/modules/aggregator/aggregator.views.inc
@@ -0,0 +1,343 @@
+  $data['aggregator_item']['table']['group']  = t('Aggregator');

One extra space before =.

+++ b/core/modules/aggregator/aggregator.views.inc
@@ -0,0 +1,343 @@
+      'click sortable' => TRUE,
+     ),
+    'argument' => array(

Another extra space here and in a dozen similar places.

+++ b/core/modules/aggregator/aggregator.views.inc
@@ -0,0 +1,343 @@
+    'argument' => array(
+      'id' => 'string',
+    ),
+    'sort' => array(
+      'id' => 'standard',
+    ),
+    'filter' => array(
+      'id' => 'string',
+    ),
+    'argument' => array(
+      'id' => 'string',
+    ),

Extra argument.

+++ b/core/modules/aggregator/lib/Drupal/aggregator/Plugin/views/argument/CategoryCid.php
@@ -0,0 +1,38 @@
+    $result = db_query("SELECT title FROM {aggregator_category} where cid IN (:cid)", array(':cid' => $this->value))->fetchAssoc();
+    foreach ($result as $category) {
+      $titles[] = check_plain($category->title);

I think ->fetchAssoc() does not work here.

+++ b/core/modules/aggregator/lib/Drupal/aggregator/Plugin/views/field/Category.php
@@ -0,0 +1,81 @@
+      $this->options['alter']['path'] = "aggregator/category/$cid";

Path is aggregator/categories/$cid.

+++ b/core/modules/aggregator/lib/Drupal/aggregator/Plugin/views/filter/CategoryCid.php
@@ -0,0 +1,40 @@
+    $result = db_query('SELECT * FROM {aggregator_category} ORDER BY title');
+    foreach ($result as $category) {
+      $this->value_options[$category->cid] = $category->title;

SELECT * -> SELECT cid, title

+++ b/core/modules/aggregator/tests/modules/aggregator_test_views/test_views/views.view.test_aggregator_items.yml
@@ -0,0 +1,114 @@
+  feed_1:

This is not used in tests. Should it use webtest if it is impossible to test feed until #1956220: Allow to use xpath on drupal unit tests/unit tests?

Status:Needs work» Needs review
StatusFileSize
new3.35 KB
new31 KB
PASSED: [[SimpleTest]]: [MySQL] 54,106 pass(es).
[ View ]

One extra space before =.

Wow you are an eagle!

Couldn't we use fetchAllKeyed(0, 1) instead?

Instead of fetchAllKeyed() or instead of fetchCol?

Is the other one commitable in a reasonable timeframe?

it is no problem, whichever gets committed first, since views integration files are not touched from the other patch..this patch here just introduce new files...i can easily git rm the categories one...just please dont give any extra attention to the categories integration..

it will die soonish:D
so if olli's concerns are resolved this should go back to rtbc

I'd like to set this back to rtbc, but I found these:
Warning: Invalid argument supplied for foreach() in Drupal\aggregator\Plugin\views\row\Rss->render() (line 98)
Notice: Trying to get property of non-object in Drupal\aggregator\Plugin\views\argument\CategoryCid->title_query() (line 33)

StatusFileSize
new1.93 KB
new31.01 KB
PASSED: [[SimpleTest]]: [MySQL] 54,056 pass(es).
[ View ]

Good catches, the first warning I'm not sure about, because I don't know when elements would be there? It's not declared as a property or anything? I've wrapped it for now, but it could be removed?

I have also changed the argument title_query method as fetchCol will return a flat array of values, not an object. So $row->title will never exist. We can just use that array and return a mapped version of that.

I fixed all consers beside the tests, as yeah the category integration will be dropped soon.

+++ b/core/modules/aggregator/tests/modules/aggregator_test_views/test_views/views.view.test_aggregator_items.yml
@@ -0,0 +1,114 @@
+human_name: test_aggregator_items

label

I don't know when elements would be there? It's not declared as a property or anything? I've wrapped it for now, but it could be removed?

I think we can remove that. In the original patch (or in d7) that is $item->elements which is set right before the foreach so there is no way elements could be there.

StatusFileSize
new806 bytes
new30.73 KB
PASSED: [[SimpleTest]]: [MySQL] 54,017 pass(es).
[ View ]

Yep, that's what I thought, and it seems there is no plan to actually ever need it. Let's rip it out!

StatusFileSize
new714 bytes
new30.72 KB
PASSED: [[SimpleTest]]: [MySQL] 53,994 pass(es).
[ View ]

Just fixing the label as well.

Status:Needs review» Reviewed & tested by the community

Great!

Status:Reviewed & tested by the community» Needs work
Issue tags:+Needs tests

+++ b/core/modules/aggregator/lib/Drupal/aggregator/Tests/Views/IntegrationTest.phpundefined
@@ -0,0 +1,117 @@
+      $expected_author = filter_xss($items[$iid]->author->value, array());

Should be using aggregator_filter_xss()

And I think we a test to ensure aggregator_filter_xss() is being used as expected on the aggregrator item author and description.

StatusFileSize
new2.31 KB
new32.11 KB
FAILED: [[SimpleTest]]: [MySQL] 56,158 pass(es), 31 fail(s), and 150 exception(s).
[ View ]

Here's the requested updates, assertions are failing now. I'd like some oversight on this.

Status:Needs work» Needs review

Status:Needs review» Needs work

The last submitted patch, drupal-1821844-55.patch, failed testing.

@wamilton, I think there has been some changes in the annotations:

+++ b/core/modules/aggregator/lib/Drupal/aggregator/Plugin/views/argument/CategoryCid.php
@@ -0,0 +1,36 @@
+ * @Plugin(
+ *   id = "aggregator_category_cid",
+ *   module = "aggregator"
+ * )

These have changed to @PluginID("aggregator_category_cid")

+++ b/core/modules/aggregator/lib/Drupal/aggregator/Plugin/views/row/Rss.php
@@ -0,0 +1,105 @@
+ *   type = "feed"

This is using display_types = {"feed"}

In addition to those changes:

+++ b/core/modules/aggregator/lib/Drupal/aggregator/Tests/Views/IntegrationTest.php
@@ -0,0 +1,114 @@
+      $expected_author = filter_xss($items[$iid]->author->value, array());

This should also use aggregator_filter_xss() as pointed out by @alexpott.

+++ b/core/modules/aggregator/lib/Drupal/aggregator/Tests/Views/IntegrationTest.php
@@ -0,0 +1,114 @@
+      $expected_description = aggregator_filter_xss($items[$iid]->description->value, array());

That aggregator_filter_xss() takes only one parameter.

Status:Needs work» Needs review
StatusFileSize
new13.84 KB
new31.03 KB
PASSED: [[SimpleTest]]: [MySQL] 55,518 pass(es).
[ View ]

Fixed #58 but #54 is still pending.

Assigned:dawehner» Unassigned
Issue tags:-Needs tests

Thank you!

+++ b/core/modules/aggregator/aggregator.views.incundefined
@@ -0,0 +1,337 @@
+      'click sortable' => TRUE,
...
+      'click sortable' => TRUE,
...
+      'click sortable' => TRUE,
...
+      'click sortable' => TRUE,
...
+      'click sortable' => TRUE,
...
+      'click sortable' => FALSE,
...
+      'click sortable' => TRUE,
...
+      'click sortable' => TRUE,
...
+      'click sortable' => TRUE,
...
+      'click sortable' => TRUE,
...
+      'click sortable' => TRUE,
...
+      'click sortable' => FALSE,
...
+      'click sortable' => TRUE,
...
+      'click sortable' => TRUE,
...
+      'click sortable' => TRUE,

As mentioned in the other issue, you can drop it.

+++ b/core/modules/aggregator/lib/Drupal/aggregator/Plugin/views/argument/CategoryCid.phpundefined
@@ -0,0 +1,33 @@
+    $titles = db_query("SELECT title FROM {aggregator_category} where cid IN (:cid)", array(':cid' => $this->value))->fetchCol();

What about injecting the db connection as well?

+++ b/core/modules/aggregator/lib/Drupal/aggregator/Plugin/views/argument/CategoryCid.phpundefined
@@ -0,0 +1,33 @@
+      return check_plain($title);
+++ b/core/modules/aggregator/lib/Drupal/aggregator/Plugin/views/argument/Fid.phpundefined
@@ -0,0 +1,35 @@
+      $titles[] = check_plain($feed->label());

check_plain should be replaced by String::checkPlain now

+++ b/core/modules/aggregator/lib/Drupal/aggregator/Plugin/views/argument/Fid.phpundefined
@@ -0,0 +1,35 @@
+    $feeds = \Drupal::service('plugin.manager.entity')->getStorageController('aggregator_feed')->load($this->value);

Even more injections.

+++ b/core/modules/aggregator/lib/Drupal/aggregator/Plugin/views/argument/Iid.phpundefined
@@ -0,0 +1,35 @@
+    $items = \Drupal::service('plugin.manager.entity')->getStorageController('aggregator_item')->load($this->value);
...
+      $titles[] = check_plain($feed->label());

Same here.

StatusFileSize
new12.25 KB
new35.57 KB
PASSED: [[SimpleTest]]: [MySQL] 55,233 pass(es).
[ View ]

Thanks @dawehner and @olli for the review. Fixed everything in #60.

Looking pretty good!

+++ b/core/modules/aggregator/aggregator.views.incundefined
@@ -0,0 +1,322 @@
+      'id' => 'aggregator_xss',
...
+      'id' => 'xss',

I guess aggregator_xss is the correct id? or is the item description intentionally not using the Aggregator xss implementation on purpose?

Before, when we have the 'click sortable' keys in the data, that would disable click sorting for both description fields. The aggregator specific Xss field handler can just implement its own click_sortable method and return FALSE. If using views' xss handler for item description is correct then 'click sortable' => FALSE will need to be added back to the data.

+++ b/core/modules/aggregator/lib/Drupal/aggregator/Plugin/views/field/Xss.phpundefined
@@ -0,0 +1,34 @@
+  public function sanitizeValue($value, $type = NULL) {
+    if ($type == 'xss') {
+      return aggregator_filter_xss($value);
+    }
+    else {
+      return parent::sanitizeValue($value, $type);
+    }

If this is extending Drupal\views\Plugin\views\field\Xss then sanitize value will always be passed 'xss' the type. So why have the fallback? I guess the actual type is pretty much irrelevant here, as we always want it run though this.

StatusFileSize
new1.67 KB
new35.54 KB
PASSED: [[SimpleTest]]: [MySQL] 57,290 pass(es).
[ View ]

Converted 'id' => 'xss', for guid to 'id' => 'standard', as per @dawehner suggestion.
Added back 'click sortable' => FALSE, that change was unnecessary. Good catch @damiankloip.
Simplified sanitizeValue.

+++ b/core/modules/aggregator/lib/Drupal/aggregator/Plugin/views/argument/CategoryCid.phpundefined
@@ -0,0 +1,79 @@
+   * Creates an instance of the plugin.
+   *
+   * @param \Symfony\Component\DependencyInjection\ContainerInterface $container
+   *    The DI Container.
+   * @param array $configuration
+   *   A configuration array containing information about the plugin instance.
+   * @param string $plugin_id
+   *   The plugin ID for the plugin instance.
+   * @param array $plugin_definition
+   *   The plugin implementation definition.
+   *
+   * @return static
+   *   Returns an instance of this plugin.
+++ b/core/modules/aggregator/lib/Drupal/aggregator/Plugin/views/argument/Fid.phpundefined
@@ -0,0 +1,81 @@
+  /**
+   * Creates an instance of the plugin.
+   *
+   * @param \Symfony\Component\DependencyInjection\ContainerInterface $container
+   *    The DI Container.
+   * @param array $configuration
+   *   A configuration array containing information about the plugin instance.
+   * @param string $plugin_id
+   *   The plugin ID for the plugin instance.
+   * @param array $plugin_definition
+   *   The plugin implementation definition.
+   *
+   * @return static
+   *   Returns an instance of this plugin.
+++ b/core/modules/aggregator/lib/Drupal/aggregator/Plugin/views/argument/Iid.phpundefined
@@ -0,0 +1,81 @@
+  /**
+   * Creates an instance of the plugin.
+   *
+   * @param \Symfony\Component\DependencyInjection\ContainerInterface $container
+   *    The DI Container.
+   * @param array $configuration
+   *   A configuration array containing information about the plugin instance.
+   * @param string $plugin_id
+   *   The plugin ID for the plugin instance.
+   * @param array $plugin_definition
+   *   The plugin implementation definition.
+   *
+   * @return static
+   *   Returns an instance of this plugin.

Should be {@inheritdoc}. While you are here, __construct should be better above, as that's THE important function here.

+++ b/core/modules/aggregator/lib/Drupal/aggregator/Plugin/views/field/Category.phpundefined
@@ -0,0 +1,78 @@
+   * Data should be made XSS safe prior to calling this function.
+   */
+  protected function render_link($data, $values) {

Needs docs, sorry.

+++ b/core/modules/aggregator/lib/Drupal/aggregator/Plugin/views/field/TitleLink.phpundefined
@@ -0,0 +1,82 @@
+   * Renders aggregator item's title as link.
...
+   * Data should be made XSS safe prior to calling this function.
...
+  protected function render_link($data, $values) {

Needs docs.

+++ b/core/modules/aggregator/lib/Drupal/aggregator/Plugin/views/filter/CategoryCid.phpundefined
@@ -0,0 +1,34 @@
+class CategoryCid extends InOperator {
...
+    $this->value_options = db_query('SELECT cid, title FROM {aggregator_category} ORDER BY title')->fetchAllKeyed();

This one also could get a injection?

+++ b/core/modules/aggregator/lib/Drupal/aggregator/Plugin/views/row/Rss.phpundefined
@@ -0,0 +1,105 @@
+      $item->{$name} = $value->value;
...
+        'value' => gmdate('r', $entity->timestamp->value),
...
+        'value' => $entity->author->value,
...
+        'value' => $entity->guid->value,
...
+    return theme($this->themeFunctions(), array(

so views_view_row_css takes care about the escaping here.

StatusFileSize
new10.64 KB
new35.71 KB
PASSED: [[SimpleTest]]: [MySQL] 57,405 pass(es).
[ View ]

Thanks you @dawehner for the review and help. Fixed everything in #64.

Status:Needs review» Reviewed & tested by the community

We have at least some basic testing, so that's good to go! Thank you very much for bring this home.

+++ b/core/modules/aggregator/lib/Drupal/aggregator/ItemStorageController.phpundefined
@@ -24,8 +24,9 @@ class ItemStorageController extends DatabaseStorageControllerNG {
+    if (!isset($entity->timestamp->value)) {
+      $entity->timestamp->value = REQUEST_TIME;
+    }

Howcome we have added this to this patch? Doesn't seem like views integration :)

Howcome we have added this to this patch? Doesn't seem like views integration :)

In order to test the timestamp rendering we have to be able to force a certain value, but the ItemStorageController just forced you to use the request all the time, so this seems to be a totally fine adapation.

OK, makes sense. We should consider moving (follow up) this for all created properties as there have been similar problems in tests recently.

Agreed, let's get a followup issue for that. :)

Issue tags:+Needs followup

Status:Reviewed & tested by the community» Needs work
Issue tags:+Needs reroll

curl https://drupal.org/files/1821844-65.patch | git a
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 36565  100 36565    0     0  19731      0  0:00:01  0:00:01 --:--:-- 28700
error: patch failed: core/modules/aggregator/lib/Drupal/aggregator/ItemStorageController.php:24
error: core/modules/aggregator/lib/Drupal/aggregator/ItemStorageController.php: patch does not apply

Status:Needs work» Needs review
Issue tags:-Needs reroll
StatusFileSize
new35.89 KB
FAILED: [[SimpleTest]]: [MySQL] 57,918 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Rerolled.

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

The last submitted patch, 1821844-73.patch, failed testing.

Status:Needs work» Needs review

#73: 1821844-73.patch queued for re-testing.

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

The last submitted patch, 1821844-73.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new1.59 KB
new35.63 KB
PASSED: [[SimpleTest]]: [MySQL] 58,083 pass(es).
[ View ]

Status:Needs review» Reviewed & tested by the community

Thank you for the rerole!

Status:Reviewed & tested by the community» Fixed

Committed to 8.x. Thanks.

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