Skip to content

Commit

Permalink
Report MIT email for student members
Browse files Browse the repository at this point in the history 
We *require* students to provide a valid `@mit.edu` email address in
order to pay dues as students -- this is valuable for identifying
somebody's Kerberos even if they choose to have, say, a Gmail address as
their preferred address.
  • Loading branch information
@DavidCain
DavidCain committed May 7, 2024
1 parent d8978f8 commit 891e5f3
Show file tree
Hide file tree
Showing 3 changed files with 122 additions and 2 deletions.
6 changes: 6 additions & 0 deletions ws/api_views.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -645,6 +645,8 @@ class MemberInfo(TypedDict):
email: str
affiliation: str
num_rentals: int
# Only reported if they are an MIT student!
mit_email: str | None
# Fields from TripInformation, if found
is_leader: NotRequired[bool]
num_trips_attended: NotRequired[int]
Expand All @@ -660,6 +662,7 @@ def _flat_members_info(
for info in members:
flat_info: MemberInfo = {
"email": info.email,
"mit_email": info.mit_email,
"affiliation": info.affiliation,
"num_rentals": info.num_rentals,
}
Expand All @@ -675,6 +678,9 @@ def _flat_members_info(
"num_discounts": info.trips_information.num_discounts,
}
)
# If there's a verified MIT email address from the trips site, use it!
if info.trips_information.verified_mit_email is not None:
flat_info["mit_email"] = info.trips_information.verified_mit_email
yield flat_info

def get(self, request: HttpRequest, *args: Any, **kwargs: Any) -> JsonResponse:
Expand Down
79 changes: 78 additions & 1 deletion ws/tests/views/test_api_views.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -659,6 +659,15 @@ def test_fetches_async_by_default(self):
verified=True,
primary=False,
)
# Reflect reality -- to be an MIT student, you *must* have a verified mit.edu
self.participant.affiliation = "MU"
self.participant.save()
factories.EmailAddressFactory.create(
user=self.participant.user,
verified=True,
primary=False,
email="[email protected]",
)
factories.SignUpFactory.create(participant=self.participant, on_trip=True)

