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).
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Chi created an issue. See original summary.

Chi’s picture

Issue summary: View changes
Status: Active » Postponed
Chi’s picture

Issue summary: View changes
Chi’s picture

Issue summary: View changes
Chi’s picture

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

Status: Postponed » Active
andypost’s picture

Version: 8.5.x-dev » 8.6.x-dev
Issue summary: View changes
Status: Active » Needs review
FileSize
3.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

Issue tags: +KharkivCS
idebr’s picture

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

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

Status: Needs work » Needs review
FileSize
1020 bytes
3.78 KB

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

andypost’s picture

FileSize
1000 bytes
4.02 KB

Ah it's about variable name, then makes sense

pifagor’s picture

Look good

borisson_’s picture

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

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

Status: Needs work » Needs review
FileSize
3.95 KB
andypost’s picture

Status: Needs review » Reviewed & tested by the community

let's go without comment

alexpott’s picture

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.