url extended param not pass to LRS

[wzhonggo]

Hi I use launch server and want to pass some param(ex: a=b&c=d) to launch url. But the xAPIWrapper.js is not pass extended param to LRS. I find #14 , it is delete pass param code. I do not know why? Anyone can help me

[vbhayden]

Good morning,

Is this issue that the wrapper is incorrectly changing the URL you're passing, or that the parameters you give are being ignored?

Do you have steps to recreate the issue?

[wzhonggo]

Hi

1. clone xapi-launch and deploy in Ubuntu.
2. upload demo.zip file as a app in xapi-launch server .
3. launch the link and click send statment button . I see send statment is ok, but not see Query String Parameters in Chrome console.

If add below code in xAPIWrapper.js , will see xAPILaunchKey:add6de73-0bbf-4375-8c6a-ec153ad285f3 xAPILaunchService:http://127.0.0.1:8099/ in Chrome console Query String Parameters.

// add extended LMS-specified values to the URL     
if (lrs !== null && lrs.extended !== undefined) {
    extended = new Array();
    for (prop in lrs.extended) {
        extended.push(prop + "=" + encodeURIComponent(lrs.extended[prop]));
    }
    if (extended.length > 0) {
        url += (url.indexOf("?") > -1 ? "&" : "?") + extended.join("&");
    }
}

send statemnt with param send statemnt with no param

[FlorianTolk]

What version of xAPI wrapper did you use?

Looking at the xapiwrapper.min.js, it doesn't look like a minified version of what we have on the git. if you want to use query string parameters, you'll have to edit the version that is currently in the zip file.

I do get this problem as well, but I am also replacing your minified file with the xapi wrapper in this repository, and as soon as I do that, the statements no longer send.

My theory is that the wrapper included in the demo.zip was built specifically for it, and so when you replace it with xapiwrapper.js, the functions no longer match, and that is what is causing your problems.

[wzhonggo]

Hi FlorianTolk

The xAPI wrapper version is v 1.15.0, I clone this repository and use nodejs build the minified version.

The reason you can not send statemnt after replace xapiwrapper.js is the minified version need mutil js file . But xapiwrapper.js is the just one of list. You can look at Gruntfile.js. If you repalce with xapiwrapper.min.js , it will be ok.

code from Gruntfile.js

 'build': {
        files: {
          'dist/xapiwrapper.min.js': [
            'lib/cryptojs_v3.1.2.js',
            'src/activitytypes.js',
            'src/verbs.js',
            'src/xapiwrapper.js',
            'src/xapistatement.js',
            'src/xapi-util.js',
            'src/xapi-launch.js'
          ]
        }
      }

[vbhayden]

Morning wz,

By "it will be ok", are you saying that using the updated version of the wrapper corrects the issue?

If this isn't an issue with this specific version of the JS xAPI wrapper, then it sounds more like an issue with the xapi-launch project.

[wzhonggo]

Hi vbhayden

"it will be ok" is for @FlorianTolk , FlorianTolk test with demo.zip and the statements no longer send after replacing minified file in demo.zip with xapiwrapper.js.

My previous description is not clear， I have update the comment.

I add below code in xapiwrapper.js and rebuild xapiwrapper.min.js , it is pass extended param to LRS.

// add extended LMS-specified values to the URL     
if (lrs !== null && lrs.extended !== undefined) {
    extended = new Array();
    for (prop in lrs.extended) {
        extended.push(prop + "=" + encodeURIComponent(lrs.extended[prop]));
    }
    if (extended.length > 0) {
        url += (url.indexOf("?") > -1 ? "&" : "?") + extended.join("&");
    }
}

[vbhayden]

Where are you adding that in xapiwrapper.js, to the ADL.XHR_request definition?

[wzhonggo]

Hi vbhayden

The code add in to the ADL.XHR_request function. See add extended LMS-specified values to the URL

