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

Scoring not run when instrument saved from the API #7460

Closed
driusan opened this issue Jun 3, 2021 · 8 comments
Closed

Scoring not run when instrument saved from the API #7460

driusan opened this issue Jun 3, 2021 · 8 comments
Labels
Bug PR or issue introducing/requiring bug fixes (not mutually exclusive with the Feature label)

Comments

@driusan
Copy link
Collaborator

driusan commented Jun 3, 2021

Describe the bug
When a PUT request is handled to save instrument data for the API, it calls the _save function. This bypasses the scoring, which is done through the front end with save (no underscore).

To Reproduce
Save an instrument which uses scoring with the API. Look in frontend to see if scoring was run.

What did you expect to happen?
Scoring should happen.

@driusan driusan added the Bug PR or issue introducing/requiring bug fixes (not mutually exclusive with the Feature label) label Jun 3, 2021
@xlecours
Copy link
Contributor

xlecours commented Jun 3, 2021

If there is scoring happening then I think it should be a post request. But, the documentation clearly state what is to be expected when a score field is provided in the request body:
A PUT request MAY not specify score columns that will be calculated/overwriten by server-side scoring. If the client attempted to score fields client-side and the value passed by the PUT request conflict with the server-side calculation of the score, the server-side calculation will win. Any keys specified in the document PUT that do not match a corresponding field in the form MAY be ignored.

@xlecours
Copy link
Contributor

xlecours commented Jun 3, 2021

Is the same fix expected for PATCH requests as well?

@GabrielPelletier
Copy link

GabrielPelletier commented Jun 4, 2021

I believe that POST is not supported for ... /candidates/$CandID/$VisitLabel/instruments/$InstrumentName.

To clarify, the issue occurs with a PUT or PATCH request without any Score fields provided in the request body. I expected the PUT or PATCH to run the scoring function, using the data that is loaded in the different fields to calculate scores and populate the score fields. The API documentation actually does not mention that a PUT/PATCH request runs the score function, so it should be added, I guess. (Yes, for both PUT and PATCH requests)

@driusan
Copy link
Collaborator Author

driusan commented Jun 7, 2021

Why should it be a POST request? If you PUT the same content twice you should get the same score, so it is still idempotent.

I would expect the same behaviour from PATCH

@ridz1208
Copy link
Collaborator

@driusan @xlecours

$instrumentname = $this->_instrument->testName;
$this->_instrument->clearInstrument();
$this->_instrument->_save($data[$instrumentname]);

This is currently what the instrument endpoint does. I see 2 options here

  1. call $this->_instrument->save() Which is called before _save() and has all the proper process (calculate candidate age, validate, .....)
  2. add a line $this->_instrument->score(); ad the end of this section to only run the scoring.

I think #1 is the way to go but the potential issue with it is that the save() function does not expect any parameters with the instrument data like _save($data) does. If you wanna take a look here are the functions save() and _save()

@driusan
Copy link
Collaborator Author

driusan commented Jun 16, 2021

Option 1 will not work.

save starts with:

       if ($this->form->validate()) {
            $this->form->process([&$this, '_saveValues'], true);
            $this->score();

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

(the else handles printing an error if validation fails). All it does is wrap Quick/LorisForm's validate function and call _saveValues and score. Calling it directly without data posted will just result in the validation failure logic always getting run.

@ridz1208
Copy link
Collaborator

So we should probable reproduce the functionality of save in the API class

@driusan
Copy link
Collaborator Author

driusan commented Jun 16, 2021

Yes.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Bug PR or issue introducing/requiring bug fixes (not mutually exclusive with the Feature label)
Projects
None yet
Development

No branches or pull requests

4 participants