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

Feature TR-6169 set Interaction metadata to ResponseValidityConstraint #397

Merged
merged 8 commits into from
Jun 20, 2024

Conversation

kilatib
Copy link
Contributor

@kilatib kilatib commented Jun 6, 2024

set Interaction metadata to ResponseValidityConstraint TR-6169

https://oat-sa.atlassian.net/browse/TR-6169

Description

Adds extraData to ResponseValidityConstraint that is optionally provided by Interaction upon ResponseValidityConstraint instantiation

Related PRs

@kilatib kilatib requested review from wazelin and poyuki June 6, 2024 11:16
@kilatib kilatib force-pushed the fix/6169/disable-vilidation-for-xtml-format branch from 860023c to d89c1ab Compare June 6, 2024 11:58
@kilatib kilatib requested a review from wazelin June 6, 2024 11:59
@kilatib kilatib requested a review from poyuki June 14, 2024 10:20
Comment on lines +511 to +516
[
'qtiClassName' => $this->getQtiClassName(),
'options' => [
'format' => $this->getFormat(),
],
]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if instead of loosely-typed extraData we provisioned a reference to the whole instance of the current Interaction here?

Suggested change
[
'qtiClassName' => $this->getQtiClassName(),
'options' => [
'format' => $this->getFormat(),
],
]
$this

Instead of extraData property on the constraint level, you can then have private ?Interaction = null.

Do you foresee any problems with that? If so, or if you don't think it makes any sense, ignore this comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it was my first idea, but we have several problems with it on serialized and unserialized actions for the final session. It is not super practice, but it is better to keep here simple data.

@kilatib kilatib requested a review from wazelin June 18, 2024 21:58
Copy link
Member

@wazelin wazelin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a tiny nitpick, and we're good-to-go!

@@ -473,7 +475,7 @@ public function hasExpectedLines(): bool
/**
* Set the format of the text entered by the candidate.
*
* @param int $format A value from the TextFormat enumeration.
* @param int $format A value from the TextFormat enumeration.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need to align the docblock elements like that.
I'd rather have all of them consistent and with a single space as a delimiter.

Copy link
Contributor

@poyuki poyuki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM nicely done. Approving it as doc block changes are requested to be rolled back.

@kilatib kilatib requested a review from wazelin June 19, 2024 17:11
*/
private $associationValidityConstraints;

/**
* Provide additional information about the ResponseValidityConstraint.
* Metadata defined by @see \qtism\data\content\interactions\Interaction instantiating this ResponseValidityConstraint
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🫡

@wazelin wazelin merged commit a40ea3f into develop Jun 20, 2024
5 of 6 checks passed
@wazelin wazelin deleted the fix/6169/disable-vilidation-for-xtml-format branch June 20, 2024 11:15
@wazelin wazelin changed the title fix: not validate maxWords for XHTML extendTextInteractions Feature TR-6169 set Interaction metada to ResponseValidityConstraint Jun 20, 2024
@wazelin wazelin changed the title Feature TR-6169 set Interaction metada to ResponseValidityConstraint Feature TR-6169 set Interaction metadata to ResponseValidityConstraint Jun 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants