Skip to content

Conversation

xabbuh
Copy link
Contributor

@xabbuh xabbuh commented Mar 17, 2017

This fixes #48.

@xabbuh
Copy link
Contributor Author

xabbuh commented Mar 17, 2017

ping @Lctrs What do you think?

*/
public function __construct(StatementId $id = null, Actor $actor, Verb $verb, Object $object, Result $result = null, Actor $authority = null, \DateTime $created = null, \DateTime $stored = null, Context $context = null, array $attachments = null)
public function __construct(StatementId $id = null, Actor $actor, Verb $verb, Object $object, Result $result = null, Actor $authority = null, \DateTime $created = null, \DateTime $stored = null, Context $context = null, array $attachments = null, $version = '1.0.3')
Copy link

Choose a reason for hiding this comment

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

Default should be 1.0.0 instead of 1.0.3.

From the spec :

Statements returned by an LRS MUST retain the version they are accepted with. If they lack a version, the version MUST be set to 1.0.0.

And

Note: The requirement for the "version" property to contain the value 1.0.0, rather than the latest patch version is deliberate since Statements in any version of 1.0.x conform to the 1.0.0 data model. In fact, a single Statement may be included in multiple requests over time, each following a different patch version of the specification. The patch version of the specification being followed can be determined from the "X-Experience-API-Version" header being used in each request.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for reading the spec so carefully. Fixed it and added a spec test to make sure we don't break this in the future.

@Lctrs
Copy link

Lctrs commented Mar 17, 2017

Otherwise, LGTM 👍

@Lctrs
Copy link

Lctrs commented Mar 20, 2017

If possible, this should be backported to 1.x since we support both versions in other libraries.

@xabbuh xabbuh merged commit 33f7e08 into php-xapi:master Mar 20, 2017
@xabbuh xabbuh deleted the issue-48 branch March 20, 2017 10:02
@xabbuh
Copy link
Contributor Author

xabbuh commented Mar 20, 2017

good point, see #53

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.

Missing "version" property in Statement model
2 participants