-
Notifications
You must be signed in to change notification settings - Fork 3
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
Add sqlite database #5
Changes from 9 commits
0df9b10
f418a3b
77362f4
6f7084c
de92a95
1fbab7b
40c820e
6498ecb
1f03e6c
9eb455b
7dffd50
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,3 +2,4 @@ Flask==0.9 | |
twilio==3.3.10 | ||
mock==0.8.0 | ||
nose==1.1.2 | ||
sqlalchemy==0.9.1 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,122 @@ | ||
|
||
import datetime | ||
import re | ||
import sqlalchemy | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Import nitpicks:
|
||
from sqlalchemy import Boolean, Column, Integer, String, Text, DateTime, ForeignKey | ||
from sqlalchemy.ext.declarative import declarative_base | ||
from sqlalchemy.orm import sessionmaker, Session | ||
|
||
from phone_directory_actions import PhoneDirectoryActions | ||
from util import validate_phone_number | ||
|
||
|
||
# Creating the base class for database mapped objects, | ||
# see: http://docs.sqlalchemy.org/en/rel_0_9/orm/extensions/declarative.html | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good comment! |
||
Base = declarative_base() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's happening here? |
||
|
||
# A helper for binding a database engine for use with declaritive base | ||
def bind_engine(engine): | ||
Base.metadata.create_all(engine) | ||
return sessionmaker(bind=engine, autoflush=True) | ||
|
||
class User(Base): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Docstring, please |
||
""" User model representing a user with a user_id, name, and phone number """ | ||
__tablename__ = 'users' | ||
|
||
user_id = Column('user_id', Integer, primary_key=True) | ||
phone = Column('crm_type', String(100)) | ||
name = Column('name', String(100)) | ||
updated_at = Column('updated_at', DateTime, onupdate=datetime.datetime.now, default=datetime.datetime.now, index=True) | ||
created_at = Column('created_at', DateTime, default=datetime.datetime.now, index=True) | ||
|
||
|
||
class PhoneHistory(Base): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Docstring please! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added |
||
""" PhoneHistory, we store a PhoneHistory entry for each change """ | ||
__tablename__ = 'phone_history' | ||
id = Column('id', Integer, index=True, primary_key=True) | ||
user_id = Column('user_id', Integer, index=True) | ||
phone = Column('phone', String(100)) | ||
updated_at = Column('updated_at', DateTime, onupdate=datetime.datetime.now, default=datetime.datetime.now, index=True) | ||
created_at = Column('created_at', DateTime, default=datetime.datetime.now, index=True) | ||
|
||
|
||
class SqlitePhoneDirectoryActions(PhoneDirectoryActions): | ||
''' | ||
Data stored in a sqliteDB. | ||
''' | ||
|
||
def __init__(self, database="sqlite:///:memory:"): | ||
engine = sqlalchemy.create_engine( | ||
database # TODO: better path to database | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For each TODO checked in, make a corresponding issue describing exactly what you mean is to be done. Refer to the issue number in the TODO comment. |
||
) | ||
|
||
self._SessionMaker = bind_engine(engine) | ||
|
||
def _get_user_by_id(self, user_id): | ||
session = self._SessionMaker() | ||
users_returned = session.query(User).filter(User.user_id == user_id).all() | ||
session.close() | ||
|
||
if len(users_returned) != 1: | ||
raise KeyError("User {} not found".format(user_id)) | ||
else: | ||
return users_returned[0] | ||
|
||
def user_exists(self, user_id): | ||
''' | ||
Return true if user_id exists, false if not | ||
''' | ||
user = self._get_user_by_id(user_id) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make sure that _get_user_by_id returns a value if user_id doesn't exist. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it raises an exception. KeyError There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
return (user != None) # truthy | ||
|
||
def get_phone_number(self, user_id): | ||
''' | ||
Return phone number for given user_id. Throw KeyError if user doesn't exist. | ||
''' | ||
user = self._get_user_by_id(user_id) | ||
return user.phone | ||
|
||
def get_name(self, user_id): | ||
''' | ||
Return username for given user_id. Throw KeyError if user doesn't exist. | ||
''' | ||
user = self._get_user_by_id(user_id) | ||
return user.name | ||
|
||
def update_phone_number(self, user_id, new_number): | ||
''' | ||
If user_id exists, update its phone number and archive the old number. | ||
If user_id does not exist, throw KeyError | ||
If new phone number is not valid, throw ValueError | ||
''' | ||
user = self._get_user_by_id(user_id) | ||
validate_phone_number(new_number) | ||
old_number = user.phone | ||
user.phone = new_number | ||
|
||
session = self._SessionMaker() | ||
|
||
history = PhoneHistory() | ||
history.user_id = user.user_id | ||
history.phone = old_number | ||
|
||
session.add(user) | ||
session.add(history) | ||
session.commit() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this session need to be closed? Or does commit close it too? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed. |
||
session.close() | ||
|
||
def _add_a_user(self, user_id, phone, name): | ||
validate_phone_number(phone) | ||
|
||
session = self._SessionMaker() | ||
|
||
user = User() | ||
user.name = name | ||
user.user_id = user_id | ||
user.phone = phone | ||
|
||
session.add(user) | ||
session.commit() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this session need to be closed? Or does commit close it too? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good point, I missed that. |
||
session.close() | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
import unittest | ||
import sys | ||
|
||
sys.path.append("src") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not the ideal method, but will work fine if people are using nosetests. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yay testing! Could you update the readme saying how to run these tests? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done. |
||
|
||
from sqlite_phone_directory import SqlitePhoneDirectoryActions | ||
|
||
class TestSqlitePhoneDirectory(unittest.TestCase): | ||
|
||
def setUp(self): | ||
self._db = SqlitePhoneDirectoryActions(database="sqlite:///:memory:") | ||
self._db._add_a_user(user_id=1122, phone='1'*10, name='ben') | ||
|
||
def test_user_exists(self): | ||
assert self._db.user_exists(1122) | ||
|
||
def test_get_name(self): | ||
assert self._db.get_name(1122) == 'ben' | ||
|
||
def test_get_phone_number(self): | ||
assert self._db.get_phone_number(1122) == '1'*10 | ||
|
||
def test_update_phone_number(self): | ||
self._db.update_phone_number(user_id=1122, new_number='2'*10) | ||
assert self._db.get_phone_number(1122) == '2'*10 | ||
|
||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! 👍 Some minor comments.