launchpad-reviewers team mailing list archive

[Guruprasad <mp+424477@xxxxxxxxxxxxxxxxxx>]

Guruprasad has proposed merging ~lgp171188/launchpad:send-team-expiration-warnings-earlier into launchpad:master.

Commit message:
Update the team membership expiration warning schedule

Now send the notification once a week from 4 weeks before expiry to 2
weeks before expiry and then send the notification daily in the last
week.

LP: #1935692


Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1935692 in Launchpad itself: "Teams expiration warning emails could start earlier"
  https://bugs.launchpad.net/launchpad/+bug/1935692

For more details, see:
https://code.launchpad.net/~lgp171188/launchpad/+git/launchpad/+merge/424477
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~lgp171188/launchpad:send-team-expiration-warnings-earlier into launchpad:master.

diff --git a/cronscripts/flag-expired-memberships.py b/cronscripts/flag-expired-memberships.py
index f458df7..cfb6812 100755
--- a/cronscripts/flag-expired-memberships.py
+++ b/cronscripts/flag-expired-memberships.py
@@ -7,19 +7,10 @@
 
 import _pythonpath  # noqa: F401
 
-from datetime import (
-    datetime,
-    timedelta,
-    )
-
-import pytz
 from zope.component import getUtility
 
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
-from lp.registry.interfaces.teammembership import (
-    DAYS_BEFORE_EXPIRATION_WARNING_IS_SENT,
-    ITeamMembershipSet,
-    )
+from lp.registry.interfaces.teammembership import ITeamMembershipSet
 from lp.services.config import config
 from lp.services.scripts.base import (
     LaunchpadCronScript,
@@ -34,7 +25,7 @@ class ExpireMemberships(LaunchpadCronScript):
         """Flag expired team memberships and warn about impending expiration.
 
         Flag expired team memberships and send warnings for members whose
-        memberships are going to expire in one week (or less) from now.
+        team memberships are expiring soon.
         """
         membershipset = getUtility(ITeamMembershipSet)
         self.txn.begin()
@@ -42,11 +33,10 @@ class ExpireMemberships(LaunchpadCronScript):
         membershipset.handleMembershipsExpiringToday(reviewer, self.logger)
         self.txn.commit()
 
-        min_date_for_warning = datetime.now(pytz.timezone('UTC')) + timedelta(
-            days=DAYS_BEFORE_EXPIRATION_WARNING_IS_SENT)
         self.txn.begin()
-        for membership in membershipset.getMembershipsToExpire(
-            min_date_for_warning):
+        memberships_to_warn = membershipset.getExpiringMembershipsToWarn()
+        print(memberships_to_warn)
+        for membership in memberships_to_warn:
             membership.sendExpirationWarningEmail()
             self.logger.debug("Sent warning email to %s in %s team."
                           % (membership.person.name, membership.team.name))
diff --git a/lib/lp/registry/interfaces/teammembership.py b/lib/lp/registry/interfaces/teammembership.py
index b45bf73..10eef52 100644
--- a/lib/lp/registry/interfaces/teammembership.py
+++ b/lib/lp/registry/interfaces/teammembership.py
@@ -44,10 +44,12 @@ from zope.schema import (
 from lp import _
 
 
-# One week before a membership expires we send a notification to the member,
+# Four weeks before a membership expires we send a notification to the member,
 # either inviting them to renew their own membership or asking them to get a
-# team admin to do so, depending on the team's renewal policy.
-DAYS_BEFORE_EXPIRATION_WARNING_IS_SENT = 7
+# team admin to do so, depending on the team's renewal policy. We repeat these
+# notifications weekly till the week in which the membership expires and then
+# we send daily notifications till the expiry date.
+DAYS_BEFORE_EXPIRATION_WARNING_IS_SENT = 28
 
 
 class TeamMembershipStatus(DBEnumeratedType):
@@ -270,6 +272,16 @@ class ITeamMembershipSet(Interface):
         equal to :when: and its status is either ADMIN or APPROVED.
         """
 
+    def getMembershipsExpiringOnDates(dates):
+        """Return all TeamMemberships expiring on the given dates."""
+
+    def getExpiringMembershipsToWarn():
+        """Return all TeamMemberships to be warned about expiration.
+
+        This includes the TeamMemberships expiring in <= 1 week and
+        TeamMemberships expiring exactly in 2 to the number of weeks in
+        DAYS_BEFORE_EXPIRATION_WARNING_IS_SENT from now."""
+
     def new(person, team, status, user, dateexpires=None, comment=None):
         """Create and return a TeamMembership for the given person and team.
 
diff --git a/lib/lp/registry/model/teammembership.py b/lib/lp/registry/model/teammembership.py
index ab4c4cb..23bce62 100644
--- a/lib/lp/registry/model/teammembership.py
+++ b/lib/lp/registry/model/teammembership.py
@@ -14,6 +14,7 @@ from datetime import (
     )
 
 import pytz
+from storm.expr import Func
 from storm.info import ClassAlias
 from storm.store import Store
 from zope.component import getUtility
@@ -341,6 +342,36 @@ class TeamMembershipSet:
             ]
         return IStore(TeamMembership).find(TeamMembership, *conditions)
 
+    def getExpiringMembershipsToWarn(self):
+        """See `ITeamMembershipSet`,"""
+        now = datetime.now(pytz.UTC)
+        min_date_for_daily_warning = now + timedelta(days=7)
+        memberships_to_warn = set()
+        memberships_to_warn.update(
+            list(self.getMembershipsToExpire(min_date_for_daily_warning))
+        )
+        weekly_reminder_dates = [
+            (now + timedelta(days=weeks*7)).replace(
+                hour=0, minute=0, second=0, microsecond=0
+            )
+            for weeks in range(
+                2, DAYS_BEFORE_EXPIRATION_WARNING_IS_SENT // 7 + 1
+            )
+        ]
+        memberships_to_warn.update(
+            list(self.getMembershipsExpiringOnDates(weekly_reminder_dates))
+        )
+        return memberships_to_warn
+
+    def getMembershipsExpiringOnDates(self, dates):
+        """See `ITeamMembershipSet`."""
+        return IStore(TeamMembership).find(
+            TeamMembership,
+            Func("date_trunc", "day", TeamMembership.dateexpires).is_in(dates),
+            TeamMembership.status.is_in(
+                [TeamMembershipStatus.ADMIN, TeamMembershipStatus.APPROVED]),
+        )
+
     def deactivateActiveMemberships(self, team, comment, reviewer):
         """See `ITeamMembershipSet`."""
         now = datetime.now(pytz.timezone('UTC'))
diff --git a/lib/lp/registry/tests/test_teammembership.py b/lib/lp/registry/tests/test_teammembership.py
index 3f3b58b..6b9d1e8 100644
--- a/lib/lp/registry/tests/test_teammembership.py
+++ b/lib/lp/registry/tests/test_teammembership.py
@@ -36,6 +36,7 @@ from lp.registry.interfaces.persontransferjob import (
     )
 from lp.registry.interfaces.teammembership import (
     CyclicalTeamMembershipError,
+    DAYS_BEFORE_EXPIRATION_WARNING_IS_SENT,
     ITeamMembershipSet,
     TeamMembershipStatus,
     )
@@ -136,6 +137,16 @@ class TestTeamMembershipSet(TestCaseWithFactory):
         self.membershipset = getUtility(ITeamMembershipSet)
         self.personset = getUtility(IPersonSet)
 
+    def addToTeamSetExpiryDate(self, person, team, expiry_date):
+        team.addMember(person, team.teamowner)
+        membership = self.membershipset.getByPersonAndTeam(person, team)
+        self.assertIsNotNone(membership)
+        removeSecurityProxy(membership).dateexpires = expiry_date
+        return membership
+
+    def getMidnightTime(self, when):
+        return when.replace(hour=0, minute=0, second=0, microsecond=0)
+
     def test_membership_creation(self):
         marilize = self.personset.getByName('marilize')
         ubuntu_team = self.personset.getByName('ubuntu-team')
@@ -260,6 +271,65 @@ class TestTeamMembershipSet(TestCaseWithFactory):
             [superteam], list(targetteam.teamowner.teams_participated_in))
         self.assertEqual([], list(member.teams_participated_in))
 
