diff --git a/swh/web/auth/mailmap.py b/swh/web/auth/mailmap.py --- a/swh/web/auth/mailmap.py +++ b/swh/web/auth/mailmap.py @@ -3,6 +3,8 @@ # License: GNU Affero General Public License version 3, or any later version # See top-level LICENSE file for more information +import json + from django.conf.urls import url from django.db import IntegrityError from django.db.models import Q @@ -17,7 +19,7 @@ from rest_framework.request import Request from rest_framework.response import Response -from swh.web.auth.models import UserMailmap +from swh.web.auth.models import UserMailmap, UserMailmapEvent from swh.web.auth.utils import MAILMAP_PERMISSION @@ -41,6 +43,12 @@ if not request.user.has_perm(MAILMAP_PERMISSION): return HttpResponseForbidden() + event = UserMailmapEvent.objects.create( + user_id=str(request.user.id), + request_type="add", + request=json.dumps(request.data), + ) + from_email = request.data.pop("from_email", None) if not from_email: return HttpResponseBadRequest("'from_email' must be provided and non-empty.") @@ -51,9 +59,11 @@ ) except IntegrityError: return HttpResponseBadRequest("This 'from_email' already exists.") - mm = UserMailmap.objects.get( - user_id=str(request.user.id), from_email=from_email - ) + + event.successful = True + event.save() + + mm = UserMailmap.objects.get(user_id=str(request.user.id), from_email=from_email) return Response(UserMailmapSerializer(mm).data) @@ -62,6 +72,12 @@ if not request.user.has_perm(MAILMAP_PERMISSION): return HttpResponseForbidden() + event = UserMailmapEvent.objects.create( + user_id=str(request.user.id), + request_type="update", + request=json.dumps(request.data), + ) + from_email = request.data.pop("from_email", None) if not from_email: return HttpResponseBadRequest("'from_email' must be provided and non-empty.") @@ -82,6 +98,9 @@ to_update.save() + event.successful = True + event.save() + mm = UserMailmap.objects.get(user_id=user_id, from_email=from_email) return Response(UserMailmapSerializer(mm).data) diff --git a/swh/web/auth/migrations/0005_usermailmapevent.py b/swh/web/auth/migrations/0005_usermailmapevent.py new file mode 100644 --- /dev/null +++ b/swh/web/auth/migrations/0005_usermailmapevent.py @@ -0,0 +1,39 @@ +# Generated by Django 2.2.27 on 2022-02-07 16:02 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("swh_web_auth", "0004_usermailmap"), + ] + + operations = [ + migrations.CreateModel( + name="UserMailmapEvent", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("timestamp", models.DateTimeField(auto_now=True)), + ("user_id", models.CharField(max_length=50)), + ("request_type", models.CharField(max_length=50)), + ("request", models.TextField()), + ("successful", models.BooleanField(default=False)), + ], + options={"db_table": "user_mailmap_event",}, + ), + migrations.AddIndex( + model_name="usermailmapevent", + index=models.Index( + fields=["timestamp"], name="user_mailma_timesta_1f7aef_idx" + ), + ), + ] diff --git a/swh/web/auth/models.py b/swh/web/auth/models.py --- a/swh/web/auth/models.py +++ b/swh/web/auth/models.py @@ -91,3 +91,33 @@ return "%s <%s>" % (self.display_name, self.to_email) else: return self.display_name + + +class UserMailmapEvent(models.Model): + """ + Represents an update to a mailmap object + """ + + timestamp = models.DateTimeField(auto_now=True, null=False) + """Timestamp of the moment the event was submitted""" + + user_id = models.CharField(max_length=50, null=False) + """User id from Keycloak of the user who changed the mailmap. + (Not necessarily the one who the mail belongs to.)""" + + request_type = models.CharField(max_length=50, null=False) + """Either ``add`` or ``update``.""" + + request = models.TextField(null=False) + """JSON dump of the request received.""" + + successful = models.BooleanField(default=False, null=False) + """If False, then the request failed or crashed before completing, + and may or may not have altered the database's state.""" + + class Meta: + indexes = [ + models.Index(fields=["timestamp"]), + ] + app_label = "swh_web_auth" + db_table = "user_mailmap_event" diff --git a/swh/web/tests/auth/test_mailmap.py b/swh/web/tests/auth/test_mailmap.py --- a/swh/web/tests/auth/test_mailmap.py +++ b/swh/web/tests/auth/test_mailmap.py @@ -4,6 +4,7 @@ # See top-level LICENSE file for more information import datetime +import json from io import StringIO from typing import Dict @@ -14,7 +15,7 @@ from django.db import transaction from swh.model.model import Person -from swh.web.auth.models import UserMailmap +from swh.web.auth.models import UserMailmap, UserMailmapEvent from swh.web.auth.utils import MAILMAP_PERMISSION from swh.web.common.utils import reverse from swh.web.tests.utils import ( @@ -40,13 +41,13 @@ @pytest.mark.django_db(transaction=True) def test_mailmap_endpoints_user_with_permission(api_client, mailmap_user): api_client.force_login(mailmap_user) + + request_data = {"from_email": "bar@example.org", "display_name": "bar"} + for view_name in ("profile-mailmap-add", "profile-mailmap-update"): url = reverse(view_name) check_api_post_response( - api_client, - url, - data={"from_email": "bar@example.org", "display_name": "bar"}, - status_code=200, + api_client, url, data=request_data, status_code=200, ) # FIXME: use check_api_get_responses; currently this crashes without @@ -61,6 +62,13 @@ assert resp[0]["from_email"] == "bar@example.org" assert resp[0]["display_name"] == "bar" + events = UserMailmapEvent.objects.order_by("timestamp").all() + assert len(events) == 2 + assert events[0].request_type == "add" + assert json.loads(events[0].request) == request_data + assert events[1].request_type == "update" + assert json.loads(events[1].request) == request_data + @pytest.mark.django_db(transaction=True) def test_mailmap_add_duplicate(api_client, mailmap_user): @@ -108,6 +116,12 @@ assert len(resp) == 1 assert resp[0].items() >= request_data.items() + events = UserMailmapEvent.objects.all() + assert len(events) == 1 + assert events[0].request_type == "add" + assert json.loads(events[0].request) == request_data + assert events[0].successful + @pytest.mark.django_db(transaction=True) def test_mailmap_endpoints_error_response(api_client, mailmap_user): @@ -121,6 +135,17 @@ resp = check_api_post_response(api_client, url, status_code=400) assert b"from_email" in resp.content + events = UserMailmapEvent.objects.order_by("timestamp").all() + assert len(events) == 2 + + assert events[0].request_type == "add" + assert json.loads(events[0].request) == {} + assert not events[0].successful + + assert events[1].request_type == "update" + assert json.loads(events[1].request) == {} + assert not events[1].successful + @pytest.mark.django_db(transaction=True) def test_mailmap_update(api_client, mailmap_user): @@ -175,6 +200,12 @@ assert mailmaps[1].display_name == "Display Name 2", mailmaps assert before_add <= mailmaps[1].last_update_date <= after_add + events = UserMailmapEvent.objects.order_by("timestamp").all() + assert len(events) == 3 + assert events[0].request_type == "add" + assert events[1].request_type == "add" + assert events[2].request_type == "update" + def populate_mailmap(): for (verified, activated) in (