Skip to content

Commit

Permalink
Feature/443 user audit fields (#603)
Browse files Browse the repository at this point in the history
* Add user audit fields.

* fix broken test.

* display N/a when modify values are null or empty.
  • Loading branch information
john-labbate authored Jul 23, 2024
1 parent 63b9f3b commit c86b281
Show file tree
Hide file tree
Showing 12 changed files with 185 additions and 16 deletions.
31 changes: 31 additions & 0 deletions alembic/versions/291331bea272_user_audit_fields.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
"""user-audit-fields
Revision ID: 291331bea272
Revises: a5fbdcd0e719
Create Date: 2024-07-19 09:06:27.754024
"""
from alembic import op
import sqlalchemy as sa


# revision identifiers, used by Alembic.
revision = '291331bea272'
down_revision = 'a5fbdcd0e719'
branch_labels = None
depends_on = None


def upgrade() -> None:
op.add_column('users', sa.Column('created_on', sa.DateTime, nullable=False, server_default=sa.func.current_timestamp()))
op.add_column('users', sa.Column('created_by', sa.String(), nullable=False, server_default='Migrated'))
op.alter_column('users', 'created_by', server_default=None)
op.add_column('users', sa.Column('modified_on', sa.DateTime, nullable=True))
op.add_column('users', sa.Column('modified_by', sa.String(), nullable=True))


def downgrade() -> None:
op.drop_column('users', 'created_on')
op.drop_column('users', 'created_by')
op.drop_column('users', 'modified_on')
op.drop_column('users', 'modified_by')
1 change: 0 additions & 1 deletion data/seedsdata.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -966,7 +966,6 @@ AOPSs:
email: [email protected]
agency: Native American Tribal Governments
bureau: Sierra Tribal Consortium, INC.
bureau: null
reporting_agencies:
- report_agency: Native American Tribal Governments
report_bureau: Sierra Tribal Consortium, INC.
Expand Down
87 changes: 87 additions & 0 deletions training-front-end/src/components/AdminEditReporting.vue
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,41 @@ async function updateUser(successMessage) {
function cancelUpdate() {
editing.value = false;
}
function formatDate(dateStr) {
// Check if the string is null, undefined, or empty
if (!dateStr || dateStr.trim() === '') {
return 'N/A';
}
const date = new Date(dateStr);
// Check if the date is valid
if (isNaN(date.getTime())) {
return 'N/A';
}
//doing it this way to reduce dependancies
const monthNames = [
'January', 'February', 'March', 'April', 'May', 'June',
'July', 'August', 'September', 'October', 'November', 'December'
];
const month = monthNames[date.getMonth()];
const day = date.getDate();
const year = date.getFullYear();
return `${month} ${day}, ${year}`;
}
function formatString(str) {
// Check if the string is null, undefined, or empty
if (!str || str.trim() === '') {
return 'N/A';
}
return str;
}
</script>

<template>
Expand Down Expand Up @@ -90,6 +125,20 @@ function cancelUpdate() {
{{ user.name }}
</dd>
</div>
<div class="tablet:grid-col">
<dt class="font-sans-xs">
Created By
</dt>
<dd
id="user-created-by-value"
:aria-label="'Created By: ' + user.created_by"
class="margin-left-0 text-bold font-sans-sm"
>
{{ user.created_by }}
</dd>
</div>
</div>
<div class="grid-row grid-gap padding-top-2">
<div class="tablet:grid-col">
<dt class="font-sans-xs">
Email
Expand All @@ -102,6 +151,18 @@ function cancelUpdate() {
{{ user.email }}
</dd>
</div>
<div class="tablet:grid-col">
<dt class="font-sans-xs">
Created On
</dt>
<dd
id="user-created-on-value"
:aria-label="'Created On: ' + formatDate(user.created_on)"
class="margin-left-0 text-bold font-sans-sm"
>
{{ formatDate(user.created_on) }}
</dd>
</div>
</div>
<div class="grid-row grid-gap padding-top-2">
<div class="tablet:grid-col">
Expand All @@ -116,6 +177,20 @@ function cancelUpdate() {
{{ user.agency.name }}
</dd>
</div>
<div class="tablet:grid-col">
<dt class="font-sans-xs">
Last Modified By
</dt>
<dd
id="user-modified-by-value"
:aria-label="'Last Modified By: ' + formatString(user.modified_by)"
class="margin-left-0 text-bold font-sans-sm"
>
{{ formatString(user.modified_by) }}
</dd>
</div>
</div>
<div class="grid-row grid-gap padding-top-2">
<div class="tablet:grid-col">
<dt class="font-sans-xs">
Sub-Agency, Organization, or Bureau
Expand All @@ -128,6 +203,18 @@ function cancelUpdate() {
{{ user.agency.bureau }}
</dd>
</div>
<div class="tablet:grid-col">
<dt class="font-sans-xs">
Last Modified On
</dt>
<dd
id="user-modified-on-value"
:aria-label="'Last Modified On: ' + formatDate(user.modified_on)"
class="margin-left-0 text-bold font-sans-sm"
>
{{ formatDate(user.modified_on) }}
</dd>
</div>
</div>
<div class="margin-top-3">
<button
Expand Down
7 changes: 4 additions & 3 deletions training/api/api_v1/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ def edit_user_for_reporting(
user=Depends(RequireRole(["Admin"]))
):
try:
updated_user = repo.edit_user_for_reporting(user_id, agency_id_list)
updated_user = repo.edit_user_for_reporting(user_id, agency_id_list, user['name'])
logging.info(f"{user['email']} granted user {updated_user.email} reporting for agencies: {agency_id_list}")
return updated_user
return User.model_validate(updated_user)
except ValueError:
raise HTTPException(
status_code=status.HTTP_400_BAD_REQUEST,
Expand Down Expand Up @@ -109,7 +109,8 @@ def update_user_by_id(
)
try:
logging.info(f"{user['email']} updated user {updated_user.email} user profile")
return repo.update_user(user_id, updated_user)
db_user = repo.update_user(user_id, updated_user, user["name"])
return User.model_validate(db_user)
except ValueError:
raise HTTPException(
status_code=status.HTTP_400_BAD_REQUEST,
Expand Down
4 changes: 3 additions & 1 deletion training/api/deps.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from training.services import QuizService, GspcService
from training.database import SessionLocal
from sqlalchemy.orm import Session
import logging


def db() -> Generator[Session, None, None]:
Expand All @@ -15,7 +16,8 @@ def db() -> Generator[Session, None, None]:
try:
yield session
session.commit()
except Exception:
except Exception as e:
logging.error(f"Error in DB session: {e}")
session.rollback()
finally:
session.close()
Expand Down
7 changes: 6 additions & 1 deletion training/models/user.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
from training.models import Base
from sqlalchemy.orm import Mapped, mapped_column
from sqlalchemy import ForeignKey
from sqlalchemy import ForeignKey, DateTime, func
from sqlalchemy.orm import relationship
from training.models.agency import Agency
from training.models.role import Role
from datetime import datetime


class User(Base):
Expand All @@ -16,3 +17,7 @@ class User(Base):
agency: Mapped[Agency] = relationship()
roles: Mapped[list[Role]] = relationship(secondary="users_x_roles")
report_agencies: Mapped[list[Agency]] = relationship(secondary="report_users_x_agencies")
created_on: Mapped[datetime] = mapped_column(DateTime(timezone=True), default=func.now(), nullable=False)
created_by: Mapped[str] = mapped_column(nullable=False)
modified_on: Mapped[datetime] = mapped_column(DateTime(timezone=True))
modified_by: Mapped[str] = mapped_column()
11 changes: 8 additions & 3 deletions training/repositories/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from training import models, schemas
from training.schemas import UserQuizCompletionReportData, UserSearchResult
from .base import BaseRepository
from datetime import datetime


class UserRepository(BaseRepository[models.User]):
Expand All @@ -11,15 +12,15 @@ def __init__(self, session: Session):
super().__init__(session, models.User)

def create(self, user: schemas.UserCreate) -> models.User:
return self.save(models.User(email=user.email.lower(), name=user.name, agency_id=user.agency_id))
return self.save(models.User(email=user.email.lower(), name=user.name, agency_id=user.agency_id, created_by=user.name))

def find_by_email(self, email: str) -> models.User | None:
return self._session.query(models.User).filter(models.User.email == email.lower()).first()

def find_by_agency(self, agency_id: int) -> list[models.User]:
return self._session.query(models.User).filter(models.User.agency_id == agency_id).all()

def edit_user_for_reporting(self, user_id: int, report_agencies_list: list[int]) -> models.User:
def edit_user_for_reporting(self, user_id: int, report_agencies_list: list[int], modified_by: str) -> models.User:
# edit_user_for_reporting allow admin to assign report role and associate report agencies to specific user
db_user = self._session.query(models.User).filter(models.User.id == user_id).first()
if db_user is None:
Expand Down Expand Up @@ -47,6 +48,8 @@ def edit_user_for_reporting(self, user_id: int, report_agencies_list: list[int])
db_user.report_agencies.append(agency)
else:
raise ValueError("invalid agency associated with this user")
db_user.modified_by = modified_by
db_user.modified_on = datetime.now()
self._session.commit()
return db_user

Expand Down Expand Up @@ -77,7 +80,7 @@ def get_users(self, searchText: str, page_number: int) -> UserSearchResult:
user_search_result = UserSearchResult(users=search_results, total_count=count)
return user_search_result

def update_user(self, user_id: int, user: schemas.UserUpdate) -> models.User:
def update_user(self, user_id: int, user: schemas.UserUpdate, modified_by: str) -> models.User:
"""
Updates user name and agency values
:param user_id: User's ID to update
Expand All @@ -89,5 +92,7 @@ def update_user(self, user_id: int, user: schemas.UserUpdate) -> models.User:
raise ValueError("invalid user id")
db_user.name = user.name
db_user.agency_id = user.agency_id
db_user.modified_by = modified_by
db_user.modified_on = datetime.now()
self._session.commit()
return db_user
22 changes: 22 additions & 0 deletions training/schemas/user.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
from pydantic import ConfigDict, BaseModel, EmailStr, field_validator
from training.schemas.agency import Agency
from training.schemas.role import Role
from typing import Optional
from datetime import datetime


class UserBase(BaseModel):
Expand All @@ -23,13 +25,23 @@ class User(UserBase):
agency: Agency
roles: list[Role]
report_agencies: list[Agency]
created_on: str
created_by: str
modified_on: Optional[str] = None
modified_by: Optional[str] = None

def is_admin(self) -> bool:
role_names = [role.name.upper() for role in self.roles]
return "Admin".upper() in role_names

model_config = ConfigDict(from_attributes=True)

@field_validator('created_on', 'modified_on', mode='before')
def convert_datetime(cls, input):
if isinstance(input, datetime):
return input.isoformat()
return input


class UserJWT(User):
# Provides a user object that is appropriate for encoding into a JWT.
Expand All @@ -45,4 +57,14 @@ def convert_roles(cls, input) -> list[str]:
class UserSearchResult(BaseModel):
users: list[User]
total_count: int

@field_validator('users', mode='before')
def convert_user_datetimes(cls, input):
for user in input:
if isinstance(user.created_on, datetime):
user.created_on = user.created_on.isoformat()
if user.modified_on and isinstance(user.modified_on, datetime):
user.modified_on = user.modified_on.isoformat()
return input

model_config = ConfigDict(from_attributes=True)
2 changes: 1 addition & 1 deletion training/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ def db_with_data(db: Session, testdata: dict):
agency_ids.append(agency.id)

for index, user in enumerate(testdata["users"]):
user = models.User(email=user["email"], name=user["name"], agency_id=agency_ids[index % 2])
user = models.User(email=user["email"], name=user["name"], agency_id=agency_ids[index % 2], created_by=user["created_by"])
db.add(user)
db.commit()
db.refresh(user)
Expand Down
6 changes: 4 additions & 2 deletions training/tests/test_api_loginless_flow.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def fake_user_repo():
@pytest.fixture
def user_complete():
return {"name": "Stephen Dedalus", "email": "[email protected]", "agency_id": 3, "agency": {"name": 'test name', "id": 3, "bureau": None}, "roles": [],
"report_agencies": []}
"report_agencies": [], "created_on": "2024-07-19T13:46:23.004418", "created_by": "Stephen Dedalus"}


@pytest.fixture
Expand All @@ -61,7 +61,9 @@ def authorized_complete():
agency_id=3,
agency=Agency(id=3, name='test name', bureau="test"),
roles=[Role(id=1, name='Wizard')],
report_agencies=[]
report_agencies=[],
created_on="2024-07-19T13:46:23.004418",
created_by="Stephen Dedalus"
)


Expand Down
Loading

0 comments on commit c86b281

Please sign in to comment.