Incidents API update (#95)

* Update incidents-list (/api/incidents)

* Update incidents with PUT /incidents/<pk>

* Tidy up tests

* Update incident lead via API

* /incidents list should be sorted by report date

* Return primary key in incident response

* Change deprecated parameter

* Add docs for incidents endpoint

requirements-dev.txt

@@ -15,6 +15,8 @@ django-bootstrap4==0.0.8
 djangorestframework==3.10.1
 docopt==0.6.2
 emoji-data-python==1.1.0
+factory-boy>=2.12.0
+faker>=2.0.0
 idna==2.8
 importlib-metadata==0.18
 markdown2==2.3.8

response/core/serializers.py

@@ -8,36 +8,61 @@
 from django.contrib.auth.models import User
 
 
-class ExternalUserSerializer(serializers.HyperlinkedModelSerializer):
-    owner = serializers.PrimaryKeyRelatedField(queryset=User.objects.all(), required=False)
-
+class ExternalUserSerializer(serializers.ModelSerializer):
     class Meta:
         model = ExternalUser
-        fields = ('app_id', 'external_id', 'owner', 'display_name')
+        fields = ("app_id", "external_id", "display_name")
 
 
-class ActionSerializer(serializers.HyperlinkedModelSerializer):
+class ActionSerializer(serializers.ModelSerializer):
     # Serializers define the API representation.
-    incident = serializers.PrimaryKeyRelatedField(queryset=Incident.objects.all(), required=False)
-    user = serializers.PrimaryKeyRelatedField(queryset=ExternalUser.objects.all(), required=False)
+    incident = serializers.PrimaryKeyRelatedField(
+        queryset=Incident.objects.all(), required=False
+    )
+    user = serializers.PrimaryKeyRelatedField(
+        queryset=ExternalUser.objects.all(), required=False
+    )
 
     class Meta:
         model = Action
-        fields = ('pk', 'details', 'done', 'incident', 'user')
+        fields = ("pk", "details", "done", "incident", "user")
 
 
-class IncidentSerializer(serializers.HyperlinkedModelSerializer):
-    reporter = serializers.PrimaryKeyRelatedField(queryset=ExternalUser.objects.all(), required=False)
-    lead = serializers.PrimaryKeyRelatedField(queryset=ExternalUser.objects.all(), required=False)
+class IncidentSerializer(serializers.ModelSerializer):
+    reporter = ExternalUserSerializer(read_only=True)
+    lead = ExternalUserSerializer()
 
     class Meta:
         model = Incident
-        fields = ('pk','report', 'reporter', 'lead', 'start_time', 'end_time', 'report_time', 'action_set')
+        fields = (
+            "action_set",
+            "end_time",
+            "impact",
+            "lead",
+            "pk",
+            "report",
+            "report_time",
+            "reporter",
+            "severity",
+            "start_time",
+            "summary",
+        )
+
+    def update(self, instance, validated_data):
+        instance.end_time = validated_data.get("end_time", instance.end_time)
+        instance.impact = validated_data.get("impact", instance.impact)
+
+        new_lead = validated_data.get("lead", None)
+        if new_lead:
+            instance.lead = ExternalUser.objects.get(
+                display_name=new_lead["display_name"],
+                external_id=new_lead["external_id"],
+            )
 
-    def __init__(self, *args, **kwargs):
-        super(IncidentSerializer, self).__init__(*args, **kwargs)
-        request = kwargs['context']['request']
-        expand = request.GET.get('expand', "").split(',')
+        instance.report = validated_data.get("report", instance.report)
+        instance.start_time = validated_data.get("start_time", instance.start_time)
+        instance.summary = validated_data.get("summary", instance.summary)
+        instance.severity = validated_data.get("severity", instance.severity)
 
-        if 'actions' in expand:
-            self.fields['action_set'] = ActionSerializer(many=True, read_only=True)
+        instance.save()
+        return instance

response/core/urls.py

@@ -5,7 +5,7 @@
 
 # Routers provide an easy way of automatically determining the URL conf.
 router = routers.DefaultRouter()
-router.register(r'incidents', IncidentViewSet, base_name='Incidents')
+router.register(r'incidents', IncidentViewSet, basename='incident')
 router.register(r'actions', ActionViewSet)
 router.register(r'ExternalUser', ExternalUserViewSet)
 

response/core/views.py

@@ -1,9 +1,9 @@
-from rest_framework import viewsets
+from rest_framework import pagination, viewsets
 
 from response.core.models.incident import Incident
 from response.core.models.action import Action
 from response.core.models.user_external import ExternalUser
-from response.core.serializers import ExternalUserSerializer, ActionSerializer, IncidentSerializer
+from response.core import serializers
 
 from datetime import datetime
 from calendar import monthrange
@@ -12,35 +12,24 @@
 class ExternalUserViewSet(viewsets.ModelViewSet):
     # ViewSets define the view behavior.
     queryset = ExternalUser.objects.all()
-    serializer_class = ExternalUserSerializer
+    serializer_class = serializers.ExternalUserSerializer
 
 
 class ActionViewSet(viewsets.ModelViewSet):
     # ViewSets define the view behavior.
     queryset = Action.objects.all()
-    serializer_class = ActionSerializer
+    serializer_class = serializers.ActionSerializer
 
 
-# Will return the incidents of the current month
-# Can pass ?start=2019-05-28&end=2019-06-03 to change range
 class IncidentViewSet(viewsets.ModelViewSet):
-    # ViewSets define the view behavior.
-
-    serializer_class = IncidentSerializer
-    pagination_class = None  # Remove pagination
-
-    def get_queryset(self):
-        # Same query is used to get single items so we check if pk is passed
-        # incident/2/ if we use the filter below we would have to have correct time range
-        if 'pk' in self.kwargs:
-            return Incident.objects.filter(pk=self.kwargs['pk'])
+    """
+    Allows getting a list of Incidents (sorted by report time from newest to
+    oldest), and updating existing ones.
 
-        today = datetime.today()
-        first_day_of_current_month = datetime(today.year, today.month, 1)
-        days_in_month = monthrange(today.year, today.month)[1]
-        last_day_of_current_month = datetime(today.year, today.month, days_in_month)
+    Note that Incidents can only be created via the Slack workflow.
+    """
 
-        start = self.request.GET.get('start', first_day_of_current_month)
-        end = self.request.GET.get('end', last_day_of_current_month)
+    queryset = Incident.objects.order_by("-report_time")
 
-        return Incident.objects.filter(start_time__gte=start, start_time__lte=end)
+    serializer_class = serializers.IncidentSerializer
+    pagination_class = pagination.LimitOffsetPagination

tests/api/conftest.py

@@ -0,0 +1,14 @@
+import pytest
+from rest_framework.test import APIRequestFactory
+
+from tests.factories import UserFactory, ExternalUserFactory
+
+@pytest.fixture
+def arf():
+    return APIRequestFactory()
+
+
+@pytest.fixture()
+def api_user(transactional_db):
+    e = ExternalUserFactory()
+    return e.owner

tests/api/test_incidents.py

@@ -0,0 +1,138 @@
+from django.urls import reverse
+from faker import Faker
+from rest_framework.test import force_authenticate
+import json
+import pytest
+import random
+
+from response.models import Incident, ExternalUser
+from response import serializers
+from response.core.views import IncidentViewSet
+from tests.factories import IncidentFactory
+
+faker = Faker()
+
+
+def test_list_incidents(arf, api_user):
+    persisted_incidents = IncidentFactory.create_batch(5)
+
+    req = arf.get(reverse("incident-list"))
+    force_authenticate(req, user=api_user)
+    response = IncidentViewSet.as_view({"get": "list"})(req)
+
+    assert response.status_code == 200, "Got non-200 response from API"
+    content = json.loads(response.rendered_content)
+
+    assert "results" in content, "Response didn't have results key"
+    incidents = content["results"]
+    assert len(incidents) == len(
+        persisted_incidents
+    ), "Didn't get expected number of incidents back"
+
+    for idx, incident in enumerate(incidents):
+        assert incident["report_time"]
+
+        # incidents should be in order of newest to oldest
+        if idx != len(incidents) - 1:
+            assert (
+                incident["report_time"] >= incidents[idx + 1]["report_time"]
+            ), "Incidents are not in order of newest to oldest by report time"
+
+        assert "end_time" in incident  # end_time can be null for open incidents
+        assert incident["impact"]
+        assert incident["report"]
+        assert incident["start_time"]
+        assert incident["summary"]
+        assert incident["severity"]
+
+        reporter = incident["reporter"]
+        assert reporter["display_name"]
+        assert reporter["external_id"]
+
+        lead = incident["lead"]
+        assert lead["display_name"]
+        assert lead["external_id"]
+
+        # TODO: verify actions are serialised inline
+
+
+@pytest.mark.parametrize(
+    "update_key,update_value",
+    (
+        ("", ""),  # no update
+        ("impact", faker.paragraph(nb_sentences=2, variable_nb_sentences=True)),
+        ("report", faker.paragraph(nb_sentences=1, variable_nb_sentences=True)),
+        ("summary", faker.paragraph(nb_sentences=3, variable_nb_sentences=True)),
+        (
+            "start_time",
+            faker.date_time_between(start_date="-3d", end_date="now", tzinfo=None),
+        ),
+        (
+            "end_time",
+            faker.date_time_between(start_date="-3d", end_date="now", tzinfo=None),
+        ),
+        ("severity", str(random.randint(1, 4))),
+    ),
+)
+def test_update_incident(arf, api_user, update_key, update_value):
+    """
+    Tests that we can PUT /incidents/<pk> and mutate fields that get saved to
+    the DB.
+    """
+    persisted_incidents = IncidentFactory.create_batch(5)
+
+    incident = persisted_incidents[0]
+    serializer = serializers.IncidentSerializer(incident)
+    serialized = serializer.data
+
+    updated = serialized
+    del updated["reporter"]  # can't update reporter
+    if update_key:
+        updated[update_key] = update_value
+
+    req = arf.put(
+        reverse("incident-detail", kwargs={"pk": incident.pk}), updated, format="json"
+    )
+    force_authenticate(req, user=api_user)
+
+    response = IncidentViewSet.as_view({"put": "update"})(req, pk=incident.pk)
+    print(response.rendered_content)
+    assert response.status_code == 200, "Got non-200 response from API"
+
+    if update_key:
+        new_incident = Incident.objects.get(pk=incident.pk)
+        assert (
+            getattr(new_incident, update_key) == update_value
+        ), "Updated value wasn't persisted to the DB"
+
+
+def test_update_incident_lead(arf, api_user):
+    """
+    Tests that we can update the incident lead by name
+    """
+    persisted_incidents = IncidentFactory.create_batch(5)
+
+    incident = persisted_incidents[0]
+    serializer = serializers.IncidentSerializer(incident)
+    updated = serializer.data
+
+    users = ExternalUser.objects.all()
+
+    new_lead = users[0]
+    while new_lead == incident.lead:
+        new_lead = random.choices(users)
+
+    updated["lead"] = serializers.ExternalUserSerializer(new_lead).data
+    del updated["reporter"]  # can't update reporter
+
+    req = arf.put(
+        reverse("incident-detail", kwargs={"pk": incident.pk}), updated, format="json"
+    )
+    force_authenticate(req, user=api_user)
+
+    response = IncidentViewSet.as_view({"put": "update"})(req, pk=incident.pk)
+    print(response.rendered_content)
+    assert response.status_code == 200, "Got non-200 response from API"
+
+    new_incident = Incident.objects.get(pk=incident.pk)
+    assert new_incident.lead == new_lead

tests/conftest.py

@@ -12,9 +12,13 @@
 from response.slack.authentication import generate_signature
 from response.slack.client import SlackClient
 
-@pytest.fixture()
+@pytest.fixture(autouse=True)
 def mock_slack(monkeypatch):
     mock_slack = MagicMock(spec=SlackClient(""))
+    mock_slack.send_or_update_message_block.return_value = {
+        "ok": True,
+        "ts": 1234,
+    }
     monkeypatch.setattr(settings, "SLACK_CLIENT", mock_slack)
     return mock_slack
 

tests/factories/__init__.py

@@ -0,0 +1,2 @@
+from .incident import *
+from .user import *

tests/factories/incident.py

@@ -0,0 +1,42 @@
+import random
+
+from django.db.models.signals import post_save
+import factory
+from faker import Factory
+
+from response.core.models import Incident, ExternalUser
+
+faker = Factory.create()
+
+
+@factory.django.mute_signals(post_save)
+class IncidentFactory(factory.DjangoModelFactory):
+    class Meta:
+        model = Incident
+
+    impact = factory.LazyFunction(
+        lambda: faker.paragraph(nb_sentences=1, variable_nb_sentences=True)
+    )
+    report = factory.LazyFunction(
+        lambda: faker.paragraph(nb_sentences=3, variable_nb_sentences=True)
+    )
+    report_time = factory.LazyFunction(
+        lambda: faker.date_time_between(start_date="-3d", end_date="now", tzinfo=None)
+    )
+
+    reporter = factory.SubFactory("tests.factories.ExternalUserFactory")
+    lead = factory.SubFactory("tests.factories.ExternalUserFactory")
+
+    start_time = factory.LazyFunction(
+        lambda: faker.date_time_between(start_date="-3d", end_date="now", tzinfo=None)
+    )
+
+    if random.random() > 0.5:
+        end_time = factory.LazyAttribute(
+            lambda a: faker.date_time_between(start_date=a.start_time, end_date="now")
+        )
+
+    severity = factory.LazyFunction(lambda: str(random.randint(1, 4)))
+    summary = factory.LazyFunction(
+        lambda: faker.paragraph(nb_sentences=3, variable_nb_sentences=True)
+    )

tests/factories/user.py

@@ -0,0 +1,26 @@
+from django.contrib.auth.models import User
+import factory
+from faker import Factory
+
+
+from response.core.models import ExternalUser
+
+faker = Factory.create()
+
+
+class UserFactory(factory.DjangoModelFactory):
+    class Meta:
+        model = User
+
+    username = factory.LazyFunction(faker.user_name)
+    password = factory.LazyFunction(faker.password)
+
+
+class ExternalUserFactory(factory.DjangoModelFactory):
+    class Meta:
+        model = ExternalUser
+
+    app_id = "slack"
+    display_name = factory.LazyFunction(faker.name)
+    external_id = factory.LazyFunction(lambda: "U" + faker.word())
+    owner = factory.SubFactory("tests.factories.UserFactory")