(api) Enrich incoming statements

[Leobouloc]

Purpose

The xAPI specification states that statements SHOULD or MUST be enriched with the following fields:

• "id" (https://github.com/adlnet/xAPI-Spec/blob/master/xAPI-Data.md#24-statement-properties)
• "timestamp" (https://github.com/adlnet/xAPI-Spec/blob/master/xAPI-Data.md#247-timestamp)
• "stored" (https://github.com/adlnet/xAPI-Spec/blob/1.0.3/xAPI-Data.md#248-stored)
• "authority" (https://github.com/adlnet/xAPI-Spec/blob/master/xAPI-Data.md#249-authority)

Currently, only the "id" field is being added. This PR aims improve compliance by enriching incoming statments with the fields above. Adding "authority" is also a necessary step to implement permissions.

Proposal

The main change besides enrichment is testing. Currently, tests check that incoming and retrieved statements are equal. The proposal changes this in an equivalency. Proposed equivalency between statements is defined as:

    To be equivalent, they must be identical on all fields not modified on input by the
    LRS and idententical on other fields when these fields are present in both
    statements. For example, if an "authority" field is present in only one statement,
    they may still be equivalent.

• write and test an equivalency function
• write enrichment functions for each field and add them to POST and PUT functions
• adapt existing tests to test for equivalency
• write tests for enrichment

NB: This proposal creates a new tests/helpers.py file for utilities common to multiple test files. We may refactor tests to include statement generation in this file (for example).

[wilbrdt]

[SergioSim]

This looks nice!
Have only minor questions/suggestions)
Maybe we could include stored in the forwarded statements? WDYT?
Also, do we need to add the version field in the returned statements from a get request?

[Leobouloc]

Also, do we need to add the version field in the returned statements from a get request?

Not sure I understand. Are you suggesting we remove this field if it is stored in a statement ?

[SergioSim]

Also, do we need to add the version field in the returned statements from a get request?

Not sure I understand. Are you suggesting we remove this field if it is stored in a statement ?

It's related to enriching outcoming statements (get request) with the version number, as in the spec:

Statements returned by an LRS MUST retain the version they are accepted with. If they lack a version, the version MUST be set to 1.0.0.

I think, we could add the version field prior statement storage, or, we could not enforce the storage of the version number and add it dynamically to the statements returned from a get request.

[Leobouloc]

I think, we could add the version field prior statement storage, or, we could not enforce the storage of the version number and add it dynamically to the statements returned from a get request.

Mhmm, I'm not sure how to interpret the spec since the version parameter is marked as "Not Recommended" (here: https://github.com/adlnet/xAPI-Spec/blob/master/xAPI-Data.md#24-statement-properties).

[SergioSim]

I think, we could add the version field prior statement storage, or, we could not enforce the storage of the version number and add it dynamically to the statements returned from a get request.

Mhmm, I'm not sure how to interpret the spec since the version parameter is marked as "Not Recommended" (here: https://github.com/adlnet/xAPI-Spec/blob/master/xAPI-Data.md#24-statement-properties).

It's indeed not recommended for learning record providers to send statements containing the version field:

Learning Record Providers SHOULD NOT set the Statement version.

However, in my understanding, this doesn't contradict the LRS requirement to return statements with a version field.

[Leobouloc]

I think, we could add the version field prior statement storage, or, we could not enforce the storage of the version number and add it dynamically to the statements returned from a get request.

Mhmm, I'm not sure how to interpret the spec since the version parameter is marked as "Not Recommended" (here: https://github.com/adlnet/xAPI-Spec/blob/master/xAPI-Data.md#24-statement-properties).

It's indeed not recommended for learning record providers to send statements containing the version field:

Learning Record Providers SHOULD NOT set the Statement version.

However, in my understanding, this doesn't contradict the LRS requirement to return statements with a version field.

I see. Then perhaps we can open a separate issue, as seems to be beyond the scope of the current PR.

EDIT: Issue opened here: #424

[SergioSim]

Looks awesome) 👍
Have only a few final minor questions/suggestions)

[wilbrdt]

[SergioSim]

LGTM) 👍
Just the CI linter complains for some reason, could you check it?