Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

[Instrument API] expand saving functionality to include Candidate age calculations and scoring #7485

Merged
merged 3 commits into from
Jun 16, 2021
Merged

[Instrument API] expand saving functionality to include Candidate age calculations and scoring #7485

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bcf5d75
[Instrument API] expand saving functionality to include Candidate age…
ridz1208 Jun 16, 2021
aab3aa0
its own function
ridz1208 Jun 16, 2021
64080d4
Update NDB_BVL_Instrument.class.inc
ridz1208 Jun 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions modules/api/php/endpoints/candidate/visit/instrument/instrument.class.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,8 @@ class Instrument extends Endpoint implements \LORIS\Middleware\ETagCalculator
try {
$instrumentname = $this->_instrument->testName;
$this->_instrument->clearInstrument();
$this->_instrument->_save($data[$instrumentname]);
$this->_instrument->_saveValues($data[$instrumentname]);
$this->_instrument->score();
Copy link
Collaborator

Choose a reason for hiding this comment

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

The only thing missing from the validate code path of the save() function seems to be:

// determine the data entry completion status, and store that in
// the database
$dataEntryCompletionStatus = $this->_determineDataEntryCompletionStatus();
$this->_setDataEntryCompletionStatus(
    $dataEntryCompletionStatus
);

why not just add that?

You could also try moving the code into a final public class method and calling that from both save and the api, to ensure that they're always doing the same thing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

can I just move the data entry completion status logic to save values instead ? or you prefer not ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I looked into the IBIS and CCNA repositories and on IBIS there's 19 instrument classes that override _saveValues, mostly to implement instruments that have an uploader. (I think it might be the only way to do it?)

CCNA, on the other hand, has a bunch of scripts that call _saveValues but no instruments that override it.

It's probably safer to keep it where it is to ensure that it doesn't get missed.

} catch (\Throwable $e) {
error_log($e->getMessage());
return new \LORIS\Http\Response\JSON\InternalServerError();
Expand Down Expand Up @@ -208,7 +209,8 @@ class Instrument extends Endpoint implements \LORIS\Middleware\ETagCalculator

try {
$instrumentname = $this->_instrument->testName;
$this->_instrument->_save($data[$instrumentname]);
$this->_instrument->_saveValues($data[$instrumentname]);
$this->_instrument->score();
} catch (\Throwable $e) {
error_log($e->getMessage());
return new \LORIS\Http\Response\JSON\InternalServerError();
Expand Down