The following issues need to be fixed:

  1. Snake_case for declaring public properties. property inherited from parent plugin
  2. Absence of documentation for some methods. - added in #12
  3. Unnecessary braces ($clause declaration).
  4. Meaningless variable names ($ces).
CommentFileSizeAuthor
#21 2605714-21.patch3.95 KBUtkarsh_Mishra
#17 2605714-17.patch4.02 KBandypost
#17 2605714-interdiff-16.txt1000 bytesandypost
#16 2605714-16.patch3.78 KBandypost
#16 2605714-interdiff-12.txt1020 bytesandypost
#12 2605714-12.patch3.12 KBandypost

Comments

Chi created an issue. See original summary.

chi’s picture

chi commented
Issue summary: View changes
Status: Active » Postponed

Postpone this till #2591147: Filter criteria for views, option "Content is updated" - causes a SQL syntax error is fixed to prevent broken patches.

chi’s picture

chi commented
Issue summary: View changes
chi’s picture

chi commented
Issue summary: View changes
chi’s picture

chi commented
Title: Cleanup history_user_timestamp views filter handler. » Cleanup HistoryUserTimestamp views filter handler

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Postponed » Active
andypost’s picture

andypost
he/him
Primary language Russian
commented
Version: 8.5.x-dev » 8.6.x-dev
Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new 2605714-12.patch3.12 KB

Checked state, both field & filter needs fixes
#1 removed cos no longer here
#2 can't be fixed because inherited
#3 doc blocks fixed

andypost’s picture

andypost
he/him
Primary language Russian
commented
Issue tags: +KharkivCS
idebr’s picture

idebr commented
Status: Needs review » Needs work

Point 3 and 4 from the issue summary have yet to be addressed. These points target these lines in ./core/modules/history/src/Plugin/views/filter/HistoryUserTimestamp.php

    if ($ces = $this->query->ensureTable('comment_entity_statistics', $this->relationship)) {
      $clause = ("OR $ces.last_comment_timestamp > (***CURRENT_TIME*** - $limit)");
      $clause2 = "OR $field < $ces.last_comment_timestamp";
    }
andypost’s picture

andypost
he/him
Primary language Russian
commented

I'm not sure it is good idea to change ces to anything else because all over in comment module we use this abbreviation for comment_entity_statistics

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs work » Needs review
StatusFileSize
new 2605714-interdiff-12.txt1020 bytes
new 2605714-16.patch3.78 KB

Fixed #3 and still prefer not fix $ces cos it common all over core code

andypost’s picture

andypost
he/him
Primary language Russian
commented
StatusFileSize
new 2605714-interdiff-16.txt1000 bytes
new 2605714-17.patch4.02 KB

Ah it's about variable name, then makes sense

pifagor’s picture

pifagor
Location 🇺🇦 Rivne
commented

Look good

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented
Status: Needs review » Reviewed & tested by the community

All of the issues mentioned in the IS are now fixed. Looks like a good improvement.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/modules/history/src/Plugin/views/filter/HistoryUserTimestamp.php
@@ -20,7 +20,11 @@ class HistoryUserTimestamp extends FilterPluginBase {
+  /**
+   * {@inheritdoc}
+   *
+   * Don't display empty space where the operator would be.
+   */

Mixing inheritdoc with extra docs is not supported as far as I know. Given that the docs on the parent are Disable the possibility to use operators. I think the extra docs are either redundant or moved to the parent.

Utkarsh_Mishra’s picture

Utkarsh_Mishra commented
Status: Needs work » Needs review
StatusFileSize
new 2605714-21.patch3.95 KB
andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs review » Reviewed & tested by the community

let's go without comment

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Gave @idebr a review credit for ensuring the issue summary was carried out. And @Chi a credit for credit the issue with a clear instruction of what to do.

Backported to 8.5.x because there is no change here and 90% docs. So handy to keep everything in sync.

  • alexpott committed 5bd2dd5 on 8.6.x 
    Issue #2605714 by andypost, Utkarsh_Mishra, Chi, idebr: Cleanup...

  • alexpott committed c2c2fdc on 8.5.x 
    Issue #2605714 by andypost, Utkarsh_Mishra, Chi, idebr: Cleanup...

Status: Fixed » Closed (fixed)

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