ADL.XHR_request = function(lrs, url, method, data, auth, callback, callbackargs, ignore404, extraHeaders, withCredentials, strictCallbacks)
    {
        "use strict";

        var xhr,
            finished = false,
            xDomainRequest = false,
            ieXDomain = false,
            ieModeRequest,
            urlparts = url.toLowerCase().match(/^(.+):\/\/([^:\/]*):?(\d+)?(\/.*)?$/),
            location = window.location,
            urlPort,
            result,
            extended,
            prop,
            until;

        //Consolidate headers
        var headers = {};
        headers["Content-Type"] = "application/json";
        headers["Authorization"] = auth;
        headers['X-Experience-API-Version'] = ADL.XAPIWrapper.xapiVersion;
        if(extraHeaders !== null){
            for(var headerName in extraHeaders){
                headers[headerName] = extraHeaders[headerName];
            }
        }

        //See if this really is a cross domain
        xDomainRequest = (location.protocol.toLowerCase() !== urlparts[1] || location.hostname.toLowerCase() !== urlparts[2]);
        if (!xDomainRequest) {
            urlPort = (urlparts[3] === null ? ( urlparts[1] === 'http' ? '80' : '443') : urlparts[3]);
            xDomainRequest = (urlPort === location.port);
        }

        // add extended LMS-specified values to the URL
        if (lrs !== null && lrs.extended !== undefined) {
            extended = new Array();
            for (prop in lrs.extended) {
                extended.push(prop + "=" + encodeURIComponent(lrs.extended[prop]));
            }
            if (extended.length > 0) {
                url += (url.indexOf("?") > -1 ? "&" : "?") + extended.join("&");
            }
        }

        //If it's not cross domain or we're not using IE, use the usual XmlHttpRequest
        var windowsVersionCheck = window.XDomainRequest && (window.XMLHttpRequest && new XMLHttpRequest().responseType === undefined);
        if (!xDomainRequest || windowsVersionCheck === undefined || windowsVersionCheck===false) {
            xhr = new XMLHttpRequest();
            xhr.withCredentials = withCredentials; //allow cross domain cookie based auth
            xhr.open(method, url, callback != null);
            for(var headerName in headers){
                xhr.setRequestHeader(headerName, headers[headerName]);
            }
        }
        //Otherwise, use IE's XDomainRequest object
        else {
            ieXDomain = true;
            ieModeRequest = ie_request(method, url, headers, data);
            xhr = new XDomainRequest();
            xhr.open(ieModeRequest.method, ieModeRequest.url);
        }

        //Setup request callback
        function requestComplete() {
            if(!finished){
                // may be in sync or async mode, using XMLHttpRequest or IE XDomainRequest, onreadystatechange or
                // onload or both might fire depending upon browser, just covering all bases with event hooks and
                // using 'finished' flag to avoid triggering events multiple times
                finished = true;
                var notFoundOk = (ignore404 && xhr.status === 404);
                if (xhr.status === undefined || (xhr.status >= 200 && xhr.status < 400) || notFoundOk) {
                    if (callback) {
                        if(callbackargs){
                            strictCallbacks ? callback(null, xhr, callbackargs) : callback(xhr, callbackargs);
                        }
                        else {
                          var body;

                            try {
                                body = JSON.parse(xhr.responseText);
                            }
                            catch(e){
                                body = xhr.responseText;
                            }

                          strictCallbacks ? callback(null, xhr, body) : callback(xhr,body);
                        }
                    } else {
                        result = xhr;
                        return xhr;
                    }
                } else {
                    var warning;
                    try {
                        warning = "There was a problem communicating with the Learning Record Store. ( "
                            + xhr.status + " | " + xhr.response+ " )" + url
                    } catch (ex) {
                        warning = ex.toString();
                    }
                    ADL.XAPIWrapper.log(warning);
                    ADL.xhrRequestOnError(xhr, method, url, callback, callbackargs, strictCallbacks);
                    result = xhr;
                    return xhr;
                }
            } else {
                return result;
            }
        };

        xhr.onreadystatechange = function () {
            if (xhr.readyState === 4) {
               return requestComplete();
            }
        };

        xhr.onload = requestComplete;
        xhr.onerror = requestComplete;
        //xhr.onerror =  ADL.xhrRequestOnError(xhr, method, url);

        xhr.send(ieXDomain ? ieModeRequest.data : data);

        if (!callback) {
            // synchronous
            if (ieXDomain) {
                // synchronous call in IE, with no asynchronous mode available.
                until = 1000 + new Date();
                while (new Date() < until && xhr.readyState !== 4 && !finished) {
                    delay();
                }
            }
            return requestComplete();
        }
    };

