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

Issue when rendering feedback #8

Closed
dailez opened this issue Jul 7, 2015 · 3 comments
Closed

Issue when rendering feedback #8

dailez opened this issue Jul 7, 2015 · 3 comments
Assignees

Comments

@dailez
Copy link

dailez commented Jul 7, 2015

There is a "bug" or better "not covered case" in the rendering routine in the qtism-sdk.

The problem occurs when the outcomeDeclaration of the feedback has cardinality=multiple (i.e. feedback_adaptive.xml).

The affected file is qtism/qtism/src/qtism/common/collections/Container.php -> public function equals($obj).

In the feedback example above the $obj type is "string" and the workaround is to add an elseif at the line 150:
elseif (gettype($obj) === 'string') {
foreach (array_keys($this->getDataPlaceHolder()) as $key) {
if ($obj === $this[$key]->getValue()) {
return true;
}
}
return false;
}

Would you please so kind to check it and let me know if I misunderstood something.

Kindest regards

@dailez dailez changed the title Rendering of feedback Issue when rendering feedback Jul 7, 2015
@bugalot
Copy link
Contributor

bugalot commented Jul 10, 2015

Thanks @dailez for your report. Actually, your identification of the issue seems good. However, the fix should not go in the equals() method of the Container class but rather in the rendering engine itself which should also try to check whether or not the Container->contains() the identifier.

Indeed, changing the implementation of Container->contains() could have impact on much code of the SDK.

I will make a patch to this. Thanks a lot for your feedback. It is much appreciated!

Question for you:
Which version of QTI-SDK do you use? Are you dependent on the master branch or on a particular release?

@bugalot
Copy link
Contributor

bugalot commented Jul 10, 2015

The following commit on the master branch should make it 13ed421

If you need the 0.9.x branch to be updated quick, just let me know.

@bugalot bugalot self-assigned this Jul 10, 2015
@dailez
Copy link
Author

dailez commented Jul 14, 2015

Thank you very much! It is now working like a charm.

@bugalot bugalot closed this as completed Jul 14, 2015
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