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 Examiner Select #9452

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

skarya22
Copy link
Contributor

@skarya22 skarya22 commented Nov 6, 2024

Brief summary of changes

  • Includes a CCNA override that makes recreate_conflicts work when there are many examiners

CCNA OVERRIDE PR

@driusan
Copy link
Collaborator

driusan commented Nov 7, 2024

Can you split this in 2?

The test_names looks like an easy bug fix, the "DISTINCT" seems more questionable. Why aren't they already distinct?

@skarya22
Copy link
Contributor Author

skarya22 commented Nov 7, 2024

Can you split this in 2?

The test_names looks like an easy bug fix, the "DISTINCT" seems more questionable. Why aren't they already distinct?

I'll split it up. I am not sure why they are not already distinct, but this is an override that we needed in CCNA as without it the recreate_conflicts script fails:
image

@skarya22 skarya22 changed the title [Recreate_Conflicts] Fix script [NDB_BVL_Instrument] Modify Examiner Select Nov 11, 2024
@@ -1053,7 +1053,7 @@ abstract class NDB_BVL_Instrument extends NDB_Page
['tst_name' => $this->testName]
);
$results = $db->pselectWithIndexKey(
"SELECT c.examinerID, e.full_name, u.Email
"SELECT DISTINCT c.examinerID, e.full_name, u.Email
Copy link
Collaborator

Choose a reason for hiding this comment

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

@skarya22 I don't think this DISTINCT is necessary since you are querying by CenterID but I don't see any way it would hurt so.. I'm okai with it

@maximemulder maximemulder added Area: Instruments PR or issue related instruments Project: CCNA Issue or PR related to the CCNA project and removed Priority: Projects labels Nov 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Instruments PR or issue related instruments Project: CCNA Issue or PR related to the CCNA project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants