Institution feature implementation. #670
Conversation
167ca3f to
f5c778d
Compare
15f56ef to
f765628
Compare
6573d56 to
13c777e
Compare
|
@aebruno Overhauled this slightly, it's now finalised and ready for review. Thanks :) |
60064d8 to
964bef3
Compare
aae0f80 to
8b47e29
Compare
|
@aebruno Rebased PR and updated formatting with |
8b47e29 to
3333060
Compare
coldfront/core/project/tests.py
Outdated
| class TestInstitution(TransactionTestCase): | ||
| """Tear down database after each run to prevent conflicts across cases""" |
There was a problem hiding this comment.
I am not sure from looking at this if TransactionTestCase is necessary for this test. Could you explain why you are using this over TestCase? Or did you just copy-paste from the one above this and it is not needed? I would rather not take the speed hit on tests if it is not necessary, also if this code is not doing anything with transactions it might be confusing to anyone who reads this test and thinks there is more going on than there actually is.
There was a problem hiding this comment.
@Eric-Butcher Testing isn't my strongest area - I based this on the previous test and welcome the change to TestCase. Apologies for this, thanks for the spot.
| self.user.email = "user@inst.edu.com" | ||
| self.user.save() | ||
| project_institution_three = self.create_project_with_institution("Project 3", PROJECT_INSTITUTION_EMAIL_MAP) | ||
| self.assertEqual(project_institution_three, "EDU") |
There was a problem hiding this comment.
Since add_automated_institution_choice(project, institution_dict) (or whatever you rename it to) does not make changes to the database I believe it would be beneficial to add a test case to ensure that that is the case. If someone gets confused down the road and does not realize that and changes the function later, that test should ding the change made to the API for add_automated_institution_choice.
e142807 to
3ecb604
Compare
|
Many thanks again for your thorough review @Eric-Butcher,
|
coldfront/core/project/migrations/0006_historicalproject_institution_project_institution.py
Outdated
Show resolved
Hide resolved
Signed-off-by: Connor Brock <BrockC2@cardiff.ac.uk>
cca0b6c to
36a46f7
Compare
Overview
Implemented project institution feature, enabling sites with multiple institutions to easily identify which project belongs to which institution. This feature automates institution assignment based on the PI's email address.
Changes Made
models.py- Addedinstitutionattribute into theProjectclass.core.py- Added environment variable,PROJECT_INSTITUTION_EMAIL_MAP.view.py- Implemented logic intoform_validfunction to handle institution assignment./templates-project_detail,project_list,project_archived_listaltered to showinstitutionfrom GUI.tests.py- Tests added forinstitutionfeature.config.md- Added basic documentation to describePROJECT_INSTITUTION_EMAIL_MAPadmin.md- Overhauledget_list_displayto add multiple dynamic columns intolist_display, therefore showinginstitutionandproject_codealongside one another./migrations- Migration file added./management- Management commands added to retrospectively assign institution codes to preexisting projects,--dry-runoption included too.Examples*
Feature uses
PROJECT_INSTITUTION_EMAIL_MAPto check the PIs email address against a dictionary of key-value pairs, set by the user. e.g.PROJECT_INSTITUTION_EMAIL_MAP=cardiff.ac.uk=CDF,bangor.ac.uk=BGR,swansea.ac.uk=SWANIf PIs email matches a key, the value will be stored into the database.
The screenshots below use a
cardiff.ac.ukemail address and make theCDFinstitution available in the GUI.project_listproject_detailProject Admin Panel