From 9e8f1c7591c9c13857a05efb51a5035855031ef0 Mon Sep 17 00:00:00 2001 From: Hannah Cushman Garland Date: Wed, 11 Sep 2024 15:18:30 -0500 Subject: [PATCH 1/2] Update pupa clean so that Jurisdiction, Division, and Post objects are not deleted --- pupa/cli/commands/clean.py | 5 ++-- pupa/tests/clean/test_clean.py | 44 ++++++++++++++++++++++++---------- pupa/tests/django_settings.py | 12 ++++++---- setup.py | 2 +- 4 files changed, 42 insertions(+), 21 deletions(-) diff --git a/pupa/cli/commands/clean.py b/pupa/cli/commands/clean.py index c105077..77e39f2 100644 --- a/pupa/cli/commands/clean.py +++ b/pupa/cli/commands/clean.py @@ -62,10 +62,11 @@ def get_stale_objects(self, window): ocd_apps = ["core", "legislative"] # Check all subclasses of OCDBase models = get_subclasses(ocd_apps, OCDBase) + # Top-level models are protected from deletion + protected_models = ("Division", "Jurisdiction", "Post") for model in models: - # Jurisdictions are protected from deletion - if "Jurisdiction" not in model.__name__: + if model.__name__ not in protected_models: cutoff_date = datetime.now(tz=timezone.utc) - timedelta(days=window) yield from model.objects.filter(last_seen__lte=cutoff_date).iterator() diff --git a/pupa/tests/clean/test_clean.py b/pupa/tests/clean/test_clean.py index 0ca9e32..d544cfa 100644 --- a/pupa/tests/clean/test_clean.py +++ b/pupa/tests/clean/test_clean.py @@ -3,9 +3,9 @@ from datetime import datetime, timezone, timedelta from freezegun import freeze_time -from opencivicdata.core.models import Person, Organization, Jurisdiction, Division +from opencivicdata.core.models import Person, Organization, Jurisdiction, Division, Post -from pupa.cli.commands.clean import Command +from pupa.cli.commands.clean import Command as CleanCommand @pytest.fixture @@ -25,14 +25,23 @@ def subparsers(): @pytest.fixture -def jurisdiction(): - Division.objects.create(id="ocd-division/country:us", name="USA") - return Jurisdiction.objects.create(id="jid", division_id="ocd-division/country:us") +def division(): + return Division.objects.create(id="ocd-division/country:us", name="USA") + + +@pytest.fixture +def jurisdiction(division): + return Jurisdiction.objects.create(id="jid", division=division) @pytest.fixture def organization(jurisdiction): - return Organization.objects.create(name="WWE", jurisdiction_id="jid") + return Organization.objects.create(name="WWE", jurisdiction=jurisdiction) + + +@pytest.fixture +def post(organization): + return Post.objects.create(organization=organization, label="Some post", role="Some post") @pytest.fixture @@ -52,17 +61,24 @@ def build(self, **kwargs): @pytest.mark.django_db -def test_get_stale_objects(subparsers, organization, person): +def test_get_stale_objects(subparsers, division, jurisdiction, organization, post, person): stale_person = person.build() membership = stale_person.memberships.create(organization=organization) + protected_objects = {division, jurisdiction, post} expected_stale_objects = {stale_person, organization, membership} a_week_from_now = datetime.now(tz=timezone.utc) + timedelta(days=7) with freeze_time(a_week_from_now): fresh_person = person.build(name="Thomas Jefferson", family_name="Jefferson") fresh_person.memberships.create(organization=organization) - assert set(Command(subparsers).get_stale_objects(7)) == expected_stale_objects + + stale_objects = set(CleanCommand(subparsers).get_stale_objects(7)) + assert stale_objects == expected_stale_objects + + # This is implied by the above check, but it's important, so we'll check + # for it explicitly. + assert protected_objects not in stale_objects @pytest.mark.django_db @@ -77,7 +93,7 @@ def test_remove_stale_objects(subparsers, organization, person): fresh_person = person.build(name="Thomas Jefferson", family_name="Jefferson") fresh_person.memberships.create(organization=organization) - Command(subparsers).remove_stale_objects(7) + CleanCommand(subparsers).remove_stale_objects(7) for obj in expected_stale_objects: was_deleted = not type(obj).objects.filter(id=obj.id).exists() assert was_deleted @@ -97,7 +113,7 @@ def test_clean_command(subparsers, organization, person): organization.save() # Update org's last_seen field # Call clean command - Command(subparsers).handle( + CleanCommand(subparsers).handle( argparse.Namespace(report=False, window=7, yes=True, max=10), [] ) @@ -118,23 +134,25 @@ def test_clean_command_failsafe(subparsers, organization, person): for p in stale_people: p.memberships.create(organization=organization) + cmd = CleanCommand(subparsers) + a_week_from_now = datetime.now(tz=timezone.utc) + timedelta(days=7) with freeze_time(a_week_from_now): with pytest.raises(SystemExit): # Should trigger failsafe exist when deleting more than 10 objects - Command(subparsers).handle( + cmd.handle( argparse.Namespace(report=False, window=7, yes=False, max=10), [] ) with pytest.raises(SystemExit): # Should trigger failsafe exist when deleting more than 10 objects, # even when yes is specified - Command(subparsers).handle( + cmd.handle( argparse.Namespace(report=False, window=7, yes=True, max=10), [] ) # Should proceed without error, since max is increased (1 organization, # 20 people, 20 memberships) - Command(subparsers).handle( + cmd.handle( argparse.Namespace(report=False, window=7, max=41, yes=True), [] ) diff --git a/pupa/tests/django_settings.py b/pupa/tests/django_settings.py index 7ab0e20..4c914fb 100644 --- a/pupa/tests/django_settings.py +++ b/pupa/tests/django_settings.py @@ -1,4 +1,6 @@ # django settings for tests +import os + SECRET_KEY = 'test' INSTALLED_APPS = ('django.contrib.contenttypes', 'opencivicdata.core.apps.BaseConfig', @@ -7,11 +9,11 @@ DATABASES = { 'default': { 'ENGINE': 'django.contrib.gis.db.backends.postgis', - 'NAME': 'test', - 'USER': 'test', - 'PASSWORD': 'test', - 'HOST': 'localhost', - 'PORT': 5432, + 'NAME': os.getenv('POSTGRES_DB', 'test'), + 'USER': os.getenv('POSTGRES_USER', 'test'), + 'PASSWORD': os.getenv('POSTGRES_PASSWORD', 'test'), + 'HOST': os.getenv('POSTGRES_HOST', 'localhost'), + 'PORT': os.getenv('POSTGRES_PORT', 5432), } } MIDDLEWARE_CLASSES = () diff --git a/setup.py b/setup.py index 3307975..1562d3e 100644 --- a/setup.py +++ b/setup.py @@ -18,7 +18,7 @@ entry_points='''[console_scripts] pupa = pupa.cli.__main__:main''', install_requires=[ - 'Django>=2.2', + 'Django>=2.2,<5', 'opencivicdata>=3.3.0', 'dj_database_url>=0.3.0', 'scrapelib>=1.0', From 9087629032c92ea249781f1adc252e78b5c806de Mon Sep 17 00:00:00 2001 From: Hannah Cushman Garland Date: Wed, 11 Sep 2024 15:23:51 -0500 Subject: [PATCH 2/2] flake it up --- pupa/tests/clean/test_clean.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pupa/tests/clean/test_clean.py b/pupa/tests/clean/test_clean.py index d544cfa..721640e 100644 --- a/pupa/tests/clean/test_clean.py +++ b/pupa/tests/clean/test_clean.py @@ -72,7 +72,7 @@ def test_get_stale_objects(subparsers, division, jurisdiction, organization, pos with freeze_time(a_week_from_now): fresh_person = person.build(name="Thomas Jefferson", family_name="Jefferson") fresh_person.memberships.create(organization=organization) - + stale_objects = set(CleanCommand(subparsers).get_stale_objects(7)) assert stale_objects == expected_stale_objects