diff --git a/backend/geonature/core/notifications/models.py b/backend/geonature/core/notifications/models.py index 5b605d4ede..498c2fde02 100644 --- a/backend/geonature/core/notifications/models.py +++ b/backend/geonature/core/notifications/models.py @@ -3,9 +3,11 @@ """ import datetime +import sqlalchemy as sa from sqlalchemy import ForeignKey from sqlalchemy.sql import select from sqlalchemy.orm import relationship +from flask import g from utils_flask_sqla.serializers import serializable from pypnusershub.db.models import User @@ -85,6 +87,28 @@ class Notification(db.Model): user = db.relationship(User) +class NotificationRuleQuery(db.Query): + def filter_by_role_with_defaults(self, id_role=None): + if id_role is None: + id_role = g.current_user.id_role + cte = ( + NotificationRule.query.filter( + sa.or_( + NotificationRule.id_role.is_(None), + NotificationRule.id_role == id_role, + ) + ) + .distinct(NotificationRule.code_category, NotificationRule.code_method) + .order_by( + NotificationRule.code_category.desc(), + NotificationRule.code_method.desc(), + NotificationRule.id_role.asc(), + ) + .cte("cte") + ) + return self.filter(NotificationRule.id == cte.c.id) + + @serializable class NotificationRule(db.Model): __tablename__ = "t_notifications_rules" @@ -92,16 +116,28 @@ class NotificationRule(db.Model): db.UniqueConstraint( "id_role", "code_method", "code_category", name="un_role_method_category" ), + db.Index( + "un_method_category", + "code_method", + "code_category", + unique=True, + postgresql_ops={ + "where": sa.text("id_role IS NULL"), + }, + ), {"schema": "gn_notifications"}, ) + query_class = NotificationRuleQuery + id = db.Column(db.Integer, primary_key=True) - id_role = db.Column(db.Integer, ForeignKey(User.id_role), nullable=False) + id_role = db.Column(db.Integer, ForeignKey(User.id_role), nullable=True) code_method = db.Column(db.Unicode, ForeignKey(NotificationMethod.code), nullable=False) code_category = db.Column( db.Unicode, ForeignKey(NotificationCategory.code), nullable=False, ) + subscribed = db.Column(db.Boolean, nullable=False) method = relationship(NotificationMethod) category = relationship(NotificationCategory) diff --git a/backend/geonature/core/notifications/routes.py b/backend/geonature/core/notifications/routes.py index 407bc10221..aa38e7261e 100644 --- a/backend/geonature/core/notifications/routes.py +++ b/backend/geonature/core/notifications/routes.py @@ -80,28 +80,20 @@ def update_notification(id_notification): @routes.route("/rules", methods=["GET"]) @permissions.login_required def list_notification_rules(): - rules = ( - NotificationRule.query.filter(NotificationRule.id_role == g.current_user.id_role) - .order_by( - NotificationRule.code_category.desc(), - NotificationRule.code_method.desc(), - ) - .options( - joinedload("method"), - joinedload("category"), - ) + rules = NotificationRule.query.filter_by_role_with_defaults().options( + joinedload("method"), + joinedload("category"), ) result = [ rule.as_dict( fields=[ - "id", - "id_role", "code_method", "code_category", "method.label", "method.description", "category.label", "category.description", + "subscribed", ] ) for rule in rules.all() @@ -121,39 +113,45 @@ def delete_all_notifications(): # add rule for user -@routes.route("/rules", methods=["PUT"]) +@routes.route( + "/rules/category//method//subscribe", + methods=["POST"], + defaults={"subscribe": True}, +) +@routes.route( + "/rules/category//method//unsubscribe", + methods=["POST"], + defaults={"subscribe": False}, +) @permissions.login_required -def create_rule(): - - requestData = request.get_json() - if requestData is None: - raise BadRequest("Empty request data") - - code_method = requestData.get("code_method") - if not code_method: - raise BadRequest("Missing method") - if not db.session.query( - NotificationMethod.query.filter_by(code=str(code_method)).exists() - ).scalar(): - raise BadRequest("Invalid method") - - code_category = requestData.get("code_category") - if not code_category: - raise BadRequest("Missing category") +def update_rule(code_category, code_method, subscribe): if not db.session.query( NotificationCategory.query.filter_by(code=str(code_category)).exists() ).scalar(): raise BadRequest("Invalid category") + if not db.session.query( + NotificationMethod.query.filter_by(code=str(code_method)).exists() + ).scalar(): + raise BadRequest("Invalid method") # Create new rule for current user - new_rule = NotificationRule( + rule = NotificationRule.query.filter_by( id_role=g.current_user.id_role, code_method=code_method, code_category=code_category, - ) - db.session.add(new_rule) + ).one_or_none() + if rule: + rule.subscribed = subscribe + else: + rule = NotificationRule( + id_role=g.current_user.id_role, + code_method=code_method, + code_category=code_category, + subscribed=subscribe, + ) + db.session.add(rule) db.session.commit() - return jsonify(new_rule.as_dict()) + return jsonify(rule.as_dict(fields=["code_method", "code_category", "subscribed"])) # Delete all rules for current user @@ -167,18 +165,6 @@ def delete_all_rules(): return jsonify(nbRulesDeleted) -# Delete a specific rule -@routes.route("/rules/", methods=["DELETE"]) -@permissions.login_required -def delete_rule(id): - rule = NotificationRule.query.get_or_404(id) - if rule.user != g.current_user: - raise Forbidden - db.session.delete(rule) - db.session.commit() - return "", 204 - - # Get all availabe method for notification @routes.route("/methods", methods=["GET"]) @permissions.login_required diff --git a/backend/geonature/core/notifications/utils.py b/backend/geonature/core/notifications/utils.py index 90df7fbc16..f8d8d61ea6 100644 --- a/backend/geonature/core/notifications/utils.py +++ b/backend/geonature/core/notifications/utils.py @@ -2,6 +2,7 @@ from jinja2 import Template from flask import current_app +import sqlalchemy as sa from pypnusershub.db.models import User @@ -40,9 +41,9 @@ def dispatch_notification(category, role, title=None, url=None, *, content=None, # add role, title and url to rendering context context = {"role": role, "title": title, "url": url, **context} - rules = NotificationRule.query.filter( - NotificationRule.id_role == role.id_role, + rules = NotificationRule.query.filter_by_role_with_defaults(role.id_role).filter( NotificationRule.code_category == category.code, + NotificationRule.subscribed.is_(sa.true()), ) for rule in rules.all(): if content: diff --git a/backend/geonature/tests/test_notifications.py b/backend/geonature/tests/test_notifications.py index aaae0767c3..4fecb842e9 100644 --- a/backend/geonature/tests/test_notifications.py +++ b/backend/geonature/tests/test_notifications.py @@ -23,6 +23,16 @@ log = logging.getLogger() +@pytest.fixture() +def notifications_enabled(monkeypatch): + monkeypatch.setitem(current_app.config, "NOTIFICATIONS_ENABLED", True) + + +@pytest.fixture(scope="class") +def clear_default_notification_rules(): + NotificationRule.query.filter(NotificationRule.id_role.is_(None)).delete() + + @pytest.fixture() def notification_data(users): with db.session.begin_nested(): @@ -98,17 +108,18 @@ def notification_rule(users, rule_method, rule_category): id_role=users["admin_user"].id_role, code_method=rule_method.code, code_category=rule_category.code, + subscribed=True, ) db.session.add(new_notification_rule) return new_notification_rule -@pytest.fixture() -def notifications_enabled(monkeypatch): - monkeypatch.setitem(current_app.config, "NOTIFICATIONS_ENABLED", True) - - -@pytest.mark.usefixtures("client_class", "temporary_transaction") +@pytest.mark.usefixtures( + "client_class", + "temporary_transaction", + "notifications_enabled", + "clear_default_notification_rules", +) class TestNotification: def test_list_database_notification(self, users, notification_data): # Init data for test @@ -237,47 +248,79 @@ def test_list_notification_rules(self, users, notification_rule): data = response.get_json() assert len(data) == 1 - def test_create_rule_ko(self, users, rule_method, rule_category): - # Init data for test - url = "notifications.create_rule" - log.debug("Url d'appel %s", url_for(url)) - - # TEST NO USER - response = self.client.put(url_for(url), content_type="application/json") - assert response.status_code == 401 + # def test_create_rule_ko(self, users, rule_method, rule_category): + # # Init data for test + # url = "notifications.create_rule" + # log.debug("Url d'appel %s", url_for(url)) + + # # TEST NO USER + # response = self.client.put(url_for(url), content_type="application/json") + # assert response.status_code == 401 + + # # TEST CONNECTED USER WITHOUT DATA + # set_logged_user_cookie(self.client, users["admin_user"]) + # response = self.client.put(url_for(url)) + # assert response.status_code == 400 + + # # TEST CONNECTED USER WITH DATA BUT WRONG KEY + # set_logged_user_cookie(self.client, users["admin_user"]) + # data = {"method": rule_method.code, "categorie": rule_category.code} + # response = self.client.put(url_for(url), json=data, content_type="application/json") + # assert response.status_code == BadRequest.code + + # # TEST CONNECTED USER WITH DATA BUT WRONG VALUE + # set_logged_user_cookie(self.client, users["admin_user"]) + # data = {"code_method": 1, "code_category": rule_category.code} + # response = self.client.put(url_for(url), json=data, content_type="application/json") + # assert response.status_code == BadRequest.code + + def test_update_rule(self, users, rule_method, rule_category): + role = users["user"] + subscribe_url = url_for( + "notifications.update_rule", + code_method=rule_method.code, + code_category=rule_category.code, + subscribe=True, + ) + unsubscribe_url = url_for( + "notifications.update_rule", + code_method=rule_method.code, + code_category=rule_category.code, + subscribe=False, + ) - # TEST CONNECTED USER WITHOUT DATA - set_logged_user_cookie(self.client, users["admin_user"]) - response = self.client.put(url_for(url)) - assert response.status_code == 400 + assert not db.session.query( + NotificationRule.query.filter_by( + id_role=role.id_role, + method=rule_method, + category=rule_category, + ).exists() + ).scalar() - # TEST CONNECTED USER WITH DATA BUT WRONG KEY - set_logged_user_cookie(self.client, users["admin_user"]) - data = {"method": rule_method.code, "categorie": rule_category.code} - response = self.client.put(url_for(url), json=data, content_type="application/json") - assert response.status_code == BadRequest.code + response = self.client.post(subscribe_url) + assert response.status_code == Unauthorized.code, response.data - # TEST CONNECTED USER WITH DATA BUT WRONG VALUE - set_logged_user_cookie(self.client, users["admin_user"]) - data = {"code_method": 1, "code_category": rule_category.code} - response = self.client.put(url_for(url), json=data, content_type="application/json") - assert response.status_code == BadRequest.code + set_logged_user_cookie(self.client, role) - def test_create_rule_ok(self, users, rule_method, rule_category): + response = self.client.post(subscribe_url) + assert response.status_code == 200, response.data - url = "notifications.create_rule" - log.debug("Url d'appel %s", url_for(url)) + rule = NotificationRule.query.filter_by( + id_role=role.id_role, + method=rule_method, + category=rule_category, + ).one() + assert rule.subscribed - # TEST SUCCESSFULL RULE CREATION - set_logged_user_cookie(self.client, users["user"]) - data = {"code_method": rule_method.code, "code_category": rule_category.code} - response = self.client.put(url_for(url), json=data, content_type="application/json") + response = self.client.post(unsubscribe_url) assert response.status_code == 200, response.data - newRule = response.get_json() - assert newRule.get("code_method") == rule_method.code - assert newRule.get("code_category") == rule_category.code - assert newRule.get("id_role") == users["user"].id_role + rule = NotificationRule.query.filter_by( + id_role=role.id_role, + method=rule_method, + category=rule_category, + ).one() + assert not rule.subscribed def test_delete_all_rules(self, users, notification_rule): # Init data for test @@ -312,35 +355,6 @@ def test_delete_all_rules(self, users, notification_rule): ).exists() ).scalar() - def test_delete_rule(self, users, notification_rule): - # Init data for test - url = "notifications.delete_rule" - log.debug("Url d'appel %s", url_for(url, id=1)) - - # TEST NO USER - response = self.client.delete(url_for(url, id=1)) - assert response.status_code == Unauthorized.code - - # TEST CONNECTED USER WITHOUT RULE - set_logged_user_cookie(self.client, users["user"]) - response = self.client.delete(url_for(url, id=notification_rule.id)) - assert response.status_code == Forbidden.code - assert db.session.query( - NotificationRule.query.filter_by( - id=notification_rule.id, - ).exists() - ).scalar() - - # TEST CONNECTED USER WITH RULE - set_logged_user_cookie(self.client, users["admin_user"]) - response = self.client.delete(url_for(url, id=notification_rule.id)) - assert response.status_code == 204 - assert not db.session.query( - NotificationRule.query.filter_by( - id=notification_rule.id, - ).exists() - ).scalar() - def test_list_methods(self, users, rule_method): # Init data for test @@ -397,6 +411,7 @@ def test_dispatch_notifications_database_with_like( id_role=role.id_role, code_method="DB", code_category=rule_category_1.code, + subscribed=True, ) db.session.add(new_rule) @@ -428,7 +443,7 @@ def test_dispatch_notifications_database_with_like( assert notif.code_status == "UNREAD" def test_dispatch_notifications_database_with_like( - self, users, rule_category, rule_category_1, rule_template, notifications_enabled + self, users, rule_category, rule_category_1, rule_template ): role = users["user"] @@ -438,6 +453,7 @@ def test_dispatch_notifications_database_with_like( id_role=role.id_role, code_method="DB", code_category=rule_category_1.code, + subscribed=True, ) db.session.add(new_rule) @@ -469,7 +485,7 @@ def test_dispatch_notifications_database_with_like( assert notif.code_status == "UNREAD" def test_dispatch_notifications_mail_with_template( - self, users, rule_category, rule_mail_template, notifications_enabled, celery_eager + self, users, rule_category, rule_mail_template, celery_eager ): with db.session.begin_nested(): users["user"].email = "user@geonature.fr" @@ -479,6 +495,7 @@ def test_dispatch_notifications_mail_with_template( id_role=users["user"].id_role, code_method="EMAIL", code_category=rule_category.code, + subscribed=True, ) ) db.session.add( @@ -486,6 +503,7 @@ def test_dispatch_notifications_mail_with_template( id_role=users["admin_user"].id_role, code_method="EMAIL", code_category=rule_category.code, + subscribed=True, ) ) diff --git a/frontend/src/app/components/notification/notification-data.service.ts b/frontend/src/app/components/notification/notification-data.service.ts index dd2e580bea..0c66b100e6 100644 --- a/frontend/src/app/components/notification/notification-data.service.ts +++ b/frontend/src/app/components/notification/notification-data.service.ts @@ -43,6 +43,7 @@ export interface NotificationRule { code_method: string; method?: NotificationMethod; category?: NotificationCategory; + subscribed: boolean; } @Injectable() @@ -75,13 +76,6 @@ export class NotificationDataService { ); } - // Create rules - createRule(data) { - return this._api.put(`${AppConfig.API_ENDPOINT}/notifications/rules`, data, { - headers: new HttpHeaders().set('Content-Type', 'application/json'), - }); - } - // returns all rules for current user getRules() { return this._api.get(`${AppConfig.API_ENDPOINT}/notifications/rules`); @@ -99,11 +93,21 @@ export class NotificationDataService { return this._api.get(`${AppConfig.API_ENDPOINT}/notifications/methods`); } - deleteRules() { - return this._api.delete<{}>(`${AppConfig.API_ENDPOINT}/notifications/rules`); + subscribe(category, method) { + return this._api.post( + `${AppConfig.API_ENDPOINT}/notifications/rules/category/${category}/method/${method}/subscribe`, + null + ); + } + + unsubscribe(category, method) { + return this._api.post( + `${AppConfig.API_ENDPOINT}/notifications/rules/category/${category}/method/${method}/unsubscribe`, + null + ); } - deleteRule(id: number) { - return this._api.delete<{}>(`${AppConfig.API_ENDPOINT}/notifications/rules/${id}`); + clearSubscriptions() { + return this._api.delete(`${AppConfig.API_ENDPOINT}/notifications/rules`); } } diff --git a/frontend/src/app/components/notification/rules/rules.component.html b/frontend/src/app/components/notification/rules/rules.component.html index c1d4010d5d..8b3be03be1 100644 --- a/frontend/src/app/components/notification/rules/rules.component.html +++ b/frontend/src/app/components/notification/rules/rules.component.html @@ -75,8 +75,8 @@

Gestion des règles de notification

-
diff --git a/frontend/src/app/components/notification/rules/rules.component.ts b/frontend/src/app/components/notification/rules/rules.component.ts index 49c0e204e0..bca73b0ecf 100644 --- a/frontend/src/app/components/notification/rules/rules.component.ts +++ b/frontend/src/app/components/notification/rules/rules.component.ts @@ -55,23 +55,24 @@ export class RulesComponent implements OnInit { * Create a rule for un user * data inclue code_category and code_method */ - createRule(data) { - this.notificationDataService.createRule(data).subscribe((response) => {}); + subscribe(category, method) { + this.notificationDataService.subscribe(category, method).subscribe((response) => {}); } /** * delete one rule with its id */ - deleteRule(idRule) { - this.notificationDataService.deleteRule(idRule).subscribe((response) => {}); + unsubscribe(category, method) { + this.notificationDataService.unsubscribe(category, method).subscribe((response) => {}); } /** * delete all user rules */ - deleteRules() { - this.notificationDataService.deleteRules().subscribe((response) => { + clearSubscriptions() { + this.notificationDataService.clearSubscriptions().subscribe((response) => { // refresh rules values + //this.getRules(); -- this does not trigger update of checkboxes this.ngOnInit(); }); } @@ -79,22 +80,16 @@ export class RulesComponent implements OnInit { /** * Action from checkbox to create or delete a rule depending on checkbox value * - * @param categorie notification code_category + * @param category notification code_category * @param method notification code_method * @param event event to get checkbox */ - updateRule(categorie, method, event) { + updateRule(category, method, event) { // if checkbox is checked add rule if (event.target.checked) { - this.createRule({ code_method: method, code_category: categorie }); + this.subscribe(category, method); } else { - // if checkbox not checked remove rule - for (var rule of this.userRules) { - if (rule.code_category == categorie && rule.code_method == method) { - this.deleteRule(rule.id); - break; - } - } + this.unsubscribe(category, method); } } @@ -105,12 +100,11 @@ export class RulesComponent implements OnInit { * @returns boolean */ hasUserSubscribed(categorie, method) { - let checked: boolean = false; for (var rule of this.userRules) { if (rule.code_category == categorie && rule.code_method == method) { - return (checked = true); + return rule.subscribed; } } - return checked; + return false; } }