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 ralph for xAPI statements generation in backend #2582

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

quitterie-lcs
Copy link
Contributor

Purpose

We want to use Ralph to generate xAPI statements for videos, documents, webinar
and lives. Based on Pydantic models, it would strengthen validation to avoid potential
errors when statements are sent to a LRS.

Proposal

  • Replace hard coded partial statements in backends
  • Adapt tests
  • Use LRS backends from Ralph ?

Finally, we have integrated Ralph in marsha to help generate xAPI statements
with Pydantic! However the integration is partial as statements are initialized
from the frontend.
@quitterie-lcs quitterie-lcs force-pushed the xapi/integrate-ralph branch from ccab22a to 45c9636 Compare May 7, 2024 12:30
@@ -41,10 +42,12 @@ def _statement_from_lti(
xapi_logger = logging.getLogger(f"xapi.{consumer_site.domain}")

# xapi statement enriched with video and jwt_token information
lti = LTI(request, object_instance)
Copy link
Member

Choose a reason for hiding this comment

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

As you are not in a LTI request you will have no relevant information.
IMO if you need to have an information from the LTI request, you have to set it in the JWT token and then retrieve it in it when you need it.
And to add the origin_url in the jwt token you should add it here I think

token.payload.update(
{
"context_id": lti.context_id,
"consumer_site": str(lti.get_consumer_site().id),
}
)

Comment on lines +54 to +58
return (
jwt_token.payload["user"].get("username")
if jwt_token.payload.get("user")
else 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
return (
jwt_token.payload["user"].get("username")
if jwt_token.payload.get("user")
else None
)
return jwt_token.payload.get("user", {}).get("username")

}
return BaseXapiAgentWithAccount(
account=BaseXapiAccount(homePage=homepage, name=str(user.id)),
name=user.username,
Copy link
Contributor

Choose a reason for hiding this comment

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

name field of the actor is optionnal. Should we avoid setting it, facilitating our future work on anonymization?
If we decide to keep it, should'nt it be a full name instead of username?

"account": {"name": user_id, "homePage": homepage},
}
return BaseXapiAgentWithAccount(
name=username, account=BaseXapiAccount(homePage=homepage, name=user_id)
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

Comment on lines 113 to +130
statement = self.build_common_statement_properties(
statement, homepage, user=user, user_id=user_id
statement, homepage, user=user, user_id=user_id, username=username
)

statement["context"].update(
{"contextActivities": {"category": [{"id": "https://w3id.org/xapi/lms"}]}}
{
"contextActivities": {
"category": [LMSProfileActivity().model_dump(exclude_none=True)]
}
}
)

statement["object"] = {
"definition": {
"type": "http://id.tincanapi.com/activitytype/document",
"name": {self.get_locale(): document.title},
},
"id": f"uuid://{document.id}",
"objectType": "Activity",
}
statement["object"] = DocumentActivity(
definition=DocumentActivityDefinition(
name={self.get_locale(): document.title}
),
id=f"uuid://{document.id}",
).model_dump(exclude_none=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be possible to use BaseXapiStatement from ralph to build these core properties?

Comment on lines +222 to +239
if statement["verb"]["id"] == "http://id.tincanapi.com/verb/downloaded":
statement["context"].update(
{
"contextActivities": {
"category": [LMSProfileActivity().model_dump(exclude_none=True)]
}
}
)
else:
statement["context"].update(
{
"contextActivities": {
"category": [
VideoProfileActivity().model_dump(exclude_none=True)
]
}
}
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, would it be possible to work with ralph pydantic models instead of modifying a dict directly?
We could use them without any validation using a .construct()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants