Skip to content

Commit

Permalink
Merge pull request #354 from opencivicdata/hcg/protect-post
Browse files Browse the repository at this point in the history
Update pupa clean so that Jurisdiction, Division, and Post objects are not deleted
  • Loading branch information
hancush authored Sep 11, 2024
2 parents a6517de + 9087629 commit 2f7847c
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 21 deletions.
5 changes: 3 additions & 2 deletions pupa/cli/commands/clean.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
44 changes: 31 additions & 13 deletions pupa/tests/clean/test_clean.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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), []
)

Expand All @@ -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), []
)
12 changes: 7 additions & 5 deletions pupa/tests/django_settings.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
# django settings for tests
import os

SECRET_KEY = 'test'
INSTALLED_APPS = ('django.contrib.contenttypes',
'opencivicdata.core.apps.BaseConfig',
Expand All @@ -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 = ()
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down

0 comments on commit 2f7847c

Please sign in to comment.