+    def test_getMembershipsExpiringOnDates(self):
+        now = datetime.now(pytz.UTC)
+        datetime1 = now + timedelta(days=1)
+        datetime2 = now + timedelta(days=2)
+        datetime3 = now + timedelta(days=3)
+        team = self.factory.makeTeam(name='super')
+        login_celebrity('admin')
+        member1 = self.factory.makePerson()
+        membership1 = self.addToTeamSetExpiryDate(
+            member1, team, datetime1
+        )
+        member2 = self.factory.makePerson()
+        membership2 = self.addToTeamSetExpiryDate(
+            member2,
+            team,
+            datetime2.replace(
+                hour=23, minute=59, second=59
+            )
+        )
+        member3 = self.factory.makePerson()
+        _ = self.addToTeamSetExpiryDate(
+            member3, team, datetime3
+        )
+        self.assertEqual(
+            {membership1, membership2},
+            set(
+                self.membershipset.getMembershipsExpiringOnDates(
+                    [
+                        self.getMidnightTime(datetime1),
+                        self.getMidnightTime(datetime2),
+                    ]
+                )
+            )
+        )
+
+    def test_getExpiringMembershipsToWarn(self):
+        team = self.factory.makeTeam(name='super')
+        team_members = []
+        memberships = []
+        now = datetime.now(pytz.UTC)
+        login_celebrity('admin')
+        member = self.factory.makePerson()
+        memberships.append(
+            self.addToTeamSetExpiryDate(member, team, now + timedelta(days=2))
+        )
+        for weeks in range(2, DAYS_BEFORE_EXPIRATION_WARNING_IS_SENT // 7 + 2):
+            person = self.factory.makePerson()
+            memberships.append(
+                self.addToTeamSetExpiryDate(
+                    person, team, now + timedelta(days=weeks*7)
+                )
+            )
+            team_members.append(person)
+
+        self.assertEqual(
+            set(memberships[:-1]),
+            set(self.membershipset.getExpiringMembershipsToWarn())
+        )
+
 
 class TeamParticipationTestCase(TestCaseWithFactory):
     """Tests for team participation using 5 teams."""