search

yetanalytics / lrsql

A SQL-based Learning Record Store
https://www.sqllrs.com
Apache License 2.0
89 stars 18 forks source link

xAPI Statement Validation Issue - empty contextActivities object #357

Closed GraemeSMiller closed 8 months ago

GraemeSMiller commented 8 months ago

I have just setup lrsql so a partner (a popular e-learing course builder) can have us act as a Custom LRS. It was so easy - got it all working and had statements being sent in less than 30 minutes. Great out the box experience. However posting statements from partner LMS resulted in 400 - I captured the response and statement fails validation beacuse they send "contextActivities": {} as an empty object. If I remove the contextActivties and repost it in postman it instantly works.

I looked that xAPI specification and I wasn't clear whether empty object was valid or not. It says it is an optional map. Extensions property also seems like an optional map and lrsql accepts it as an empty object. Though contextActitives calls out the only valid types - where extensions doesn't (as it is for domain specific data). Context Activities says "There are four valid context types. All, any or none of these MAY be used in a given Statement". Also "Every key in the contextActivities Object MUST be one of parent, grouping, category, or other." - so I guess I can see partner reading that as empty object is acceptable as it is Empty and there are no keys. Other statement validators pass it - but I assume yours fails it by design?

The provider is unable/unwilling to change the posted statement body. As far as I can see you use your other libraries to do the validation. Is there any way to configure the validation rules for statement so we can bypass this validation issue? Can I configure contextActivities to allow empty object? I assume not as you probably don't support bypassing what you consider valid statement?

I could potentially rewrite the body with a proxy, or fork your repo - but I wanted to check whether there was an easier option.

cliffcaseyyet commented 8 months ago

@GraemeSMiller Thanks for bringing this up. We will look into this and get back to you with an answer today.

Thanks, Cliff

cliffcaseyyet commented 8 months ago

@GraemeSMiller I confirmed your observation in testing and we are discussing internally. Is it possible to also share which LMS is issuing empty object bodies (if it's not a custom/internal one)? As this is a public post, you could also email me if you prefer.

Thanks, Cliff

cliffcaseyyet commented 8 months ago

@GraemeSMiller after some internal discussion we decided to move forward with a fix. It is ambiguous spec-wise that an LRS support empty maps but it turns out you are not the first to request. We are testing a small patch now and I will get back to you with version # when it is pushed.

cliffcaseyyet commented 8 months ago

@GraemeSMiller okay, that turned out to be pretty quick. Check out this release and let us know if it resolves your LMS issues.

-Cliff

GraemeSMiller commented 8 months ago

The update works perfectly. Thanks for the super fast turn around time.