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.

Sign up for GitHub

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

feat: add extra fields to ORA staff graded serializer #2076

Closed
wants to merge 1 commit into from
Closed

feat: add extra fields to ORA staff graded serializer #2076

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
42 changes: 42 additions & 0 deletions openassessment/data.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,48 @@ def map_anonymized_ids_to_usernames(anonymized_ids):

return anonymous_id_to_username_mapping

def map_anonymized_ids_to_emails(anonymized_ids):
Copy link
Contributor

Choose a reason for hiding this comment

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

Here's an idea, rather than having these all be separate, and adding a new function every time we want more user data, why not do something like

def map_anonymized_ids_to_user_data(anonymized_ids):
    """
    Args:
        anonymized_ids - list of anonymized user ids.
    Returns:
        dict {
           <anonymous_user_id> : {
               'email': (str) <user.email>
               'username': (str) <user.username>
               'fullname': (str) <user.profile.name>
            }
        }
        """
       User = get_user_model()

    users = _use_read_replica(
        User.objects.filter(anonymoususerid__anonymous_user_id__in=anonymized_ids)
        .select_related("profile")
        .annotate(
            anonymous_id=F("anonymoususerid__anonymous_user_id")),
            fullname=F("profile__name"),
        )
        .values("username", "email", "fullname", "anonymous_id")
    )

    anonymous_id_to_user_info_mappnig = {
        user["anonymous_id"]: user for user in users
    }
    return anonymous_id_to_user_info_mapping

Copy link
Member

@mariajgrimaldi mariajgrimaldi Oct 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I like that idea. If we make this change, we'd also need to modify: https://github.com/openedx/edx-ora2/pull/2076/files#diff-4708da322d3d3b03cd269f5aa9574e7b1708cd6e665f1c25b2ba2570ca18dfefR98-R113 (and probably more) so we keep the implementation consistent.

"""
Args:
anonymized_ids - list of anonymized user ids.
Returns:
dictionary, that contains mapping between anonymized user ids and
actual user emails.
"""
User = get_user_model()

users = _use_read_replica(
User.objects.filter(anonymoususerid__anonymous_user_id__in=anonymized_ids)
.annotate(anonymous_id=F("anonymoususerid__anonymous_user_id"))
.values("email", "anonymous_id")
)
anonymous_id_to_email_mapping = {
user["anonymous_id"]: user["email"] for user in users
}
return anonymous_id_to_email_mapping


def map_anonymized_ids_to_fullname(anonymized_ids):
"""
Args:
anonymized_ids - list of anonymized user ids.
Returns:
dictionary, that contains mapping between anonymized user ids and
actual user fullname.
"""
User = get_user_model()

users = _use_read_replica(
User.objects.filter(anonymoususerid__anonymous_user_id__in=anonymized_ids)
.select_related("profile")
.annotate(anonymous_id=F("anonymoususerid__anonymous_user_id"))
.values("profile__name", "anonymous_id")
)

anonymous_id_to_fullname_mapping = {
user["anonymous_id"]: user["profile__name"] for user in users
}
return anonymous_id_to_fullname_mapping

