Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions apps/contrib/management/base/lookups.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,14 +86,21 @@ def data_type(self) -> str:


class FKByName(BaseLookup):
"""Resolves a foreign key by matching a human-friendly field (e.g. name) against the DB."""
"""
Resolves a foreign key by matching a human-friendly field (e.g. name) against the DB.

An ambiguous name fails the row rather than resolving to whichever match came back first.
``M2MByName`` has always done this; the two now agree, and both match what ADR 0006 says the
framework does. Pass ``error_on_multiple=False`` only where picking arbitrarily is genuinely
acceptable.
"""

def __init__(
self,
field: str,
model,
lookup_field: str = "name",
error_on_multiple: bool = False,
error_on_multiple: bool = True,
list_values: bool = True,
):
self.field = field
Expand Down Expand Up @@ -391,6 +398,12 @@ def __init__(self, field: str, model, split: str = ";", list_values: bool = Fals
self.split = split
self.list_values = list_values

def clear_value(self):
# A list field empties to [], as it does for the other multi-value lookups. Without this
# the base's None reaches a many-to-many serializer field, which refuses it — leaving an
# id-based relation settable but never clearable.
return []

def resolve(self, value):
if is_empty(value):
return []
Expand Down
40 changes: 40 additions & 0 deletions apps/organization/tests/test_import_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -254,3 +254,43 @@ def test_blacklisted_column_rejected_on_import(self):
with self.assertRaises(CommandError):
call_command("import_organizations", path)
self.assertFalse(Organization.objects.filter(name="X").exists())

def test_an_ambiguous_organization_kind_fails_the_row(self):
# ADR 0006 says a name matching multiple rows raises rather than guessing. FKByName
# defaulted to picking the first match, unlike M2MByName, so it silently linked one of them.
OrganizationKindFactory.create(name="Ambiguous Kind")
OrganizationKindFactory.create(name="Ambiguous Kind")

path = write_sheet(
["name", "category", "methodology", "organization_kind"],
[
{
"name": "Probe Org",
"category": "INTERNATIONAL",
"methodology": "probe methodology",
"organization_kind": "Ambiguous Kind",
}
],
)
out = StringIO()
with self.assertRaises(CommandError):
call_command("import_organizations", path, stdout=out)

self.assertIn("cannot disambiguate", out.getvalue())
self.assertEqual(Organization.objects.filter(name="Probe Org").count(), 0)

def test_an_unambiguous_name_still_resolves(self):
path = write_sheet(
["name", "category", "methodology", "organization_kind"],
[
{
"name": "Plain Org",
"category": "INTERNATIONAL",
"methodology": "probe methodology",
"organization_kind": "Government",
}
],
)
call_command("import_organizations", path, stdout=StringIO())

self.assertEqual(Organization.objects.get(name="Plain Org").organization_kind, self.kind)
23 changes: 20 additions & 3 deletions apps/report/management/commands/import_reports.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
M2MById,
M2MByName,
)
from apps.contrib.serializers import IntegerIDField, UpdateSerializerMixin
from apps.country.models import Country, CountryRegion, GeographicalGroup
from apps.crisis.models import Crisis
from apps.entry.models import Figure, FigureTag
Expand All @@ -17,7 +18,23 @@
ViolenceSubType,
)
from apps.report.models import Report
from apps.report.serializers import ReportSerializer, ReportUpdateSerializer
from apps.report.serializers import ReportSerializer


class ReportImportSerializer(ReportSerializer):
"""Report fields an operator may write from a sheet.

`is_pfa_visible_in_gidd` is writable here but not on `ReportSerializer`, whose fields become the
report mutation inputs: the GraphQL surface gates the flag behind `setPfaVisibleInGidd` and its
own admin permission, which a field on the create/update inputs would bypass.
"""

class Meta(ReportSerializer.Meta):
fields = ReportSerializer.Meta.fields + ["is_pfa_visible_in_gidd"]


class ReportImportUpdateSerializer(UpdateSerializerMixin, ReportImportSerializer):
id = IntegerIDField(required=True)


