sartography
/
cr-connect-workflow
mirror of https://github.com/sartography/cr-connect-workflow.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Refactor of the way we store and return details about users - All the details about a user, or individual associated with a study is returned within in an Ldap model. I've removed duplication between these models. This required some cleanup of the tests, and a migration that will drop the user details.

This commit is contained in:
Dan 2021-09-22 13:16:25 -04:00
parent 4b591a076f
commit 19104303de
10 changed files with 70 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
crc/api/user.py
View File

@ -228,20 +228,17 @@ def _handle_login(user_info: LdapModel, redirect_url=None):
return auth_token
def _upsert_user(user_info):
user = session.query(UserModel).filter(UserModel.uid == user_info.uid).first()
def _upsert_user(ldap_info):
user = session.query(UserModel).filter(UserModel.uid == ldap_info.uid).first()
if user is None:
# Add new user
user = UserModel()
else:
user = session.query(UserModel).filter(UserModel.uid == user_info.uid).with_for_update().first()
user = session.query(UserModel).filter(UserModel.uid == ldap_info.uid).with_for_update().first()
user.uid = user_info.uid
user.display_name = user_info.display_name
user.email_address = user_info.email_address
user.affiliation = user_info.affiliation
user.title = user_info.title
user.uid = ldap_info.uid
user.ldap_info = ldap_info
session.add(user)
session.commit()

7
crc/models/study.py
View File

@ -10,6 +10,7 @@ from sqlalchemy import func
from crc import db, ma
from crc.api.common import ApiErrorSchema, ApiError
from crc.models.file import FileModel, SimpleFileSchema, FileSchema
from crc.models.ldap import LdapModel, LdapSchema
from crc.models.protocol_builder import ProtocolBuilderStatus, ProtocolBuilderStudy
from crc.models.workflow import WorkflowSpecCategoryModel, WorkflowState, WorkflowStatus, WorkflowSpecModel, \
WorkflowModel
@ -76,13 +77,17 @@ class StudyAssociated(db.Model):
role = db.Column(db.String, nullable=True)
send_email = db.Column(db.Boolean, nullable=True)
access = db.Column(db.Boolean, nullable=True)
ldap_info = db.relationship(LdapModel)
class StudyAssociatedSchema(ma.Schema):
class Meta:
fields=['uid', 'role', 'send_email', 'access']
fields=['uid', 'role', 'send_email', 'access', 'ldap_info']
model = StudyAssociated
unknown = INCLUDE
ldap_info = fields.Nested(LdapSchema, dump_only=True)
class StudyEvent(db.Model):
__tablename__ = 'study_event'

15
crc/models/user.py
View File

@ -6,21 +6,14 @@ from marshmallow_sqlalchemy import SQLAlchemyAutoSchema
from crc import db, app
from crc.api.common import ApiError
from crc.models.ldap import LdapSchema
class UserModel(db.Model):
__tablename__ = 'user'
id = db.Column(db.Integer, primary_key=True)
uid = db.Column(db.String, unique=True)
email_address = db.Column(db.String)
display_name = db.Column(db.String)
affiliation = db.Column(db.String, nullable=True)
eppn = db.Column(db.String, nullable=True)
first_name = db.Column(db.String, nullable=True)
last_name = db.Column(db.String, nullable=True)
title = db.Column(db.String, nullable=True)
# TODO: Add Department and School
uid = db.Column(db.String, db.ForeignKey('ldap_model.uid'), unique=True)
ldap_info = db.relationship("LdapModel")
def is_admin(self):
# Currently admin abilities are set in the configuration, but this
@ -65,7 +58,9 @@ class UserModelSchema(SQLAlchemyAutoSchema):
model = UserModel
load_instance = True
include_relationships = True
uid = fields.String()
is_admin = fields.Method('get_is_admin', dump_only=True)
ldap_info = fields.Nested(LdapSchema)
def get_is_admin(self, obj):
return obj.is_admin()

4
crc/services/study_service.py
View File

