tunapanda / funzo-app

Cordova app running ember and h5p
2 stars 3 forks source link

include gps and debug info in xapi #106

Closed usernamenumber closed 7 years ago

usernamenumber commented 7 years ago

@mcantor @Jakeii

I completely redid how the xapi statements are structured, and included a bunch more useful data, including optional location data per Certell's request. Still hoping to figure out a better way to balance making it clear whether students are using the app at school or not, but for now I've addressed it by making the gps accuracy configurable and defaulting it to something middle-of-the-road. Maybe the book metadata could include coords of the institution's campus(es), and the xapi could just include how far the user is from the closest one?

I should push this out sooner rather than later, but since it's a big change if either of you have a sec to look it over before I merge, that would be great. So you can see how the xapi stuff gets built if nothing else.

Here's an example of what a statement looks like as of this PR:

{
    "version": "1.0.0",
    "id": "37ced4de-80a0-4220-9df9-a31f6c211dbe",
    "timestamp": "2016-10-01T19:58:27.655Z",
    "actor": {
        "objectType": "Agent",
        "account": {
            "name": "2E85AEBE-97D7-831A-AA2B-371DCF22E23F",
            "homePage": "http://funzo.tunapanda.org/xapi/extensions/institution/testing-university"
        }
    },
    "verb": {
        "id": "http://adlnet.gov/expapi/verbs/experienced",
        "display": {
            "en-US": "experienced"
        }
    },
    "context": {
        "platform": "cse_testing-university",
        "extensions": {
            "http://tunapanda.org/xapi/extensions/debug": {
                "book": "cse_testing-university",
                "userAgent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/53.0.2785.116 Safari/537.36",
                "platform": "web",
                "messages": []
            },
            "http://tunapanda.org/xapi/extensions/institution": "testing-university",
            "http://tunapanda.org/xapi/extensions/location": {
                "lat": "XX.XXX  (redacted since this PR is publicly viewable)",
                "lng": "YY.YYY"
            },
            "http://tunapanda.org/xapi/extensions/regname": "foo"
        },
        "contextActivities": {
            "parent": [
                {
                    "objectType": "Activity",
                    "id": "http://localhost:4200/#/book/cse_testing-university/section/00Preface"
                }
            ]
        }
    },
    "object": {
        "objectType": "Activity",
        "id": "http://commonsenseeconomics.com/",
        "definition": {
            "type": "http://funzo.tunapanda.org/xapi/activity/link",
            "name": {
                "en-US": "Third Edition"
            }
        }
    },
    "authority": {
        "objectType": "Agent",
        "name": "New Client",
        "mbox": "mailto:hello@learninglocker.net"
    },
    "stored": "2016-10-01T19:58:29.534100+00:00"
}
Jakeii commented 7 years ago

Instead of using setIfUnset, I'd recommend using just a default object:


populateXAPI(statement_data) {
  const default_statement_data = {
    timestamp: new Date().toISOString(),
    version: '1.0.0',
    ...
  };

  statement_data = Object.assign({}, default_statement_data, statement_data);

}

Other than that looks good to me!

usernamenumber commented 7 years ago

Argh, I knew there must have been a nicer way to do that in JS. Ok, will update and then merge.

usernamenumber commented 7 years ago

...actually, I don't think that will work. The point of the default statement is to store a bunch of values that most statements will not need to change. The "context" subdict is a good example of one that mixes settings that statements probably won't need to update with ones they probably will.

Unfortunately, the Object.assign technique seems to only merge at the shallowest level, so if you override anything in a subdict you have to override everything in it, which defeats the point.

var default_statement_data = { "context" : { "var1":"val1", "var2":"val2", "var3":"val3" } }
var statement_data = { "context": { "var2":"overidden" } }
console.log(Object.assign({}, default_statement_data, statement_data))
// { context: { var2: "overridden" } }

We could recurse through and only update leaf nodes, but at that point I think the setIfUnset method looks cleaner.

Thoughts?