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

Agentetagnone #90

Merged
merged 4 commits into from
Mar 5, 2019
Merged

Conversation

garemoko
Copy link
Contributor

@garemoko garemoko commented Sep 7, 2017

@brianjmiller this is ready for you to review.

It solves a problem where if the etag options property is not set, both saveActivityProfile and saveAgentProfile will not set the etag header, which is a bad request and will result in a conformant LRS returning 400. This means that in scenarios where there is no etag (e.g. the resource is being PUT for the first time), there's no way to make a valid request.

I did consider that since this is a change in behavior, we should add an additional 'if-none' options property to allow people to explicitly set the if-none-match header, however since a conformant LRS will always reject requests without that header, I didn't see the point in supporting not setting that header.

This PR does nothing to address the related issue that there is no way to set the If-None-Match header for PUT requests to the State API. In that case, the etag header is optional so we would need to continue to support sending no-etag-header requests to avoid a breaking change.

This PR has been tested with the Moodle xAPI launch plugin and I have confirmed that this change fixes davidpesce/moodle-mod_tincanlaunch#140.

@WillSkates
Copy link

Hey @garemoko. You will need the fixes in #83 for this.

@davidpesce
Copy link

Any word on the status of this PR? This looks like it'll fix the issue I'm running into with the new version of LL.

@garemoko
Copy link
Contributor Author

Any news on getting this merged? I'm planning another release of the Moodle launch plugin in the next couple of months and would rather not fork/edit TinCanPHP in that release.

@brianjmiller brianjmiller merged commit 9758b3e into RusticiSoftware:master Mar 5, 2019
@garemoko garemoko deleted the agentetagnone branch March 6, 2019 11:14
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.

eTag headers not being set
4 participants