Skip to content
Snippets Groups Projects
Commit 40f520a3 authored by Simon Urli's avatar Simon Urli
Browse files

XWIKI-19070: Change the notifications locations inclusion default: don't...

XWIKI-19070: Change the notifications locations inclusion default: don't consider that the whole wiki is watched when nothing is watched

  * Change algorithm of the ScopeNotificationFilter so that it works
    properly with the new default value (wiki not watched by default)
even with target events
  * Fix tests and perform minor improvments in tests
parent 922adf1e
No related branches found
No related tags found
No related merge requests found
Showing
with 27 additions and 24 deletions
......@@ -31,7 +31,7 @@
<name>XWiki Platform - Mentions - Notifications</name>
<packaging>jar</packaging>
<properties>
<xwiki.jacoco.instructionRatio>0.77</xwiki.jacoco.instructionRatio>
<xwiki.jacoco.instructionRatio>0.76</xwiki.jacoco.instructionRatio>
<!-- This component must be installed at the farm level because
org.xwiki.eventstream.store.internal.RecordableEventListener.convertEvent uses
org.xwiki.component.manager.ComponentManager. To be removed once XWIKI-17359 is fixed. -->
......
......@@ -80,19 +80,27 @@ public FilterPolicy filterEvent(Event event, DocumentReference user,
// targets an user. We still check the exclusive one in case the user would want to avoid spam.
boolean checkInclusiveFilters = event.getTarget() == null || event.getTarget().isEmpty();
// We dismiss the event if the location is not watched or if the starting date of the location is after
// the date of the event.
// Note: the filtering on the date is not handled on the HQL-side because the request used to be too long and
// used to generate stack overflows. So we won't make it worse by adding a date condition on each different
// scope preference.
WatchedLocationState state
= stateComputer.isLocationWatched(filterPreferences, eventEntity, event.getType(), format, false,
checkInclusiveFilters);
if (!state.isWatched() || (state.getStartingDate() != null
// We only filter if the inclusive filter is strictly after the event date.
// Event that occurs at the same date that the inclusive filters are kept.
&& state.getStartingDate().after(event.getDate()))) {
return FilterPolicy.FILTER;
// We dismiss the event if:
// 1. the location is not watched without any starting date (default behaviour in case of no filter, but
// only if it's not a target event)
// 2. the location is not watched because of a filter that has been added before the event date
// 3. the location is now watched because of a filter, but the event has been sent before the filter exists
if (state.getStartingDate() == null) {
return (!state.isWatched() && checkInclusiveFilters) ? FilterPolicy.FILTER : FilterPolicy.NO_EFFECT;
} else {
if (state.isWatched() && state.getStartingDate().after(event.getDate())) {
return FilterPolicy.FILTER;
} else if (!state.isWatched() && event.getDate().before(event.getDate())) {
return FilterPolicy.FILTER;
}
}
// Otherwise, we have nothing to say
......
......@@ -114,8 +114,8 @@ public WatchedLocationState isLocationWatched(Collection<NotificationFilterPrefe
Iterator<ScopeNotificationFilterPreference> it = preferences.getInclusiveFiltersThatHasNoParents();
if (!checkInclusiveFilters || !it.hasNext()) {
// No inclusive filters == we get everything, so the location is watched
return new WatchedLocationState(true);
// No inclusive filters == we get nothing, so it's not watched.
return new WatchedLocationState(false);
}
boolean match = false;
......@@ -155,6 +155,7 @@ private WatchedLocationState handleExclusiveFilters(EntityReference location,
// If the exclusive filter match the event location...
if (match(pref, location) && deepLevel > deepestLevel) {
state = WatchedState.NOT_WATCHED;
startingDate = pref.getStartingDate();
deepestLevel = deepLevel;
// then we watch the location if there is at least an inclusive filter child matching it
......
......@@ -233,11 +233,11 @@ void complexCase1() throws Exception
assertEquals(NotificationFilter.FilterPolicy.NO_EFFECT,
this.scopeNotificationFilter.filterEvent(event4, user, filterPreferences, NotificationFormat.ALERT));
// Test with wikiB:SpaceH.DocumentI - kept because nothing match and there is no top level inclusive filter
// Test with wikiB:SpaceH.DocumentI - filtered because nothing match and there is no top level inclusive filter
Event event5 = mock(Event.class);
when(event5.getDocument()).thenReturn(new DocumentReference("wikiB", "SpaceH", "DocumentI"));
when(event5.getDate()).thenReturn(new Date(100));
assertEquals(NotificationFilter.FilterPolicy.NO_EFFECT,
assertEquals(NotificationFilter.FilterPolicy.FILTER,
this.scopeNotificationFilter.filterEvent(event5, user, filterPreferences, NotificationFormat.ALERT));
}
......
......@@ -159,8 +159,8 @@ public void notificationsEmails(TestUtils testUtils) throws Exception
DocumentReference page1 = new DocumentReference("xwiki", NOTIFICATIONS_EMAIL_TEST, "Page1");
DocumentReference page2 = new DocumentReference("xwiki", NOTIFICATIONS_EMAIL_TEST, "Page2");
testUtils.createPage(NOTIFICATIONS_EMAIL_TEST, "Page1", "Content 1", "Title 1");
testUtils.createPage(NOTIFICATIONS_EMAIL_TEST, "Page2", "Content 2", "Title 2");
testUtils.createPage(page1, "Content 1", "Title 1");
testUtils.createPage(page2, "Content 2", "Title 2");
// Wait for the notifications to be handled.
testUtils.login(SECOND_USER_NAME, SECOND_USER_PASSWORD);
......
......@@ -92,6 +92,7 @@ public class NotificationsIT
@BeforeEach
public void setup(TestUtils setup) throws Exception
{
setup.loginAsSuperAdmin();
// Create the two users we will be using
setup.createUser(FIRST_USER_NAME, FIRST_USER_PASSWORD, "", "");
setup.createUser(SECOND_USER_NAME, SECOND_USER_PASSWORD, "", "");
......@@ -122,6 +123,7 @@ public void tearDown(TestUtils setup)
{
setup.deletePage("XWiki", FIRST_USER_NAME);
setup.deletePage("XWiki", SECOND_USER_NAME);
setup.forceGuestUser();
}
@Test
......
......@@ -509,7 +509,7 @@ void globalAndOtherUserSettings(TestUtils testUtils, TestReference testReference
assertEquals(UNDETERMINED, notificationsUserProfilePage.getApplicationState(SYSTEM, EMAIL_FORMAT));
assertEquals(ON, notificationsUserProfilePage.getEventTypeState(SYSTEM, ADD_COMMENT, ALERT_FORMAT));
assertEquals(OFF, notificationsUserProfilePage.getEventTypeState(SYSTEM, ADD_COMMENT, EMAIL_FORMAT));
assertEquals(ON, notificationsUserProfilePage.getEventTypeState(SYSTEM, CREATE, ALERT_FORMAT));
assertEquals(OFF, notificationsUserProfilePage.getEventTypeState(SYSTEM, CREATE, ALERT_FORMAT));
assertEquals(ON, notificationsUserProfilePage.getEventTypeState(SYSTEM, CREATE, EMAIL_FORMAT));
assertEquals(ON, notificationsUserProfilePage.getEventTypeState(SYSTEM, DELETE, ALERT_FORMAT));
assertEquals(ON, notificationsUserProfilePage.getEventTypeState(SYSTEM, DELETE, EMAIL_FORMAT));
......
......@@ -19,15 +19,12 @@
*/
package org.xwiki.platform.notifications.test.po;
import org.xwiki.stability.Unstable;
/**
* Represents the Notifications preferences in administration.
*
* @version $Id$
* @since 13.2R1
*/
@Unstable
public class NotificationsAdministrationPage extends AbstractNotificationsSettingsPage
{
/**
......
......@@ -227,6 +227,7 @@ public void clearAllNotifications()
*/
public void showNotificationTray()
{
getDriver().scrollTo(this.watchListButton);
if (!isMenuOpen()) {
this.watchListButton.click();
getDriver().waitUntilCondition(webDriver -> isMenuOpen());
......
......@@ -25,7 +25,6 @@
import org.openqa.selenium.By;
import org.openqa.selenium.WebElement;
import org.xwiki.platform.notifications.test.po.AbstractNotificationsSettingsPage;
import org.xwiki.stability.Unstable;
import org.xwiki.test.ui.XWikiWebDriver;
import org.xwiki.test.ui.po.BootstrapSwitch;
......@@ -35,7 +34,6 @@
* @version $Id$
* @since 13.2RC1
*/
@Unstable
public abstract class AbstractNotificationFilterPreference
{
private AbstractNotificationsSettingsPage parentPage;
......
......@@ -25,7 +25,6 @@
import org.openqa.selenium.By;
import org.openqa.selenium.WebElement;
import org.xwiki.platform.notifications.test.po.AbstractNotificationsSettingsPage;
import org.xwiki.stability.Unstable;
import org.xwiki.test.ui.XWikiWebDriver;
import org.xwiki.test.ui.po.ConfirmationBox;
......@@ -36,7 +35,6 @@
* @version $Id$
* @since 13.2RC1
*/
@Unstable
public class CustomNotificationFilterPreference extends AbstractNotificationFilterPreference
{
/**
......
......@@ -22,7 +22,6 @@
import org.openqa.selenium.By;
import org.openqa.selenium.WebElement;
import org.xwiki.platform.notifications.test.po.AbstractNotificationsSettingsPage;
import org.xwiki.stability.Unstable;
import org.xwiki.test.ui.XWikiWebDriver;
/**
......@@ -31,7 +30,6 @@
* @version $Id$
* @since 13.2RC1
*/
@Unstable
public class SystemNotificationFilterPreference extends AbstractNotificationFilterPreference
{
private String description;
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment