From 8.x-3.x.

CommentFileSizeAuthor
#77 drupal-1821844-77.patch35.63 KBolli
#77 interdiff.txt1.59 KBolli
#73 1821844-73.patch35.89 KBdamiankloip
#65 1821844-65.patch35.71 KBjibran
#65 interdiff.txt10.64 KBjibran
#63 1821844-63.patch35.54 KBjibran
#63 interdiff.txt1.67 KBjibran
#61 1821844-61.patch35.57 KBjibran
#61 interdiff.txt12.25 KBjibran
#59 1821844-59.patch31.03 KBjibran
#59 interdiff.txt13.84 KBjibran
#55 drupal-1821844-55.patch32.11 KBwamilton
#55 interdiff.txt2.31 KBwamilton
#52 drupal-1821844-52.patch30.72 KBdawehner
#52 interdiff.txt714 bytesdawehner
#51 1821844-51.patch30.73 KBdamiankloip
#51 interdiff.txt806 bytesdamiankloip
#48 1821844-48.patch31.01 KBdamiankloip
#48 interdiff.txt1.93 KBdamiankloip
#41 drupal-1821844-41.patch31 KBdawehner
#41 interdiff.txt3.35 KBdawehner
#37 drupal-1821844-37.patch31.18 KBdawehner
#37 interdiff.txt1.71 KBdawehner
#34 drupal-1821844-34.patch31.13 KBdawehner
#34 interdiff.txt3.45 KBdawehner
#33 interdiff.txt981 bytesdawehner
#32 drupal-1821844-32.patch30.99 KBdawehner
#30 interdiff.txt2.06 KBdawehner
#29 drupal-1821844-29.patch31.31 KBdawehner
#27 drupal-1821844-26.patch31.38 KBdawehner
#22 interdiff.txt1.35 KBdawehner
#22 drupal-1821844-22.patch22.11 KBdawehner
#20 drupal-1821844-20.patch22.1 KBdawehner
#20 interdiff.txt5.25 KBdawehner
#17 drupal-1821844-17.patch22.44 KBParisLiakos
#17 interdiff.txt608 bytesParisLiakos
#14 drupal-1821844-14.patch22.32 KBdawehner
#12 drupal-1821844-12.patch22.09 KBdawehner
#10 drupal-1821844-10.patch22.84 KBdawehner
#4 aggregator-1821844-4.patch23.72 KBxjm
#4 interdiff.txt879 bytesxjm
#3 aggregator-1821844-3.patch23.65 KBxjm
#3 interdiff.txt832 bytesxjm
aggregator-views.patch22.84 KBxjm
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

xjm’s picture

Component: views_ui.module » aggregator.module
Issue tags: +VDC
xjm’s picture

Status: Active » Needs review

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

xjm’s picture

FileSize
832 bytes
23.65 KB

This should be better for reviewing the test coverage.

xjm’s picture

FileSize
879 bytes
23.72 KB

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

xjm’s picture

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

Berdir’s picture

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...

xjm’s picture

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.

xjm’s picture

Status: Needs review » Needs work

Making that more explicit. :)

xjm’s picture

Assigned: Unassigned » xjm
dawehner’s picture

FileSize
22.84 KB

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.

xjm’s picture

Status: Needs work » Needs review

Sending to the bot.

dawehner’s picture

FileSize
22.09 KB

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.

dawehner’s picture

Status: Needs work » Needs review
FileSize
22.32 KB

Fix these errors: Init method has been wrong.

xjm’s picture

Assigned: xjm » Unassigned

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

Berdir’s picture

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

ParisLiakos’s picture

FileSize
608 bytes
22.44 KB

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

dawehner’s picture

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

ParisLiakos’s picture

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

dawehner’s picture

Status: Needs work » Needs review
FileSize
5.25 KB
22.1 KB

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

ParisLiakos’s picture

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

dawehner’s picture

Status: Needs work » Needs review
FileSize
22.11 KB
1.35 KB

There we go.

ParisLiakos’s picture

do we need tests here?

dawehner’s picture

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

dawehner’s picture

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.

dawehner’s picture

Status: Needs work » Needs review
FileSize
31.38 KB

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.

ParisLiakos’s picture

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

dawehner’s picture

Status: Needs work » Needs review
FileSize
31.31 KB
dawehner’s picture

FileSize
2.06 KB

Thanks for the review, here is an interdiff.

ParisLiakos’s picture

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

dawehner’s picture

Status: Needs work » Needs review
FileSize
30.99 KB

There we go.

dawehner’s picture

FileSize
981 bytes

/me sighs about myself.

dawehner’s picture

FileSize
3.45 KB
31.13 KB

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.

ParisLiakos’s picture

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

dawehner’s picture

Status: Needs work » Needs review
FileSize
1.71 KB
31.18 KB

Thanks for the review. Fixed the exception as well.

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

ParisLiakos’s picture

Status: Needs review » Reviewed & tested by the community

this looks awesome now:) thanks
RTBC!

webchick’s picture

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

olli’s picture

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?

dawehner’s picture

Status: Needs work » Needs review
FileSize
3.35 KB
31 KB

One extra space before =.

Wow you are an eagle!

damiankloip’s picture

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

dawehner’s picture

Instead of fetchAllKeyed() or instead of fetchCol?

twistor’s picture

dawehner’s picture

Is the other one commitable in a reasonable timeframe?

ParisLiakos’s picture

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

olli’s picture

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)

damiankloip’s picture

FileSize
1.93 KB
31.01 KB

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.

dawehner’s picture

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

olli’s picture

+++ 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.

damiankloip’s picture

FileSize
806 bytes
30.73 KB

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

dawehner’s picture

FileSize
714 bytes
30.72 KB

Just fixing the label as well.

olli’s picture

Status: Needs review » Reviewed & tested by the community

Great!

alexpott’s picture

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.

wamilton’s picture

FileSize
2.31 KB
32.11 KB

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

wamilton’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

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

olli’s picture

@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.

jibran’s picture

Status: Needs work » Needs review
FileSize
13.84 KB
31.03 KB

Fixed #58 but #54 is still pending.

dawehner’s picture

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.

jibran’s picture

FileSize
12.25 KB
35.57 KB

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

damiankloip’s picture

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.

jibran’s picture

FileSize
1.67 KB
35.54 KB

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.

dawehner’s picture

+++ 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.

jibran’s picture

FileSize
10.64 KB
35.71 KB

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

dawehner’s picture

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.

damiankloip’s picture

+++ 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 :)

dawehner’s picture

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.

damiankloip’s picture

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

xjm’s picture

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

xjm’s picture

Issue tags: +Needs followup
alexpott’s picture

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
damiankloip’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
35.89 KB

Rerolled.

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

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

ParisLiakos’s picture

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.

olli’s picture

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Thank you for the rerole!

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 8.x. Thanks.

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