Merge pull request #102 from MerginMaps/remove_unnecessary_emails_#86…

…1n3wzau

remove unimportant emails #861n3wzau

server/mergin/auth/db_events.py

@@ -2,56 +2,9 @@
 #
 # SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial
 
-from flask import render_template, current_app
-from sqlalchemy import event
-
 from .app import force_delete_user
 from .. import db
-from ..auth.models import UserProfile, User
-
-
-def before_user_profile_updated(mapper, connection, target):
-    """Before profile updated, inform user by sending email about that profile that changed
-    Just send email if user want to receive notifications
-    """
-    from ..celery import send_email_async
-
-    if target.receive_notifications and target.user.verified_email:
-        state = db.inspect(target)
-        changes = {}
-
-        for attr in state.attrs:
-            hist = attr.load_history()
-            if not hist.has_changes():
-                continue
-
-            before = hist.deleted[0]
-            after = hist.added[0]
-            field = attr.key
-
-            # if boolean, show Yes or No
-            if before is not None and isinstance(before, bool):
-                before = "Yes" if before is True else "No"
-            if after is not None and isinstance(after, bool):
-                after = "Yes" if after is True else "No"
-
-            profile_key = field.title().replace("_", " ")
-            changes[profile_key] = {"before": before, "after": after}
-
-        # inform user
-        if changes:
-            email_data = {
-                "subject": "Profile has been changed",
-                "html": render_template(
-                    "email/profile_changed.html",
-                    subject="Profile update",
-                    user=target.user,
-                    changes=changes,
-                ),
-                "recipients": [target.user.email],
-                "sender": current_app.config["MAIL_DEFAULT_SENDER"],
-            }
-            send_email_async.delay(**email_data)
+from ..auth.models import User
 
 
 def permanently_delete_user(user: User):
@@ -61,10 +14,8 @@ def permanently_delete_user(user: User):
 
 
 def register_events():
-    event.listen(UserProfile, "after_update", before_user_profile_updated)
     force_delete_user.connect(permanently_delete_user)
 
 
 def remove_events():
-    event.remove(UserProfile, "after_update", before_user_profile_updated)
     force_delete_user.disconnect(permanently_delete_user)

server/mergin/sync/db_events.py

@@ -7,9 +7,8 @@
 from sqlalchemy import event
 
 from .. import db
-from ..auth.models import User, UserProfile
+from ..auth.models import User
 from .models import Project, ProjectAccess
-from .public_api_controller import project_deleted
 
 
 def remove_user_references(mapper, connection, user):  # pylint: disable=W0612
@@ -35,39 +34,6 @@ def filter_user(ids):
             )
 
 
-def project_post_delete_actions(project: Project) -> None:  # pylint: disable=W0612
-    """After project is deleted inform users by sending email"""
-    from ..celery import send_email_async
-
-    if not project.access:
-        return
-    users_ids = list(
-        set(project.access.owners + project.access.writers + project.access.readers)
-    )
-    users_profiles = UserProfile.query.filter(UserProfile.user_id.in_(users_ids)).all()
-    project_workspace = project.workspace
-    for profile in users_profiles:
-        # skip the user who triggered deletion
-        if profile.user.username == project.removed_by:
-            continue
-
-        if not (profile.receive_notifications and profile.user.verified_email):
-            continue
-
-        email_data = {
-            "subject": f'Mergin project {"/".join([project_workspace.name, project.name])} has been deleted',
-            "html": render_template(
-                "email/removed_project.html",
-                subject="Project deleted",
-                project=project,
-                username=profile.user.username,
-            ),
-            "recipients": [profile.user.email],
-            "sender": current_app.config["MAIL_DEFAULT_SENDER"],
-        }
-        send_email_async.delay(**email_data)
-
-
 def check(session):
     if os.path.isfile(current_app.config["MAINTENANCE_FILE"]):
         abort(503, "Service unavailable due to maintenance, please try later")
@@ -76,10 +42,8 @@ def check(session):
 def register_events():
     event.listen(User, "before_delete", remove_user_references)
     event.listen(db.session, "before_commit", check)
-    project_deleted.connect(project_post_delete_actions)
 
 
 def remove_events():
     event.remove(User, "before_delete", remove_user_references)
     event.remove(db.session, "before_commit", check)
-    project_deleted.disconnect(project_post_delete_actions)

server/mergin/sync/private_api_controller.py

@@ -266,31 +266,6 @@ def unsubscribe_project(id):  # pylint: disable=W0612
     project.access.unset_role(current_user.id)
     db.session.add(project)
     db.session.commit()
-    # notify owners and the user who unsubscribed
-    project_path = get_project_path(project)
-    recipients = (
-        UserProfile.query.filter(
-            UserProfile.user_id.in_(project.access.owners + [current_user.id])
-        )
-        .filter(UserProfile.receive_notifications)
-        .all()
-    )
-    for profile in recipients:
-        if not profile.user.verified_email:
-            continue
-        html = render_template(
-            "email/project_unsubscribe.html",
-            project_path=project_path,
-            recipient=profile.user.username,
-            username=current_user.username,
-        )
-        email_data = {
-            "subject": f"Access to mergin project {project_path} has been modified",
-            "html": html,
-            "recipients": [profile.user.email],
-            "sender": current_app.config["MAIL_DEFAULT_SENDER"],
-        }
-        send_email_async.delay(**email_data)
     return NoContent, 200
 
 