with mock.patch.object(tasks.update_member_stats, "delay") as update:
Expand All @@ -680,6 +689,7 @@ def test_fetches_async_by_default(self):
"num_trips_attended": 1,
"num_trips_led": 0,
"num_discounts": 0,
"mit_email": "[email protected]",
},
],
},
Expand Down Expand Up @@ -712,6 +722,8 @@ def test_no_matching_participant(self):
self._expect_members(
{
"email": "[email protected]",
# We still report the MIT email from the geardb!
"mit_email": "[email protected]",
"affiliation": "MIT undergrad",
"num_rentals": 3,
}
Expand Down Expand Up @@ -761,6 +773,7 @@ def test_matches_on_verified_emails_only(self) -> None:
{
# We report their trips email as preferred!
"email": "[email protected]",
"mit_email": None,
"affiliation": "Non-MIT undergrad",
# Found a matching account!
"is_leader": False,
Expand All @@ -773,10 +786,61 @@ def test_matches_on_verified_emails_only(self) -> None:
{
"affiliation": "MIT affiliate",
"email": "[email protected]",
"mit_email": None,
"num_rentals": 0,
},
)

@responses.activate
def test_ignores_possibly_old_mit_edu(self):
with freeze_time("2019-02-22 12:25:00 EST"):
cached = models.MembershipStats.load()
cached.response = [
{
"id": 37,
"affiliation": "MIT alum (former student)",
"alternate_emails": ["[email protected]"],
"email": "[email protected]",
"num_rentals": 3,
}
]
cached.save()

# Simulate an old MIT email they may no longer own!
factories.EmailAddressFactory.create(
user=self.participant.user,
email="[email protected]",
verified=True,
primary=False,
)
# MIT alum -- they *used* to own [email protected]
self.participant.affiliation = "ML"
self.participant.save()

with mock.patch.object(tasks.update_member_stats, "delay"):
response = self.client.get("/stats/membership.json") # No cache_strategy

self.assertEqual(
response.json(),
{
# We used the cached information from the geardb
"retrieved_at": "2019-02-22T12:25:00-05:00",
"members": [
{
"email": self.participant.email,
"affiliation": "MIT alum (former student)",
"num_rentals": 3,
"is_leader": True,
"num_trips_attended": 0,
"num_trips_led": 0,
"num_discounts": 0,
# We do *not* report the mit.edu email -- it may be old
"mit_email": None,
},
],
},
)

@responses.activate
def test_trips_data_included(self):
responses.get(
Expand Down Expand Up @@ -812,6 +876,15 @@ def test_trips_data_included(self):
verified=True,
primary=False,
)
# Reflect reality -- to be an MIT student, you *must* have a verified mit.edu
self.participant.affiliation = "MU"
self.participant.save()
factories.EmailAddressFactory.create(
user=self.participant.user,
verified=True,
primary=False,
email="[email protected]",
)
bob = factories.ParticipantFactory.create(email="[email protected]")
factories.EmailAddressFactory.create(
user=bob.user, email="[email protected]", verified=True, primary=False
Expand Down Expand Up @@ -843,6 +916,7 @@ def test_trips_data_included(self):
self._expect_members(
{
"email": "[email protected]", # Preferred email!
"mit_email": None,
"affiliation": "Non-MIT undergrad",
"num_rentals": 0,
"is_leader": False, # Not presently a leader!
Expand All @@ -852,6 +926,7 @@ def test_trips_data_included(self):
},
{
"email": self.participant.email,
"mit_email": "[email protected]",
"affiliation": "MIT undergrad",
"num_rentals": 3,
"is_leader": True,
Expand All @@ -863,6 +938,7 @@ def test_trips_data_included(self):
{
"affiliation": "MIT affiliate",
"email": "[email protected]",
"mit_email": None,
"num_rentals": 0,
},
)
Expand All @@ -878,5 +954,6 @@ def test_trips_data_included(self):
# 1. Count trips per participant (separate to avoid double-counting)
# 2. Count discounts, trips led, per participant
# 3. Get all emails (lowercased, for mapping back to participant records)
with self.assertNumQueries(3):
# 4. Get MIT email addresses
with self.assertNumQueries(4):
stats.with_trips_information()
39 changes: 38 additions & 1 deletion ws/utils/member_stats.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,13 @@
from allauth.account.models import EmailAddress
from django.db.models import Count, Exists, OuterRef
from django.db.models.functions import Lower
from mitoc_const import affiliations
from typing_extensions import assert_never

from ws import models, tasks

if TYPE_CHECKING:
from collections.abc import Collection
from collections.abc import Collection, Iterator


logger = logging.getLogger(__name__)
Expand All @@ -32,10 +33,15 @@ class TripsInformation(NamedTuple):
# Email address as given on Participant object
# (We assume this is their preferred current email)
email: str
# If they are a student & have verified an mit.edu email, we report it
verified_mit_email: str | None


class MembershipInformation(NamedTuple):
email: str
# In order to claim MIT student status, you must verify you own an @mit.edu email.
# We'll only report this for *current* students (since many others will lack).
mit_email: str | None
alternate_emails: list[str]
person_id: int
affiliation: str
Expand Down Expand Up @@ -133,9 +139,26 @@ def fetch_geardb_stats_for_all_members(
else:
assert_never(cache_strategy)

def _mit_emails(member: dict[str, Any]) -> Iterator[str]:
for email in {member["email"], *member["alternate_emails"]}:
if email.lower().endswith("@mit.edu"): # Exclude alum.mit.edu!
yield email.lower()

info = [
MembershipInformation(
email=member["email"],
# If they are a current MIT student, assume they own the first mit.edu.
# To pay dues on the trips site, we *require* they own an MIT email.
# However, it's possible a desk worker manually added the membership.
mit_email=(
next(_mit_emails(member), None)
if member["affiliation"]
in {
affiliations.MIT_UNDERGRAD.VALUE,
affiliations.MIT_GRAD_STUDENT.VALUE,
}
else None
),
alternate_emails=member["alternate_emails"],
person_id=int(member["id"]),
affiliation=member["affiliation"],
Expand All @@ -161,6 +184,19 @@ def _get_trip_stats_by_user() -> dict[int, TripsInformation]:
.values_list("participant_id", "num_trips")
)

# Technically, yes, one could have multiple verified mit.edu addresses.
# But that's going to be exceptionally rare, and the dict will handle dupes.
mit_email_for_students: dict[int, str] = dict(
models.Participant.objects.filter(
affiliation__in=[
affiliations.MIT_UNDERGRAD.CODE,
affiliations.MIT_GRAD_STUDENT.CODE,
],
user__emailaddress__verified=True,
user__emailaddress__email__iendswith="@mit.edu",
).values_list("pk", "user__emailaddress__email")
)

additional_stats = (
models.Participant.objects.all()
.annotate(
Expand All @@ -187,6 +223,7 @@ def _get_trip_stats_by_user() -> dict[int, TripsInformation]:
return {
par["user_id"]: TripsInformation(
email=par["email"],
verified_mit_email=mit_email_for_students.get(par["pk"]),
is_leader=par["is_leader"],
num_trips_attended=trips_per_participant.get(par["pk"], 0),
num_trips_led=par["num_trips_led"],
Expand Down

0 comments on commit 891e5f3

Please sign in to comment.