Skip to content

Conversation

@mariajgrimaldi
Copy link
Member

@mariajgrimaldi mariajgrimaldi commented Jun 8, 2021

Description:
This PR adds class definitions for the objects that will be sent by Open edX events, they are based in the library

JIRA:
https://edunext.atlassian.net/browse/PS2021-629?atlOrigin=eyJpIjoiODIwNDA5M2MxZGRlNGQ2NjhjNjJhZWIwNWZiNWNlYjUiLCJwIjoiaiJ9

Dependencies:
This PR depends on #7

Testing instructions:
In your Open edX devstack

  1. Go to lms shell make lms-shell
  2. Install openedx-events specifying the events definition branch:
    pip install git+https://github.com/eduNEXT/openedx-events.git@MJG/event_data#egg=event_data
  3. Go to the lms Django shell: python manage.py lms shell
  4. Now, you can import the platform objects and map them to attrs classes. For example, you could do:
>>> from common.djangoapps.student.models import CourseEnrollment
>>> from openedx_events.learning.data import CourseEnrollmentData, UserData, UserPersonalData, UserNonPersonalData, CourseOverviewData
>>> course_enrollment = CourseEnrollment.objects.get(id=1)

*** MAPPING CourseEnrollment to CourseEnrollmentData ***
>>> course_enrollment_data = CourseEnrollmentData(
...                             user=UserData(
...                                 user_pii=(
...                                       username=course_enrollment.user.username,
...                                       email=course_enrollment.user.email,
...                                       name=course_enrollment.user.profile.name                                 
...                                  ),
...                                 user_non_pii=(
...                                       id=course_enrollment,ent.user.id,
...                                       is_active=course_enrollment.user.is_active
...                                 ),
...                             ),
...                             course=CourseOverviewData(
...                                 course_key=course_enrollment.course.id,
...                                 display_name=course_enrollment.course.display_name
...                             ),
...                             mode=course_enrollment.mode,
...                             creation_date=course_enrollment.created,
...                             is_active=course_enrollment.is_active
...                         )
>>> print(course_enrollment_data)
>>> import attr
>>> attr.asdict(course_enrollment_data)

Reviewers:

  • tag reviewer
  • tag reviewer

Merge checklist:

  • All reviewers approved
  • CI build is green
  • Version bumped
  • Changelog record added
  • Documentation updated (not only docstrings)
  • Commits are squashed

Post merge:

  • Create a tag
  • Check new version is pushed to PyPI after tag-triggered build is
    finished.
  • Delete working branch (if not needed anymore)

Author concerns: List any concerns about this PR - inelegant
solutions, hacks, quick-and-dirty implementations, concerns about
migrations, etc.

@mariajgrimaldi mariajgrimaldi changed the title Mjg/event data [WIP] Mjg/event data Jun 8, 2021
@mariajgrimaldi mariajgrimaldi force-pushed the MJG/events_tooling branch 6 times, most recently from 67c8323 to d7879cd Compare June 9, 2021 02:45
@mariajgrimaldi mariajgrimaldi force-pushed the MJG/event_data branch 2 times, most recently from 44e043b to 91d3576 Compare June 9, 2021 02:56
@mariajgrimaldi mariajgrimaldi changed the title [WIP] Mjg/event data feat: add data attributes definition for learning subdomain Jun 9, 2021
@mariajgrimaldi mariajgrimaldi force-pushed the MJG/events_tooling branch 2 times, most recently from e6c9f96 to 1f8a798 Compare June 9, 2021 16:02
@mariajgrimaldi mariajgrimaldi force-pushed the MJG/event_data branch 3 times, most recently from aa94458 to 5eb16c4 Compare June 9, 2021 17:48
@mariajgrimaldi mariajgrimaldi marked this pull request as ready for review June 10, 2021 18:31
@mariajgrimaldi mariajgrimaldi force-pushed the MJG/event_data branch 2 times, most recently from d3ae682 to 861c1c5 Compare July 21, 2021 16:01
@mariajgrimaldi mariajgrimaldi force-pushed the MJG/event_data branch 2 times, most recently from 6b4a1cd to ca605d7 Compare July 22, 2021 19:44
email = attr.ib(type=str)
is_active = attr.ib(type=bool, default=True)
meta = attr.ib(type=Dict[str, str], factory=dict)
name = attr.ib(type=str, factory=str)
Copy link
Member Author

Choose a reason for hiding this comment

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

Hey @ormsbee! We followed your suggestions. Now, the user is divided into two parts with PII and non-PII information

course = attr.ib(type=CourseData)
mode = attr.ib(type=str)
grade = attr.ib(type=str)
previous_status = attr.ib(type=str, factory=str)
Copy link
Member Author

Choose a reason for hiding this comment

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

Hey again @ormsbee! We added the previous status for the certificate!

We'll make a mental note to review this again when using this data class in the platform! Thank you for the suggestion

email = attr.ib(type=str)
is_active = attr.ib(type=bool, default=True)
meta = attr.ib(type=Dict[str, str], factory=dict)
name = attr.ib(type=str, factory=str)
Copy link
Member Author

Choose a reason for hiding this comment

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

Hey @feanil!
We also changed StudentData to UserData :)

"""
Attributes defined for Open edX student object.
"""

Copy link
Member

Choose a reason for hiding this comment

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

I see you put it in the UserNonPersonalData attr class. Given the "LMS user_id can be used for public events" portion of OEP-32 I think we are in the clear

Information.
"""

user_non_pii = attr.ib(type=UserNonPersonalData)
Copy link
Member

Choose a reason for hiding this comment

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

I'd like to keep the access to the user data a little less convoluted.
With this, to access the ID, one would need to call user_data_obj.user_non_pii.id

We could do something like:

@attr.s(frozen=True)
class UserNonPiiData:
    id = attr.ib(type=int)
    is_active = attr.ib(type=bool)


@attr.s(frozen=True)
class UserPiiData:
    username = attr.ib(type=str)
    email = attr.ib(type=str)
    name = attr.ib(type=str, factory=str)


@attr.s(frozen=True)
class UserData(UserNonPersonalData):
    pii = attr.ib(type=UserPersonalData)


user_data_obj = UserData(
    pii=UserPiiData(
        username="course_enrollment.user.username",
        email="course_enrollment.user.email",
        name="course_enrollment.user.profile.name"
    ),
    id=1,
    is_active=True
)

So the access is more direct, but we still make a strong distinction for pii data.

Copy link
Member

Choose a reason for hiding this comment

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

I had to remove the is_active default when testing on the interactive console

@mariajgrimaldi mariajgrimaldi force-pushed the MJG/events_tooling branch 4 times, most recently from e827c18 to 4ed2f5e Compare July 28, 2021 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants