Skip to content

Commit

Permalink
Fixed bugs:
Browse files Browse the repository at this point in the history
1) don't use schema defaults when filling in empty values - this was overwriting "false" with the default value, since false === empty
2) remove redundant logic that updates the tool consumer guid if one isn't set or if the info is different in the request. For Practera 1, protect will need to be off for all entries.
3) Switch the name of the consumer to the experience name, not the institution name. This is just for Practera internal reference and the user will never see it. Also ensure the stored value doesn't exceed DB field length in Practera. Note that this admin_add screen is very Practera 1 specific, which is not useful for anyone else who might try to use this plugin.
  • Loading branch information
jazzmind committed Jan 5, 2021
1 parent 197beb3 commit 1d71322
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 18 deletions.
19 changes: 9 additions & 10 deletions Controller/Component/LtiRequestComponent.php
Original file line number Diff line number Diff line change
Expand Up @@ -109,18 +109,15 @@ public function validate($cohort=null) {
$this->Consumer->$k = $v;
}

if ($this->Consumer->protect) {
if ($this->Consumer->protect === true) {
if (empty($data['tool_consumer_instance_guid'])) {
return $this->Provider->reason = 'A tool consumer GUID must be included in the launch request.';
}
if (empty($data['tool_consumer_instance_guid'])) {
return $this->Provider->reason = 'Request is from an invalid tool consumer: ' . $this->Consumer->consumer_guid;
}
if (empty($this->Consumer->consumer_guid) or ($this->Consumer->consumer_guid != $data['tool_consumer_instance_guid'])) {
// update the tool consumer GUID
$this->Consumer->consumer_guid = $result['Consumer']['consumer_guid'] = $data['tool_consumer_instance_guid'];
$this->Consumer->id = $result['Consumer']['id'];
$this->Consumer->save($result['Consumer']);
return $this->Provider->reason = 'Tool consumer ID in request is different from what is in Practera and protect mode is on, preventing us from updating the value automatically: ' . $this->Consumer->consumer_guid;
}
}

Expand Down Expand Up @@ -395,8 +392,9 @@ public function setUser() {
if (empty($this->LtiUser->data['LtiUser']['lis_person_sourcedid'])) {
$this->LtiUser->data['LtiUser']['lis_person_sourcedid'] = $data['lis_person_sourcedid'];
}

$this->LtiUser->id = $this->LtiUser->data['LtiUser']['id'];
if (!empty($this->LtiUser->data['LtiUser']['id'])) {
$this->LtiUser->id = $this->LtiUser->data['LtiUser']['id'];
}
$this->LtiUser->save($this->LtiUser->data['LtiUser']);
$this->LtiUser->data = $this->LtiUser->find('first', ['conditions' => $conditions]);

Expand All @@ -414,6 +412,7 @@ public function setConsumer() {
#
$doSaveConsumer = FALSE;
$data = $this->controller->request->data;

$now = time();
$today = date('Y-m-d', $now);
if (empty($this->Consumer->data['last_access'])) {
Expand Down Expand Up @@ -451,7 +450,7 @@ public function setConsumer() {
}

if (isset($data['tool_consumer_instance_guid'])) {
if (is_null($this->Consumer->data['consumer_guid'])) {
if (empty($this->Consumer->data['consumer_guid'])) {
$this->Consumer->data['consumer_guid'] = $data['tool_consumer_instance_guid'];
$doSaveConsumer = TRUE;
} else if (!$this->Consumer->data['protect']) {
Expand Down Expand Up @@ -479,10 +478,10 @@ public function setConsumer() {
#
### Persist changes to consumer
#
if ($doSaveConsumer) {
$this->Consumer->id = $this->Consumer->data['id'];
if ($doSaveConsumer && $this->Consumer->id) {
$this->Consumer->save($this->Consumer->data);
}
$this->Consumer->read();
}

/**
Expand Down
6 changes: 3 additions & 3 deletions Controller/ConsumersController.php
Original file line number Diff line number Diff line change
Expand Up @@ -325,16 +325,16 @@ public function admin_index() {

public function admin_add() {
$user = $this->Auth->user();
$name = $user['Institution']['name'];
$name = $user['Experience']['name'];
$secret = strtoupper(substr(sha1(strtotime("now") . uniqid()), 1, 4) . '-' . substr(sha1(strtotime("now") . uniqid()), 1, 4). '-' . substr(sha1(strtotime("now") . uniqid()), 1, 4));
$data = [
'name' => $name,
'name' => substr($name, 0, 44),
'secret' => $secret,
'lti_version' => 'LTI-1p0',
'consumer_name' => $name . " LMS",
'consumer_guid' => null,
'consumer_version' => 1,
'protect' => true,
'protect' => false,
'enabled' => true,
'institution_id' => $user['institution_id']
];
Expand Down
6 changes: 1 addition & 5 deletions Model/LtiAppModel.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,13 @@ public function afterFind($results, $primary = false) {
}

foreach ($set as $key => $value) {
// if there's an empty value, set the default value listed in the schema
// this is bad? it means the value in the DB and the value returned might not be the same
if (empty($value) and isset($this->_schema[$key]['default'])) {
$set[$key] = $value = $this->_schema[$key]['default'];
}
// set each result key as an object variable
$this->{$key} = $value;
}
return $results;
}


// public function beforeSave($options = []) {

// if (empty($this->_schema)) {
Expand Down

0 comments on commit 1d71322

Please sign in to comment.