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

feat(JISC): Adds jisc extensions behind send_jisc_data setting. #547

Merged
merged 3 commits into from
May 17, 2019
Merged

Conversation

ryasmi
Copy link
Member

@ryasmi ryasmi commented May 15, 2019

Description
Adds jisc extensions behind send_jisc_data setting.

{
  "http://xapi.jisc.ac.uk/sessionId": "sesskey()",
  "http://id.tincanapi.com/extension/ip-address": "$event->ip",
  "http://xapi.jisc.ac.uk/courseArea": {
    "http://xapi.jisc.ac.uk/vle_mod_id": "$course->shortname"
  },
  "http://xapi.jisc.ac.uk/statementCat": "VLE"
}

Related Issues

PR Type

  • Enhancement

@ryasmi ryasmi added the feat label May 15, 2019
@ryasmi
Copy link
Member Author

ryasmi commented May 15, 2019

@garemoko / @davidpesce you guys happy with this?

@ryasmi ryasmi requested a review from davidpesce May 15, 2019 12:56
@ryasmi ryasmi self-assigned this May 15, 2019
@davidpesce
Copy link
Collaborator

I think this looks good. The only concern that I'm remembering is with the session key. It looks like you made it unique by including the base URL for JISC's platform as the first part, then appended the sessionId.

I'm not sure where we landed on whether the sessionIds could be re-used (or collisions occurring). But with it being in an extension I don't see any technical concerns.

@ryasmi
Copy link
Member Author

ryasmi commented May 15, 2019

I think the collisions are ok, I think it can be combined with other values to make it more unique.

@ryasmi
Copy link
Member Author

ryasmi commented May 15, 2019

Thanks for taking a look @davidpesce 👍

@ryasmi ryasmi merged commit da6298a into master May 17, 2019
@ryasmi ryasmi deleted the jisc branch May 17, 2019 11:01
@HT2Bot
Copy link
Member

HT2Bot commented May 17, 2019

🎉 This PR is included in version 4.3.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add JISC context extensions to statements.
3 participants