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 extensions to components #55

Closed EBusch closed 2 years ago

brian-learningpool commented 5 years ago

@EBusch, can I ask what is your use case here? I think this one potentially requires a bigger conversation.

  1. Do we expose URLs which resolve for Adapt vocabulary, such as is done at http://xapi.vocab.pub/?
  2. Should we allow the user to define the content of an extension attribute?
  3. Do we just go with arbitrary URLs?

@danielstorey I think there might be a knock-on effect on any code which uses xapi:preSendStatement to add extensions.

@moloko or @oliverfoster I'm keen to get your thoughts here.

danielstorey commented 5 years ago

I think I can see the benefit of reporting on the component type.

Another option re urls is to submit them to https://registry.tincanapi.com/ ...

Re knock on effects - any plugins which explicitly define the extensions property on a component statement would end up the value defined here. I can only think of one instance for which I've used extensions. It's not a component statement so wouldn't conflict with the changes here. Nevertheless, it would be good practice going forward to check for and extend the existing extensions property, before defining it.

The code in this PR would not need to change to allow this as the statement would be intercepted after the extensions property is defined here. As far as I'm concerned the only blocker in this PR is deciding on and configuring the URL for the component-type extension key

oliverfoster commented 5 years ago

having the hard coded url which must include the component name seems a bit odd. it might be easier if we could:

  1. configure and lookup the url from the component type (globals?)
  2. override / specify it on the component (local attribute?)

we could then list all of the core component definitions in the xapi plugin and allow users to specify in their own plugins.

just need to pick a stable registry we're happy using?

danielstorey commented 5 years ago

@oliverfoster I agree with not hard coding i.e. having a config object for urls but I don't think the url needs to be dynamic. In the same way as some of these extensions are defined this one might be something like

Title: Component Type URI: http://id.tincanapi.com/extension/component-type Description: A value representing the type of component the learner has interacted with e.g. text, graphic, media etc.

If we want to create more Adapt-specific definitions it might be worth hosting on adaptlearning.org

oliverfoster commented 5 years ago

Cool. That makes sense.

We have an adaptlearning.github.io repo if you want to host them there?

The url definitely needs to be a config option even if it's not dynamic

brian-learningpool commented 5 years ago

The rest of URIs used in the code are taken from the ADL constants. Wouldn't the same suffice here if we're hosting it in public on something like adaptlearning.github.io? It's going to add to configuration if we go down the route of allowing the user to set a URI for referencing an article, block, component, etc.

As a core maintained extension we have control over this.

danielstorey commented 5 years ago

@brian-learningpool yeah I agree. Sorry I don't think I was clear.

I meant that URL should be stored in an object and referenced rather than random strings included as and when they're needed. I don't think this needs to be configurable by the user

oliverfoster commented 2 years ago

very stale, please redo if still needed, apologies