diff --git a/swh/web/add_forge_now/migrations/0006_request_add_new_fields.py b/swh/web/add_forge_now/migrations/0006_request_add_new_fields.py new file mode 100644 --- /dev/null +++ b/swh/web/add_forge_now/migrations/0006_request_add_new_fields.py @@ -0,0 +1,42 @@ +# Generated by Django 2.2.28 on 2022-08-12 15:08 + +from django.db import migrations, models + + +def _set_new_fields_value_to_requests(apps, schema_editor): + AddForgeNowRequest = apps.get_model("swh_web_add_forge_now", "Request") + AddForgeNowRequestHistory = apps.get_model( + "swh_web_add_forge_now", "RequestHistory" + ) + for request in AddForgeNowRequest.objects.all(): + history = AddForgeNowRequestHistory.objects.filter(request=request) + history = history.order_by("id") + if history: + request.last_modified_date = history.last().date + last_history_with_moderator = history.filter(actor_role="MODERATOR").last() + if last_history_with_moderator: + request.last_moderator = last_history_with_moderator.actor + request.save() + + +class Migration(migrations.Migration): + + dependencies = [ + ("swh_web_add_forge_now", "0005_prepare_inbound_email"), + ] + + operations = [ + migrations.AddField( + model_name="request", + name="last_moderator", + field=models.TextField(default="None"), + ), + migrations.AddField( + model_name="request", + name="last_modified_date", + field=models.DateTimeField(null=True), + ), + migrations.RunPython( + _set_new_fields_value_to_requests, migrations.RunPython.noop + ), + ] diff --git a/swh/web/add_forge_now/models.py b/swh/web/add_forge_now/models.py --- a/swh/web/add_forge_now/models.py +++ b/swh/web/add_forge_now/models.py @@ -112,6 +112,9 @@ help_text="Where did you find this contact information (url, ...)", ) + last_moderator = models.TextField(default="None") + last_modified_date = models.DateTimeField(null=True) + class Meta: app_label = APP_LABEL db_table = "add_forge_request" diff --git a/swh/web/api/views/add_forge_now.py b/swh/web/api/views/add_forge_now.py --- a/swh/web/api/views/add_forge_now.py +++ b/swh/web/api/views/add_forge_now.py @@ -9,7 +9,6 @@ from django.core.exceptions import ObjectDoesNotExist from django.core.paginator import Paginator from django.db import transaction -from django.db.models.query import QuerySet from django.forms import CharField, ModelForm from django.http import HttpResponseBadRequest from django.http.request import HttpRequest @@ -66,37 +65,10 @@ inbound_email_address = serializers.CharField() forge_domain = serializers.CharField() - last_moderator = serializers.SerializerMethodField() - last_modified_date = serializers.SerializerMethodField() - history: Dict[int, QuerySet] = {} - class Meta: model = AddForgeRequest fields = "__all__" - def _gethistory(self, request): - if request.id not in self.history: - self.history[request.id] = AddForgeNowRequestHistory.objects.filter( - request=request - ).order_by("id") - return self.history[request.id] - - def get_last_moderator(self, request): - last_history_with_moderator = ( - self._gethistory(request).filter(actor_role="MODERATOR").last() - ) - return ( - last_history_with_moderator.actor if last_history_with_moderator else "None" - ) - - def get_last_modified_date(self, request): - last_history = self._gethistory(request).last() - return ( - last_history.date.isoformat().replace("+00:00", "Z") - if last_history - else None - ) - class AddForgeNowRequestPublicSerializer(serializers.ModelSerializer): """Serializes AddForgeRequest without private fields.""" @@ -205,6 +177,9 @@ request_history.actor_role = AddForgeNowRequestActorRole.SUBMITTER.name request_history.save() + add_forge_request.last_modified_date = request_history.date + add_forge_request.save() + data = AddForgeNowRequestSerializer(add_forge_request).data return Response(data=data, status=201) @@ -292,7 +267,10 @@ if request_history.new_status is not None: add_forge_request.status = request_history.new_status - add_forge_request.save() + + add_forge_request.last_moderator = request_history.actor + add_forge_request.last_modified_date = request_history.date + add_forge_request.save() data = AddForgeNowRequestSerializer(add_forge_request).data return Response(data=data, status=200) diff --git a/swh/web/tests/add_forge_now/test_migration.py b/swh/web/tests/add_forge_now/test_migration.py --- a/swh/web/tests/add_forge_now/test_migration.py +++ b/swh/web/tests/add_forge_now/test_migration.py @@ -12,6 +12,8 @@ MIGRATION_0001 = "0001_initial" MIGRATION_0002 = "0002_authorized_null_comment" MIGRATION_0003 = "0003_request_submitter_forward_username" +MIGRATION_0005 = "0005_prepare_inbound_email" +MIGRATION_0006 = "0006_request_add_new_fields" def now() -> datetime: @@ -99,7 +101,6 @@ def test_add_forge_now_store_submitter_forward_username(migrator): - """Basic migration test to check new model authorized empty comment""" state = migrator.apply_tested_migration((APP_LABEL, MIGRATION_0002)) requestModel = state.apps.get_model(APP_LABEL, "Request") @@ -109,3 +110,59 @@ requestModel2 = state.apps.get_model(APP_LABEL, "Request") assert hasattr(requestModel2, "submitter_forward_username") + + +def test_add_forge_now_add_new_fields_to_request(migrator): + + state = migrator.apply_tested_migration((APP_LABEL, MIGRATION_0005)) + Request = state.apps.get_model(APP_LABEL, "Request") + RequestHistory = state.apps.get_model(APP_LABEL, "RequestHistory") + assert not hasattr(Request, "last_moderator") + assert not hasattr(Request, "last_modified_date") + + from swh.web.add_forge_now.models import RequestActorRole, RequestStatus + + req = Request( + status=RequestStatus.PENDING, + submitter_name="dudess", + submitter_email="dudess@orga.org", + forge_type="cgit", + forge_url="https://example.org/forge", + forge_contact_email="forge@//example.org", + forge_contact_name="forge", + forge_contact_comment=( + "Discovered on the main forge homepag, following contact link." + ), + ) + req.save() + + req_history = RequestHistory( + request=req, + text="some comment from the submitter", + actor="submitter", + actor_role=RequestActorRole.SUBMITTER.name, + new_status=None, + ) + req_history.save() + + req_history = RequestHistory( + request=req, + text="some comment from the moderator", + actor="moderator", + actor_role=RequestActorRole.MODERATOR.name, + new_status=None, + ) + req_history.save() + + state = migrator.apply_tested_migration((APP_LABEL, MIGRATION_0006)) + Request = state.apps.get_model(APP_LABEL, "Request") + RequestHistory = state.apps.get_model(APP_LABEL, "RequestHistory") + + assert hasattr(Request, "last_moderator") + assert hasattr(Request, "last_modified_date") + + for request in Request.objects.all(): + history = RequestHistory.objects.filter(request=request) + history = history.order_by("id") + assert request.last_modified_date == history.last().date + assert request.last_moderator == history.last().actor diff --git a/swh/web/tests/add_forge_now/test_views.py b/swh/web/tests/add_forge_now/test_views.py --- a/swh/web/tests/add_forge_now/test_views.py +++ b/swh/web/tests/add_forge_now/test_views.py @@ -25,12 +25,16 @@ "forge_contact_name": f"gitlab.example{i:02d}.org admin", "forge_contact_comment": "user marked as owner in forge members", } - create_add_forge_request( - client, - regular_user, - data=request, + + requests.append( + json.loads( + create_add_forge_request( + client, + regular_user, + data=request, + ).content + ) ) - requests.append(request) request = { "forge_type": "gitea", @@ -39,12 +43,16 @@ "forge_contact_name": f"gitea.example{i:02d}.org admin", "forge_contact_comment": "user marked as owner in forge members", } - create_add_forge_request( - client, - regular_user2, - data=request, + + requests.append( + json.loads( + create_add_forge_request( + client, + regular_user2, + data=request, + ).content + ) ) - requests.append(request) return requests @@ -107,19 +115,26 @@ assert data["data"][0]["id"] == NB_FORGE_TYPE * NB_FORGES_PER_TYPE assert data["data"][-1]["id"] == NB_FORGE_TYPE * NB_FORGES_PER_TYPE - length + 1 assert "submitter_name" in data["data"][0] + assert "last_moderator" in data["data"][0] + assert "last_modified_date" in data["data"][0] + + return data @pytest.mark.django_db(transaction=True, reset_sequences=True) +@pytest.mark.parametrize("order_field", ["forge_url", "last_modified_date"]) def test_add_forge_request_list_datatables_ordering( - client, regular_user, regular_user2 + client, add_forge_moderator, admin_user, order_field ): - requests = create_add_forge_requests(client, regular_user, regular_user2) - requests_sorted = list(sorted(requests, key=lambda d: d["forge_url"])) - forge_urls_asc = [request["forge_url"] for request in requests_sorted] + requests = create_add_forge_requests(client, add_forge_moderator, admin_user) + requests_sorted = list(sorted(requests, key=lambda d: d[order_field])) + forge_urls_asc = [request[order_field] for request in requests_sorted] forge_urls_desc = list(reversed(forge_urls_asc)) length = 10 + client.force_login(admin_user) + for direction in ("asc", "desc"): for i in range(4): url = reverse( @@ -130,11 +145,10 @@ "start": i * length, "order[0][column]": 2, "order[0][dir]": direction, - "columns[2][name]": "forge_url", + "columns[2][name]": order_field, }, ) - client.force_login(regular_user) resp = check_http_get_response(client, url, status_code=200) data = json.loads(resp.content) @@ -143,7 +157,7 @@ assert data["recordsTotal"] == NB_FORGE_TYPE * NB_FORGES_PER_TYPE assert len(data["data"]) == length - page_forge_urls = [request["forge_url"] for request in data["data"]] + page_forge_urls = [request[order_field] for request in data["data"]] if direction == "asc": expected_forge_urls = forge_urls_asc[i * length : (i + 1) * length] else: