Skip to content

Commit

Permalink
Switch archivable member selection from delta to year-based cutoff
Browse files Browse the repository at this point in the history
Fixes an oversight in the previous implementation of #470

Also, expand on the test suite substantially while retaining a decent
runtime.

Refs #67
  • Loading branch information
lukasjuhrich committed Apr 27, 2023
1 parent 00f7855 commit 0891278
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 34 deletions.
23 changes: 15 additions & 8 deletions pycroft/lib/user_deletion.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,13 @@
This module contains methods concerning user archival and deletion.
"""
from __future__ import annotations
from datetime import timedelta, datetime
from datetime import datetime
from typing import Protocol, Sequence, cast

from sqlalchemy import func, and_, not_
from sqlalchemy.future import select
from sqlalchemy.orm import joinedload, Session
from sqlalchemy.sql import Select
from sqlalchemy.sql.functions import current_timestamp

from pycroft.model.property import CurrentProperty
from pycroft.model.user import User
Expand All @@ -25,7 +24,9 @@ class ArchivableMemberInfo(Protocol):
mem_end: datetime


def select_archivable_members(delta: timedelta) -> Select: # Select[Tuple[User, int, datetime]]
def select_archivable_members(
current_year: int,
) -> Select: # Select[Tuple[User, int, datetime]]
# last_mem: (user_id, mem_id, mem_end)
last_mem = select_user_and_last_mem().cte("last_mem")
return (
Expand All @@ -40,7 +41,7 @@ def select_archivable_members(delta: timedelta) -> Select: # Select[Tuple[User,
# …and use that to filter out the `do-not-archive` occurrences.
.filter(CurrentProperty.property_name.is_(None))
.join(User, User.id == last_mem.c.user_id)
.filter(last_mem.c.mem_end < current_timestamp() - delta) # type: ignore[no-untyped-call]
.filter(func.extract("year", last_mem.c.mem_end) + 2 <= current_year) # type: ignore[no-untyped-call]
.order_by(last_mem.c.mem_end)
.add_columns(
User,
Expand All @@ -51,7 +52,8 @@ def select_archivable_members(delta: timedelta) -> Select: # Select[Tuple[User,


def get_archivable_members(
session: Session, delta: timedelta = timedelta(days=14)
session: Session,
current_year: int | None = None,
) -> Sequence[ArchivableMemberInfo]:
"""Return all the users that qualify for being archived right now.
Expand All @@ -66,13 +68,18 @@ def get_archivable_members(
- current_properties_maybe_denied
:param session:
:param delta: how far back the end of membership has to lie (positive timedelta).
:param current_year: dependency injection of the current year.
defaults to the current year.
"""
return cast(
list[ArchivableMemberInfo],
session.execute(
select_archivable_members(delta)
.options(
select_archivable_members(
# I know we're sloppy with time zones,
# but ±2h around new year's eve don't matter.
current_year=current_year
or datetime.now().year
).options(
joinedload(User.hosts),
# joinedload(User.current_memberships),
joinedload(User.account, innerjoin=True),
Expand Down
88 changes: 62 additions & 26 deletions tests/lib/user/test_deletion.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,18 @@
# This file is part of the Pycroft project and licensed under the terms of
# the Apache License, Version 2.0. See the LICENSE file for details
from datetime import datetime, date
from typing import Sequence

import pytest

from pycroft.helpers.interval import closed, closedopen
from pycroft.lib.user_deletion import get_archivable_members, archive_users
from pycroft.helpers.utc import with_min_time
from pycroft.lib.user_deletion import (
get_archivable_members,
archive_users,
ArchivableMemberInfo,
)
from pycroft.model.user import User
from tests.factories import UserFactory, ConfigFactory, MembershipFactory, \
PropertyGroupFactory, \
HostFactory
Expand All @@ -26,27 +33,49 @@ def test_users_without_membership_not_in_list(session):
assert get_archivable_members(session) == []


def assert_archivable_members(members, expected_user, expected_end_date):
match members:
case [(user, mem_id, mem_end)]:
assert user == expected_user
assert mem_id is not None
assert mem_end.date() == expected_end_date
case _:
pytest.fail()
def filter_members(members, user):
return [(u, id, end) for u, id, end in members if u == user]


class TestUserDeletion:
def assert_member_present(
members: Sequence[ArchivableMemberInfo],
expected_user: User,
expected_end_date: date,
):
relevant_members = filter_members(members, expected_user)
assert len(relevant_members) == 1
[(_, mem_id, mem_end)] = relevant_members
assert mem_id is not None
assert mem_end.date() == expected_end_date


def assert_member_absent(
members: Sequence[ArchivableMemberInfo],
expected_absent_user: User,
):
assert not filter_members(members, expected_absent_user)


class TestArchivableUserSelection:
@pytest.fixture(
scope="class",
params=[date(2020, 3, 1), date(2020, 1, 1), date(2020, 12, 15)],
)
def end_date(self, request):
return request.param

@pytest.fixture(scope='class')
def do_not_archive_group(self, class_session):
return PropertyGroupFactory(granted={'do-not-archive'})

@pytest.fixture(scope='class')
def old_user(self, class_session, config, do_not_archive_group):
@pytest.fixture(scope="class")
def old_user(self, class_session, config, do_not_archive_group, end_date) -> User:
user = UserFactory.create(
registered_at=datetime(2000, 1, 1),
with_membership=True,
membership__active_during=closed(datetime(2020, 1, 1), datetime(2020, 3, 1)),
membership__active_during=closed(
with_min_time(date(2020, 1, 1)), with_min_time(end_date)
),
membership__group=config.member_group,
)
MembershipFactory.create(
Expand All @@ -66,31 +95,38 @@ def do_not_archive_membership(self, session, old_user, do_not_archive_group):
active_during=closedopen(datetime(2020, 1, 1), None),
)

def test_old_users_in_deletion_list(self, session, old_user):
members = get_archivable_members(session)
assert_archivable_members(members, old_user, date(2020, 3, 1))
@pytest.mark.parametrize("year", [2022, 2023, 2024])
def test_old_users_in_deletion_list_after(self, session, old_user, year, end_date):
members = get_archivable_members(session, current_year=year)
assert_member_present(members, old_user, end_date)

def test_old_user_not_in_list_with_long_delta(self, session, old_user):
delta = date.today() - date(2020, 1, 1) # before 2020-03-01
assert get_archivable_members(session, delta) == []
@pytest.mark.parametrize("year", [2019, 2020, 2021])
def test_old_user_not_in_list_before(self, session, old_user, year):
assert_member_absent(
get_archivable_members(session, current_year=year), old_user
)

def test_user_with_do_not_archive_not_in_list(self, session, old_user,
do_not_archive_membership):
assert get_archivable_members(session) == []
@pytest.mark.parametrize("year", list(range(2018, 2023)))
def test_user_with_do_not_archive_not_in_list(
self, session, old_user, do_not_archive_membership, year
):
assert_member_absent(
get_archivable_members(session, current_year=year), old_user
)

@pytest.mark.parametrize('num_hosts', [0, 2])
def test_user_with_host_in_list(self, session, old_user, num_hosts):
def test_user_with_host_in_list(self, session, old_user, num_hosts, end_date):
if num_hosts:
HostFactory.create_batch(num_hosts, owner=old_user)
members = get_archivable_members(session)
assert_archivable_members(members, old_user, date(2020, 3, 1))
assert_member_present(members, old_user, end_date)

def test_user_with_room_in_list(self, session, old_user):
def test_user_with_room_in_list(self, session, old_user, end_date):
with session.begin_nested():
old_user.room = None
session.add(old_user)
members = get_archivable_members(session)
assert_archivable_members(members, old_user, date(2020, 3, 1))
assert_member_present(members, old_user, end_date)


class TestUserArchival:
Expand Down

0 comments on commit 0891278

Please sign in to comment.