search

xapijs / xapi

Communicate over xAPI using JavaScript.
https://www.xapijs.dev/xapi-wrapper-library
MIT License
43 stars 21 forks source link

IEEE 9274.1.1 Support #296

Open CookieCookson opened 1 year ago

CookieCookson commented 1 year ago

General Updates

g1 Specification Reorganization

No changes required

g2 MUST to SHALL

No changes required

g3 Minor Spelling and Grammar

No changes required

g4 Version update to 2.0.0

“Should *” Updates

s1-s4 Additional Properties

No changes required

s5 Including Display on Query with Format of ids

No changes required

s6 Canonical Display on Query with Format of canonical

No changes required

s7 Returning Canonical Language Map on Query with Format of canonical

s8 Return One Language within Each Language Map

No changes required

s9 Response Pattern Character Limits

No changes required

s10 correctResponsesPattern Array Length Limit

No changes required

s11 Setting Timestamp to Stored

No changes required

s12 Timestamp with Greater Value than Current Time

No changes required

s13, s14 JWS Compact Serialization to Create JSON Web Signature

No changes required

s15 Using IRIs a Metadata Provider Controls

No changes required

s16 Re-using Identifiers

No changes required

s17, s18 IRI Simple String Comparison

No changes required

s19 Timestamp in RFC 3339

s20 Timestamp Including Timezone

s21 Returning Timestamp with Different Time Zone

No changes required

s22 Timestamp in UTC

No changes required

s23 Truncating Durations

No changes required, but improvements can be made to calculateISO8601Duration:

s24, s25 Duration Comparison Precision

No changes required

s26-s33 Alternate Request Syntax Removal

s34 Accepting Batches with Document Type multipart/mixed

No changes

s35 Accepting Batches with Attachment Objects with fileUrl

No changes

s36, s37 Rejecting Statement Batch with ID Collision

No changes

s38 Last-Modified Matching Stored

No changes

s39 Last-Modified Header on Single Activity State Document Return

No changes

s40 Last-Modified Header on Single Agent Profile Document Return

No changes

s41 Last-Modified Header on Single Activity Profile Document Return

No changes

s42 Last-Modified on multiple document return

No changes

s43 Return an Activity Object when unknown

No changes

s44-s46 Concurrency Header Use on PUT and POST Document Endpoints

CookieCookson commented 10 months ago

TODO: Convert this into issues in the GH Project.

mawi12345 commented 6 months ago

I would like to assist. Can you tell me where or what I can best contribute. I have implemented an LRS that passes all v2.0.0 tests from lrs-conformance-test-suite.

To pass the test, the LRS timestamp must reject the time zones in the format "+01:00". However, I could not find anything about this in the specification. I think there is a bug in the tests and my LRS implementation accepts all valid timestamps and converts them to UTC.

CookieCookson commented 6 months ago

Hi @mawi12345 thanks for reaching out and offering your support on this issue!

This ticket was initially put together before the full proposal for xAPI 2.0 was available. Since then I've not had the time available to do the analysis on what's different between the versions from a client perspective.

I've also been thinking of what structure/pattern to use in the repo so that the library is backwards compatible with 1.0.3 and only exposes the new properties for users targeting the 2.0 version. I have an idea of passing the version string in the constructor as an argument and overloading the class interface response but this needs investigation into it's viability.

I've also got some breaking changes coming in with #382 so it would be good to release these changes together.

I'll see if I can get the ball rolling on this over the weekend, the plan is to put all the breaking changes onto the next branch which will become the next major version of xAPI.js.

For now your assistance would be appreciated by identifying all of the changes required from the spec e.g. new properties, headers, changes to date formatting etc.

Once we have the versionable code structure/pattern in place we can work through implementing these on a case by case basis, and if you have an xAPI 2 conformant LRS available that would be beneficial for manual / automated testing.

mawi12345 commented 6 months ago

I manually compared the types and couldn't find many differences. I started this branch to record the changes. It looks to me like version 1.0.x statements are compatible with version 2.0.0. The biggest difference are the new properties "contextAgents" and "contextGroups" (both optional). A versionable code structure/pattern will most likely not be necessary for 2.0.0 support. So far I have only checked the types, not the behavior (concurrency controls).

mawi12345 commented 5 months ago

