Remove Notification, NotificationHistory status labels:

Replace labels by adding a key kwarg in the model for status.

We still need this as sqlalchemy attmempts to look for `notification_status`
on the model (Notification/NotificationHistory). To achieve true ORM mapping
(map status -> notification_status) we need the key kwarg.

More here:
http://docs.sqlalchemy.org/en/latest/core/metadata.html#sqlalchemy.schema.Column#key
This commit is contained in:
Imdad Ahad
2017-07-06 14:20:24 +01:00
parent c8c47f44a9
commit a9c1338873
6 changed files with 16 additions and 40 deletions

View File

@@ -17,7 +17,7 @@ from app.statsd_decorators import statsd
def dao_get_notification_outcomes_for_job(service_id, job_id):
query = db.session.query(
func.count(NotificationHistory.status).label('count'),
NotificationHistory.status.label('status')
NotificationHistory.status
)
return query \

View File

@@ -262,8 +262,7 @@ def fetch_todays_total_message_count(service_id):
def _stats_for_service_query(service_id):
return db.session.query(
Notification.notification_type,
# see dao_fetch_todays_stats_for_all_services for why we have this label
Notification.status.label('status'),
Notification.status,
func.count(Notification.id).label('count')
).filter(
Notification.service_id == service_id,
@@ -281,8 +280,7 @@ def dao_fetch_monthly_historical_stats_by_template_for_service(service_id, year)
start_date, end_date = get_financial_year(year)
sq = db.session.query(
NotificationHistory.template_id,
# see dao_fetch_todays_stats_for_all_services for why we have this label
NotificationHistory.status.label('status'),
NotificationHistory.status,
month.label('month'),
func.count().label('count')
).filter(
@@ -298,7 +296,7 @@ def dao_fetch_monthly_historical_stats_by_template_for_service(service_id, year)
Template.id.label('template_id'),
Template.name,
Template.template_type,
sq.c.status.label('status'),
sq.c.status,
sq.c.count.label('count'),
sq.c.month
).join(
@@ -316,8 +314,7 @@ def dao_fetch_monthly_historical_stats_for_service(service_id, year):
start_date, end_date = get_financial_year(year)
rows = db.session.query(
NotificationHistory.notification_type,
# see dao_fetch_todays_stats_for_all_services for why we have this label
NotificationHistory.status.label('status'),
NotificationHistory.status,
month,
func.count(NotificationHistory.id).label('count')
).filter(
@@ -356,9 +353,7 @@ def dao_fetch_monthly_historical_stats_for_service(service_id, year):
def dao_fetch_todays_stats_for_all_services(include_from_test_key=True):
query = db.session.query(
Notification.notification_type,
# this label is necessary as the column has a different name under the hood (_status_enum / _status_fkey),
# if we query the Notification object there is a hybrid property to translate, but here there isn't anything.
Notification.status.label('status'),
Notification.status,
Notification.service_id,
func.count(Notification.id).label('count')
).filter(
@@ -388,8 +383,7 @@ def fetch_stats_by_date_range_for_all_services(start_date, end_date, include_fro
query = db.session.query(
table.notification_type,
# see dao_fetch_todays_stats_for_all_services for why we have this label
table.status.label('status'),
table.status,
table.service_id,
func.count(table.id).label('count')
).filter(

View File

@@ -31,7 +31,7 @@ def timeout_job_counts(notifications_type, timeout_start):
results = db.session.query(
JobStatistics.job_id.label('job_id'),
func.count(Notification.status).label('count'),
Notification.status.label('status')
Notification.status
).filter(
Notification.notification_type == notifications_type,
JobStatistics.job_id == Notification.job_id,

View File

@@ -788,14 +788,14 @@ class Notification(db.Model):
unique=False,
nullable=True,
onupdate=datetime.datetime.utcnow)
_status_enum = db.Column('status', NOTIFICATION_STATUS_TYPES_ENUM, index=True, nullable=True, default='created')
_status_fkey = db.Column(
status = db.Column(
'notification_status',
db.String,
db.ForeignKey('notification_status_types.name'),
index=True,
nullable=True,
default='created'
default='created',
key='status' # http://docs.sqlalchemy.org/en/latest/core/metadata.html#sqlalchemy.schema.Column
)
reference = db.Column(db.String, nullable=True, index=True)
client_reference = db.Column(db.String, index=True, nullable=True)
@@ -817,14 +817,6 @@ class Notification(db.Model):
created_by = db.relationship('User')
created_by_id = db.Column(UUID(as_uuid=True), db.ForeignKey('users.id'), nullable=True)
@hybrid_property
def status(self):
return self._status_fkey
@status.setter
def status(self, status):
self._status_fkey = status
@property
def personalisation(self):
if self._personalisation:
@@ -998,14 +990,14 @@ class NotificationHistory(db.Model, HistoryModel):
sent_at = db.Column(db.DateTime, index=False, unique=False, nullable=True)
sent_by = db.Column(db.String, nullable=True)
updated_at = db.Column(db.DateTime, index=False, unique=False, nullable=True)
_status_enum = db.Column('status', NOTIFICATION_STATUS_TYPES_ENUM, index=True, nullable=True, default='created')
_status_fkey = db.Column(
status = db.Column(
'notification_status',
db.String,
db.ForeignKey('notification_status_types.name'),
index=True,
nullable=True,
default='created'
default='created',
key='status' # http://docs.sqlalchemy.org/en/latest/core/metadata.html#sqlalchemy.schema.Column
)
reference = db.Column(db.String, nullable=True, index=True)
client_reference = db.Column(db.String, nullable=True)
@@ -1027,14 +1019,6 @@ class NotificationHistory(db.Model, HistoryModel):
super().update_from_original(original)
self.status = original.status
@hybrid_property
def status(self):
return self._status_fkey
@status.setter
def status(self, status):
self._status_fkey = status
INVITED_USER_STATUS_TYPES = ['pending', 'accepted', 'cancelled']

View File

@@ -410,7 +410,7 @@ def test_should_by_able_to_update_status_by_id(sample_template, sample_job, mmg_
data = _notification_json(sample_template, job_id=sample_job.id, status='sending')
notification = Notification(**data)
dao_create_notification(notification)
assert notification._status_fkey == 'sending'
assert notification.status == 'sending'
assert Notification.query.get(notification.id).status == 'sending'
@@ -421,7 +421,7 @@ def test_should_by_able_to_update_status_by_id(sample_template, sample_job, mmg_
assert updated.updated_at == datetime(2000, 1, 2, 12, 0, 0)
assert Notification.query.get(notification.id).status == 'delivered'
assert notification.updated_at == datetime(2000, 1, 2, 12, 0, 0)
assert notification._status_fkey == 'delivered'
assert notification.status == 'delivered'
def test_should_not_update_status_by_id_if_not_sending_and_does_not_update_job(notify_db, notify_db_session):

View File

@@ -463,8 +463,6 @@ def test_get_all_notifications_for_job_returns_correct_format(
assert len(resp['notifications']) == 1
assert resp['notifications'][0]['id'] == str(sample_notification_with_job.id)
assert resp['notifications'][0]['status'] == sample_notification_with_job.status
assert '_status_fkey' not in resp['notifications'][0]
assert '_status_enum' not in resp['notifications'][0]
def test_get_job_by_id(notify_api, sample_job):