@ -143,7 +143,9 @@ class StudyService(object):
raise ApiError('study_not_found', 'No study found with id = %d' % study_id)
people = db.session.query(StudyAssociated).filter(StudyAssociated.study_id == study_id).all()
owner = StudyAssociated(uid=study.user_uid, role='owner', send_email=True, access=True)
ldap_info = LdapService.user_info(study.user_uid)
owner = StudyAssociated(uid=study.user_uid, role='owner', send_email=True, access=True,
ldap_info=ldap_info)
people.append(owner)
return people

5
crc/services/workflow_service.py
View File

@ -27,6 +27,7 @@ from crc.api.common import ApiError
from crc.models.api_models import Task, MultiInstanceType, WorkflowApi
from crc.models.data_store import DataStoreModel
from crc.models.file import LookupDataModel, FileModel, File, FileSchema
from crc.models.ldap import LdapModel
from crc.models.study import StudyModel
from crc.models.task_event import TaskEventModel
from crc.models.user import UserModel, UserModelSchema
@ -69,6 +70,7 @@ class WorkflowService(object):
if not user:
user = db.session.query(UserModel).filter_by(uid="test").first()
if not user:
db.session.add(LdapModel(uid="test"))
db.session.add(UserModel(uid="test"))
db.session.commit()
user = db.session.query(UserModel).filter_by(uid="test").first()
@ -93,6 +95,9 @@ class WorkflowService(object):
for study in db.session.query(StudyModel).filter(StudyModel.user_uid == "test"):
StudyService.delete_study(study.id)
user = db.session.query(UserModel).filter_by(uid="test").first()
ldap = db.session.query(LdapModel).filter_by(uid="test").first()
if ldap:
db.session.delete(ldap)
if user:
db.session.delete(user)
db.session.commit()

4
example_data.py
View File

@ -335,10 +335,10 @@ class ExampleDataLoader:
file.close()
def load_default_user(self):
user = UserModel(uid="dhf8r", email_address="dhf8r@virginia.edu", display_name="Development User")
ldap_info = LdapModel(uid="dhf8r", email_address="dhf8r@virginia.edu", display_name="Development User")
db.session.add(user)
user = UserModel(uid="dhf8r", ldap_info=ldap_info)
db.session.add(ldap_info)
db.session.add(user)
db.session.commit()
def ldap(): return "x";

34
tests/base_test.py
View File