As a content author, I want my content to work in as many environments as possible. That's why we try to use as few functions of the LRS as possible. I believe that many other content authors do the same. The same situation already existed with SCORM, hardly any content uses navigation, almost all are single SCO. I suspect that this is exactly why context.team/instructor/contextGroups/contextAgents etc. will hardly be used by any content/tool.

I think it would be best for the users of the library if the about endpoint of the LRS is used to select the xAPI version. In this case, the types could only contain the intersection of the API version (equivaltent to version 1.0.3 since 2.0.0 only adds new properties).

What is your opinion on a setting that automatically determines the version at runtime?

CookieCookson commented 4 months ago

@mawi12345 Sorry for the late reply, I have been on annual leave and caught a bug on my travels which has meant I have been out of action for quite some time!

Thank you so much for creating the branch with the v2 type changes, it's very helpful to understand the scope of the changes and help make decisions on how we can handle backwards compatibility.

I agree from my past e-learning adventures I have generally used the smallest amount of xAPI properties as possible for compatibility, unfortunately(?) the xAPI spec was designed to be much broader and cater for things beyond just traditional e-learning modules. With this in mind the overall idea for this library is to be an unopinionated layer of abstraction to simplify making any kind of xAPI request, so I don't think having it automatically set the xAPI version would be in keeping with that ethos.

On the flip side if you have an idea for a non-intrusive helper method that determines the version at runtime, a pull request would be appreciated as I am sure many people would find that to be useful functionality - especially with creating content that is future-proof/backwards compatible from v2 to v1.x.

In regards to the design decision for a versionable code structure, it is certainly not a direction I would like to go into as it adds a lot more complexity. With some of the identified changes e.g. the option to have a single object instead of an array for contextActivity properties we don't want 1.0.3 users to think that is valid through the interfaces when it is not. In cases like this we could potentially add in some payload transformers which automatically wrap them in an array if the version is not compatible. This kind of goes against the unopinionated ethos of not messing with user's payloads and letting them just make the request so I am in two minds about how we best handle these kinds of changes.

mawi12345 commented 4 months ago

I support your point of view that it is better to solve the automatic version selection and abstraction on another level. I use the library as part of another abstraction layer that provides a minimal interface to functions available in both SCORM 1.2/1.3 and xAPI 1.0.x. This layer is definitely better suited to abstract over xAPI 2.0.0 as well.

I would suggest creating several packages in a mono repo.

This would change little to nothing for users of xAPI 1.x (apart from the changes necessary for the fetch abstraction) and both xAPI versions can use a strictly specification-defined interface.

I currently use this snippet to query the version of the LRS:

try {
  const res = await fetch(`${endpoint}/about`);
  const about = await res.json();
  if (about.version && Array.isArray(about.version)) {
    const lrsVersions: string[] = about.version;
    const matchedVersion = SUPPORTED_VERSIONS.find((v) => {
      return lrsVersions.includes(v);
    });
    return matchedVersion;
  }
} catch (e) {}
CookieCookson commented 4 months ago

I really like the sound of this but I just want to play devil's advocate against this approach. It would lead to an amount of code duplication as we would essentially be doubling up on everything (handlers, interfaces, tests etc) with some minor differences compared to the size of the overall codebase. If there were any newer versions of 2.x we would have to keep it in the same package and use deprecated tags where appropriate.

A (possibly worse) alternative to this could be having a form of base class(es) that contain the v1 implementation and where necessary extend the classes and override with any v2 modifications. This approach would reduce how much duplication there is but could cause a codebase that is harder to maintain.

On the flip-side again having the packages entirely separate would mean a separation of concerns and we wouldn't be worried that changing one may impact the other.

With the work in #383 it would be ideal to have the new fetcher logic in its own common/core package and it can be isolated from the xAPI version specific code.

In terms of next steps to implement this as a monorepo, I'm thinking firstly split the current 1.0.3 implementation (with new customisable fetchers) into two packages core and xapi (xAPI 1.x), and once that is in place the additional package xapi2 can be added in alongside for the xAPI 2.x spec.

Thinking aloud, it could be that the xapi package eventually becomes responsible for the version switching, with a new xapi-1 package that contains the xAPI 1.x code so people would get the benefit of supporting multiple versions out of the box by default but can migrate to the v1 or v2 packages if they wish to have more granular control.