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

[NDB_BVL_Instrument] Modify InstanceData Setting #9476

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

victori444
Copy link
Contributor

Brief summary of changes

In the Instrument class' _save() function, updates the instance data if the instrument has an SQL table so that the instance data only needs to be saved once to calculate the correct score

This also updates the score_instrument.php script to resolve issue relating to the Query class update

Testing instructions (if applicable)

  1. Run the scoring script for a candidate with the bmi instrument and verify that the scores are correct
  2. Modify the instrument data in the instrument table only, not in data_instrument
  3. Re-run the scoring script and verify that the scores are updated according to the changes made

Link(s) to related issue(s)

CCNA Override
CCNA Override 2

@victori444 victori444 added the Priority: Projects PR or issue is a priority for at least one project and should be a higher priority for LORIS label Nov 18, 2024
@@ -879,6 +879,9 @@ abstract class NDB_BVL_Instrument extends NDB_Page
['CommentID' => $this->getCommentID()],
);

if (!$this->jsonData) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't understand why you're doing this here. It seems to be duplicating line 855 but more inefficiently. I think you just need to update line 846 to fix the fact that it's not dealing with table data correctly?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: Projects PR or issue is a priority for at least one project and should be a higher priority for LORIS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants