-
Notifications
You must be signed in to change notification settings - Fork 27
feat: add data attributes definition for learning subdomain #8
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
Conversation
2f3fba7 to
2251e2e
Compare
67c8323 to
d7879cd
Compare
44e043b to
91d3576
Compare
e6c9f96 to
1f8a798
Compare
aa94458 to
5eb16c4
Compare
1f8a798 to
effffbb
Compare
5eb16c4 to
6683f06
Compare
effffbb to
0df3e69
Compare
1744cbd to
da85803
Compare
0df3e69 to
c4738eb
Compare
da85803 to
c1f9694
Compare
c4738eb to
de25a1f
Compare
c1f9694 to
c3f8b8a
Compare
d3ae682 to
861c1c5
Compare
6b4a1cd to
ca605d7
Compare
| 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) |
There was a problem hiding this comment.
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
openedx_events/learning/data.py
Outdated
| course = attr.ib(type=CourseData) | ||
| mode = attr.ib(type=str) | ||
| grade = attr.ib(type=str) | ||
| previous_status = attr.ib(type=str, factory=str) |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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 :)
80ad2c0 to
28502ff
Compare
d3dcca8 to
5a9cf69
Compare
c28f334 to
2e2e532
Compare
5a9cf69 to
871f98c
Compare
2e2e532 to
071c741
Compare
871f98c to
8707785
Compare
| """ | ||
| Attributes defined for Open edX student object. | ||
| """ | ||
|
|
There was a problem hiding this comment.
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
openedx_events/learning/data.py
Outdated
| Information. | ||
| """ | ||
|
|
||
| user_non_pii = attr.ib(type=UserNonPersonalData) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
0daedf3 to
ed2c6c0
Compare
e827c18 to
4ed2f5e
Compare
feat: add necessary tooling for Open edX events
ed2c6c0 to
1a193c2
Compare
1a193c2 to
e1de0c2
Compare
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
make lms-shellpip install git+https://github.com/eduNEXT/openedx-events.git@MJG/event_data#egg=event_datapython manage.py lms shellReviewers:
Merge checklist:
Post merge:
finished.
Author concerns: List any concerns about this PR - inelegant
solutions, hacks, quick-and-dirty implementations, concerns about
migrations, etc.