class Command(BaseImportCommand):
Expand All @@ -28,8 +45,8 @@ class Command(BaseImportCommand):
)

model = Report
create_serializer = ReportSerializer
update_serializer = ReportUpdateSerializer
create_serializer = ReportImportSerializer
update_serializer = ReportImportUpdateSerializer
lookups = [
# Enum arrays
EnumArrayLookup("filter_figure_categories", Figure.FIGURE_CATEGORY_TYPES),
Expand Down
18 changes: 16 additions & 2 deletions apps/report/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -259,12 +259,26 @@ def validate(self, attrs) -> dict:
raise serializers.ValidationError(errors)
return attrs

def create(self, validated_data):
instance = super().create(validated_data)
return self.clear_pfa_visibility_if_unqualified(instance)

def update(self, instance, validated_data):
validated_data["last_modified_by"] = self.context["request"].user
instance = super().update(instance, validated_data)
if check_is_pfa_visible_in_gidd(instance):
return self.clear_pfa_visibility_if_unqualified(instance)

@staticmethod
def clear_pfa_visibility_if_unqualified(instance):
"""Drop the GIDD visibility flag from a report that does not qualify to carry it.

Runs after the save because the checks read the country m2m, which is not written until
then. `is_pfa_visible_in_gidd` publishes a figure on an unauthenticated endpoint, so a
report that asks for visibility without meeting every condition is silenced, not refused.
"""
if instance.is_pfa_visible_in_gidd and check_is_pfa_visible_in_gidd(instance):
instance.is_pfa_visible_in_gidd = False
instance.save()
instance.save(update_fields=["is_pfa_visible_in_gidd"])
return instance


Expand Down
17 changes: 17 additions & 0 deletions apps/report/tests/test_import_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,3 +158,20 @@ def test_make_template_warns_on_separator_in_value(self):
message = err.getvalue()
self.assertIn("filter_figure_regions", message)
self.assertIn("';' separator", message)

def test_clear_empties_an_id_based_m2m(self):
# M2MById inherited the base clear value of None, which a many-to-many serializer field
# refuses, so an id-based relation could be set but never emptied.
report = Report.objects.create(
name="Has crises",
created_by=self.admin,
filter_figure_start_after=datetime.date(2020, 1, 1),
filter_figure_end_before=datetime.date(2020, 12, 31),
)
report.filter_figure_crises.set([self.crisis])

path = write_sheet(["id", "filter_figure_crises"], [{"id": report.id, "filter_figure_crises": "<clear>"}])
call_command("import_reports", path, "--user-email", self.admin.email)

report.refresh_from_db()
self.assertEqual(report.filter_figure_crises.count(), 0)
100 changes: 100 additions & 0 deletions apps/report/tests/test_import_reports_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@
from apps.crisis.models import Crisis
from apps.entry.models import Figure
from apps.report.management.commands.import_reports import Command as ImportReportsCommand
from apps.report.management.commands.import_reports import ReportImportSerializer
from apps.report.models import Report
from apps.report.serializers import ReportSerializer
from apps.users.enums import USER_ROLE
from utils.factories import CountryFactory, CrisisFactory, TagFactory, UserFactory, ViolenceSubTypeFactory
from utils.tests import HelixTestCase, create_user_with_role
Expand Down Expand Up @@ -311,6 +313,104 @@ def test_an_unknown_user_email_is_refused(self):
with self.assertRaises(CommandError):
call_command("import_reports", path, user_email="nobody@example.com", stdout=StringIO())

# ----- GIDD visibility -----

PFA_COLUMNS = [
"id",
"name",
"filter_figure_countries",
"filter_figure_start_after",
"filter_figure_end_before",
"filter_figure_categories",
"filter_figure_crisis_types",
"is_public",
"is_pfa_visible_in_gidd",
]

def _pfa_row(self, **overrides):
row = {
"id": None,
"name": "GRID 2021 - Nepal (ND) - C",
"filter_figure_countries": "NPL",
"filter_figure_start_after": "2020-01-01",
"filter_figure_end_before": "2020-12-31",
"filter_figure_categories": "NEW_DISPLACEMENT",
"filter_figure_crisis_types": "CONFLICT",
"is_public": "Yes",
"is_pfa_visible_in_gidd": "Yes",
}
row.update(overrides)
return row

def test_a_created_report_that_qualifies_becomes_visible_in_gidd(self):
path = write_sheet(self.PFA_COLUMNS, [self._pfa_row()])
call_command("import_reports", path, user_email=self.editor.email, stdout=StringIO())

report = Report.objects.get(name="GRID 2021 - Nepal (ND) - C")
self.assertTrue(report.is_pfa_visible_in_gidd)

def test_a_created_report_that_does_not_qualify_is_silenced_not_refused(self):
# No category: a PFA total is defined for exactly one of IDPs / Internal Displacements.
path = write_sheet(
self.PFA_COLUMNS,
[self._pfa_row(name="No Category Report", filter_figure_categories=None)],
)
out = StringIO()
call_command("import_reports", path, user_email=self.editor.email, stdout=out)

self.assertIn("Created 1, updated 0.", out.getvalue())
report = Report.objects.get(name="No Category Report")
self.assertFalse(report.is_pfa_visible_in_gidd)

def test_a_second_country_cannot_be_named_in_the_country_cell(self):
# filter_figure_countries is declared list_values=False, so the sheet cannot express the
# multi-country case the PFA rules reject - the cell fails to resolve first.
path = write_sheet(
self.PFA_COLUMNS,
[self._pfa_row(name="Two Country Report", filter_figure_countries="NPL,IND")],
)
with self.assertRaises(CommandError):
call_command("import_reports", path, user_email=self.editor.email, stdout=StringIO())
self.assertFalse(Report.objects.filter(name="Two Country Report").exists())

def test_a_non_public_report_does_not_become_visible_in_gidd(self):
path = write_sheet(self.PFA_COLUMNS, [self._pfa_row(name="Private Report", is_public="No")])
call_command("import_reports", path, user_email=self.editor.email, stdout=StringIO())

report = Report.objects.get(name="Private Report")
self.assertFalse(report.is_pfa_visible_in_gidd)

def test_a_part_year_report_does_not_become_visible_in_gidd(self):
path = write_sheet(
self.PFA_COLUMNS,
[self._pfa_row(name="Half Year Report", filter_figure_end_before="2020-06-30")],
)
call_command("import_reports", path, user_email=self.editor.email, stdout=StringIO())

report = Report.objects.get(name="Half Year Report")
self.assertFalse(report.is_pfa_visible_in_gidd)

def test_an_update_can_turn_visibility_on(self):
report = self._report(name="Later A PFA")
report.filter_figure_categories = [Figure.FIGURE_CATEGORY_TYPES.IDPS]
report.filter_figure_crisis_types = [Crisis.CRISIS_TYPE.DISASTER]
report.is_public = True
report.save()
self.assertFalse(report.is_pfa_visible_in_gidd)

path = write_sheet(["id", "is_pfa_visible_in_gidd"], [{"id": report.id, "is_pfa_visible_in_gidd": "Yes"}])
call_command("import_reports", path, user_email=self.editor.email, stdout=StringIO())

report.refresh_from_db()
self.assertTrue(report.is_pfa_visible_in_gidd)

def test_the_visibility_flag_is_writable_only_from_the_sheet(self):
# ReportSerializer's fields become the report mutation inputs, where the flag would bypass
# setPfaVisibleInGidd's admin permission.
self.assertNotIn("is_pfa_visible_in_gidd", ReportSerializer().fields)
self.assertIn("is_pfa_visible_in_gidd", ReportImportSerializer().fields)
self.assertIn("is_pfa_visible_in_gidd", ImportReportsCommand().import_columns())

# ----- template -----

def test_make_template_writes_the_expected_columns(self):
Expand Down