server/mergin/sync/public_api_controller.py

@@ -18,7 +18,6 @@
 from connexion import NoContent, request
 from flask import (
     abort,
-    render_template,
     current_app,
     send_from_directory,
     jsonify,
@@ -35,7 +34,7 @@
 from werkzeug.exceptions import HTTPException
 from .. import db
 from ..auth import auth_required
-from ..auth.models import User, UserProfile
+from ..auth.models import User
 from .models import Project, ProjectAccess, ProjectVersion, Upload
 from .schemas import (
     ProjectSchema,
@@ -73,7 +72,6 @@
     get_path_from_files,
     get_project_path,
 )
-from ..celery import send_email_async
 from .errors import StorageLimitHit
 from ..utils import format_time_delta
 
@@ -644,49 +642,6 @@ def update_project(namespace, project_name):  # noqa: E501  # pylint: disable=W0
         db.session.add(project)
         db.session.commit()
 
-    # send email notifications about changes to users
-    user_profiles = UserProfile.query.filter(
-        UserProfile.user_id.in_(list(id_diffs))
-    ).all()
-    project_path = "/".join([namespace, project.name])
-    web_link = f"{request.url_root.strip('/')}/projects/{project_path}"
-    for user_profile in user_profiles:
-        if not (
-            user_profile.receive_notifications and user_profile.user.verified_email
-        ):
-            continue
-        privileges = []
-        if user_profile.user.id in project.access.owners:
-            privileges += ["edit", "remove"]
-        if user_profile.user.id in project.access.writers:
-            privileges.append("upload")
-        if user_profile.user.id in project.access.readers:
-            privileges.append("download")
-        subject = "Project access modified"
-        if len(privileges):
-            html = render_template(
-                "email/modified_project_access.html",
-                subject=subject,
-                project=project,
-                user=user_profile.user,
-                privileges=privileges,
-                link=web_link,
-            )
-        else:
-            html = render_template(
-                "email/removed_project_access.html",
-                subject=subject,
-                project=project,
-                user=user_profile.user,
-            )
-
-        email_data = {
-            "subject": f"Access to mergin project {project_path} has been modified",
-            "html": html,
-            "recipients": [user_profile.user.email],
-            "sender": current_app.config["MAIL_DEFAULT_SENDER"],
-        }
-        send_email_async.delay(**email_data)
     # partial success
     if error:
         return jsonify(**error.to_dict(), project=ProjectSchema().dump(project)), 207

server/mergin/tests/test_auth.py

@@ -5,7 +5,7 @@
 from datetime import datetime, timedelta
 import pytest
 import json
-from flask import url_for, current_app
+from flask import url_for
 from itsdangerous import URLSafeTimedSerializer
 from sqlalchemy import desc
 from unittest.mock import patch

server/mergin/tests/test_celery.py

@@ -10,12 +10,14 @@
 
 from .. import db
 from ..config import Configuration
-from ..sync.models import Project
+from ..sync.models import Project, AccessRequest
 from ..celery import send_email_async
 from ..sync.tasks import remove_temp_files, remove_projects_backups
 from ..sync.storages.disk import move_to_tmp
 from . import test_project, test_workspace_name, test_workspace_id
-from .utils import cleanup, add_user
+from .utils import add_user, create_workspace, create_project, login
+from ..auth.models import User
+from . import json_headers
 
 
 def test_send_email(app):
@@ -55,23 +57,26 @@ def test_send_email(app):
 @patch("mergin.celery.send_email_async.apply_async")
 def test_send_email_from_flask(send_email_mock, client):
     """Test correct data are passed to celery task which is called from endpoint."""
-    usr = add_user("test1", "test")
-    project = Project.query.filter_by(
-        workspace_id=test_workspace_id, name="test"
-    ).first()
-    readers = project.access.readers.copy()
-    readers.append(usr.id)
-    project.access.readers = readers
-    db.session.commit()
+    user = User.query.filter(User.username == "mergin").first()
+    test_workspace = create_workspace()
+    p = create_project("testx", test_workspace, user)
+    user2 = add_user("test_user", "ilovemergin")
+    login(client, "test_user", "ilovemergin")
     email_data = {
-        "subject": "Mergin project mergin/test has been deleted",
-        "recipients": [usr.email],
+        "subject": "Project access requested",
+        "recipients": [user.email],
         "sender": current_app.config["MAIL_DEFAULT_SENDER"],
     }
-    resp = client.delete("/v1/project/{}/{}".format("mergin", "test"))
+    resp = client.post(
+        f"/app/project/access-request/{test_workspace.name}/{p.name}",
+        headers=json_headers,
+    )
+    access_request = AccessRequest.query.filter(
+        AccessRequest.project_id == p.id
+    ).first()
     assert resp.status_code == 200
-    # cleanup files
-    cleanup(client, [project.storage.project_dir])
+    assert access_request.user.username == "test_user"
+
     assert send_email_mock.called
     call_args, _ = send_email_mock.call_args
     _, kwargs = call_args