search

adaptlearning / adapt-contrib-xapi

TinCan/xAPI extension for the Adapt Framework
GNU General Public License v3.0
12 stars 28 forks source link

Add parent activities #53

Closed EBusch closed 4 years ago

EBusch commented 5 years ago

50

brian-learningpool commented 5 years ago

Hi @EBusch, thanks for this. It would be great if this was linked to a new configuration option on the xAPI extension, e.g. _shouldSendParent, similar to https://github.com/adaptlearning/adapt-contrib-xapi/blob/master/properties.schema#L147-L155, as this may not always be required.

garemoko commented 5 years ago

@EBusch @brian-learningpool We have a client that's using adapt and I'm noticing quiz question statements that have no parent context activities. This is causing a problem for reporting.

It looks like this PR could solve this issue, except that they would also need to add statement.addParentActivity(this.getParentActivity(model)); to the onQuestionInteraction function on line 603.

Three questions:

  1. Does it sound like that would work?
  2. Could it be added to this PR, or should I make a PR to EBusch:feature/parents or to adaptlearning:master?
  3. What is the status of this PR and timescale for merging?

@brian-learningpool I'm having trouble imagining a use case where you would not want context activities to be included in statements. Is there a specific use case in mind?

brian-learningpool commented 5 years ago

Hi @garemoko.

  1. That does sound like it would work.
  2. I haven't heard back from @EBusch regarding my previous comment. Feel free to pick it up and add to adaptlearning:master please. The point I was making was that it would be good if it was up to the instructional designer whether they toggled this functionality or not. There are a multitude of existing settings we've added already it configurable for as many use cases as possible.
  3. This PR has been sitting for some time. If it were ready to go in, i.e. sending parent context was configurable, then it could be merged very quickly, possibly as soon as next week as I'm keen to get #59 merged ASAP also.

It's not that there is a use case where context activities should not be sent. It's just they currently aren't being set and it has never been raised as an issue until @EBusch raised it.

On your specific use-case regarding question statements, would you expect the context to be the Assessment (article) or the component's parent, i.e. the block?

garemoko commented 5 years ago

Thanks for the quick reply @brian-learningpool. I'll see if I can make some time to do a PR. I'm not sure I will be able to test though since I have no experience with Adapt and don't have it set up. Is that likely to be a problem?

In order to answer your question, can you briefly outline the hierarchy of elements in adapt? Can you explain what you mean by the terms "assessment", "article", "component" and "block"?

In Watershed, we have a specific report type called an Activity Report that's designed to provide all the visualizations and data people typically want to see from an e-learning assessment. For courses produced in tools like Storyline, Lectora etc. Watershed report builders can select the course they want to report on, and then they see details of question responses for all questions in that course. This is possible because the course is listed as a context activity.

So as a minimum, I would want the e-learning course package to be either a parent or grouping contextActivity, but there could also be value in having other levels in the hierarchy also listed.

brian-learningpool commented 5 years ago

Hi @garemoko, don't worry, I can give it a test here or I can make the change myself, if you'll excuse the wall of text to follow, once we establish what that parent in this case should be.

A quick outline of the Adapt course structure would be:

- course 
-- contentobject (page/menu)
--- article                                <- assessment
---- block
----- component                            <- question(s)

