Skip to content

Commit

Permalink
XWIKI-19751: Move the "watch" button for a page to the page content m…
Browse files Browse the repository at this point in the history 
…enu (#3059)

Goal of this change is to simplify usage of the watch page feature.
It comes with important changes:
   - the watch location switches in the bell menu area have been removed
   - a new button with a logo and a status has been introduced next to edit button
   - a new modal has been introduced when clicking on the button providing various choices
   - the watch page API have been modified to not just provide boolean value watch / not watched, but to provide a larger panel of possible statuses
   - a REST resource have been provided to allow watching / blocking / removing a filter on a location
  

* Start providing REST API for watch pages to simplify operations
  * Distinguish more cases for watched / ignored pages to improve UI
  * Provide implementation for REST API for watching pages
  * Improve WatchEntitiesManager API to support more operations
  * Improve WatchedEntityReference API to use UserReference
  * Provide a dedicated page to handle watch settings
  * Start writing javascript code to handle choices
  * Fix issue in WatchedEntitiesManager
  * Start providing doc / fixing checkstyle
  * Handle a bit more actions in UI
  * Fix a bug in state computer related to the fact that the scoe filter
    hierarchy is not really a hierarchy
  * Provide a new API to allow computing the reference of the immediate
    ancestor of a page for which a filter exists
  * Improve UI to base possible actions on multiple criteria: current
    watch status, presence or not of a filter on an ancestor, and check
if the page is terminal or not
  * Encode all options for the watch settings modal
  * Change the URL scheme for the REST API
  * Minor improvments
  * Provide test for notification rest module
  * Fix checkstyle
  * Put revapi ignores
  * Start fixing since
  * Provide translations and improve UIX code
  * Provide page object and fix integration tests
  * Ensure to not display the watch status for guest
  * Fix a few since and some problems in UIX
  * Fix API of WatchedUserReference
  * Fix bad escaping
  * Better exception handling for creating references
  * Various improvments in translations (active voice + use of ignore)
  * Improve a bit the UI
  * Improve UI when a page is ignored/followed by an ancestor and check
    proper rights
  * Review available options and improve javascript to allow chaining
    operations: following space when the page is followed implies to
first unfollow the page to not keep unnecessary filters and mess up the
UI
  * Improve translations
  * Clean up some unnecessary code
  * Make the UI a bit more clear
  * Fix some stuff after rebsae
  * Slightly improve UX
  * Fix missing unstable
  * Fix bad deprecated API call
  • Loading branch information
@surli
surli authored Jun 12, 2024
1 parent 44b7467 commit 5f90563
Show file tree
Hide file tree
Showing 30 changed files with 2,674 additions and 703 deletions.
67 changes: 65 additions & 2 deletions xwiki-platform-core/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,71 @@
</item>
</differences>
</revapi.differences>


<revapi.differences>
<justification>
The REST API shouldn't be called directly from Java but through HTTP requests. Adding a new query string
parameter to an existing REST resource doesn't break existing code that calls this resource through HTTP.
</justification>
<criticality>allowed</criticality>
<differences>
<item>
<ignore>true</ignore>
<code>java.method.numberOfParametersChanged</code>
<old>method org.xwiki.rest.model.jaxb.Pages org.xwiki.rest.resources.pages.PageChildrenResource::getPageChildren(java.lang.String, java.lang.String, java.lang.String, java.lang.Integer, java.lang.Integer, java.lang.Boolean) throws org.xwiki.rest.XWikiRestException</old>
<new>method org.xwiki.rest.model.jaxb.Pages org.xwiki.rest.resources.pages.PageChildrenResource::getPageChildren(java.lang.String, java.lang.String, java.lang.String, java.lang.Integer, java.lang.Integer, java.lang.Boolean, java.lang.String, java.lang.String) throws org.xwiki.rest.XWikiRestException</new>
</item>
</differences>
</revapi.differences>
<revapi.differences>
<justification>Changes in unstable API of notification for watching pages.</justification>
<criticality>allowed</criticality>
<differences>
<item>
<ignore>true</ignore>
<code>java.field.removed</code>
<old>field org.xwiki.notifications.filters.watch.WatchedEntityReference.WatchedStatus.NOT_WATCHED</old>
</item>
<item>
<ignore>true</ignore>
<code>java.field.removed</code>
<old>field org.xwiki.notifications.filters.watch.WatchedEntityReference.WatchedStatus.WATCHED_FOR_SOME_EVENTS_OR_FORMATS</old>
</item>
<item>
<ignore>true</ignore>
<code>java.method.parameterTypeChanged</code>
<old>parameter org.xwiki.notifications.filters.watch.WatchedEntityReference.WatchedStatus org.xwiki.notifications.filters.watch.WatchedEntityReference::getWatchedStatus(===org.xwiki.model.reference.DocumentReference===) throws org.xwiki.notifications.NotificationException</old>
<new>parameter org.xwiki.notifications.filters.watch.WatchedEntityReference.WatchedStatus org.xwiki.notifications.filters.watch.WatchedEntityReference::getWatchedStatus(===org.xwiki.user.UserReference===) throws org.xwiki.notifications.NotificationException</new>
<parameterIndex>0</parameterIndex>
</item>
<item>
<ignore>true</ignore>
<code>java.method.parameterTypeChanged</code>
<old>parameter org.xwiki.notifications.filters.watch.WatchedEntityReference.WatchedStatus org.xwiki.notifications.filters.watch.WatchedEntityReference::getWatchedStatus(===org.xwiki.model.reference.DocumentReference===) throws org.xwiki.notifications.NotificationException @ org.xwiki.notifications.filters.watch.WatchedUserReference</old>
<new>parameter org.xwiki.notifications.filters.watch.WatchedEntityReference.WatchedStatus org.xwiki.notifications.filters.watch.WatchedUserReference::getWatchedStatus(===org.xwiki.user.UserReference===) throws org.xwiki.notifications.NotificationException</new>
<parameterIndex>0</parameterIndex>
</item>
</differences>
</revapi.differences>
<revapi.differences>
<justification>Watch page API changes to be able to compute it more exactly</justification>
<criticality>highlight</criticality>
<differences>
<item>
<ignore>true</ignore>
<code>java.method.numberOfParametersChanged</code>
<old>method void org.xwiki.notifications.filters.watch.WatchedLocationReference::&lt;init&gt;(org.xwiki.model.reference.EntityReference, java.lang.String, org.xwiki.model.reference.EntityReferenceResolver&lt;java.lang.String&gt;, org.xwiki.notifications.filters.internal.scope.ScopeNotificationFilterLocationStateComputer, org.xwiki.notifications.filters.NotificationFilterPreferenceManager)</old>
<new>method void org.xwiki.notifications.filters.watch.WatchedLocationReference::&lt;init&gt;(org.xwiki.model.reference.EntityReference, org.xwiki.component.manager.ComponentManager) throws org.xwiki.component.manager.ComponentLookupException</new>
<justification>Simplify future extensions of this API.</justification>
</item>
<item>
<ignore>true</ignore>
<code>java.method.numberOfParametersChanged</code>
<old>method void org.xwiki.notifications.filters.watch.WatchedUserReference::&lt;init&gt;(java.lang.String, org.xwiki.notifications.filters.internal.user.EventUserFilterPreferencesGetter, org.xwiki.notifications.filters.NotificationFilterPreferenceManager)</old>
<new>method void org.xwiki.notifications.filters.watch.WatchedUserReference::&lt;init&gt;(java.lang.String, org.xwiki.component.manager.ComponentManager) throws org.xwiki.component.manager.ComponentLookupException</new>
<justification>Simplify future extensions of this API.</justification>
</item>
</differences>
</revapi.differences>
</analysisConfiguration>
</configuration>
</plugin>
Expand Down
2 changes: 1 addition & 1 deletion ...form-legacy-notifications-filters/xwiki-platform-legacy-notifications-filters-api/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
By setting this property, we make sure this application would be automatically uninstalled if the administrator
installs the old Activity Stream Application. -->
<xwiki.extension.features>org.xwiki.platform:xwiki-platform-notifications-filters-api</xwiki.extension.features>
<xwiki.jacoco.instructionRatio>0.08</xwiki.jacoco.instructionRatio>
<xwiki.jacoco.instructionRatio>0.07</xwiki.jacoco.instructionRatio>
</properties>
<dependencies>
<!-- Trigger xwiki-platform-notifications-filters-api dependencies (but without
Expand Down
75 changes: 45 additions & 30 deletions ...src/main/java/org/xwiki/notifications/filters/internal/scope/ScopeNotificationFilter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import javax.inject.Named;
import javax.inject.Singleton;

import org.slf4j.Logger;
import org.xwiki.component.annotation.Component;
import org.xwiki.eventstream.Event;
import org.xwiki.model.reference.DocumentReference;
Expand Down Expand Up @@ -65,46 +66,60 @@ public class ScopeNotificationFilter implements NotificationFilter
@Inject
private ScopeNotificationFilterExpressionGenerator expressionGenerator;

@Inject
private Logger logger;

@Override
public FilterPolicy filterEvent(Event event, DocumentReference user,
Collection<NotificationFilterPreference> filterPreferences,
NotificationFormat format)
{
final EntityReference eventEntity = getEventEntity(event);
if (eventEntity == null) {
// We don't handle events that are not related to a particular location
return FilterPolicy.NO_EFFECT;
FilterPolicy result = FilterPolicy.NO_EFFECT;
if (eventEntity != null) {
// We don't check the inclusive filters if the target is specified since it means the notification already
// 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();

// 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, false);

result = getFilterPolicy(event, user, state, checkInclusiveFilters);
}
return result;
}

// We don't check the inclusive filters if the target is specified since it means the notification already
// 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();


// 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, false);

// 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() && state.getStartingDate().before(event.getDate())) {
return FilterPolicy.FILTER;
private FilterPolicy getFilterPolicy(Event event, DocumentReference user, WatchedLocationState state,
boolean checkInclusiveFilters)
{
FilterPolicy result = FilterPolicy.NO_EFFECT;
switch (state.getState()) {
case BLOCKED, BLOCKED_BY_ANCESTOR, BLOCKED_WITH_CHILDREN -> {
if (state.getStartingDate() != null && state.getStartingDate().before(event.getDate())) {
result = FilterPolicy.FILTER;
}
}
}

// Otherwise, we have nothing to say
return FilterPolicy.NO_EFFECT;
case WATCHED, WATCHED_BY_ANCESTOR, WATCHED_WITH_CHILDREN -> {
if (state.getStartingDate() != null && state.getStartingDate().after(event.getDate())) {
result = FilterPolicy.FILTER;
}
}

case CUSTOM ->
this.logger.error("Filtering of event should never return custom. Event: [{}]. User: [{}] ",
event, user);

default -> {
if (checkInclusiveFilters) {
result = FilterPolicy.FILTER;
}
}
}
return result;
}

@Override
Expand Down
Loading

0 comments on commit 5f90563

Please sign in to comment.