This is my first time building anything remotely like a robust database. I'm midway through designing it and I would be grateful if you have suggestions or have found any logical errors.

Goal: allow authenticated users to enter data for multiple hospitals.

My database has three tables: User, Hospital, and Data.

A User might have many Hospitals (and vise-versa). Because of this, I have created a many-to-many scheme. A Hospital might have many Datas so I created a one to many relationship.

from petalapp import db

from datetime import datetime
ROLE_USER = 0

#TODO:rename?
hospitals = db.Table('hospitals',
db.Column('hospital_id', db.Integer, db.ForeignKey('hospital.id')),
db.Column('user_id', db.Integer, db.ForeignKey('user.id'))
)
# tags bmarks time
class User(db.Model):
"""User has a many-to-many relationship with Hospital"""

id = db.Column(db.Integer, primary_key=True)
nickname = db.Column(db.String(64), unique = True)
email = db.Column(db.String(150), unique=True)
role = db.Column(db.SmallInteger, default=ROLE_USER)

hospitals = db.relationship('Hospital', secondary=hospitals,
backref=db.backref('users', lazy='dynamic'))

def __init__(self, nickname, email, role=ROLE_USER):
self.nickname= nickname
self.role = role
self.email = email

#TODO what information to show?
def __repr__(self):
return '<Name : %r>' % (self.nickname)

def is_authenticated(self):
return True

def is_active(self):
return True

def is_anonymous(self):
return False

def get_id(self):
return unicode(self.id)

if not (hospital in self.hospitals):
self.hospitals.append(hospital)

def remove_hospital(self, hospital):
if not (hospital in self.hospital):
self.hospitals.remove(hospital)

@staticmethod
def make_unique_nickname(nickname):
if User.query.filter_by(nickname = nickname).first() == None:
return nickname
version = 2
while True:
new_nickname = nickname + str(version)
if User.query.filter_by(nickname = new_nickname).first() == None:
break
version += 1
return new_nickname

class Hospital(db.Model):
"""Hospital's has a one-to-many relationship with DATA and a
many-to-many relationship with User"""

id = db.Column(db.Integer, primary_key=True)
name = db.Column(db.String(80))
data = db.relationship('Data', backref='hospital', lazy = 'dynamic')

def __init__(self, name):
self.name = name

def __repr__(self):
return '<Name %r>' % self.name

class Data(db.Model):
"""Data has a many-to-one relationship with Hospital"""

id = db.Column(db.Integer, primary_key=True)

standard_form = db.Column(db.Integer)
marketing_education = db.Column(db.Integer)
record_availability = db.Column(db.Integer)
family_centerdness = db.Column(db.Integer)
pc_networking = db.Column(db.Integer)
education_and_training = db.Column(db.Integer)
team_funding = db.Column(db.Integer)
coverage = db.Column(db.Integer)
pc_for_expired_pts = db.Column(db.Integer)
hospital_pc_screening  = db.Column(db.Integer)
pc_follow_up = db.Column(db.Integer)
post_discharge_services = db.Column(db.Integer)
bereavement_contacts = db.Column(db.Integer)
certification = db.Column(db.Integer)
team_wellness = db.Column(db.Integer)
care_coordination = db.Column(db.Integer)

timestamp = db.Column(db.DateTime)

hospital_id = db.Column(db.Integer, db.ForeignKey('hospital.id'))

def __init__(self, standard_form=0,
marketing_education=0, record_availability=0, family_centerdness=0,
pc_networking=0, education_and_training=0, team_funding=0,
coverage=0, pc_for_expired_pts=0, hospital_pc_screening=0,
pc_follow_up=0, post_discharge_services=0, bereavement_contacts=0,
certification=0, team_wellness=0, care_coordination=0, timestamp=datetime.utcnow()):

self.standard_form = standard_form
self.marketing_education =  marketing_education
self.record_availability = record_availability
self.family_centerdness =  family_centerdness
self.pc_networking =  pc_networking
self.education_and_training =  education_and_training
self.team_funding =  team_funding
self.coverage =  coverage
self.pc_for_expired_pts =  pc_for_expired_pts
self.hospital_pc_screening  =  hospital_pc_screening
self.pc_follow_up =  pc_follow_up
self.post_discharge_services =  post_discharge_services
self.bereavement_contacts =  bereavement_contacts
self.certification =  certification
self.team_wellness =  team_wellness
self.care_coordination = care_coordination
self.timestamp = timestamp

def __repr__(self):
return """
<standard_form : %r>\n
<marketing_education : %r>\n
<record_availability : %r>\n
<family_centerdness : %r>\n
<pc_networking : %r>\n
<education_and_training : %r>\n
<team_funding : %r>\n
<coverage : %r>\n
<pc_for_expired_pts : %r>\n
<hospital_pc_screening  : %r>\n
<pc_follow_up : %r>\n
<post_discharge_services : %r>\n
<bereavement_contacts : %r>\n
<certification : %r>\n
<team_wellness : %r>\n
<care_coordination : %r>\n
<datetime_utc  : %r>""" % (
self.standard_form,
self.marketing_education,
self.record_availability,
self.family_centerdness,
self.pc_networking,
self.education_and_training,
self.team_funding,
self.coverage,
self.pc_for_expired_pts,
self.hospital_pc_screening ,
self.pc_follow_up,
self.post_discharge_services,
self.bereavement_contacts,
self.certification,
self.team_wellness,
self.care_coordination,
self.timestamp)


