Hi,
First of all I want to say that this is a really nice library you've build here.

Now while I'm doing development and I'm listing all the errors, I've notice a few E_NOTICE that appear in the tincanapi_get_object function because the $internal_path its not being parse properly:

$path = explode('/', drupal_get_normal_path($internal_path));

e.g. drupal_get_normal_path($internal_path) can return /node/[node_id]?query=params
That way in $path[1] you will not get just the node_id but the node_id with the query_params and that is causing the above notices

A quick fix for this would be using php internal parse_url function:

$path = explode('/', drupal_get_normal_path(parse_url($internal_path, PHP_URL_PATH)));

I've also attach a patch file for this.

Thanks,
Cornel Les

CommentFileSizeAuthor
#4 tincanapi-get_object_fix-2907968-4.patch548 byteskenianbei
fixing_e_notice_in_tincanapi_get_object.patch700 bytescornel.les

Comments

cornel.les created an issue. See original summary.

darren oh’s picture

darren oh
Primary language English
Location Lakeland, Florida
commented
Status: Active » Reviewed & tested by the community

The patch fixes this bug correctly.

kenianbei’s picture

kenianbei commented

I think a better change would be to rather check that $path[1] is_numberic. That way subContent will preserve their id.

kenianbei’s picture

kenianbei commented
StatusFileSize
new tincanapi-get_object_fix-2907968-4.patch548 bytes
kenianbei’s picture

kenianbei commented
Status: Reviewed & tested by the community » Needs review
darren oh’s picture

darren oh
Primary language English
Location Lakeland, Florida
commented
Status: Needs review » Reviewed & tested by the community

The patch in #4 doesn’t help us get the correct value for $path[1] if the URL contains a query string. The original patch is still the correct solution.

kenianbei’s picture

kenianbei commented

Sorry, I should have outlined in more detail what my purpose was.

For the H5P module, there is also subcontent, which will look like this:
http://localhost/node/1?subContentId=xxxxx-xxxx-xxxx-xxxx-xxxxxxxx

The first patch will strip this id out of the url, and the LRS will have no record or way to group by subcontent. With the first patch the object is saved in the LRS without any subcontent info, like this:

{
  "object": {
    "id": "http://localhost/node/1",
    "definition": {
      "name": {
        "en-US": "Interactive content: Test Content"
      },
      "type": "http://orw.iminds.be/tincan/content/type/h5p_content"
    },
    "objectType": "Activity"
  }
}

If you use the second patch, the LRS record will look like this:

{
  "object": {
    "id": "http://localhost/node/1?subContentId=xxxxx-xxxx-xxxx-xxxx-xxxxxxxx",
    "objectType": "Activity",
    "definition": {
      "extensions": {
        "http://h5p&46;org/x-api/h5p-local-content-id": 20711,
        "http://h5p&46;org/x-api/h5p-subContentId": "xxxxx-xxxx-xxxx-xxxx-xxxxxxxx"
      },
      "name": {
        "en-US": "Does 1 + 1 = 2?\n"
      }
    }
  }
}

It seems like tincanapi is processing subcontent farther down in the process otherwise the finished LRS record would be just:

{
  "object": {
    "id": "http://localhost/node/1?subContentId=xxxxx-xxxx-xxxx-xxxx-xxxxxxxx",
  }
}

Does that make sense? Perhaps we could also get some feedback from the tincanapi module maintainers?

kenianbei’s picture

kenianbei commented
Status: Reviewed & tested by the community » Needs review