@ -10,16 +10,16 @@ import unittest
import urllib.parse
import datetime
from flask import g
from sqlalchemy import Sequence
from crc import app, db, session
from crc.models.api_models import WorkflowApiSchema, MultiInstanceType
from crc.models.file import FileModel, FileDataModel, CONTENT_TYPES
from crc.models.task_event import TaskEventModel
from crc.models.study import StudyModel, StudyStatus
from crc.models.data_store import DataStoreModel
from crc.models.ldap import LdapModel
from crc.models.user import UserModel
from crc.models.workflow import WorkflowSpecModel, WorkflowSpecCategoryModel
from crc.services.ldap_service import LdapService
from crc.services.file_service import FileService
from crc.services.study_service import StudyService
from crc.services.user_service import UserService
@ -45,27 +45,14 @@ class BaseTest(unittest.TestCase):
auths = {}
test_uid = "dhf8r"
# These users correspond to the ldap details available in our mock ldap service.
users = [
{
'uid': 'dhf8r',
'email_address': 'dhf8r@virginia.EDU',
'display_name': 'Daniel Harold Funk',
'affiliation': 'staff@virginia.edu;member@virginia.edu',
'eppn': 'dhf8r@virginia.edu',
'first_name': 'Daniel',
'last_name': 'Funk',
'title': 'SOFTWARE ENGINEER V'
},
{
'uid': 'lbd3p',
'email_address': 'lbd3p@virginia.EDU',
'display_name': 'Laura Barnes',
'affiliation': 'staff@virginia.edu;member@virginia.edu',
'eppn': 'lbd3p@virginia.edu',
'first_name': 'Laura',
'last_name': 'Barnes',
'title': 'Associate Professor of Systems and Information Engineering'
},
'uid': 'lb3dp',
}
]
studies = [
@ -127,7 +114,7 @@ class BaseTest(unittest.TestCase):
self.assertTrue(str.startswith(rv.location, redirect_url))
user_model = session.query(UserModel).filter_by(uid=uid).first()
self.assertIsNotNone(user_model.display_name)
self.assertIsNotNone(user_model.ldap_info.display_name)
self.assertEqual(user_model.uid, uid)
self.assertTrue('user' in g, 'User should be in Flask globals')
user = UserService.current_user(allow_admin_impersonate=True)
@ -150,10 +137,12 @@ class BaseTest(unittest.TestCase):
ExampleDataLoader.clean_db()
# If in production mode, only add the first user.
if app.config['PRODUCTION']:
session.add(UserModel(**self.users[0]))
ldap_info = LdapService.user_info(self.users[0]['uid'])
session.add(UserModel(uid=self.users[0]['uid'], ldap_info=ldap_info))
else:
for user_json in self.users:
session.add(UserModel(**user_json))
ldap_info = LdapService.user_info(user_json['uid'])
session.add(UserModel(uid=user_json['uid'], ldap_info=ldap_info))
if use_crc_data:
ExampleDataLoader().load_all()
@ -267,7 +256,8 @@ class BaseTest(unittest.TestCase):
def create_user(self, uid="dhf8r", email="daniel.h.funk@gmail.com", display_name="Hoopy Frood"):
user = session.query(UserModel).filter(UserModel.uid == uid).first()
if user is None:
user = UserModel(uid=uid, email_address=email, display_name=display_name)
ldap_user = LdapService.user_info(uid)
user = UserModel(uid=uid, ldap_info=ldap_user)
session.add(user)
session.commit()
return user

17
tests/study/test_study_associate_script.py
View File

@ -8,6 +8,8 @@ from crc.services.user_service import UserService
from crc import session, app
from crc.models.study import StudyModel
from crc.models.ldap import LdapSchema
from crc.services.ldap_service import LdapService
from crc.models.user import UserModel
from crc.api.study import user_studies
from crc.services.study_service import StudyService
@ -53,17 +55,20 @@ class TestSudySponsorsScript(BaseTest):
self.assertIn('sponsors', data)
self.assertIn('out', data)
print(data['out'])
self.assertDictEqual({'uid': 'dhf8r', 'role': 'owner', 'send_email': True, 'access': True},
dhf8r_info = LdapSchema().dump(LdapService.user_info('dhf8r'))
lb3dp_info = LdapSchema().dump(LdapService.user_info('lb3dp'))
self.assertDictEqual({'uid': 'dhf8r', 'role': 'owner', 'send_email': True, 'access': True, 'ldap_info': dhf8r_info},
data['out'][1])
self.assertDictEqual({'uid': 'lb3dp', 'role': 'SuperDude', 'send_email': False, 'access': True},
self.assertDictEqual({'uid': 'lb3dp', 'role': 'SuperDude', 'send_email': False, 'access': True, 'ldap_info': lb3dp_info},
data['out'][0])
self.assertDictEqual({'uid': 'lb3dp', 'role': 'SuperDude', 'send_email': False, 'access': True},
self.assertDictEqual({'uid': 'lb3dp', 'role': 'SuperDude', 'send_email': False, 'access': True, 'ldap_info': lb3dp_info},
data['out2'])
self.assertDictEqual({'uid': 'dhf8r', 'role': 'owner', 'send_email': True, 'access': True},
self.assertDictEqual({'uid': 'dhf8r', 'role': 'owner', 'send_email': True, 'access': True, 'ldap_info': dhf8r_info},
data['out3'][1])
self.assertDictEqual({'uid': 'lb3dp', 'role': 'SuperGal', 'send_email': False, 'access': True},
self.assertDictEqual({'uid': 'lb3dp', 'role': 'SuperGal', 'send_email': False, 'access': True, 'ldap_info': lb3dp_info},
data['out3'][0])
self.assertDictEqual({'uid': 'lb3dp', 'role': 'SuperGal', 'send_email': False, 'access': True},
self.assertDictEqual({'uid': 'lb3dp', 'role': 'SuperGal', 'send_email': False, 'access': True, 'ldap_info': lb3dp_info},
data['out4'])
self.assertEqual(3, len(data['sponsors']))

17
tests/study/test_study_service.py
View File

@ -5,15 +5,14 @@ from unittest.mock import patch
from tests.base_test import BaseTest
from crc import db, app
from crc.models.protocol_builder import ProtocolBuilderStatus
from crc.models.study import StudyModel, StudyStatus
from crc.models.study import StudyModel, StudyStatus, StudyAssociatedSchema
from crc.models.user import UserModel
from crc.models.workflow import WorkflowModel, WorkflowStatus, \
WorkflowSpecCategoryModel
from crc.services.file_service import FileService
from crc.services.ldap_service import LdapService
from crc.services.study_service import StudyService
from crc.services.workflow_processor import WorkflowProcessor
from example_data import ExampleDataLoader
class TestStudyService(BaseTest):
@ -33,7 +32,8 @@ class TestStudyService(BaseTest):
self.load_test_spec("top_level_workflow", master_spec=True, category_id=cat.id)
user = db.session.query(UserModel).filter(UserModel.uid == "dhf8r").first()
if not user:
user = UserModel(uid="dhf8r", email_address="whatever@stuff.com", display_name="Stayathome Smellalots")
ldap = LdapService.user_info('dhf8r')
user = UserModel(uid="dhf8r", ldap_info=ldap)
db.session.add(user)
db.session.commit()
else:
@ -257,3 +257,12 @@ class TestStudyService(BaseTest):
studies = StudyService().get_studies_for_user(user)
# study_details has an invalid REVIEW_TYPE, so we should get 0 studies back
self.assertEqual(0, len(studies))
def test_study_associates(self):
user = self.create_user_with_study_and_workflow()
study = db.session.query(StudyModel).first()
associates = StudyService.get_study_associates(study.id)
self.assertEquals(1, len(associates))
assoc_json = StudyAssociatedSchema(many=True).dump(associates)
print(assoc_json)
self.assertEquals("Dan", assoc_json[0]['ldap_info']['given_name'])

15
tests/test_authentication.py
View File

@ -9,6 +9,7 @@ from crc import app, session
from crc.api.common import ApiError
from crc.models.protocol_builder import ProtocolBuilderStatus
from crc.models.study import StudySchema, StudyModel, StudyStatus
from crc.services.ldap_service import LdapService
from crc.models.user import UserModel
from unittest.mock import patch
@ -63,7 +64,7 @@ class TestAuthentication(BaseTest):
def test_non_production_auth_creates_user(self):
new_uid = self.non_admin_uid ## Assure this user id is in the fake responses from ldap.
self.load_example_data()
# self.load_example_data()
user = session.query(UserModel).filter(UserModel.uid == new_uid).first()
self.assertIsNone(user)
@ -78,8 +79,8 @@ class TestAuthentication(BaseTest):
user = session.query(UserModel).filter(UserModel.uid == new_uid).first()
self.assertIsNotNone(user)
self.assertIsNotNone(user.display_name)
self.assertIsNotNone(user.email_address)
self.assertIsNotNone(user.ldap_info.display_name)
self.assertIsNotNone(user.ldap_info.email_address)
# Hitting the same endpoint again with the same info should not cause an error
rv_2 = self.app.get(url, follow_redirects=False)
@ -119,7 +120,7 @@ class TestAuthentication(BaseTest):
self.assert_success(rv)
# User must exist in the mock ldap responses.
user = UserModel(uid="dhf8r", first_name='Dan', last_name='Funk', email_address='dhf8r@virginia.edu')
user = UserModel(uid="dhf8r", ldap_info=LdapService.user_info("dhf8r"))
rv = self.app.get('/v1.0/user', headers=self.logged_in_headers(user, redirect_url='http://omg.edu/lolwut'))
self.assert_success(rv)
user_data = json.loads(rv.get_data(as_text=True))
@ -318,7 +319,7 @@ class TestAuthentication(BaseTest):
self.assertFalse(user.is_admin())
self.assertIsNotNone(user)
self.assertEqual(self.non_admin_uid, user.uid)
self.assertEqual("Laura Barnes", user.display_name)
self.assertEqual("lb3dp@virginia.edu", user.email_address)
self.assertEqual("E0:Associate Professor of Systems and Information Engineering", user.title)
self.assertEqual("Laura Barnes", user.ldap_info.display_name)
self.assertEqual("lb3dp@virginia.edu", user.ldap_info.email_address)
self.assertEqual("E0:Associate Professor of Systems and Information Engineering", user.ldap_info.title)
return user