Here are my tests so far:

'''
File: db_test_many_create.py
Date: 2012-12-06
Author: Drew Verlee
Description: functions to build a many-to-many relationship
'''
import unittest
from petalapp.database.models import User, Hospital, Data, ROLE_USER
from petalapp import db

class BuildDestroyTables(unittest.TestCase):

def setUp(self):
db.drop_all()
db.create_all()

def tearDown(self):
db.session.remove()
db.drop_all()

def test_user_setup(self):

user_test_1 = User("test_user_nickname","user_email",ROLE_USER)
db.session.commit()

def test_data_setup(self):

data_test_1 = Data(1)
db.session.commit()

def test_hospital_setup(self):

hospital_test_1 = Hospital("test_hospital_1")
db.session.commit()

def test_make_unique_nickname(self):
u = User(nickname = 'john', email = 'john@example.com')
db.session.commit()
nickname = User.make_unique_nickname('john')
assert nickname != 'john'
u = User(nickname = nickname, email = 'susan@example.com')
db.session.commit()
nickname2 = User.make_unique_nickname('john')
assert nickname2 != 'john'
assert nickname2 != nickname

#create
drew = User(nickname= "Drew", email="Drew@gmail.com",role=ROLE_USER)
mac_hospital = Hospital("Mac_hospital")
pro_hospital = Hospital("pro_hospital")
mac_data = Data(1,2,3)
pro_data = Data(10,9,8)

#commit
db.session.commit()

mac_hospital.data.append(mac_data)
pro_hospital.data.append(pro_data)
drew.hospitals.append(mac_hospital)
drew.hospitals.append(pro_hospital)
db.session.commit()

johns_hospital_data = Data('johns_hospital_data')
johns_hospital = Hospital('johns_hospital')
john = User('john', 'john@gmail.com')

#TODO make a function for this?
johns_hospital.append(johns_hospital_data)
#do  i need a commit?
db.session.commit()

self.assertEqual(john.remove_hospital(johns_hospital), None)

db.session.commit()

self.assertEqual(len(john.hospitals), 1)

if __name__ == "__main__":
unittest.main()

-
I'm looking for exactly the same kind of best practice. If you have become confident in how to do it, could you share in a self-answer? –  kobejohn Oct 10 '13 at 5:18

## Models - General

The comments on the model definitions don't add much value. I could figure out the relationships from the db schema; tell me instead something about what the classes are meant to represent, any invariants they have, anything not made explicit in the code.

## Models - User

When defining a backreference, as you do in User, I like to go to the other class and add a comment saying something along the lines of "A backreference 'users' is created to the Users class". With that said, I would avoid naming the variable users; what does the variable name 'users' tell me when reading the class? It tells me the type of the object I'm getting, but nothing about what significance those objects have. Something like registered_users, patients (or whatever makes sense) might be a better option.

You could replace your is_active, is_anonymous methods with read only properties, but that's personal preference. The get_id method seems very odd, not sure I see the value.

I question also the value of the add_hospital and remove_hospital methods, though that may be due to lack of knowledge of your use case. Regardless, the remove_hospital is incorrect; it should be:

def remove_hospital(self, hospital):
if hospital in self.hospitals: # You have if not (hospital in self.hospital)
self.hospitals.remove(hospital)


## Models - Data

Personally, any variable named data sets off alarm bells. As it is, the Data class seems to store a lot of "stuff", and no where do I get told why this stuff should be grouped together, what its conceptual value is, etc etc. Would something like InspectionResult (or whatever the data comes from) be more useful? Regardless, I can't help but feel this class does too much.

Your __init__ for the Data class is horrendous. Use the default parameter of the Column constructor to set defaults, and make sure to call Super(Data, self).__init__() if you are overriding the __init__ of a subclass of anything you don't control.

## Tests

I can't say I like the name of the test class; it calls itself BuildDestroyTables and then goes on to do a whole lot more than that. I've personally moved away from class based testing, but back when I used it, I would define a BaseDatabaseTest class that other tests could inherit from to absolve them of having to worry about database setup/cleanup while not violating the single responsibility principle.

You use a raw sessions; I would encourage you to use a contextmanager or something to manage your session. Here's one from a project I'm currently working on:

import contextlib

@contextlib.contextmanager
def managed_session(**kwargs):
"""
Provides "proper" session management to a block. Features:

- Rollback if an exception is raised
- Commit if no exception is raised
- Session cleanup on exit
"""
session = db_session(**kwargs) # Or however you create a new session
try:
yield session
except:
session.rollback()
raise
else:
session.commit()
finally:
session.close()


You can then use it like this:

with managed_session() as session: