Skip to content
This repository was archived by the owner on Jul 27, 2022. It is now read-only.

ISAICP-6194: Don't send out unnecessary notifications when a group is deleted #2431

Merged
merged 5 commits into from
Apr 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion tests/features/collection/notification.collection.feature
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
Feature: Notification test for the collection transitions.
In order to manage my collections
As an user that is related to the collection
I want to receive a notification an event occurs.
I want to receive a notification when an event occurs

Scenario: Notifications should be sent whenever an event is occurring related to a collection.
Given the following owner:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,4 +112,38 @@ Feature: Notification test for the discussion transitions on a post moderated pa
Examples:
| moderation | roles |
| no | |
| yes | author |
| yes | author |

Scenario: No notifications should be sent when a discussion is orphaned
Given collections:
| title | state |
| Event Horizon Telescope | validated |
And discussion content:
| title | collection | state |
| How do the jets fire into space? | Event Horizon Telescope | validated |
And users:
| Username |
| Lindsey McCray |
| Cambria Falconer |
| Monroe Fearchar |
And collection user memberships:
| collection | user | roles |
| Event Horizon Telescope | Lindsey McCray | owner, facilitator |
And discussion subscriptions:
| username | title |
| Cambria Falconer | How do the jets fire into space? |
And comments:
| message | author | parent |
| Huge magnetic fields | Monroe Fearchar | How do the jets fire into space? |

When all e-mails have been sent
And I am logged in as a moderator
And I go to the homepage of the "Event Horizon Telescope" collection
And I click "Edit" in the "Entity actions" region
And I click "Delete"
And I press "Delete"
Then the following email should have been sent:
| recipient | Lindsey McCray |
| subject | Joinup: Your collection has been deleted by the moderation team |
| body | The Joinup moderation team deleted the collection Event Horizon Telescope |
And 1 e-mail should have been sent
Original file line number Diff line number Diff line change
Expand Up @@ -199,12 +199,13 @@ protected function entityViewAccess(CommunityContentInterface $content, AccountI
$view_scheme = $this->getPermissionScheme('view');
$workflow_id = $content->getWorkflow()->getId();
$state = $content->getWorkflowState();
$roles = $view_scheme[$workflow_id][$state] ?? [];
// @todo Shouldn't we return AccessResult::neutral() instead of
// AccessResult::allowed() and only AccessResult::forbidden() should have
// cacheable metadata? Neutral means we don't make any opinion but the
// default view access on node is to allow.
// @see https://citnet.tech.ec.europa.eu/CITnet/jira/browse/ISAICP-6007
$result = $this->workflowHelper->userHasOwnAnyRoles($content, $account, $view_scheme[$workflow_id][$state]) ? AccessResult::allowed() : AccessResult::forbidden();
$result = !empty($roles) && $this->workflowHelper->userHasOwnAnyRoles($content, $account, $roles) ? AccessResult::allowed() : AccessResult::forbidden();
return $result->addCacheableDependency($content);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher;
use Drupal\changed_fields\ObserverInterface;
use Drupal\joinup_discussion\Entity\DiscussionInterface;
use Drupal\joinup_discussion\Event\DiscussionEvents;
use Drupal\joinup_discussion\Event\DiscussionUpdateEvent;

Expand Down Expand Up @@ -61,7 +62,7 @@ public function update(\SplSubject $entity_subject): void {
$changed_fields = $entity_subject->getChangedFields();
// Dispatch the update event only if there are changes of relevant fields
// and the discussion is in the 'validated' state.
if ($changed_fields && $discussion->get('field_state')->value === 'validated') {
if ($discussion instanceof DiscussionInterface && $changed_fields && $discussion->get('field_state')->value === 'validated') {
$event = new DiscussionUpdateEvent($discussion, $changed_fields);
$this->eventDispatcher->dispatch(DiscussionEvents::UPDATE, $event);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

namespace Drupal\joinup_discussion\Event;

use Drupal\node\NodeInterface;
use Drupal\joinup_discussion\Entity\DiscussionInterface;
use Symfony\Component\EventDispatcher\Event;

/**
Expand All @@ -15,27 +15,27 @@ abstract class DiscussionEventBase extends Event {
/**
* The discussion node.
*
* @var \Drupal\node\NodeInterface
* @var \Drupal\joinup_discussion\Entity\DiscussionInterface
*/
protected $node;

/**
* Creates a new discussion event object.
*
* @param \Drupal\node\NodeInterface $node
* @param \Drupal\joinup_discussion\Entity\DiscussionInterface $node
* The discussion node subject of event.
*/
public function __construct(NodeInterface $node) {
public function __construct(DiscussionInterface $node) {
$this->node = $node;
}

/**
* Returns the discussion node.
*
* @return \Drupal\node\NodeInterface
* @return \Drupal\joinup_discussion\Entity\DiscussionInterface
* The discussion node.
*/
public function getNode(): NodeInterface {
public function getNode(): DiscussionInterface {
return $this->node;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

namespace Drupal\joinup_discussion\Event;

use Drupal\node\NodeInterface;
use Drupal\joinup_discussion\Entity\DiscussionInterface;

/**
* An event to fire whenever a discussion is updated.
Expand All @@ -21,12 +21,12 @@ class DiscussionUpdateEvent extends DiscussionEventBase {
/**
* Creates a new discussion event object.
*
* @param \Drupal\node\NodeInterface $node
* @param \Drupal\joinup_discussion\Entity\DiscussionInterface $node
* The discussion node subject of event.
* @param array $changed_fields
* A list of changed fields, keyed by field name.
*/
public function __construct(NodeInterface $node, array $changed_fields) {
public function __construct(DiscussionInterface $node, array $changed_fields) {
parent::__construct($node);
$this->changedFields = $changed_fields;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use Drupal\joinup_discussion\Event\DiscussionDeleteEvent;
use Drupal\joinup_discussion\Event\DiscussionEvents;
use Drupal\joinup_discussion\Event\DiscussionUpdateEvent;
use Drupal\joinup_group\Exception\MissingGroupException;
use Drupal\joinup_notification\JoinupMessageDeliveryInterface;
use Drupal\joinup_notification\MessageArgumentGenerator;
use Drupal\joinup_subscription\JoinupDiscussionSubscriptionInterface;
Expand Down Expand Up @@ -127,6 +128,14 @@ public function notifyOnDiscussionDeletion(DiscussionDeleteEvent $event): void {
return;
}

// Don't send out notifications for orphaned discussions.
try {
$discussion->getGroup();
}
catch (MissingGroupException $e) {
return;
}

$this->sendMessage($discussion, 'discussion_delete');
}

Expand Down