class CsvWriter:
"""
Expand Down
61 changes: 55 additions & 6 deletions openassessment/staffgrader/serializers/submission_list.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@ class Meta:
'gradedBy',
'username',
'teamName',
'score'
'score',
"email",
"fullname",
]
read_only_fields = fields

Expand All @@ -40,17 +42,23 @@ class Meta:
CONTEXT_ANON_ID_TO_USERNAME = 'anonymous_id_to_username'
CONTEXT_SUB_TO_ASSESSMENT = 'submission_uuid_to_assessment'
CONTEXT_SUB_TO_ANON_ID = 'submission_uuid_to_student_id'
CONTEXT_ANON_ID_TO_EMAIL = "anonymous_id_to_email"
CONTEXT_ANON_ID_TO_FULLNAME = "anonymous_id_to_fullname"

def _verify_required_context(self, context):
"""Verify that required individual or team context is present for serialization"""
context_keys = set(context.keys())

# Required context for individual submissions
required_context = set([
self.CONTEXT_ANON_ID_TO_USERNAME,
self.CONTEXT_SUB_TO_ASSESSMENT,
self.CONTEXT_SUB_TO_ANON_ID
])
required_context = set(
[
self.CONTEXT_ANON_ID_TO_USERNAME,
self.CONTEXT_SUB_TO_ASSESSMENT,
self.CONTEXT_SUB_TO_ANON_ID,
self.CONTEXT_ANON_ID_TO_EMAIL,
self.CONTEXT_ANON_ID_TO_FULLNAME,
]
)

missing_context = required_context - context_keys
if missing_context:
Expand All @@ -70,6 +78,8 @@ def __init__(self, *args, **kwargs):
username = serializers.SerializerMethodField()
teamName = serializers.SerializerMethodField()
score = serializers.SerializerMethodField()
email = serializers.SerializerMethodField()
fullname = serializers.SerializerMethodField()

def _get_username_from_context(self, anonymous_user_id):
try:
Expand All @@ -85,6 +95,22 @@ def _get_anonymous_id_from_context(self, submission_uuid):
f"No submitter anonymous user id found for submission uuid {submission_uuid}"
) from e

def _get_email_from_context(self, anonymous_user_id):
try:
return self.context[self.CONTEXT_ANON_ID_TO_EMAIL][anonymous_user_id]
except KeyError as e:
raise MissingContextException(
f"Email not found for anonymous user id {anonymous_user_id}"
) from e

def _get_fullname_from_context(self, anonymous_user_id):
try:
return self.context[self.CONTEXT_ANON_ID_TO_FULLNAME][anonymous_user_id]
except KeyError as e:
raise MissingContextException(
f"fullname not found for anonymous user id {anonymous_user_id}"
) from e

def get_dateGraded(self, workflow):
return str(workflow.grading_completed_at)

Expand All @@ -99,6 +125,16 @@ def get_username(self, workflow):
self._get_anonymous_id_from_context(workflow.identifying_uuid)
)

def get_email(self, workflow):
return self._get_email_from_context(
self._get_anonymous_id_from_context(workflow.identifying_uuid)
)

def get_fullname(self, workflow):
return self._get_fullname_from_context(
self._get_anonymous_id_from_context(workflow.identifying_uuid)
)

def get_teamName(self, workflow): # pylint: disable=unused-argument
# For individual submissions, this is intentionally empty
return None
Expand All @@ -123,12 +159,16 @@ class TeamSubmissionListSerializer(SubmissionListSerializer):
CONTEXT_SUB_TO_ASSESSMENT = 'submission_uuid_to_assessment'
CONTEXT_SUB_TO_TEAM_ID = 'team_submission_uuid_to_team_id'
CONTEXT_TEAM_ID_TO_TEAM_NAME = 'team_id_to_team_name'
CONTEXT_ANON_ID_TO_EMAIL = "anonymous_id_to_email"
CONTEXT_ANON_ID_TO_FULLNAME = "anonymous_id_to_fullname"

REQUIRED_CONTEXT_KEYS = [
CONTEXT_ANON_ID_TO_USERNAME,
CONTEXT_SUB_TO_ASSESSMENT,
CONTEXT_SUB_TO_TEAM_ID,
CONTEXT_TEAM_ID_TO_TEAM_NAME,
CONTEXT_ANON_ID_TO_EMAIL,
CONTEXT_ANON_ID_TO_FULLNAME,
]

def _verify_required_context(self, context):
Expand Down Expand Up @@ -160,6 +200,15 @@ def get_username(self, workflow): # pylint: disable=unused-argument
# For team submissions, this is intentionally empty
return None

def get_email(self, workflow): # pylint: disable=unused-argument
# For team submissions, this is intentionally empty
return None

def get_fullname(self, workflow): # pylint: disable=unused-argument
# For team submissions, this is intentionally empty
return None


Copy link
Member

Choose a reason for hiding this comment

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

Suggested change

def get_teamName(self, workflow):
return self._get_team_name_from_context(
self._get_team_id_from_context(workflow.identifying_uuid)
Expand Down
33 changes: 26 additions & 7 deletions openassessment/staffgrader/staff_grader_mixin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@

from openassessment.assessment.models.base import Assessment, AssessmentPart
from openassessment.assessment.models.staff import StaffWorkflow, TeamStaffWorkflow
from openassessment.data import map_anonymized_ids_to_usernames, OraSubmissionAnswerFactory, VersionNotFoundException
from openassessment.data import (OraSubmissionAnswerFactory, VersionNotFoundException, map_anonymized_ids_to_emails,
map_anonymized_ids_to_fullname, map_anonymized_ids_to_usernames)
Comment on lines +18 to +19
Copy link
Contributor

Choose a reason for hiding this comment

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

style nit

Suggested change
from openassessment.data import (OraSubmissionAnswerFactory, VersionNotFoundException, map_anonymized_ids_to_emails,
map_anonymized_ids_to_fullname, map_anonymized_ids_to_usernames)
from openassessment.data import (
OraSubmissionAnswerFactory,
VersionNotFoundException,
map_anonymized_ids_to_emails,
map_anonymized_ids_to_fullname,
map_anonymized_ids_to_usernames
)

from openassessment.staffgrader.errors.submission_lock import SubmissionLockContestedError
from openassessment.staffgrader.models.submission_lock import SubmissionGradingLock
from openassessment.staffgrader.serializers import (
Expand Down Expand Up @@ -215,7 +216,15 @@ def _get_list_workflows_serializer_context(self, staff_workflows, is_team_assign
team_id_to_team_name = self.teams_service.get_team_names(course_id, topic_id)

# Do bulk lookup for scorer anonymous ids (submitting team name is a separate lookup)
anonymous_id_to_username = map_anonymized_ids_to_usernames(set(workflow_scorer_ids))
anonymous_id_to_username = map_anonymized_ids_to_usernames(
set(workflow_scorer_ids)
)
anonymous_id_to_email = map_anonymized_ids_to_emails(
set(workflow_scorer_ids)
)
anonymous_id_to_fullname = map_anonymized_ids_to_fullname(
set(workflow_scorer_ids)
)

context = {
'team_submission_uuid_to_team_id': team_submission_uuid_to_team_id,
Expand All @@ -233,6 +242,13 @@ def _get_list_workflows_serializer_context(self, staff_workflows, is_team_assign
anonymous_id_to_username = map_anonymized_ids_to_usernames(
set(submission_uuid_to_student_id.values()) | workflow_scorer_ids
)
anonymous_id_to_email = map_anonymized_ids_to_emails(
set(submission_uuid_to_student_id.values()) | workflow_scorer_ids
)

anonymous_id_to_fullname = map_anonymized_ids_to_fullname(
set(submission_uuid_to_student_id.values()) | workflow_scorer_ids
)

context = {
'submission_uuid_to_student_id': submission_uuid_to_student_id,
Expand All @@ -242,11 +258,14 @@ def _get_list_workflows_serializer_context(self, staff_workflows, is_team_assign
# Rubric, Criteria, and Option models
submission_uuid_to_assessment = self.bulk_deep_fetch_assessments(staff_workflows)

context.update({
'anonymous_id_to_username': anonymous_id_to_username,
'submission_uuid_to_assessment': submission_uuid_to_assessment,
})

context.update(
{
"anonymous_id_to_username": anonymous_id_to_username,
"anonymous_id_to_email": anonymous_id_to_email,
"anonymous_id_to_fullname": anonymous_id_to_fullname,
"submission_uuid_to_assessment": submission_uuid_to_assessment,
}
)
return context

def _bulk_fetch_annotated_staff_workflows(self, is_team_assignment=False):
Expand Down