Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add first_active field to BlossomUser model #147

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class Meta:
"id",
"username",
"gamma",
"date_joined",
"first_active",
"last_login",
"last_update_time",
"accepted_coc",
Expand Down
87 changes: 81 additions & 6 deletions api/tests/test_volunteers.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,14 @@
"""Tests to validate the behavior of the VolunteerViewSet."""
import datetime
import json

import pytz
from django.test import Client
from django.urls import reverse
from rest_framework import status

from api.models import Submission, Transcription
from api.tests.helpers import (
create_submission,
create_transcription,
create_user,
setup_user_client,
)
from api.tests.helpers import create_submission, create_user, setup_user_client
from authentication.models import BlossomUser


Expand Down Expand Up @@ -273,3 +270,81 @@ def test_accept_coc_duplicate(self, client: Client) -> None:
assert result.status_code == status.HTTP_409_CONFLICT
user.refresh_from_db()
assert user.accepted_coc is True


class TestVolunteerFirstActive:
def test_first_active_one_claim(self, client: Client) -> None:
"""Test that first_activity returns date of single claim."""
client, headers, user = setup_user_client(client)

time = datetime.datetime(
2021, 6, 2, 13, 20, tzinfo=pytz.timezone("UTC")
) # 2021-06-02 13:20
create_submission(claimed_by=user, claim_time=time)

other_time = datetime.datetime(
2021, 5, 2, 13, 20, tzinfo=pytz.timezone("UTC")
) # 2021-05-02 13:20
other_user = create_user(username="other")
create_submission(claimed_by=other_user, claim_time=other_time)

user.refresh_from_db()
assert user.first_active == time

def test_first_active_multiple_claims(self, client: Client) -> None:
"""Test that first_activity returns date of first claim."""
client, headers, user = setup_user_client(client)

times = [
datetime.datetime(
2021, 6, 2, 13, 20, tzinfo=pytz.timezone("UTC")
), # 2021-06-02 13:20
datetime.datetime(
2021, 6, 3, 12, 30, tzinfo=pytz.timezone("UTC")
), # 2021-06-03 12:30
datetime.datetime(
2021, 6, 4, 3, 10, tzinfo=pytz.timezone("UTC")
), # 2021-06-04 03:10
datetime.datetime(
2021, 6, 5, 22, 5, tzinfo=pytz.timezone("UTC")
), # 2021-06-05 22:05
]

for time in times:
create_submission(claimed_by=user, claim_time=time)

other_user = create_user(username="other")

other_times = [
datetime.datetime(
2021, 5, 2, 13, 20, tzinfo=pytz.timezone("UTC")
), # 2021-05-02 13:20
datetime.datetime(
2021, 5, 3, 12, 30, tzinfo=pytz.timezone("UTC")
), # 2021-05-03 12:30
datetime.datetime(
2021, 5, 4, 3, 10, tzinfo=pytz.timezone("UTC")
), # 2021-05-04 03:10
datetime.datetime(
2021, 5, 5, 22, 5, tzinfo=pytz.timezone("UTC")
), # 2021-05-05 22:05
]

for time in other_times:
create_submission(claimed_by=other_user, claim_time=time)

user.refresh_from_db()
assert user.first_active == times[0]

def test_first_active_no_claim(self, client: Client) -> None:
"""Test that first_activity returns None if nothing has been claimed."""
client, headers, user = setup_user_client(client)

other_time = datetime.datetime(
2021, 6, 2, 13, 20, tzinfo=pytz.timezone("UTC")
) # 2021-06-02 13:20
other_user = create_user(username="other")
create_submission(claimed_by=other_user, claim_time=other_time)

user.refresh_from_db()
assert user.first_active is None
18 changes: 18 additions & 0 deletions authentication/models.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
"""Models used within the Authentication application."""
import datetime
from typing import Optional

from django.contrib.auth.models import AbstractUser
from django.db import models
from django.utils import timezone
Expand Down Expand Up @@ -58,6 +61,21 @@ def gamma(self) -> int:
return 0 # see https://github.com/GrafeasGroup/blossom/issues/15
return Submission.objects.filter(completed_by=self).count()

@property
def first_active(self) -> Optional[datetime.datetime]:
"""
Return the date when the user was first active.

Usually, this is the date of their first transcription.

:return: the date when the user was first active or None if they haven't
transcribed yet.
"""
first_claim = (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we know anything about the performance of this query? It sounds like it could be quite detrimental to execution time if there's many submissions, especially since they have to be sorted. Wouldn't we be better off just making this an additional field of the user, since this date does not change after the initial claim?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be quite fast, but it's probably better to do it as @itsthejoker suggested and to just fix the date_joined field to actually be correct

Submission.objects.filter(claimed_by=self).order_by("claim_time").first()
)
return None if first_claim is None else first_claim.claim_time

def __str__(self) -> str:
return self.username

Expand Down