The assessment is basically an article with specific attributes, things like a pass mark, number of attempts, etc. As you can see, calling getParent() on a question (or any) component will return the container block. For most instances the block is used as a container. (There's more info at https://github.com/adaptlearning/adapt_framework/wiki/Framework-in-five-minutes#learn-your-abcs).

This is why I asked would you expect the parent context in this case to be the assessment article, the block, or the course the question belongs to? I think there's a bit of a difference between what is the "meaningful" parent -- possibly the article or the course itself -- versus what getParent() will return.

To answer your questions:

garemoko commented 5 years ago

@brian-learningpool Thanks so much for this. Really useful information.

It's actually looking like I may not get to this in the near future, but this information is really valuable for when I or somebody else gets to it.

It sounds like that the parent context activity should be the immediate parent (the block), and then keep recursively getting the parent and adding grouping context activities until getParentActivity returns undefined.

brian-learningpool commented 5 years ago

Not a problem, @garemoko. This may be something we can escalate on our side, especially since the original PR has been hanging around since February.

I've just checked the spec here (https://github.com/adlnet/xAPI-Spec/blob/master/xAPI-Data.md#2462-contextactivities-property) and the wording around "sensible parent" leads me to think that the article might be a better match, conceptually at least. The actual example further down that link is actually questions in a test.

I know we do have quite a lot of users who disable sending statements for blocks completely as it tends to muddy the waters with fairly meaningless statements in the LRS. Since v0.6 we've turned off tracking blocks by default (see https://github.com/adaptlearning/adapt-contrib-xapi/commit/793ea9c36b8b5a148ccd2cbf432e4ae0d6e7dbd6).

kevindoherty30 commented 5 years ago

Overall the ability to contain a parent would be extremely useful when it comes to working with the statements.

I would agree that blocks seem like an arbitrary thing to track as the parent, I think the course and/or article would give much more value especially when looking at multiple courses with assessments in an LRS.

garemoko commented 5 years ago

Makes sense. I don't really know what a block is, so you're judgement will be more informed than mine!

It would only be useful to have the block as the parent if blocks routinely contain multiple questions. If normally there's a 1:1 ratio of questions to blocks, then it makes sense to pretend the block isn't there and skip up to the next level.

brian-learningpool commented 5 years ago

Yeah, that's exactly my thinking. 👍

I'll hopefully get a chance to work on this towards the end of next week. Any help in testing (particularly on Watershed) would be greatly appreciated.

garemoko commented 5 years ago

@brian-learningpool absolutely. Let me know via email (andrew.downes@watershedlrs.com) once you're nearly ready and I can send you credentials for a watershed endpoint to fire at.

moloko commented 5 years ago

Block doesn't seem that useful to me either, it's really more a 'container element' for the responsive design aspect of Adapt. I would say that if the question is part of an assessment then it might be useful to show the assessment as being the parent; if not part of an assessment let's just have the page as the parent.

moloko commented 5 years ago

Sorry, having just read this comment by @garemoko :

For courses produced in tools like Storyline, Lectora etc. Watershed report builders can select the course they want to report on, and then they see details of question responses for all questions in that course. This is possible because the course is listed as a context activity.

Maybe we should be setting the course as the parent activity? Unless there's any way of having some sort of hierarchy/filtering system so that one could see:

(And always setting the course as the parent would presumably be nice and simple to do)

garemoko commented 5 years ago

Yes, in watershed at least, you can filter by either a parent or grouping context activity

brian-learningpool commented 5 years ago

@moloko, always setting the course as the parent would be very easy to do but I'm not sure how useful that would be if we apply it across the board. The grouping property of a statement already has an array containing the course as is, e.g.: 

"grouping": [
  {
    "id": "https://my-website.com/my-xapi-course",
    "objectType": "Activity"
  }
]

This isn't explicitly set in our code so it must be added by the xAPIWrapper library. . @garemoko, have you tried filtering by that?

All questions on page X could be handled by adding the current page (or 'lesson') to the grouping an additional activity. (The lesson activity type was added in https://github.com/adaptlearning/adapt-contrib-xapi/commit/0e690ece99bf7d4cbe278a48d0dbabd3131ca8ea).

Then I think it only becomes applicable to add a parent where the question is part of an assessment. This is actually the example given in the spec: https://github.com/adlnet/xAPI-Spec/blob/master/xAPI-Data.md#details-15

In almost all cases there is only one sensible parent or none, not multiple. For example: a Statement about a quiz question would have the quiz as its parent Activity.

Does this make sense?

garemoko commented 5 years ago

The statements we're getting do not include any context activities at all, but if there was a grouping, we could certainly filter by it.

The approach you're suggesting should be ok, though for clarity that example in the spec isn't intended to imply that the only parent of a question should be a quiz. I think when I wrote it I just had in mind a simple quiz containing questions.

In an ideal world question statements would always have a parent representing whatever thing contains them, whether that's an assessment or a topic or whatever. But from a practical standpoint., Watershed's reports will work with groupings just as easily as with parents.

brian-learningpool commented 5 years ago

Just to update, I started working on this but identified an issue with xAPIWrapper so will continue when https://github.com/adlnet/xAPIWrapper/pull/155 is merged.

brian-learningpool commented 5 years ago

The PR on the xAPIWrapper has just been merged so I hope to get to this shortly.

danielghost commented 4 years ago

@brian-learningpool, you may want to look at the way the context is used in https://github.com/cgkineo/adapt-xAPI (code does need updating as still aligned with v2 framework). We spent some time identifying what we thought may be useful to add to the `context and its contextActivities, to help in making the queries on the LRS more powerful, so you can either have a very focused or broad query in terms of capturing all statements for a particular context.

Maybe took @garemoko guidance on the parent attribute too literally though, and only used this on questions, with everything else only using the grouping array. As with all these things, there is a certain amount of interpretation with the spec.

Only just starting to get back to looking at this plugin again, so we should probably look to have a conversation about contributing back to this version of the plugin rather than our own - we never did this 2 years ago when we were discussing it @brian-learningpool!

brian-learningpool commented 4 years ago

Hi @danielghost, thanks for the info.

It would be fantastic if you could contribute back to this version. 👍

brian-learningpool commented 4 years ago

The PR for the changes discussed in this thread is at https://github.com/adaptlearning/adapt-contrib-xapi/pull/74.

brian-learningpool commented 4 years ago

Closing this PR as context grouping and parent were added as part of https://github.com/adaptlearning/adapt-contrib-xapi/pull/74.