Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adapt-contrib-spoor: Add ability to dynamically set the identifier in manifest.xml #2247

Closed
brian-learningpool opened this issue Oct 31, 2018 · 3 comments
Assignees

Comments

@brian-learningpool
Copy link
Member

Subject of the enhancement

In imsmanifest.xml the identifier is currently set to 'adapt_manifest'. Some LMSs require that this is a unique value. I'm proposing that we add a new property to Spoor to allow the user to specify this, with a default value of adapt_manifest in line with current behaviour.

Essentially this leaves it up to the user to configure (if required) but it means that editing the XML file by hand is no longer required.

Your environment

Adapt Framework v3.2.1

Steps to reproduce

Create any Adapt course in the framework or authoring tool.

Expected behaviour

The user should have the option to specify the manifest identifier.

Actual behaviour

The identifier is hard-coded to 'adapt_manifest'.

@moloko
Copy link
Contributor

moloko commented Oct 31, 2018

Interestingly, having looking at the SCORM docs I don't think there is a requirement for this to be completely unique - I think it's probably just a classic example of the LMS developers having interpreted the SCORM 1.2 CAM document oddly.

This is what the SCORM 1.2 CAM has to say on the matter:

An identifier, provided by an author or authoring tool, that is unique within the Manifest

That said, it's not exactly a big problem doing this so let's just go for it anyway

@moloko
Copy link
Contributor

moloko commented Oct 31, 2018

Contrary to what I said in the chat I don't think - reading the docs - that it's necessary for the default attribute of the <organizations> node and the identifier attribute of the <organization> node to be completely unique... suggest we just leave them set to "adapt_scorm" for now?

@brian-learningpool
Copy link
Member Author

PR here: adaptlearning/adapt-contrib-spoor#182

brian-learningpool added a commit that referenced this issue Jan 22, 2019
This ensures 'adapt_scorm' will be used as a default if the corresponding property is missing.
brian-learningpool added a commit that referenced this issue Jan 22, 2019
This ensures 'adapt_scorm' will be used as a default if the corresponding property is missing.
brian-learningpool added a commit that referenced this issue Jan 22, 2019
This ensures 'adapt_scorm' will be used as a default if the corresponding property is missing.
@moloko moloko closed this as completed in db636da Jan 22, 2019
brian-learningpool added a commit that referenced this issue Jan 23, 2019
This preserves backwards compatibility.
brian-learningpool added a commit that referenced this issue Jan 23, 2019
This is a backport of the v4.0.0 change.
brian-learningpool added a commit that referenced this issue Jan 29, 2019
This commit supports a corresponding change in Spoor by providing backwards
compatibility for JSONs which may be missing the 'manifiest_identifier' attribute.
brian-learningpool added a commit that referenced this issue Jan 30, 2019
)

This commit supports a corresponding change in Spoor by providing backwards
compatibility for JSONs which may be missing the 'manifiest_identifier' attribute.
brian-learningpool added a commit that referenced this issue Jan 30, 2019
This was checking a non-existent object.
brian-learningpool added a commit that referenced this issue Jan 30, 2019
* Fixes #2247 - Corrects error in Spoor 'adapt_manifest' replace

This was checking a non-existent object.
Now using @oliverfoster's proposed code.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants