launchpad-reviewers team mailing list archive

[Guruprasad <mp+430006@xxxxxxxxxxxxxxxxxx>]

Guruprasad has proposed merging ~lgp171188/launchpad:teammembership-renewal-fencepost-fix into launchpad:master.

Commit message:
Allow team membership renewal when the expiration warning is sent

This allows renewing the team membership a day earlier than
DAYS_BEFORE_EXPIRATION_WARNING_IS_SENT days before the exact expiry
time so that the users are able to perform the renewal when the first
expiration warning email is sent.

LP: #1987056


Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1987056 in Launchpad itself: "Team membership expiration warnings sent before renewal is possible"
  https://bugs.launchpad.net/launchpad/+bug/1987056

For more details, see:
https://code.launchpad.net/~lgp171188/launchpad/+git/launchpad/+merge/430006
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~lgp171188/launchpad:teammembership-renewal-fencepost-fix into launchpad:master.

diff --git a/lib/lp/registry/browser/team.py b/lib/lp/registry/browser/team.py
index 7495823..32afa9a 100644
--- a/lib/lp/registry/browser/team.py
+++ b/lib/lp/registry/browser/team.py
@@ -1414,8 +1414,12 @@ class TeamMembershipSelfRenewalView(LaunchpadFormView):
         ondemand = TeamMembershipRenewalPolicy.ONDEMAND
         admin = TeamMembershipStatus.ADMIN
         approved = TeamMembershipStatus.APPROVED
-        date_limit = datetime.now(pytz.UTC) - timedelta(
-            days=DAYS_BEFORE_EXPIRATION_WARNING_IS_SENT
+        # We add a grace period of one day to the limit to
+        # cover the fencepost error when `date_limit` is
+        # earlier than `self.dateexpires`, which happens later
+        # in the same day.
+        date_limit = datetime.now(pytz.UTC) + timedelta(
+            days=DAYS_BEFORE_EXPIRATION_WARNING_IS_SENT + 1
         )
         if context.status not in (admin, approved):
             text = "it is not active."
diff --git a/lib/lp/registry/doc/teammembership.rst b/lib/lp/registry/doc/teammembership.rst
index a7569aa..cac006e 100644
--- a/lib/lp/registry/doc/teammembership.rst
+++ b/lib/lp/registry/doc/teammembership.rst
@@ -12,7 +12,7 @@ represents all the people who are /effective members/ of the team.
 First of all, create some teams:
 
     >>> import pytz
-    >>> from datetime import datetime, timedelta
+    >>> from datetime import datetime, timedelta, timezone
     >>> from lp.registry.interfaces.person import (
     ...     TeamMembershipRenewalPolicy,
     ...     TeamMembershipPolicy,
@@ -803,6 +803,37 @@ expire soon.
     >>> print(karl_on_mirroradmins.status.title)
     Approved
 
+The membership can be renewed by the member within
+DAYS_BEFORE_EXPIRATION_WARNING_IS_SENT + 1 days, but
+not outside that.
+    >>> from lp.registry.interfaces.teammembership import (
+    ...     DAYS_BEFORE_EXPIRATION_WARNING_IS_SENT,
+    ... )
+
+    >>> membership = removeSecurityProxy(karl_on_mirroradmins)
+
+    >>> membership.dateexpires = (
+    ...     datetime.now(timezone.utc)
+    ...     + timedelta(days=DAYS_BEFORE_EXPIRATION_WARNING_IS_SENT)
+    ...     + timedelta(hours=5)
+    ... )
+    >>> membership.canBeRenewedByMember()
+    True
+    >>> membership.dateexpires = (
+    ...     datetime.now(timezone.utc)
+    ...     + timedelta(days=DAYS_BEFORE_EXPIRATION_WARNING_IS_SENT)
+    ...     + timedelta(days=1)
+    ... )
+    >>> membership.canBeRenewedByMember()
+    True
+    >>> membership.dateexpires = (
+    ...     datetime.now(timezone.utc)
+    ...     + timedelta(days=DAYS_BEFORE_EXPIRATION_WARNING_IS_SENT)
+    ...     + timedelta(days=1, minutes=1)
+    ... )
+    >>> membership.canBeRenewedByMember()
+    False
+
 
 Querying team memberships
 -------------------------
diff --git a/lib/lp/registry/model/teammembership.py b/lib/lp/registry/model/teammembership.py
index 8efef02..891b057 100644
--- a/lib/lp/registry/model/teammembership.py
+++ b/lib/lp/registry/model/teammembership.py
@@ -120,8 +120,12 @@ class TeamMembership(SQLBase):
         ondemand = TeamMembershipRenewalPolicy.ONDEMAND
         admin = TeamMembershipStatus.APPROVED
         approved = TeamMembershipStatus.ADMIN
+        # We add a grace period of one day to the limit to
+        # cover the fencepost error when `date_limit` is
+        # earlier than `self.dateexpires`, which happens later
+        # in the same day.
         date_limit = datetime.now(pytz.UTC) + timedelta(
-            days=DAYS_BEFORE_EXPIRATION_WARNING_IS_SENT
+            days=DAYS_BEFORE_EXPIRATION_WARNING_IS_SENT + 1
         )
         return (
             self.status in (admin, approved)