Problem/Motivation

https://www.drupal.org/project/globallink/issues/3344157 and inspect console. There is a JS error Uncaught (in promise) TypeError: Cannot read properties of undefined (reading 'mergeActivity')

Proposed resolution

The current code is

const afterComment = comments.get(commentTimes.find(e => e < createdAt.getTime()));
if (afterComment.mergeActivity === undefined) {

But we could better check if an object has a property (see https://stackoverflow.com/questions/11040472/how-to-check-if-object-prop...).

Remaining tasks

MR

Issue fork drupalorg-3432638

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

fjgarlin created an issue. See original summary.

fjgarlin opened merge request !234

fjgarlin’s picture

fjgarlin CreditAttribution: fjgarlin as a volunteer and at Drupal Association commented
Status: Active » Needs review

Code needs review. This page is using that code https://fjgarlin-drupal.dev.devdrupal.org/project/config_notify/issues/3..., but as it's an edge case I'm not sure we're testing that particular edge case. In any case, the change seemed really straightforward.

apaderno’s picture

apaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
CreditAttribution: apaderno as a volunteer commented

On the dev site, Toggle MR Activity works as expected, and it even shows the number of comments that would be shown/hidden.
I checked https://fjgarlin-drupal.dev.devdrupal.org/project/config_notify/issues/3... as anonymous user, if that makes any difference.

drumm made their first commit to this issue’s fork.

  • drumm committed 15221d8f on 7.x-3.x authored by fjgarlin 
    Issue #3432638: MR comments and updates not displayed due to JS error
fjgarlin’s picture

fjgarlin CreditAttribution: fjgarlin as a volunteer and at Drupal Association commented

I did not anticipate this.
Uncaught (in promise) TypeError: Cannot read properties of undefined (reading 'hasOwnProperty')

So it seems like sometimes afterComment is completely null or undefined.
Maybe we can do something like this (though I don't get why it's fully empty or undefined in this case):

if (afterComment === undefined || afterComment === null) {
  afterComment = {};
}

fjgarlin changed the visibility of the branch 7.x-3.x to hidden.

fjgarlin opened merge request !235

fjgarlin’s picture

fjgarlin CreditAttribution: fjgarlin as a volunteer and at Drupal Association commented

Created new MR https://git.drupalcode.org/project/drupalorg/-/merge_requests/235 with #7.

drumm’s picture

drumm
he/him
Location NY, US
CreditAttribution: drumm at Drupal Association commented

We’re assuming all MR activity happens after the first issue comment. GitLab seems to have pulled in “@mloayza commented on commit 151669b7 2 years ago” on https://git.drupalcode.org/project/globallink/-/merge_requests/3.

So we either discard that MR note, or set afterComment to the earliest element of comments

  • drumm committed 68909240 on 7.x-3.x authored by fjgarlin 
    Issue #3432638: If MR notes are dated before the first issue comment,...
drumm’s picture

drumm
he/him
Location NY, US
CreditAttribution: drumm at Drupal Association commented
Status: Needs review » Fixed

The second fix is deployed & confirmed working now.

  • drumm committed 68909240 on dcp-updates-mktplace-changes authored by fjgarlin 
    Issue #3432638: If MR notes are dated before the first issue comment,...

  • drumm committed 15221d8f on dcp-updates-mktplace-changes authored by fjgarlin 
    Issue #3432638: MR comments and updates not displayed due to JS error

Status: Fixed » Closed (fixed)

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