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.

Sign up for GitHub

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

Jump to bottom

getInstanceData inconsistent between JSON and table-based instruments #8804

Closed
driusan opened this issue Jun 21, 2023 · 1 comment
Closed

getInstanceData inconsistent between JSON and table-based instruments #8804

driusan opened this issue Jun 21, 2023 · 1 comment
Assignees
@driusan
Labels
Category: Bug PR or issue that aims to report or fix a bug

Comments

@driusan
Copy link
Collaborator

driusan commented Jun 21, 2023 •
edited
Loading

The getInstanceData method (and hence, the API instrument's endpoint for instrument data) has inconsistent behaviour based on if the instrument saves the data to JSON in the flag table or whether it saves it to a table for the instrument.

Inconsistency 1

Table-based data does SELECT * from $tablename WHERE CommentID=.., resulting in an array of keys set to the value NULL for every column in the table.

JSON-based data does SELECT Data FROM flag WHERE CommentID=.... since Data is nullable, it coalesces to the empty array. The result is that empty($result) is true for JSON-based data but false for table-based data.

Inconsistency 2

The CommentID is included in the table as a primary key. This does not get included in the flag instance data. (Resolved by #8805)

Needs investigation

UserID, Examiner, and Testdate may or may not be inconsistent for the same reason as point 2.
@driusan driusan added the Category: Bug PR or issue that aims to report or fix a bug label Jun 21, 2023
@driusan driusan self-assigned this Jun 21, 2023
driusan added a commit to driusan/Loris that referenced this issue Jun 21, 2023
@driusan
[instruments] Remove CommentID from getInstanceData.
f5a7e4d 
The CommentID is not part of the data, it's the foreign key used
between the flag table and the instrument table. JSON-based instruments
do not have it, and this ensures better consistency between the two
so that issues such as aces#8796 and aces#8801 will not vary based on instrument
type and will be caught sooner.

Resolves part of aces#8804 (Inconsistency #2)
@driusan driusan mentioned this issue Jun 21, 2023
Merged
driusan added a commit that referenced this issue Jun 21, 2023
@driusan
[instruments] Remove CommentID from getInstanceData. (#8805)
c2f41af 
The CommentID is not part of the data, it's the foreign key used
between the flag table and the instrument table. JSON-based instruments
do not have it, and this ensures better consistency between the two
so that issues such as #8796 and #8801 will not vary based on instrument
type and will be caught sooner.

Resolves part of #8804 (Inconsistency #2)
@driusan driusan mentioned this issue Jun 21, 2023
Merged
@driusan
Copy link
Collaborator Author

driusan commented Jul 4, 2023

Fixed by the above 2 PRs

@driusan driusan closed this as completed Jul 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@driusan driusan

Labels
Category: Bug PR or issue that aims to report or fix a bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@driusan