Simplify handling of sections in bundled notification links

This code previously ensured that the fragment identifier linking
to a section was only included if all events had the same section.
It doesn't actually seem worth the effort, since we handle scrolling
to the highlighted comments client-side anyway.

And the links were not quite correct, because we didn't parse and
strip the section title as expected by built-in Echo events. Just
use Echo's code for this.

Depends-On: Idb3a87fd18330f90a8cdc1276994d54288e17b28
Change-Id: Icae0d3654dd02109337ff8737b16f55bbd514f43
This commit is contained in:
Bartosz Dziewoński 2022-01-21 00:06:36 +01:00
parent 8f937eb9da
commit d2405cc11c
2 changed files with 9 additions and 10 deletions

View file

@ -18,6 +18,7 @@ use WikiMap;
* *
* @property \EchoEvent $event * @property \EchoEvent $event
* @property \Language $language * @property \Language $language
* @property \EchoPresentationModelSection $section
*/ */
trait DiscussionToolsEventTrait { trait DiscussionToolsEventTrait {
@ -51,7 +52,6 @@ trait DiscussionToolsEventTrait {
if ( !$this->userCan( RevisionRecord::DELETED_TEXT ) ) { if ( !$this->userCan( RevisionRecord::DELETED_TEXT ) ) {
return null; return null;
} }
$title = $this->event->getTitle();
if ( !$this->isBundled() ) { if ( !$this->isBundled() ) {
// For a single-comment notification, make a pretty(ish) direct link to the comment. // For a single-comment notification, make a pretty(ish) direct link to the comment.
// The browser scrolls and we highlight it client-side. // The browser scrolls and we highlight it client-side.
@ -59,6 +59,7 @@ trait DiscussionToolsEventTrait {
if ( !$commentId ) { if ( !$commentId ) {
return null; return null;
} }
$title = $this->event->getTitle();
return $title->createFragmentTarget( $commentId )->getFullURL(); return $title->createFragmentTarget( $commentId )->getFullURL();
} else { } else {
// For a multi-comment notification, we can't make a direct link, because we don't know // For a multi-comment notification, we can't make a direct link, because we don't know
@ -72,23 +73,21 @@ trait DiscussionToolsEventTrait {
// * Mention notifications are *never* bundled. // * Mention notifications are *never* bundled.
// Code below tries to avoid assumptions in case this behavior changes. // Code below tries to avoid assumptions in case this behavior changes.
$sectionTitles = [ $this->event->getExtraParam( 'section-title' ) ];
$commentIds = [ $this->event->getExtraParam( 'comment-id' ) ]; $commentIds = [ $this->event->getExtraParam( 'comment-id' ) ];
foreach ( $this->getBundledEvents() as $event ) { foreach ( $this->getBundledEvents() as $event ) {
$sectionTitles[] = $event->getExtraParam( 'section-title' );
$commentIds[] = $event->getExtraParam( 'comment-id' ); $commentIds[] = $event->getExtraParam( 'comment-id' );
} }
$commentIds = array_values( array_filter( $commentIds ) ); $commentIds = array_values( array_filter( $commentIds ) );
$sectionTitles = array_values( array_unique( array_filter( $sectionTitles ) ) );
if ( !$commentIds ) { if ( !$commentIds ) {
return null; return null;
} }
$params = [ 'dtnewcomments' => implode( '|', $commentIds ) ]; $params = [ 'dtnewcomments' => implode( '|', $commentIds ) ];
if ( count( $sectionTitles ) === 1 ) { // This may or may not have a fragment identifier, depending on whether it was recorded for
return $title->createFragmentTarget( $sectionTitles[0] )->getFullURL( $params ); // the first one of the bundled events. It's usually not needed because we handle scrolling
} else { // client-side, but we can keep it for no-JS users, and to reduce the jump when scrolling.
return $title->getFullURL( $params ); $titleWithOptionalSection = $this->section->getTitleWithSection();
} return $titleWithOptionalSection->getFullURL( $params );
} }
} }

View file

@ -25,7 +25,7 @@ class SubscribedNewCommentPresentationModel extends EchoEventPresentationModel {
/** /**
* @var EchoPresentationModelSection * @var EchoPresentationModelSection
*/ */
private $section; protected $section;
/** /**
* @inheritDoc * @inheritDoc