Completed milestone M1 tasks related to instructor management#45
Completed milestone M1 tasks related to instructor management#45
Conversation
Fixes #36 Renamed file getInstructorInfo.sql to addInstructorMgmt.sql and added test script testGetInstructorMgmt.sql Renamed functions so that all functions start with the prefix getInstructor Added new function getInstructors() to return details of all instructors: not required for M1, but will likely be needed soon Added a 3-arg version of function getInstructorSections to return an instructor's sections for a year-season combo Changed return type of function getInstructor(email) to TABLE to fix the issue that the return type RECORD always returned a row: even when the e-mail lookup failed Added STABLE and RETURNS NULL ON NULL INPUT to all functions
Renamed script testGetInstructorMgmt.sql to testInstructorMgmt.sql
smurthys
requested review from
ZBoylan,
afig,
bella004,
griffine,
hunterSchloss,
s-aithal,
wildtayne and
zbhujwala
smurthys
changed the title
|
I already see that function |
|
This looks good so far, and all the functions work as specified. I have made the changes to the web server locally to support the changes in this pull request, however they are very minor so I am unsure how to best integrate them. I could open a new PR with a condition that it must be merged after this one. |
|
@srrollo Making the changes in another PR is fine as long as it is done without much delay after the changes from this PR are merged. BTW, I can merge this PR as soon as I get at least two approvals. Also, it will be nice to get @zbhujwala's feedback because he is a major contributor to the initial version. |
Updated a comment in each .sql file involved in instructor mgmrt. Also removed ON CONFLICT in test script when inserting test rows to table Section.
|
I just pushed commit a1b4b22 which updates a comment in each .sql file involved in instructor mgmt. It also removes |
Added function getInstructor(instructorID) and added a test case Changed typename INTEGER to INT for consistency within the script Made comments consistent
|
Commit 783a3f3 adds function While there, I changed the type name I also edited some comments for consistency. |
wildtayne
left a comment
wildtayne
left a comment
There was a problem hiding this comment.
I agree that we should use INT over INTEGER. Also, I think this PR is good to merge.
Instructor REST calls now use the updated DB function names (From #45) The login failed message now sends the correct error code (401) Added a check for blank attendance sheets - if no records are found, a 500 error is sent Updated the attendance function to use the correct column name from the DB
afig
left a comment
afig
left a comment
There was a problem hiding this comment.
The changes look good. All tests passed in my instance of Gradebook. Standardizing the use of INT in place of INTEGER is fine by me as well.
One small issue is that the filenames in the first line of the two files and do not match the actual file names of the latest version of the files. Otherwise, everything looks okay to merge.
|
Sorry about not getting the filenames right in the prologue comment. Commit f7bf0f7 fixes it. |
Updated gradebookServer.js DB function names to match #45 + Misc. fixes
3 participants
The commits in this PR make the changes necessary to prepare instructor-management functions for milestone M1. They also fix Issue #36 and the issues discussed in PR #32.
Note: Using the changes in this PR requires simple changes in the web-server because the names of DB functions are changed.
Here is a summary of changes:
getInstructorInfo.sqltoaddInstructorMgmt.sqltestInstructorMgmt.sqlgetInstructorgetInstructors()to return details of all instructors: this function is not required for M1, but will likely be needed soongetInstructorSectionsto return an instructor's sections for a year-season combo. With this function it is possible to get course and section combos such as `CS110-72' for an instructor-year-season combogetInstructor(email)toTABLEto fix the issue that the return typeRECORDalways returned a row: even when the e-mail lookup failed. Also addedROWSattribute to set the maximum result cardinality to 1STABLEandRETURNS NULL ON NULL INPUTattributes to all functions, exceptgetInstructors()