Skip to content

Conversation

@mariajgrimaldi
Copy link
Member

@mariajgrimaldi mariajgrimaldi commented Jul 28, 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. Merged already

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, 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                                 
...                                  ),
...                                  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.
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, 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                                 
...                                  ),
...                                 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:

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)

@mariajgrimaldi mariajgrimaldi changed the title Mjg/event data feat: add data attributes definition for learning subdomain Jul 28, 2021
@mariajgrimaldi
Copy link
Member Author

mariajgrimaldi commented Jul 28, 2021

This PR was opened because #8 was a PR to an intermediate branch. Please, refer to #8 for design discussions.



@attr.s(frozen=True)
class UserData(UserNonPersonalData):
Copy link
Member Author

@mariajgrimaldi mariajgrimaldi Jul 28, 2021

Choose a reason for hiding this comment

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

Hey @felipemontoya, thanks for the recommendation! Do you think we are ready to merge?

Copy link
Member

Choose a reason for hiding this comment

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

I think we are

Copy link
Member

@felipemontoya felipemontoya left a comment

Choose a reason for hiding this comment

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

Good work, we are ready to merge this and focus on the usage now.

Btw, the example of code in the cover letter is now outdated.

@mariajgrimaldi mariajgrimaldi force-pushed the MJG/event_data branch 2 times, most recently from 5c1fbca to 3f54c48 Compare July 28, 2021 18:49
@mariajgrimaldi mariajgrimaldi merged commit 9d3dc73 into main Jul 29, 2021
@mariajgrimaldi mariajgrimaldi deleted the MJG/event_data branch August 31, 2022 20:29
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.

3 participants