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 index 00000000..01c2ce70 --- /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 index d3f06e0e..86abbe1c 100644 --- a/swh/web/add_forge_now/models.py +++ b/swh/web/add_forge_now/models.py @@ -1,138 +1,141 @@ # Copyright (C) 2022 The Software Heritage developers # See the AUTHORS file at the top-level directory of this distribution # License: GNU Affero General Public License version 3, or any later version # See top-level LICENSE file for more information from __future__ import annotations import enum from typing import List from urllib.parse import urlparse from django.db import models from ..config import get_config from ..inbound_email.utils import get_address_for_pk from .apps import APP_LABEL class RequestStatus(enum.Enum): """Request statuses. Values are used in the ui. """ PENDING = "Pending" WAITING_FOR_FEEDBACK = "Waiting for feedback" FEEDBACK_TO_HANDLE = "Feedback to handle" ACCEPTED = "Accepted" SCHEDULED = "Scheduled" FIRST_LISTING_DONE = "First listing done" FIRST_ORIGIN_LOADED = "First origin loaded" REJECTED = "Rejected" SUSPENDED = "Suspended" DENIED = "Denied" @classmethod def choices(cls): return tuple((variant.name, variant.value) for variant in cls) def allowed_next_statuses(self) -> List[RequestStatus]: next_statuses = { self.PENDING: [self.WAITING_FOR_FEEDBACK, self.REJECTED, self.SUSPENDED], self.WAITING_FOR_FEEDBACK: [self.FEEDBACK_TO_HANDLE], self.FEEDBACK_TO_HANDLE: [ self.WAITING_FOR_FEEDBACK, self.ACCEPTED, self.REJECTED, self.SUSPENDED, ], self.ACCEPTED: [self.SCHEDULED], self.SCHEDULED: [ self.FIRST_LISTING_DONE, # in case of race condition between lister and loader: self.FIRST_ORIGIN_LOADED, ], self.FIRST_LISTING_DONE: [self.FIRST_ORIGIN_LOADED], self.FIRST_ORIGIN_LOADED: [], self.REJECTED: [], self.SUSPENDED: [self.PENDING], self.DENIED: [], } return next_statuses[self] # type: ignore class RequestActorRole(enum.Enum): MODERATOR = "moderator" SUBMITTER = "submitter" FORGE_ADMIN = "forge admin" EMAIL = "email" @classmethod def choices(cls): return tuple((variant.name, variant.value) for variant in cls) class RequestHistory(models.Model): """Comment or status change. This is commented or changed by either submitter or moderator. """ request = models.ForeignKey("Request", models.DO_NOTHING) text = models.TextField() actor = models.TextField() actor_role = models.TextField(choices=RequestActorRole.choices()) date = models.DateTimeField(auto_now_add=True) new_status = models.TextField(choices=RequestStatus.choices(), null=True) message_source = models.BinaryField(null=True) class Meta: app_label = APP_LABEL db_table = "add_forge_request_history" class Request(models.Model): status = models.TextField( choices=RequestStatus.choices(), default=RequestStatus.PENDING.name, ) submission_date = models.DateTimeField(auto_now_add=True) submitter_name = models.TextField() submitter_email = models.TextField() submitter_forward_username = models.BooleanField(default=False) # FIXME: shall we do create a user model inside the webapp instead? forge_type = models.TextField() forge_url = models.TextField() forge_contact_email = models.EmailField() forge_contact_name = models.TextField() forge_contact_comment = models.TextField( null=True, 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" @property def inbound_email_address(self) -> str: """Generate an email address for correspondence related to this request.""" base_address = get_config()["add_forge_now"]["email_address"] return get_address_for_pk(salt=APP_LABEL, base_address=base_address, pk=self.pk) @property def forge_domain(self) -> str: """Get the domain/netloc out of the forge_url. Fallback to using the first part of the url path, if the netloc can't be found (for instance, if the url scheme hasn't been set). """ parsed_url = urlparse(self.forge_url) domain = parsed_url.netloc if not domain: domain = parsed_url.path.split("/", 1)[0] return domain diff --git a/swh/web/api/views/add_forge_now.py b/swh/web/api/views/add_forge_now.py index b388ddeb..83479d31 100644 --- a/swh/web/api/views/add_forge_now.py +++ b/swh/web/api/views/add_forge_now.py @@ -1,413 +1,391 @@ # Copyright (C) 2022 The Software Heritage developers # See the AUTHORS file at the top-level directory of this distribution # License: GNU Affero General Public License version 3, or any later version # See top-level LICENSE file for more information import json from typing import Any, Dict, Union 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 from django.http.response import HttpResponse, HttpResponseForbidden from rest_framework import serializers from rest_framework.request import Request from rest_framework.response import Response from swh.web.add_forge_now.models import Request as AddForgeRequest from swh.web.add_forge_now.models import RequestActorRole as AddForgeNowRequestActorRole from swh.web.add_forge_now.models import RequestHistory as AddForgeNowRequestHistory from swh.web.add_forge_now.models import RequestStatus as AddForgeNowRequestStatus from swh.web.api.apidoc import api_doc, format_docstring from swh.web.api.apiurls import api_route from swh.web.auth.utils import is_add_forge_now_moderator from swh.web.common.exc import BadInputExc from swh.web.common.utils import reverse def _block_while_testing(): """Replaced by tests to check concurrency behavior""" pass class AddForgeNowRequestForm(ModelForm): forge_contact_comment = CharField( required=False, ) class Meta: model = AddForgeRequest fields = ( "forge_type", "forge_url", "forge_contact_email", "forge_contact_name", "forge_contact_comment", "submitter_forward_username", ) class AddForgeNowRequestHistoryForm(ModelForm): new_status = CharField( max_length=200, required=False, ) class Meta: model = AddForgeNowRequestHistory fields = ("text", "new_status") class AddForgeNowRequestSerializer(serializers.ModelSerializer): 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.""" class Meta: model = AddForgeRequest fields = ("id", "forge_url", "forge_type", "status", "submission_date") class AddForgeNowRequestHistorySerializer(serializers.ModelSerializer): message_source_url = serializers.SerializerMethodField() class Meta: model = AddForgeNowRequestHistory exclude = ("request", "message_source") def get_message_source_url(self, request_history): if request_history.message_source is None: return None return reverse( "forge-add-message-source", url_args={"id": request_history.pk}, request=self.context["request"], ) class AddForgeNowRequestHistoryPublicSerializer(serializers.ModelSerializer): class Meta: model = AddForgeNowRequestHistory fields = ("id", "date", "new_status", "actor_role") @api_route( r"/add-forge/request/create/", "api-1-add-forge-request-create", methods=["POST"], ) @api_doc("/add-forge/request/create") @format_docstring() @transaction.atomic def api_add_forge_request_create(request: Union[HttpRequest, Request]) -> HttpResponse: """ .. http:post:: /api/1/add-forge/request/create/ Create a new request to add a forge to the list of those crawled regularly by Software Heritage. .. warning:: That endpoint is not publicly available and requires authentication in order to be able to request it. {common_headers} :[0-9]+)/update/", "api-1-add-forge-request-update", methods=["POST"], ) @api_doc("/add-forge/request/update", tags=["hidden"]) @format_docstring() @transaction.atomic def api_add_forge_request_update( request: Union[HttpRequest, Request], id: int ) -> HttpResponse: """ .. http:post:: /api/1/add-forge/request/update/ Update a request to add a forge to the list of those crawled regularly by Software Heritage. .. warning:: That endpoint is not publicly available and requires authentication in order to be able to request it. {common_headers} :[0-9]+)/get/", "api-1-add-forge-request-get", methods=["GET"], ) @api_doc("/add-forge/request/get") @format_docstring() def api_add_forge_request_get(request: Request, id: int): """ .. http:get:: /api/1/add-forge/request/get/ Return all details about an add-forge request. {common_headers} :param int id: add-forge request identifier :statuscode 200: request details successfully returned :statuscode 400: request identifier does not exist """ try: add_forge_request = AddForgeRequest.objects.get(id=id) except ObjectDoesNotExist: raise BadInputExc("Request id does not exist") request_history = AddForgeNowRequestHistory.objects.filter( request=add_forge_request ).order_by("id") if is_add_forge_now_moderator(request.user): data = AddForgeNowRequestSerializer(add_forge_request).data history = AddForgeNowRequestHistorySerializer( request_history, many=True, context={"request": request} ).data else: data = AddForgeNowRequestPublicSerializer(add_forge_request).data history = AddForgeNowRequestHistoryPublicSerializer( request_history, many=True ).data return {"request": data, "history": history} diff --git a/swh/web/tests/add_forge_now/test_migration.py b/swh/web/tests/add_forge_now/test_migration.py index 6eaf9dd6..f0b0fafc 100644 --- a/swh/web/tests/add_forge_now/test_migration.py +++ b/swh/web/tests/add_forge_now/test_migration.py @@ -1,111 +1,168 @@ # Copyright (C) 2022 The Software Heritage developers # See the AUTHORS file at the top-level directory of this distribution # License: GNU General Public License version 3, or any later version # See top-level LICENSE file for more information from datetime import datetime, timezone import pytest from swh.web.add_forge_now.apps import APP_LABEL 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: return datetime.now(tz=timezone.utc) def test_add_forge_now_initial_migration(migrator): """Basic migration test to check the model is fine""" state = migrator.apply_tested_migration((APP_LABEL, MIGRATION_0001)) request = state.apps.get_model(APP_LABEL, "Request") request_history = state.apps.get_model(APP_LABEL, "RequestHistory") from swh.web.add_forge_now.models import RequestActorRole, RequestStatus date_now = now() 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() assert req.submission_date > date_now req_history = request_history( request=req, text="some comment from the moderator", actor="moderator", actor_role=RequestActorRole.MODERATOR, new_status=None, ) req_history.save() assert req_history.date > req.submission_date req_history2 = request_history( request=req, text="some answer from the user", actor="user", actor_role=RequestActorRole.SUBMITTER, new_status=None, ) req_history2.save() assert req_history2.date > req_history.date def test_add_forge_now_allow_no_comment(migrator): """Basic migration test to check new model authorized empty comment""" from django.db.utils import IntegrityError state = migrator.apply_tested_migration((APP_LABEL, MIGRATION_0001)) def make_request_with_empty_comment(requestModel): return requestModel( status="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=None, ) requestModel = state.apps.get_model(APP_LABEL, "Request") req = make_request_with_empty_comment(requestModel) with pytest.raises(IntegrityError, match="violates not-null constraint"): req.save() state = migrator.apply_tested_migration((APP_LABEL, MIGRATION_0002)) requestModel2 = state.apps.get_model(APP_LABEL, "Request") req2 = make_request_with_empty_comment(requestModel2) req2.save() 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") assert not hasattr(requestModel, "submitter_forward_username") state = migrator.apply_tested_migration((APP_LABEL, MIGRATION_0003)) 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 index 48874ebf..88750eae 100644 --- a/swh/web/tests/add_forge_now/test_views.py +++ b/swh/web/tests/add_forge_now/test_views.py @@ -1,207 +1,221 @@ # Copyright (C) 2022 The Software Heritage developers # See the AUTHORS file at the top-level directory of this distribution # License: GNU Affero General Public License version 3, or any later version # See top-level LICENSE file for more information import json import pytest from swh.web.common.utils import reverse from swh.web.tests.api.views.test_add_forge_now import create_add_forge_request from swh.web.tests.utils import check_http_get_response NB_FORGE_TYPE = 2 NB_FORGES_PER_TYPE = 20 def create_add_forge_requests(client, regular_user, regular_user2): requests = [] for i in range(NB_FORGES_PER_TYPE): request = { "forge_type": "gitlab", "forge_url": f"https://gitlab.example{i:02d}.org", "forge_contact_email": f"admin@gitlab.example{i:02d}.org", "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", "forge_url": f"https://gitea.example{i:02d}.org", "forge_contact_email": f"admin@gitea.example{i:02d}.org", "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 @pytest.mark.django_db(transaction=True, reset_sequences=True) def test_add_forge_request_list_datatables_no_parameters( client, regular_user, regular_user2 ): create_add_forge_requests(client, regular_user, regular_user2) url = reverse("add-forge-request-list-datatables") resp = check_http_get_response(client, url, status_code=200) data = json.loads(resp.content) length = 10 assert data["draw"] == 0 assert data["recordsFiltered"] == NB_FORGE_TYPE * NB_FORGES_PER_TYPE assert data["recordsTotal"] == NB_FORGE_TYPE * NB_FORGES_PER_TYPE assert len(data["data"]) == length # default ordering is by descending id 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" not in data["data"][0] @pytest.mark.django_db(transaction=True, reset_sequences=True) def test_add_forge_request_list_datatables( client, regular_user, regular_user2, add_forge_moderator ): create_add_forge_requests(client, regular_user, regular_user2) length = 10 url = reverse( "add-forge-request-list-datatables", query_params={"draw": 1, "length": length, "start": 0}, ) client.force_login(regular_user) resp = check_http_get_response(client, url, status_code=200) data = json.loads(resp.content) assert data["draw"] == 1 assert data["recordsFiltered"] == NB_FORGE_TYPE * NB_FORGES_PER_TYPE assert data["recordsTotal"] == NB_FORGE_TYPE * NB_FORGES_PER_TYPE assert len(data["data"]) == length # default ordering is by descending id 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" not in data["data"][0] client.force_login(add_forge_moderator) resp = check_http_get_response(client, url, status_code=200) data = json.loads(resp.content) assert data["draw"] == 1 assert data["recordsFiltered"] == NB_FORGE_TYPE * NB_FORGES_PER_TYPE assert data["recordsTotal"] == NB_FORGE_TYPE * NB_FORGES_PER_TYPE assert len(data["data"]) == length # default ordering is by descending id 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( "add-forge-request-list-datatables", query_params={ "draw": 1, "length": length, "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) assert data["draw"] == 1 assert data["recordsFiltered"] == NB_FORGE_TYPE * NB_FORGES_PER_TYPE 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: expected_forge_urls = forge_urls_desc[i * length : (i + 1) * length] assert page_forge_urls == expected_forge_urls @pytest.mark.django_db(transaction=True, reset_sequences=True) def test_add_forge_request_list_datatables_search(client, regular_user, regular_user2): create_add_forge_requests(client, regular_user, regular_user2) url = reverse( "add-forge-request-list-datatables", query_params={ "draw": 1, "length": NB_FORGES_PER_TYPE, "start": 0, "search[value]": "gitlab", }, ) client.force_login(regular_user) resp = check_http_get_response(client, url, status_code=200) data = json.loads(resp.content) assert data["draw"] == 1 assert data["recordsFiltered"] == NB_FORGES_PER_TYPE assert data["recordsTotal"] == NB_FORGE_TYPE * NB_FORGES_PER_TYPE assert len(data["data"]) == NB_FORGES_PER_TYPE page_forge_type = [request["forge_type"] for request in data["data"]] assert page_forge_type == ["gitlab"] * NB_FORGES_PER_TYPE @pytest.mark.django_db(transaction=True, reset_sequences=True) def test_add_forge_request_list_datatables_user_requests( client, regular_user, regular_user2 ): create_add_forge_requests(client, regular_user, regular_user2) url = reverse( "add-forge-request-list-datatables", query_params={ "draw": 1, "length": NB_FORGES_PER_TYPE * NB_FORGE_TYPE, "start": 0, "user_requests_only": 1, }, ) client.force_login(regular_user2) resp = check_http_get_response(client, url, status_code=200) data = json.loads(resp.content) assert data["draw"] == 1 assert data["recordsFiltered"] == NB_FORGES_PER_TYPE assert data["recordsTotal"] == NB_FORGE_TYPE * NB_FORGES_PER_TYPE assert len(data["data"]) == NB_FORGES_PER_TYPE page_forge_type = [request["forge_type"] for request in data["data"]] assert page_forge_type == ["gitea"] * NB_FORGES_PER_TYPE