From 75dd25e2dcb9495171f3ccff2f5df33b2b7b5ef5 Mon Sep 17 00:00:00 2001 From: Montana Date: Thu, 8 Nov 2018 15:11:52 -0500 Subject: [PATCH 01/20] Update AuditLog to handle workspace role update events --- atst/domain/audit_log.py | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/atst/domain/audit_log.py b/atst/domain/audit_log.py index b5f8522c..1c15916a 100644 --- a/atst/domain/audit_log.py +++ b/atst/domain/audit_log.py @@ -24,6 +24,19 @@ class AuditLog(object): user=user, workspace_id=workspace.id, resource=resource, action=action ) + @classmethod + def log_update_workspace_role( + cls, action, user, updated_user, workspace, previous_role, new_role + ): + return cls._log( + action=action, + user=user, + updated_user=updated_user, + workspace_id=workspace.id, + previous_role_id=previous_role.id, + new_role_id=new_role.id, + ) + @classmethod def log_system_event(cls, resource, action): return cls._log(resource=resource, action=action) @@ -40,15 +53,27 @@ class AuditLog(object): return type(resource).__name__.lower() @classmethod - def _log(cls, user=None, workspace_id=None, resource=None, action=None): + def _log( + cls, + user=None, + updated_user=None, + workspace_id=None, + resource=None, + action=None, + previous_role_id=None, + new_role_id=None, + ): resource_id = resource.id if resource else None resource_type = cls._resource_type(resource) if resource else None audit_event = AuditEventQuery.create( user=user, + updated_user=updated_user, workspace_id=workspace_id, resource_id=resource_id, resource_type=resource_type, + previous_role_id=previous_role_id, + new_role_id=new_role_id, action=action, ) return AuditEventQuery.add_and_commit(audit_event) From ae3113f011a3d991656f5e6d47901260278ccebd Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Fri, 9 Nov 2018 10:57:24 -0500 Subject: [PATCH 02/20] Get previous role when workspace role updates --- atst/models/mixins/auditable.py | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/atst/models/mixins/auditable.py b/atst/models/mixins/auditable.py index 1b6ae3d2..10b252ac 100644 --- a/atst/models/mixins/auditable.py +++ b/atst/models/mixins/auditable.py @@ -1,5 +1,7 @@ -from sqlalchemy import event +from sqlalchemy import event, inspect from flask import g +from sqlalchemy.orm import class_mapper +from sqlalchemy.orm.attributes import get_history from atst.models.audit_event import AuditEvent from atst.utils import camel_to_snake, getattr_path @@ -11,12 +13,13 @@ ACTION_DELETE = "delete" class AuditableMixin(object): @staticmethod - def create_audit_event(connection, resource, action): + def create_audit_event(connection, resource, action, previous_state=None): user_id = getattr_path(g, "current_user.id") workspace_id = resource.auditable_workspace_id() request_id = resource.auditable_request_id() resource_type = resource.auditable_resource_type() display_name = resource.auditable_displayname() + previous_role_id = previous_state["role_id"] audit_event = AuditEvent( user_id=user_id, @@ -26,6 +29,7 @@ class AuditableMixin(object): resource_id=resource.id, display_name=display_name, action=action, + previous_role_id=previous_role_id, ) audit_event.save(connection) @@ -48,7 +52,14 @@ class AuditableMixin(object): @staticmethod def audit_update(mapper, connection, target): - target.create_audit_event(connection, target, ACTION_UPDATE) + previous_state = {} + inspr = inspect(target) + attrs = class_mapper(target.__class__).column_attrs + for attr in attrs: + history = getattr(inspr.attrs, attr.key).history + if history.has_changes(): + previous_state[attr.key] = get_history(target, attr.key)[2].pop() + target.create_audit_event(connection, target, ACTION_UPDATE, previous_state) def auditable_resource_type(self): return camel_to_snake(type(self).__name__) From 3b6d745955117ea1c6ec6db657c76d081545724f Mon Sep 17 00:00:00 2001 From: Montana Date: Mon, 12 Nov 2018 11:43:35 -0500 Subject: [PATCH 03/20] Add audit event attributes to workspace role model --- atst/models/audit_event.py | 7 +++- atst/models/mixins/auditable.py | 68 +++++++++++++++++++++++++++------ atst/models/workspace_role.py | 11 ++++++ 3 files changed, 74 insertions(+), 12 deletions(-) diff --git a/atst/models/audit_event.py b/atst/models/audit_event.py index 0c6403b9..0fc0494f 100644 --- a/atst/models/audit_event.py +++ b/atst/models/audit_event.py @@ -1,6 +1,7 @@ from sqlalchemy import String, Column, ForeignKey, inspect -from sqlalchemy.dialects.postgresql import UUID +from sqlalchemy.dialects.postgresql import UUID, JSON from sqlalchemy.orm import relationship +from atst.database import db from atst.models import Base, types from atst.models.mixins.timestamps import TimestampsMixin @@ -20,11 +21,15 @@ class AuditEvent(Base, TimestampsMixin): request_id = Column(UUID(as_uuid=True), ForeignKey("requests.id"), index=True) request = relationship("Request", backref="audit_events") + changed_state = Column(JSON()) + event_details = Column(JSON()) + resource_type = Column(String(), nullable=False) resource_id = Column(UUID(as_uuid=True), index=True, nullable=False) display_name = Column(String()) action = Column(String(), nullable=False) + def save(self, connection): attrs = inspect(self).dict diff --git a/atst/models/mixins/auditable.py b/atst/models/mixins/auditable.py index 10b252ac..642dd11a 100644 --- a/atst/models/mixins/auditable.py +++ b/atst/models/mixins/auditable.py @@ -3,6 +3,8 @@ from flask import g from sqlalchemy.orm import class_mapper from sqlalchemy.orm.attributes import get_history +from atst.database import db +from atst.models.workspace_role import WorkspaceRole from atst.models.audit_event import AuditEvent from atst.utils import camel_to_snake, getattr_path @@ -11,15 +13,48 @@ ACTION_UPDATE = "update" ACTION_DELETE = "delete" +class AuditableWorkspaceRole(AuditableMixin): + target = target + changed_state = get_changed_state() + event_details = get_event_details() + + @classmethod + def get_changed_state(self, resource): + previous_state = {} + inspr = inspect(target) + attrs = class_mapper(target.__class__).column_attrs + for attr in attrs: + history = getattr(inspr.attrs, attr.key).history + if history.has_changes(): + previous_state[attr.key] = get_history(target, attr.key)[2].pop() + + from_role = previous_role["role"] + to_role = target.role_displayname + return {"role": [from_role, to_role]} + + @classmethod + def get_event_details(self): + # get user that is being updated + # does this need to query the db? + ws_role_id = target.auditable_request_id() + ws_role = ( + db.session.query(WorkspaceRole) + .filter(WorkspaceRole.id == ws_role_id) + .first() + ) + return {"user": ws_role.user_name} + + class AuditableMixin(object): @staticmethod - def create_audit_event(connection, resource, action, previous_state=None): + def create_audit_event(connection, resource, action): user_id = getattr_path(g, "current_user.id") workspace_id = resource.auditable_workspace_id() request_id = resource.auditable_request_id() resource_type = resource.auditable_resource_type() display_name = resource.auditable_displayname() - previous_role_id = previous_state["role_id"] + changed_state = resource.auditable_changed_state() + event_details = resource.auditable_event_details() audit_event = AuditEvent( user_id=user_id, @@ -29,7 +64,8 @@ class AuditableMixin(object): resource_id=resource.id, display_name=display_name, action=action, - previous_role_id=previous_role_id, + changed_state=changed_state, + event_details=event_details, ) audit_event.save(connection) @@ -40,6 +76,17 @@ class AuditableMixin(object): event.listen(cls, "after_delete", cls.audit_delete) event.listen(cls, "after_update", cls.audit_update) + @staticmethod + def get_history(target): + previous_state = {} + inspr = inspect(target) + attrs = class_mapper(target.__class__).column_attrs + for attr in attrs: + history = getattr(inspr.attrs, attr.key).history + if history.has_changes(): + previous_state[attr.key] = get_history(target, attr.key)[2].pop() + return previous_state + @staticmethod def audit_insert(mapper, connection, target): """Listen for the `after_insert` event and create an AuditLog entry""" @@ -52,14 +99,13 @@ class AuditableMixin(object): @staticmethod def audit_update(mapper, connection, target): - previous_state = {} - inspr = inspect(target) - attrs = class_mapper(target.__class__).column_attrs - for attr in attrs: - history = getattr(inspr.attrs, attr.key).history - if history.has_changes(): - previous_state[attr.key] = get_history(target, attr.key)[2].pop() - target.create_audit_event(connection, target, ACTION_UPDATE, previous_state) + target.create_audit_event(connection, target, ACTION_UPDATE) + + def auditable_changed_state(self): + return getattr_path(self, "history") + + def auditable_event_details(self): + return getattr_path(self, "auditable_event_details") def auditable_resource_type(self): return camel_to_snake(type(self).__name__) diff --git a/atst/models/workspace_role.py b/atst/models/workspace_role.py index c006b87a..2942e448 100644 --- a/atst/models/workspace_role.py +++ b/atst/models/workspace_role.py @@ -41,6 +41,17 @@ class WorkspaceRole(Base, mixins.TimestampsMixin, mixins.AuditableMixin): self.role.name, self.workspace.name, self.user_id, self.id ) + @property + def history(self): + previous_state = AuditableMixin.get_history(self) + from_role = previous_role["role"] + to_role = self.role_displayname + return {"role": [from_role, to_role]} + + @property + def auditable_event_details(self): + return {"updated_user": self.user_name, "updated_user_id" = self.user_id} + @property def latest_invitation(self): if self.invitations: From 82868728bae856631c5866229b71bef7bc4a732b Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Mon, 12 Nov 2018 16:00:02 -0500 Subject: [PATCH 04/20] Fix errors from last commit --- atst/models/mixins/auditable.py | 36 +-------------------------------- atst/models/workspace_role.py | 12 ++++++----- 2 files changed, 8 insertions(+), 40 deletions(-) diff --git a/atst/models/mixins/auditable.py b/atst/models/mixins/auditable.py index 642dd11a..9606b2d7 100644 --- a/atst/models/mixins/auditable.py +++ b/atst/models/mixins/auditable.py @@ -3,8 +3,6 @@ from flask import g from sqlalchemy.orm import class_mapper from sqlalchemy.orm.attributes import get_history -from atst.database import db -from atst.models.workspace_role import WorkspaceRole from atst.models.audit_event import AuditEvent from atst.utils import camel_to_snake, getattr_path @@ -13,38 +11,6 @@ ACTION_UPDATE = "update" ACTION_DELETE = "delete" -class AuditableWorkspaceRole(AuditableMixin): - target = target - changed_state = get_changed_state() - event_details = get_event_details() - - @classmethod - def get_changed_state(self, resource): - previous_state = {} - inspr = inspect(target) - attrs = class_mapper(target.__class__).column_attrs - for attr in attrs: - history = getattr(inspr.attrs, attr.key).history - if history.has_changes(): - previous_state[attr.key] = get_history(target, attr.key)[2].pop() - - from_role = previous_role["role"] - to_role = target.role_displayname - return {"role": [from_role, to_role]} - - @classmethod - def get_event_details(self): - # get user that is being updated - # does this need to query the db? - ws_role_id = target.auditable_request_id() - ws_role = ( - db.session.query(WorkspaceRole) - .filter(WorkspaceRole.id == ws_role_id) - .first() - ) - return {"user": ws_role.user_name} - - class AuditableMixin(object): @staticmethod def create_audit_event(connection, resource, action): @@ -105,7 +71,7 @@ class AuditableMixin(object): return getattr_path(self, "history") def auditable_event_details(self): - return getattr_path(self, "auditable_event_details") + return getattr_path(self, "event_details") def auditable_resource_type(self): return camel_to_snake(type(self).__name__) diff --git a/atst/models/workspace_role.py b/atst/models/workspace_role.py index 2942e448..b536179a 100644 --- a/atst/models/workspace_role.py +++ b/atst/models/workspace_role.py @@ -10,6 +10,7 @@ from atst.database import db from atst.models.environment_role import EnvironmentRole from atst.models.project import Project from atst.models.environment import Environment +from atst.models.role import Role class Status(Enum): @@ -43,14 +44,15 @@ class WorkspaceRole(Base, mixins.TimestampsMixin, mixins.AuditableMixin): @property def history(self): - previous_state = AuditableMixin.get_history(self) - from_role = previous_role["role"] + previous_state = mixins.AuditableMixin.get_history(self) + from_role_id = previous_state["role_id"] + from_role = db.session.query(Role).filter(Role.id == from_role_id).one() to_role = self.role_displayname - return {"role": [from_role, to_role]} + return {"role": [from_role.display_name, to_role]} @property - def auditable_event_details(self): - return {"updated_user": self.user_name, "updated_user_id" = self.user_id} + def event_details(self): + return {"updated_user": self.user_name, "updated_user_id": str(self.user_id)} @property def latest_invitation(self): From adad5adfa63876da5bae06ce5f8b2776af12ea19 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Mon, 12 Nov 2018 16:20:30 -0500 Subject: [PATCH 05/20] Update template and add migration --- .../4f46aecb337f_add_columns_to_auditevent.py | 30 +++++++++++++++++++ atst/models/audit_event.py | 2 -- templates/audit_log.html | 10 +++++++ 3 files changed, 40 insertions(+), 2 deletions(-) create mode 100644 alembic/versions/4f46aecb337f_add_columns_to_auditevent.py diff --git a/alembic/versions/4f46aecb337f_add_columns_to_auditevent.py b/alembic/versions/4f46aecb337f_add_columns_to_auditevent.py new file mode 100644 index 00000000..03548f42 --- /dev/null +++ b/alembic/versions/4f46aecb337f_add_columns_to_auditevent.py @@ -0,0 +1,30 @@ +"""Add columns to AuditEvent + +Revision ID: 4f46aecb337f +Revises: 4c0b8263d800 +Create Date: 2018-11-12 16:03:55.281648 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision = '4f46aecb337f' +down_revision = '4c0b8263d800' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('audit_events', sa.Column('changed_state', postgresql.JSON(astext_type=sa.Text()), nullable=True)) + op.add_column('audit_events', sa.Column('event_details', postgresql.JSON(astext_type=sa.Text()), nullable=True)) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column('audit_events', 'event_details') + op.drop_column('audit_events', 'changed_state') + # ### end Alembic commands ### diff --git a/atst/models/audit_event.py b/atst/models/audit_event.py index 0fc0494f..0e60dcdc 100644 --- a/atst/models/audit_event.py +++ b/atst/models/audit_event.py @@ -1,7 +1,6 @@ from sqlalchemy import String, Column, ForeignKey, inspect from sqlalchemy.dialects.postgresql import UUID, JSON from sqlalchemy.orm import relationship -from atst.database import db from atst.models import Base, types from atst.models.mixins.timestamps import TimestampsMixin @@ -29,7 +28,6 @@ class AuditEvent(Base, TimestampsMixin): display_name = Column(String()) action = Column(String(), nullable=False) - def save(self, connection): attrs = inspect(self).dict diff --git a/templates/audit_log.html b/templates/audit_log.html index 8455276f..5e143aa9 100644 --- a/templates/audit_log.html +++ b/templates/audit_log.html @@ -28,6 +28,16 @@
+ {% if event.event_details %} + for User {{ event.event_details.updated_user_id }} ({{ event.event_details.updated_user }}) +
+ {% endif %} + + {% if event.changed_state %} + from {{ event.changed_state.role[0] }} to {{ event.changed_state.role[1] }} +
+ {% endif %} + {% if event.workspace %} in Workspace {{ event.workspace_id }} ({{ event.workspace.name }}) {% elif event.request %} From e51de11a2878094d260c3f2da0b006f8831a75cd Mon Sep 17 00:00:00 2001 From: Montana Date: Tue, 13 Nov 2018 11:21:12 -0500 Subject: [PATCH 06/20] capture status and no history cases on workspace role model --- atst/models/workspace_role.py | 13 +++++++++---- tests/factories.py | 1 + 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/atst/models/workspace_role.py b/atst/models/workspace_role.py index b536179a..9627c58a 100644 --- a/atst/models/workspace_role.py +++ b/atst/models/workspace_role.py @@ -45,10 +45,15 @@ class WorkspaceRole(Base, mixins.TimestampsMixin, mixins.AuditableMixin): @property def history(self): previous_state = mixins.AuditableMixin.get_history(self) - from_role_id = previous_state["role_id"] - from_role = db.session.query(Role).filter(Role.id == from_role_id).one() - to_role = self.role_displayname - return {"role": [from_role.display_name, to_role]} + auditable_previous_state = {} + if "role_id" in previous_state: + from_role_id = previous_state["role_id"] + from_role = db.session.query(Role).filter(Role.id == from_role_id).one() + to_role = self.role_displayname + auditable_previous_state["role"] = [from_role.display_name, to_role] + if "status" in previous_state: + auditable_previous_state["status"]= previous_state["status"].value + return auditable_previous_state @property def event_details(self): diff --git a/tests/factories.py b/tests/factories.py index 8d62a22d..7c7bdfd8 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -326,6 +326,7 @@ class WorkspaceRoleFactory(Base): workspace = factory.SubFactory(WorkspaceFactory) role = factory.SubFactory(RoleFactory) user = factory.SubFactory(UserFactory) + status = WorkspaceRoleStatus.PENDING class EnvironmentRoleFactory(Base): From 3f6e13f33d07c5bdb980ef02c4f2f4ac70378147 Mon Sep 17 00:00:00 2001 From: Montana Date: Tue, 13 Nov 2018 15:06:24 -0500 Subject: [PATCH 07/20] Refactor code duplication --- atst/domain/workspace_roles.py | 4 ++-- atst/domain/workspaces/workspaces.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/atst/domain/workspace_roles.py b/atst/domain/workspace_roles.py index f6942e2f..9da3224c 100644 --- a/atst/domain/workspace_roles.py +++ b/atst/domain/workspace_roles.py @@ -101,9 +101,9 @@ class WorkspaceRoles(object): return new_workspace_role @classmethod - def update_role(cls, member, workspace_id, role_name): + def update_role(cls, member, role_name): new_role = Roles.get(role_name) - workspace_role = WorkspaceRoles._get_workspace_role(member.user, workspace_id) + workspace_role = WorkspaceRoles._get_workspace_role(member.user, member.workspace_id) workspace_role.role = new_role db.session.add(workspace_role) diff --git a/atst/domain/workspaces/workspaces.py b/atst/domain/workspaces/workspaces.py index 453a1eed..6c43bc3b 100644 --- a/atst/domain/workspaces/workspaces.py +++ b/atst/domain/workspaces/workspaces.py @@ -119,7 +119,7 @@ class Workspaces(object): "edit workspace member", ) - return WorkspaceRoles.update_role(member, workspace.id, role_name) + return WorkspaceRoles.update_role(member, role_name) @classmethod def _create_workspace_role( From 88eda7cd929dabee1baab087316e088dfe8dfd72 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 13 Nov 2018 17:16:27 -0500 Subject: [PATCH 08/20] Update test assertion --- tests/models/test_workspace_role.py | 44 +++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/tests/models/test_workspace_role.py b/tests/models/test_workspace_role.py index 3622e88a..6418b72e 100644 --- a/tests/models/test_workspace_role.py +++ b/tests/models/test_workspace_role.py @@ -14,6 +14,50 @@ from tests.factories import ( ) +def test_has_no_history(session): + owner = UserFactory.create() + user = UserFactory.create() + + workspace = Workspaces.create(RequestFactory.create(creator=owner)) + workspace_role = WorkspaceRoles.add(user, workspace.id, "developer") + audit_events = ( + session.query(AuditEvent) + .filter(AuditEvent.resource_id == workspace_role.id) + .all() + ) + + create_event = [event for event in audit_events if event.action == "create"] + assert not create_event[0].changed_state + + +def test_has_history(session): + owner = UserFactory.create() + user = UserFactory.create() + + workspace = Workspaces.create(RequestFactory.create(creator=owner)) + workspace_role = WorkspaceRoles.add(user, workspace.id, "developer") + WorkspaceRoles.update_role(workspace_role, "admin") + audit_events = ( + session.query(AuditEvent) + .filter(AuditEvent.resource_id == workspace_role.id) + .all() + ) + changed_events = [event for event in audit_events if event.changed_state] + + assert changed_events[0].changed_state["role"] + + +def test_event_details(): + owner = UserFactory.create() + user = UserFactory.create() + + workspace = Workspaces.create(RequestFactory.create(creator=owner)) + workspace_role = WorkspaceRoles.add(user, workspace.id, "developer") + + assert workspace_role.event_details["updated_user"] == user.displayname + assert workspace_role.event_details["updated_user_id"] == str(user.id) + + def test_has_no_environment_roles(): owner = UserFactory.create() developer_data = { From b9033367079462ae00f67808771e9ad06491a081 Mon Sep 17 00:00:00 2001 From: Montana Date: Wed, 14 Nov 2018 10:55:06 -0500 Subject: [PATCH 09/20] Add comment to mysterious function --- atst/models/mixins/auditable.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/atst/models/mixins/auditable.py b/atst/models/mixins/auditable.py index 9606b2d7..3019fe7f 100644 --- a/atst/models/mixins/auditable.py +++ b/atst/models/mixins/auditable.py @@ -44,6 +44,16 @@ class AuditableMixin(object): @staticmethod def get_history(target): + """ + This function borrows largely from a gist: + https://gist.github.com/ngse/c20058116b8044c65d3fbceda3fdf423#file-audit_mixin-py-L106-L120 + + It returns a dictionary of the form {item: from_value}, + where 'item' is the attribute on the target that has been updated + and 'from_value' is the value of the attribute before it was updated. + + There may be more than one item in the dictionary, but that is not expected. + """ previous_state = {} inspr = inspect(target) attrs = class_mapper(target.__class__).column_attrs From f35d0add5505794a5628f69b576fde1bb5c61b87 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Wed, 14 Nov 2018 16:57:34 -0500 Subject: [PATCH 10/20] Minor fixes --- atst/models/audit_event.py | 6 +++--- atst/models/workspace_role.py | 15 ++++++++++----- templates/audit_log.html | 2 +- tests/models/test_workspace_role.py | 16 +++++++++------- 4 files changed, 23 insertions(+), 16 deletions(-) diff --git a/atst/models/audit_event.py b/atst/models/audit_event.py index 0e60dcdc..dd5f94a2 100644 --- a/atst/models/audit_event.py +++ b/atst/models/audit_event.py @@ -1,5 +1,5 @@ from sqlalchemy import String, Column, ForeignKey, inspect -from sqlalchemy.dialects.postgresql import UUID, JSON +from sqlalchemy.dialects.postgresql import UUID, JSONB from sqlalchemy.orm import relationship from atst.models import Base, types @@ -20,8 +20,8 @@ class AuditEvent(Base, TimestampsMixin): request_id = Column(UUID(as_uuid=True), ForeignKey("requests.id"), index=True) request = relationship("Request", backref="audit_events") - changed_state = Column(JSON()) - event_details = Column(JSON()) + changed_state = Column(JSONB()) + event_details = Column(JSONB()) resource_type = Column(String(), nullable=False) resource_id = Column(UUID(as_uuid=True), index=True, nullable=False) diff --git a/atst/models/workspace_role.py b/atst/models/workspace_role.py index 9627c58a..b23563a1 100644 --- a/atst/models/workspace_role.py +++ b/atst/models/workspace_role.py @@ -45,19 +45,24 @@ class WorkspaceRole(Base, mixins.TimestampsMixin, mixins.AuditableMixin): @property def history(self): previous_state = mixins.AuditableMixin.get_history(self) - auditable_previous_state = {} + change_set = {} if "role_id" in previous_state: from_role_id = previous_state["role_id"] from_role = db.session.query(Role).filter(Role.id == from_role_id).one() to_role = self.role_displayname - auditable_previous_state["role"] = [from_role.display_name, to_role] + change_set["role"] = [from_role.display_name, to_role] if "status" in previous_state: - auditable_previous_state["status"]= previous_state["status"].value - return auditable_previous_state + from_status = previous_state["status"].value + to_status = self.status.value + change_set["status"] = [from_status, to_status] + return change_set @property def event_details(self): - return {"updated_user": self.user_name, "updated_user_id": str(self.user_id)} + return { + "updated_user_name": self.user_name, + "updated_user_id": str(self.user_id), + } @property def latest_invitation(self): diff --git a/templates/audit_log.html b/templates/audit_log.html index 5e143aa9..5e679dfe 100644 --- a/templates/audit_log.html +++ b/templates/audit_log.html @@ -29,7 +29,7 @@
{% if event.event_details %} - for User {{ event.event_details.updated_user_id }} ({{ event.event_details.updated_user }}) + for User {{ event.event_details.updated_user_id }} ({{ event.event_details.updated_user_name }})
{% endif %} diff --git a/tests/models/test_workspace_role.py b/tests/models/test_workspace_role.py index 6418b72e..476ce09f 100644 --- a/tests/models/test_workspace_role.py +++ b/tests/models/test_workspace_role.py @@ -20,14 +20,15 @@ def test_has_no_history(session): workspace = Workspaces.create(RequestFactory.create(creator=owner)) workspace_role = WorkspaceRoles.add(user, workspace.id, "developer") - audit_events = ( + create_event = ( session.query(AuditEvent) - .filter(AuditEvent.resource_id == workspace_role.id) - .all() + .filter( + AuditEvent.resource_id == workspace_role.id, AuditEvent.action == "create" + ) + .one() ) - create_event = [event for event in audit_events if event.action == "create"] - assert not create_event[0].changed_state + assert not create_event.changed_state def test_has_history(session): @@ -44,7 +45,8 @@ def test_has_history(session): ) changed_events = [event for event in audit_events if event.changed_state] - assert changed_events[0].changed_state["role"] + assert changed_events[0].changed_state["role"][0] + assert changed_events[0].changed_state["role"][1] def test_event_details(): @@ -54,7 +56,7 @@ def test_event_details(): workspace = Workspaces.create(RequestFactory.create(creator=owner)) workspace_role = WorkspaceRoles.add(user, workspace.id, "developer") - assert workspace_role.event_details["updated_user"] == user.displayname + assert workspace_role.event_details["updated_user_name"] == user.displayname assert workspace_role.event_details["updated_user_id"] == str(user.id) From 49ed02ad041bfd4efaa031a6868ea2af0f2322d2 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Wed, 14 Nov 2018 17:21:08 -0500 Subject: [PATCH 11/20] wip - Add tests for status change --- tests/models/test_workspace_role.py | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/tests/models/test_workspace_role.py b/tests/models/test_workspace_role.py index 476ce09f..4fd10f9d 100644 --- a/tests/models/test_workspace_role.py +++ b/tests/models/test_workspace_role.py @@ -31,13 +31,34 @@ def test_has_no_history(session): assert not create_event.changed_state -def test_has_history(session): +def test_has_role_history(session): owner = UserFactory.create() user = UserFactory.create() workspace = Workspaces.create(RequestFactory.create(creator=owner)) workspace_role = WorkspaceRoles.add(user, workspace.id, "developer") WorkspaceRoles.update_role(workspace_role, "admin") + changed_events = ( + session.query(AuditEvent) + .filter(AuditEvent.resource_id == workspace_role.id, AuditEvent.changed_state != None) + .all() + ) + + assert changed_events[0].changed_state["role"][0] + assert changed_events[0].changed_state["role"][1] + + +def test_has_status_history(session): + owner = UserFactory.create() + user = UserFactory.create() + + workspace = Workspaces.create(RequestFactory.create(creator=owner)) + workspace_role = WorkspaceRoles.add(user, workspace.id, "developer") + session.add(workspace_role) + session.commit() + workspace_role.status = Status.ACTIVE + session.add(workspace_role) + session.commit() audit_events = ( session.query(AuditEvent) .filter(AuditEvent.resource_id == workspace_role.id) @@ -45,8 +66,10 @@ def test_has_history(session): ) changed_events = [event for event in audit_events if event.changed_state] - assert changed_events[0].changed_state["role"][0] - assert changed_events[0].changed_state["role"][1] + import ipdb; ipdb.set_trace() + + assert changed_events[0].changed_state["status"][0] + assert changed_events[0].changed_state["status"][1] def test_event_details(): From c071945b3b6e5213396f397f78cee8ad8d1c4d77 Mon Sep 17 00:00:00 2001 From: Montana Date: Thu, 15 Nov 2018 10:30:21 -0500 Subject: [PATCH 12/20] Spelling error and better name use --- atst/models/workspace_role.py | 4 ++-- tests/domain/test_audit_log.py | 2 +- tests/models/test_workspace_role.py | 5 +++-- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/atst/models/workspace_role.py b/atst/models/workspace_role.py index b23563a1..671271c2 100644 --- a/atst/models/workspace_role.py +++ b/atst/models/workspace_role.py @@ -49,8 +49,8 @@ class WorkspaceRole(Base, mixins.TimestampsMixin, mixins.AuditableMixin): if "role_id" in previous_state: from_role_id = previous_state["role_id"] from_role = db.session.query(Role).filter(Role.id == from_role_id).one() - to_role = self.role_displayname - change_set["role"] = [from_role.display_name, to_role] + to_role = self.role_name + change_set["role"] = [from_role.name, to_role] if "status" in previous_state: from_status = previous_state["status"].value to_status = self.status.value diff --git a/tests/domain/test_audit_log.py b/tests/domain/test_audit_log.py index ab03c48c..3e000dde 100644 --- a/tests/domain/test_audit_log.py +++ b/tests/domain/test_audit_log.py @@ -20,7 +20,7 @@ def test_non_admin_cannot_view_audit_log(developer): AuditLog.get_all_events(developer) -def test_ccpo_can_iview_audit_log(ccpo): +def test_ccpo_can_view_audit_log(ccpo): AuditLog.get_all_events(ccpo) diff --git a/tests/models/test_workspace_role.py b/tests/models/test_workspace_role.py index 4fd10f9d..d0b19c0c 100644 --- a/tests/models/test_workspace_role.py +++ b/tests/models/test_workspace_role.py @@ -44,8 +44,9 @@ def test_has_role_history(session): .all() ) - assert changed_events[0].changed_state["role"][0] - assert changed_events[0].changed_state["role"][1] + # changed_state["role"] returns a list [previous role, current role] + assert changed_events[0].changed_state["role"][0] == 'developer' + assert changed_events[0].changed_state["role"][1] == 'admin' def test_has_status_history(session): From 8b172ba3e2665c337a708a5be6dd59ea2f9582a7 Mon Sep 17 00:00:00 2001 From: Montana Date: Fri, 16 Nov 2018 14:18:18 -0500 Subject: [PATCH 13/20] wip for safety --- atst/domain/workspace_roles.py | 7 +++-- atst/models/mixins/auditable.py | 43 ++++++++++++++--------------- atst/models/workspace_role.py | 13 ++++++--- tests/models/test_workspace_role.py | 35 ++++++++++++----------- 4 files changed, 53 insertions(+), 45 deletions(-) diff --git a/atst/domain/workspace_roles.py b/atst/domain/workspace_roles.py index 9da3224c..60b3f4c4 100644 --- a/atst/domain/workspace_roles.py +++ b/atst/domain/workspace_roles.py @@ -153,7 +153,10 @@ class WorkspaceRoles(object): @classmethod def enable(cls, workspace_role): - workspace_role.status = WorkspaceRoleStatus.ACTIVE + ws_role = WorkspaceRoles._get_workspace_role( + workspace_role.user, workspace_role.workspace_id + ) + ws_role.status = WorkspaceRoleStatus.ACTIVE - db.session.add(workspace_role) + db.session.add(ws_role) db.session.commit() diff --git a/atst/models/mixins/auditable.py b/atst/models/mixins/auditable.py index 3019fe7f..8aee817a 100644 --- a/atst/models/mixins/auditable.py +++ b/atst/models/mixins/auditable.py @@ -1,7 +1,5 @@ from sqlalchemy import event, inspect from flask import g -from sqlalchemy.orm import class_mapper -from sqlalchemy.orm.attributes import get_history from atst.models.audit_event import AuditEvent from atst.utils import camel_to_snake, getattr_path @@ -42,27 +40,6 @@ class AuditableMixin(object): event.listen(cls, "after_delete", cls.audit_delete) event.listen(cls, "after_update", cls.audit_update) - @staticmethod - def get_history(target): - """ - This function borrows largely from a gist: - https://gist.github.com/ngse/c20058116b8044c65d3fbceda3fdf423#file-audit_mixin-py-L106-L120 - - It returns a dictionary of the form {item: from_value}, - where 'item' is the attribute on the target that has been updated - and 'from_value' is the value of the attribute before it was updated. - - There may be more than one item in the dictionary, but that is not expected. - """ - previous_state = {} - inspr = inspect(target) - attrs = class_mapper(target.__class__).column_attrs - for attr in attrs: - history = getattr(inspr.attrs, attr.key).history - if history.has_changes(): - previous_state[attr.key] = get_history(target, attr.key)[2].pop() - return previous_state - @staticmethod def audit_insert(mapper, connection, target): """Listen for the `after_insert` event and create an AuditLog entry""" @@ -77,6 +54,26 @@ class AuditableMixin(object): def audit_update(mapper, connection, target): target.create_audit_event(connection, target, ACTION_UPDATE) + def get_changes(self): + """ + This function borrows largely from a gist: + https://gist.github.com/ngse/c20058116b8044c65d3fbceda3fdf423#file-audit_mixin-py-L106-L120 + + It returns a dictionary of the form {item: [from_value, to_value]}, + where 'item' is the attribute on the target that has been updated, + 'from_value' is the value of the attribute before it was updated, + and 'to_value' is the current value of the attribute. + + There may be more than one item in the dictionary, but that is not expected. + """ + previous_state = {} + attrs = inspect(self).mapper.column_attrs + for attr in attrs: + history = getattr(inspect(self).attrs, attr.key).history + if history.has_changes(): + previous_state[attr.key] = [history.deleted.pop(), history.added.pop()] + return previous_state + def auditable_changed_state(self): return getattr_path(self, "history") diff --git a/atst/models/workspace_role.py b/atst/models/workspace_role.py index 671271c2..5fd85807 100644 --- a/atst/models/workspace_role.py +++ b/atst/models/workspace_role.py @@ -35,7 +35,9 @@ class WorkspaceRole(Base, mixins.TimestampsMixin, mixins.AuditableMixin): UUID(as_uuid=True), ForeignKey("users.id"), index=True, nullable=False ) - status = Column(SQLAEnum(Status, native_enum=False, default=Status.PENDING)) + status = Column( + SQLAEnum(Status, native_enum=False, default=Status.PENDING), nullable=False + ) def __repr__(self): return "".format( @@ -44,15 +46,18 @@ class WorkspaceRole(Base, mixins.TimestampsMixin, mixins.AuditableMixin): @property def history(self): - previous_state = mixins.AuditableMixin.get_history(self) + previous_state = self.get_changes() change_set = {} if "role_id" in previous_state: - from_role_id = previous_state["role_id"] + from_role_id = previous_state["role_id"][0] from_role = db.session.query(Role).filter(Role.id == from_role_id).one() to_role = self.role_name change_set["role"] = [from_role.name, to_role] if "status" in previous_state: - from_status = previous_state["status"].value + if previous_state["status"][0]: + from_status = previous_state["status"][0].value + else: + from_status = "pending" to_status = self.status.value change_set["status"] = [from_status, to_status] return change_set diff --git a/tests/models/test_workspace_role.py b/tests/models/test_workspace_role.py index d0b19c0c..b36d4b33 100644 --- a/tests/models/test_workspace_role.py +++ b/tests/models/test_workspace_role.py @@ -37,16 +37,19 @@ def test_has_role_history(session): workspace = Workspaces.create(RequestFactory.create(creator=owner)) workspace_role = WorkspaceRoles.add(user, workspace.id, "developer") + # why not use workspace_role = WorkspaceRoleFactory.create()? WorkspaceRoles.update_role(workspace_role, "admin") changed_events = ( session.query(AuditEvent) - .filter(AuditEvent.resource_id == workspace_role.id, AuditEvent.changed_state != None) + .filter( + AuditEvent.resource_id == workspace_role.id, + AuditEvent.changed_state != None, + ) .all() ) - # changed_state["role"] returns a list [previous role, current role] - assert changed_events[0].changed_state["role"][0] == 'developer' - assert changed_events[0].changed_state["role"][1] == 'admin' + assert changed_events[0].changed_state["role"][0] == "developer" + assert changed_events[0].changed_state["role"][1] == "admin" def test_has_status_history(session): @@ -55,22 +58,22 @@ def test_has_status_history(session): workspace = Workspaces.create(RequestFactory.create(creator=owner)) workspace_role = WorkspaceRoles.add(user, workspace.id, "developer") - session.add(workspace_role) - session.commit() - workspace_role.status = Status.ACTIVE - session.add(workspace_role) - session.commit() - audit_events = ( + import ipdb + + ipdb.set_trace() + WorkspaceRoles.enable(workspace_role) + changed_events = ( session.query(AuditEvent) - .filter(AuditEvent.resource_id == workspace_role.id) + .filter( + AuditEvent.resource_id == workspace_role.id, + AuditEvent.changed_state != None, + ) .all() ) - changed_events = [event for event in audit_events if event.changed_state] - import ipdb; ipdb.set_trace() - - assert changed_events[0].changed_state["status"][0] - assert changed_events[0].changed_state["status"][1] + # changed_state["status"] returns a list [previous status, current status] + assert changed_events[0].changed_state["status"][0] == "pending" + assert changed_events[0].changed_state["status"][1] == "active" def test_event_details(): From e73690986a940b64c56e1439be8a39753bffe3c2 Mon Sep 17 00:00:00 2001 From: Montana Date: Fri, 16 Nov 2018 14:52:59 -0500 Subject: [PATCH 14/20] Set default workspace_role status --- atst/models/workspace_role.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/atst/models/workspace_role.py b/atst/models/workspace_role.py index 5fd85807..30cebf9f 100644 --- a/atst/models/workspace_role.py +++ b/atst/models/workspace_role.py @@ -36,8 +36,7 @@ class WorkspaceRole(Base, mixins.TimestampsMixin, mixins.AuditableMixin): ) status = Column( - SQLAEnum(Status, native_enum=False, default=Status.PENDING), nullable=False - ) + SQLAEnum(Status, native_enum=False), default=Status.PENDING) def __repr__(self): return "".format( @@ -54,10 +53,7 @@ class WorkspaceRole(Base, mixins.TimestampsMixin, mixins.AuditableMixin): to_role = self.role_name change_set["role"] = [from_role.name, to_role] if "status" in previous_state: - if previous_state["status"][0]: - from_status = previous_state["status"][0].value - else: - from_status = "pending" + from_status = previous_state["status"][0].value to_status = self.status.value change_set["status"] = [from_status, to_status] return change_set From 3b2594283ffe380dbb34873818e3e3026622cd65 Mon Sep 17 00:00:00 2001 From: Montana Date: Fri, 16 Nov 2018 15:34:42 -0500 Subject: [PATCH 15/20] Use WorkspaceRoleFactory in tests --- tests/models/test_workspace_role.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/tests/models/test_workspace_role.py b/tests/models/test_workspace_role.py index b36d4b33..28c1bc22 100644 --- a/tests/models/test_workspace_role.py +++ b/tests/models/test_workspace_role.py @@ -5,6 +5,7 @@ from atst.domain.workspaces import Workspaces from atst.domain.projects import Projects from atst.domain.workspace_roles import WorkspaceRoles from atst.models.workspace_role import Status +from atst.models.role import Role from atst.models.invitation import Status as InvitationStatus from tests.factories import ( RequestFactory, @@ -36,8 +37,8 @@ def test_has_role_history(session): user = UserFactory.create() workspace = Workspaces.create(RequestFactory.create(creator=owner)) - workspace_role = WorkspaceRoles.add(user, workspace.id, "developer") - # why not use workspace_role = WorkspaceRoleFactory.create()? + role = session.query(Role).filter(Role.name == 'developer').one() + workspace_role = WorkspaceRoleFactory.create(workspace=workspace, user=user, role=role) WorkspaceRoles.update_role(workspace_role, "admin") changed_events = ( session.query(AuditEvent) @@ -57,10 +58,7 @@ def test_has_status_history(session): user = UserFactory.create() workspace = Workspaces.create(RequestFactory.create(creator=owner)) - workspace_role = WorkspaceRoles.add(user, workspace.id, "developer") - import ipdb - - ipdb.set_trace() + workspace_role = WorkspaceRoleFactory.create(workspace=workspace, user=user) WorkspaceRoles.enable(workspace_role) changed_events = ( session.query(AuditEvent) From 67a6fe4556a758c2fe11a912ad9a7e6388b784c6 Mon Sep 17 00:00:00 2001 From: Montana Date: Tue, 20 Nov 2018 10:52:16 -0500 Subject: [PATCH 16/20] Make sure the WorkspaceRoleFactory is only flushing to the DB --- atst/domain/workspace_roles.py | 10 +++------- tests/models/test_workspace_role.py | 6 +++++- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/atst/domain/workspace_roles.py b/atst/domain/workspace_roles.py index 60b3f4c4..ff9604fa 100644 --- a/atst/domain/workspace_roles.py +++ b/atst/domain/workspace_roles.py @@ -101,9 +101,8 @@ class WorkspaceRoles(object): return new_workspace_role @classmethod - def update_role(cls, member, role_name): + def update_role(cls, workspace_role, role_name): new_role = Roles.get(role_name) - workspace_role = WorkspaceRoles._get_workspace_role(member.user, member.workspace_id) workspace_role.role = new_role db.session.add(workspace_role) @@ -153,10 +152,7 @@ class WorkspaceRoles(object): @classmethod def enable(cls, workspace_role): - ws_role = WorkspaceRoles._get_workspace_role( - workspace_role.user, workspace_role.workspace_id - ) - ws_role.status = WorkspaceRoleStatus.ACTIVE + workspace_role.status = WorkspaceRoleStatus.ACTIVE - db.session.add(ws_role) + db.session.add(workspace_role) db.session.commit() diff --git a/tests/models/test_workspace_role.py b/tests/models/test_workspace_role.py index 28c1bc22..3be8f7c3 100644 --- a/tests/models/test_workspace_role.py +++ b/tests/models/test_workspace_role.py @@ -38,6 +38,7 @@ def test_has_role_history(session): workspace = Workspaces.create(RequestFactory.create(creator=owner)) role = session.query(Role).filter(Role.name == 'developer').one() + WorkspaceRoleFactory._meta.sqlalchemy_session_persistence = 'flush' workspace_role = WorkspaceRoleFactory.create(workspace=workspace, user=user, role=role) WorkspaceRoles.update_role(workspace_role, "admin") changed_events = ( @@ -53,18 +54,21 @@ def test_has_role_history(session): assert changed_events[0].changed_state["role"][1] == "admin" + def test_has_status_history(session): owner = UserFactory.create() user = UserFactory.create() workspace = Workspaces.create(RequestFactory.create(creator=owner)) + # in order to get the history, we don't want the WorkspaceRoleFactory to commit + WorkspaceRoleFactory._meta.sqlalchemy_session_persistence = 'flush' workspace_role = WorkspaceRoleFactory.create(workspace=workspace, user=user) WorkspaceRoles.enable(workspace_role) changed_events = ( session.query(AuditEvent) .filter( AuditEvent.resource_id == workspace_role.id, - AuditEvent.changed_state != None, + AuditEvent.action == 'update', ) .all() ) From 8cc9a2177034a9feca83ee305527541a5a99385b Mon Sep 17 00:00:00 2001 From: Montana Date: Wed, 21 Nov 2018 13:02:25 -0500 Subject: [PATCH 17/20] Fix audit event query --- tests/models/test_workspace_role.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/models/test_workspace_role.py b/tests/models/test_workspace_role.py index 3be8f7c3..796814db 100644 --- a/tests/models/test_workspace_role.py +++ b/tests/models/test_workspace_role.py @@ -38,6 +38,8 @@ def test_has_role_history(session): workspace = Workspaces.create(RequestFactory.create(creator=owner)) role = session.query(Role).filter(Role.name == 'developer').one() + # in order to get the history, we don't want the WorkspaceRoleFactory + # to commit after create() WorkspaceRoleFactory._meta.sqlalchemy_session_persistence = 'flush' workspace_role = WorkspaceRoleFactory.create(workspace=workspace, user=user, role=role) WorkspaceRoles.update_role(workspace_role, "admin") @@ -45,7 +47,7 @@ def test_has_role_history(session): session.query(AuditEvent) .filter( AuditEvent.resource_id == workspace_role.id, - AuditEvent.changed_state != None, + AuditEvent.action == 'update', ) .all() ) @@ -60,7 +62,8 @@ def test_has_status_history(session): user = UserFactory.create() workspace = Workspaces.create(RequestFactory.create(creator=owner)) - # in order to get the history, we don't want the WorkspaceRoleFactory to commit + # in order to get the history, we don't want the WorkspaceRoleFactory + # to commit after create() WorkspaceRoleFactory._meta.sqlalchemy_session_persistence = 'flush' workspace_role = WorkspaceRoleFactory.create(workspace=workspace, user=user) WorkspaceRoles.enable(workspace_role) From 78f3495c3fa5c699691c2b622b13850da9b5606f Mon Sep 17 00:00:00 2001 From: Montana Date: Wed, 21 Nov 2018 13:15:45 -0500 Subject: [PATCH 18/20] formatting --- atst/models/workspace_role.py | 3 +-- tests/models/test_workspace_role.py | 17 ++++++++--------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/atst/models/workspace_role.py b/atst/models/workspace_role.py index 30cebf9f..21d17a90 100644 --- a/atst/models/workspace_role.py +++ b/atst/models/workspace_role.py @@ -35,8 +35,7 @@ class WorkspaceRole(Base, mixins.TimestampsMixin, mixins.AuditableMixin): UUID(as_uuid=True), ForeignKey("users.id"), index=True, nullable=False ) - status = Column( - SQLAEnum(Status, native_enum=False), default=Status.PENDING) + status = Column(SQLAEnum(Status, native_enum=False), default=Status.PENDING) def __repr__(self): return "".format( diff --git a/tests/models/test_workspace_role.py b/tests/models/test_workspace_role.py index 796814db..dd77db93 100644 --- a/tests/models/test_workspace_role.py +++ b/tests/models/test_workspace_role.py @@ -37,17 +37,18 @@ def test_has_role_history(session): user = UserFactory.create() workspace = Workspaces.create(RequestFactory.create(creator=owner)) - role = session.query(Role).filter(Role.name == 'developer').one() + role = session.query(Role).filter(Role.name == "developer").one() # in order to get the history, we don't want the WorkspaceRoleFactory # to commit after create() - WorkspaceRoleFactory._meta.sqlalchemy_session_persistence = 'flush' - workspace_role = WorkspaceRoleFactory.create(workspace=workspace, user=user, role=role) + WorkspaceRoleFactory._meta.sqlalchemy_session_persistence = "flush" + workspace_role = WorkspaceRoleFactory.create( + workspace=workspace, user=user, role=role + ) WorkspaceRoles.update_role(workspace_role, "admin") changed_events = ( session.query(AuditEvent) .filter( - AuditEvent.resource_id == workspace_role.id, - AuditEvent.action == 'update', + AuditEvent.resource_id == workspace_role.id, AuditEvent.action == "update" ) .all() ) @@ -56,7 +57,6 @@ def test_has_role_history(session): assert changed_events[0].changed_state["role"][1] == "admin" - def test_has_status_history(session): owner = UserFactory.create() user = UserFactory.create() @@ -64,14 +64,13 @@ def test_has_status_history(session): workspace = Workspaces.create(RequestFactory.create(creator=owner)) # in order to get the history, we don't want the WorkspaceRoleFactory # to commit after create() - WorkspaceRoleFactory._meta.sqlalchemy_session_persistence = 'flush' + WorkspaceRoleFactory._meta.sqlalchemy_session_persistence = "flush" workspace_role = WorkspaceRoleFactory.create(workspace=workspace, user=user) WorkspaceRoles.enable(workspace_role) changed_events = ( session.query(AuditEvent) .filter( - AuditEvent.resource_id == workspace_role.id, - AuditEvent.action == 'update', + AuditEvent.resource_id == workspace_role.id, AuditEvent.action == "update" ) .all() ) From 7532dff97832e727a263aa378aab427bc589a8ab Mon Sep 17 00:00:00 2001 From: Montana Date: Wed, 21 Nov 2018 13:47:11 -0500 Subject: [PATCH 19/20] Re-enable system log events --- atst/domain/audit_log.py | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/atst/domain/audit_log.py b/atst/domain/audit_log.py index 1c15916a..35609c13 100644 --- a/atst/domain/audit_log.py +++ b/atst/domain/audit_log.py @@ -53,16 +53,7 @@ class AuditLog(object): return type(resource).__name__.lower() @classmethod - def _log( - cls, - user=None, - updated_user=None, - workspace_id=None, - resource=None, - action=None, - previous_role_id=None, - new_role_id=None, - ): + def _log(cls, user=None, workspace_id=None, resource=None, action=None): resource_id = resource.id if resource else None resource_type = cls._resource_type(resource) if resource else None @@ -74,6 +65,9 @@ class AuditLog(object): resource_type=resource_type, previous_role_id=previous_role_id, new_role_id=new_role_id, + workspace_id=workspace_id, + resource_id=resource_id, + resource_type=resource_type, action=action, ) return AuditEventQuery.add_and_commit(audit_event) From 21827891f7642df1b99a2ce9cfa8b1790e70ecad Mon Sep 17 00:00:00 2001 From: Montana Date: Wed, 21 Nov 2018 14:36:17 -0500 Subject: [PATCH 20/20] Fixups from rebase --- atst/domain/audit_log.py | 29 ----------------------------- tests/models/test_workspace_role.py | 1 + 2 files changed, 1 insertion(+), 29 deletions(-) diff --git a/atst/domain/audit_log.py b/atst/domain/audit_log.py index 35609c13..a334cc7d 100644 --- a/atst/domain/audit_log.py +++ b/atst/domain/audit_log.py @@ -14,29 +14,6 @@ class AuditEventQuery(Query): class AuditLog(object): - @classmethod - def log_event(cls, user, resource, action): - return cls._log(user=user, resource=resource, action=action) - - @classmethod - def log_workspace_event(cls, user, workspace, resource, action): - return cls._log( - user=user, workspace_id=workspace.id, resource=resource, action=action - ) - - @classmethod - def log_update_workspace_role( - cls, action, user, updated_user, workspace, previous_role, new_role - ): - return cls._log( - action=action, - user=user, - updated_user=updated_user, - workspace_id=workspace.id, - previous_role_id=previous_role.id, - new_role_id=new_role.id, - ) - @classmethod def log_system_event(cls, resource, action): return cls._log(resource=resource, action=action) @@ -59,12 +36,6 @@ class AuditLog(object): audit_event = AuditEventQuery.create( user=user, - updated_user=updated_user, - workspace_id=workspace_id, - resource_id=resource_id, - resource_type=resource_type, - previous_role_id=previous_role_id, - new_role_id=new_role_id, workspace_id=workspace_id, resource_id=resource_id, resource_type=resource_type, diff --git a/tests/models/test_workspace_role.py b/tests/models/test_workspace_role.py index dd77db93..d59b85cc 100644 --- a/tests/models/test_workspace_role.py +++ b/tests/models/test_workspace_role.py @@ -7,6 +7,7 @@ from atst.domain.workspace_roles import WorkspaceRoles from atst.models.workspace_role import Status from atst.models.role import Role from atst.models.invitation import Status as InvitationStatus +from atst.models.audit_event import AuditEvent from tests.factories import ( RequestFactory, UserFactory,