[brian-learningpool]

@wzhonggo, did you close this issue without creating a pull request?

It's still an issue that the extended properties aren't being passed to the endpoint.

@vbhayden can you re-open this issue and I'll patch it?

[wzhonggo]

@brian-learningpool I don't know why the code is deleted. so i not create pull request for this issue.

[f1hornet]

Hey Guys, is there a reason this function has the extended section commented out. I think its why my courses aren't working on Talent LMS and Absorb LMS

As far as I can tell the minimised version of the code has the same issue. This means that any extended variables in the launch string are being discarded and this is causing issues.

Is there a different procedure for adding the extended variables that I am missing or is this an oversight in the code?

John

[adl-trey]

Afternoon John,

That block was PR'd without due oversight and actually broke conformance as its implementation tries to attach URL query args to the statement itself. Once that was realized, I commented it out in case this came up again rather than remove it outright.

Functionally, the decision to append URL query arguments to the statement to suit specific LMS solutions isn't something that should be happening by default in this wrapper, especially if that behavior breaks conformance. Although since this seems to be an expectation of some developers writing for those LMS solutions, it might be OK to allow that as an optional argument within the wrapper.

For background, the original PR was mainly an issue with xAPI Launch's functionality, so it probably didn't belong here although we've since archived the xapi-launch repo due to inactivity.

[f1hornet]

Hi Trey Thanks for the explanation. I have reinstated that code in our framework and our course are now performing as expected on our talentLMS test platform. I am going to deliver a test package to our client on playerLync LMS tomorrow and I expect it will fix the issues they are seeing.

Thanks

John

[creighton]

I'm a bit confused with this thread. It seems to be discussing, and mixing, 2 different things.

1. Adding extended params to the xAPI request URL The first part of this thread, and the pull request, looks like it was about adding the lrs.extended params to the URL being sent to the LRS endpoint, such as https://lrs.adlnet.gov/xapi/statements?myext=foo. I thought this wasn't allowed but honestly I don't see anything in the spec right now to confirm this. If nothing in the spec prevents this, it might be fine to leave it in (which it is right now: https://github.com/adlnet/xAPIWrapper/blob/master/src/xapiwrapper.js#L1519)

2. Getting params from the URL The second part of this thread is asking why additional parameters on the current content's URL are not captured by the xAPIWrapper. That piece of code @f1hornet references is just pulling params from the content URL and adding them to the xAPIWrapper's lrs configuration object. If they weren't in that lrsProps list, they were added to the lrs.extended array, in case the content was coded to rely on it for the additional URL params. They shouldn't be added to the statement sent to the LRS, and I didn't find the code that would have done that. At most, having the code in both parts of this discussion looks like it would add the params gotten in getLRSObject to the url in XHR_request, no?

As for the xapi-launch stuff, that's all handled in the ADL Launch lib https://github.com/adlnet/xAPIWrapper/blob/master/src/xapi-launch.js. It shouldn't be impacted by either one of these issues discussed in the thread. If someone wants to use it they should look at the example in the readme https://github.com/adlnet/xAPIWrapper#xapi-launch-support .

If I'm missing something, could you please link to the source? Like I said I thought that additional params on xAPI requests might not be allowed but I can't find in the spec right now. And I didn't see where the lrs.extended was supposed to be adding those values to a statement, but again I didn't do an exhaustive search.

Thanks!

[adl-trey]

Two different things that ended up causing problems as-implemented, yeah :-\

It's been awhile and the last few months have killed my memory, but for [1] I think that actual incorporation block was kept in to allow someone to add them in manually but I don't even remember if that lrs object is accessible from outside or not.

For [2], the commented block was adding those to lrs.extended which in turn added them to the endpoint via [1]. I'm also not sure where the spec prohibits this outside of this confusingly worded blurb from the alternate syntax block:

Query string parameters:

• Any query string parameters other than "method" MUST instead be included as a form parameter with >the same name.
• The LRS MUST treat any form parameters other than "content" or the header parameters listed above as query string parameters.

I'm assuming more than just the ADL LRS interpreted that to mean the rejection of any weird query arguments, but the ADL LRS does for sure.