Activity Tracker shows 'Last updated' status as '45 years 1 week ago' for all the content added.

Steps:
1. Install / Enable Tracker Module
2. Add any content
3. Check the activity by going to user -> Activity
4. Last updated is shown as '45 years 1 week ago'

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Bug because tracker functionality is broken.
Issue priority Normal
Unfrozen changes None
Prioritized changes The main goal of this issue is fixing a bug.
Disruption None
CommentFileSizeAuthor
#29 activity_tracker_shows-2401191-29.patch2.51 KBnlisgo
#29 interdiff-2401191-25-29.txt1.2 KBnlisgo
#25 activity-tracker-time-display-2401191-25-TEST-ONLY.patch1.75 KBjhedstrom
#25 activity-tracker-time-display-2401191-25.patch2.57 KBjhedstrom
#16 activity-tracker-time-display-2401191-16.patch2.53 KBjhedstrom
#16 activity-tracker-time-display-2401191-16-TEST-ONLY.patch1.7 KBjhedstrom
#16 interdiff.txt2.94 KBjhedstrom
#4 tracker_module_fix-2401191-4.patch629 bytesa_thakur
#4 after_patch.png79.15 KBa_thakur
#4 before_patch.png79.71 KBa_thakur
Activity screen shot.jpeg101.12 KBmohrerao
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Poornima3’s picture

Poornima3 CreditAttribution: Poornima3 commented

This is particularly happening for just content type:- Basic Page (For Article it is showing the correct last updated )

Manjit.Singh’s picture

Manjit.Singh
Primary language Hindi
Location Chandigarh
CreditAttribution: Manjit.Singh commented
Status: Active » Needs review
a_thakur’s picture

a_thakur CreditAttribution: a_thakur commented
Status: Needs review » Needs work
a_thakur’s picture

a_thakur CreditAttribution: a_thakur commented
Status: Needs work » Needs review
FileSize
before_patch.png79.71 KB
after_patch.png79.15 KB
tracker_module_fix-2401191-4.patch629 bytes

Please find the attached patch. Also find the screenshots for before and after patch.

Status: Needs review » Needs work

The last submitted patch, 4: tracker_module_fix-2401191-4.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Version: 8.1.x-dev » 8.0.x-dev

Afaik it is always time to fix bugs.

a_thakur’s picture

a_thakur CreditAttribution: a_thakur commented

Not sure why test fails, is it because tracker module isn't enabled on the instance where CI is running?

Status: Needs work » Needs review

LinL queued 4: tracker_module_fix-2401191-4.patch for re-testing.

Poornima3’s picture

Poornima3 CreditAttribution: Poornima3 commented

#4

The patch works absolutely fine

mohrerao’s picture

mohrerao CreditAttribution: mohrerao commented
Issue tags: +#dcdelhi

ankur_novatree queued 4: tracker_module_fix-2401191-4.patch for re-testing.

ashutoshsngh’s picture

ashutoshsngh CreditAttribution: ashutoshsngh commented
Status: Needs review » Reviewed & tested by the community

Applied and checked worked fine.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

We should have a test for this.

RavindraSingh’s picture

RavindraSingh CreditAttribution: RavindraSingh commented
Status: Needs work » Reviewed & tested by the community

Yes, its working fine.

RavindraSingh’s picture

RavindraSingh CreditAttribution: RavindraSingh commented
Status: Reviewed & tested by the community » Needs work

what other people say?

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom commented
Status: Needs work » Needs review
Issue tags: -Needs tests
FileSize
interdiff.txt2.94 KB
activity-tracker-time-display-2401191-16-TEST-ONLY.patch1.7 KB
activity-tracker-time-display-2401191-16.patch2.53 KB

I started writing a test for this, and quickly realized that the issue only makes itself apparent when comments are disabled on a particular type. Furthermore, the changed time from the tracker data table can include comment updates, not just node time.

I've written a test that illustrates the failure, and also tweaked the fix from above to take into account comment time (but still include changed time from nodes without comments).

The last submitted patch, 16: activity-tracker-time-display-2401191-16-TEST-ONLY.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 16: activity-tracker-time-display-2401191-16.patch, failed testing.

Status: Needs work » Needs review

jhedstrom queued 16: activity-tracker-time-display-2401191-16.patch for re-testing.

jhedstrom queued 16: activity-tracker-time-display-2401191-16-TEST-ONLY.patch for re-testing.

The last submitted patch, 16: activity-tracker-time-display-2401191-16-TEST-ONLY.patch, failed testing.

mgifford queued 16: activity-tracker-time-display-2401191-16.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 16: activity-tracker-time-display-2401191-16.patch, failed testing.

idebr’s picture

idebr CreditAttribution: idebr commented
Issue tags: +Needs reroll
jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
activity-tracker-time-display-2401191-25.patch2.57 KB
activity-tracker-time-display-2401191-25-TEST-ONLY.patch1.75 KB

Re-roll of #16.

Status: Needs review » Needs work

The last submitted patch, 25: activity-tracker-time-display-2401191-25-TEST-ONLY.patch, failed testing.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom commented
Status: Needs work » Needs review

The test-only patch was expected to fail.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Status: Needs review » Needs work

Looking over the patch, it seems ok to me. But I do have some minor remarks:

  1. +++ b/core/modules/tracker/src/Tests/TrackerTest.php
@@ -8,7 +8,9 @@
+

    Nitpicking, but it's not needed here :) 

  2. +++ b/core/modules/tracker/tracker.pages.inc
@@ -61,12 +61,15 @@ function tracker_page($account = NULL) {
+      // Set last activity time from tracker data (this isn't only the node
+      // changed time, it can include comment activity, so getChangedTime() is
+      // not used.

    This is missing a closing bracket, but I would prefer to rephrase the comment altogether.
    E.g.
    Set the last activity time from tracker data. This also takes into account comment activity, so getChangedTime() is not used.

nlisgo’s picture

nlisgo CreditAttribution: nlisgo commented
Status: Needs work » Needs review
FileSize
interdiff-2401191-25-29.txt1.2 KB
activity_tracker_shows-2401191-29.patch2.51 KB

This patch addresses feedback in #28.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

Looks good to me.

Added beta eval to the summary.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick at Acquia commented
Status: Reviewed & tested by the community » Fixed

Great catch.

Committed and pushed to 8.0.x. Thanks!

  • webchick committed 3142e86 on 8.0.x 
    Issue #2401191 by jhedstrom, a_thakur, nlisgo, mohrerao: Activity...

Status: Fixed » Closed (fixed)

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

mohrerao’s picture

mohrerao CreditAttribution: mohrerao as a volunteer and commented