From e152c07f65bc2e19945d0f549bf78f4e63c8ab5c Mon Sep 17 00:00:00 2001 From: gulimabr Date: Tue, 2 Dec 2025 10:00:42 -0300 Subject: [PATCH] Changed user logic to display name and refined requirements page design --- backend/src/db_models.py | 2 + backend/src/main.py | 28 ++++++-- .../requirement_link_repository.py | 11 ++- backend/src/repositories/user_repository.py | 36 ++++++++-- backend/src/service.py | 13 ++-- frontend/src/pages/RequirementDetailPage.tsx | 32 ++++++--- frontend/src/pages/RequirementsPage.tsx | 72 ++++++++++--------- 7 files changed, 129 insertions(+), 65 deletions(-) diff --git a/backend/src/db_models.py b/backend/src/db_models.py index 37486bf..8603e76 100644 --- a/backend/src/db_models.py +++ b/backend/src/db_models.py @@ -32,6 +32,8 @@ class User(Base): id: Mapped[int] = mapped_column(Integer, primary_key=True, autoincrement=True) sub: Mapped[str] = mapped_column(Text, nullable=False, unique=True) # Keycloak subject ID + username: Mapped[str] = mapped_column(Text, nullable=False) # Keycloak preferred_username + full_name: Mapped[Optional[str]] = mapped_column(Text, nullable=True) # Keycloak name claim role_id: Mapped[int] = mapped_column(Integer, ForeignKey("roles.id"), nullable=False) created_at: Mapped[datetime] = mapped_column( DateTime(timezone=True), diff --git a/backend/src/main.py b/backend/src/main.py index 6704562..333c602 100644 --- a/backend/src/main.py +++ b/backend/src/main.py @@ -314,6 +314,20 @@ def _require_role(user, allowed_role_ids: List[int], action: str = "perform this ) +def _get_display_name(user) -> str: + """ + Get the best display name for a user. + Falls back in order: full_name -> username -> sub. + + Args: + user: The database user object + + Returns: + The best available display name for the user + """ + return user.full_name or user.username or user.sub + + async def _verify_project_membership(project_id: int, user_id: int, db: AsyncSession): """Helper to verify user is a member of a project.""" project_repo = ProjectRepository(db) @@ -570,7 +584,7 @@ async def get_project_members( return [ ProjectMemberResponse( id=member.id, - sub=member.sub, + sub=_get_display_name(member), role_id=member.role_id, role_name=member.role.role_name if member.role else "unknown", role_display_name=ROLE_DISPLAY_NAMES.get(member.role.role_name, member.role.role_name.title()) if member.role else "Unknown", @@ -647,7 +661,7 @@ async def update_member_role( return ProjectMemberResponse( id=updated_user.id, - sub=updated_user.sub, + sub=_get_display_name(updated_user), role_id=updated_user.role_id, role_name=role.role_name, role_display_name=ROLE_DISPLAY_NAMES.get(role.role_name, role.role_name.title()), @@ -772,9 +786,9 @@ def _build_requirement_response(req) -> RequirementResponse: # Get the latest validation latest_validation = max(req.validations, key=lambda v: v.created_at or req.created_at) validation_status = latest_validation.status.status_name if latest_validation.status else "Not Validated" - # Try to get username from user relationship + # Try to get display name from user relationship if latest_validation.user: - validated_by = latest_validation.user.sub + validated_by = _get_display_name(latest_validation.user) validated_at = latest_validation.created_at validation_version = latest_validation.req_version_snapshot @@ -1112,7 +1126,7 @@ async def create_validation( req_version_snapshot=validation.req_version_snapshot, comment=validation.comment, created_at=validation.created_at, - validator_username=user.sub, + validator_username=_get_display_name(user), validator_id=user.id ) @@ -1159,7 +1173,7 @@ async def get_validation_history( req_version_snapshot=v.req_version_snapshot, comment=v.comment, created_at=v.created_at, - validator_username=v.user.sub, + validator_username=_get_display_name(v.user), validator_id=v.user_id ) for v in validations @@ -1416,7 +1430,7 @@ async def create_requirement_link( "req_name": target_req.req_name, "tag_code": target_req.tag.tag_code }, - created_by_username=user.sub, + created_by_username=_get_display_name(user), created_by_id=user.id, created_at=link.created_at ) diff --git a/backend/src/repositories/requirement_link_repository.py b/backend/src/repositories/requirement_link_repository.py index bd7ed61..1a68023 100644 --- a/backend/src/repositories/requirement_link_repository.py +++ b/backend/src/repositories/requirement_link_repository.py @@ -8,6 +8,13 @@ from sqlalchemy.ext.asyncio import AsyncSession from src.db_models import RequirementLink, Requirement, RelationshipType, User +def _get_creator_display_name(user: User | None) -> str | None: + """Get the best display name for a creator user.""" + if not user: + return None + return user.full_name or user.username or user.sub + + class RequirementLinkRepository: """Repository for requirement link CRUD operations.""" @@ -66,7 +73,7 @@ class RequirementLinkRepository: "req_name": link.target_requirement.req_name, "tag_code": link.target_requirement.tag.tag_code }, - "created_by_username": link.creator.sub if link.creator else None, + "created_by_username": _get_creator_display_name(link.creator), "created_by_id": link.created_by, "created_at": link.created_at }) @@ -83,7 +90,7 @@ class RequirementLinkRepository: "req_name": link.source_requirement.req_name, "tag_code": link.source_requirement.tag.tag_code }, - "created_by_username": link.creator.sub if link.creator else None, + "created_by_username": _get_creator_display_name(link.creator), "created_by_id": link.created_by, "created_at": link.created_at }) diff --git a/backend/src/repositories/user_repository.py b/backend/src/repositories/user_repository.py index 1777f95..1b4c619 100644 --- a/backend/src/repositories/user_repository.py +++ b/backend/src/repositories/user_repository.py @@ -111,18 +111,20 @@ class UserRepository: ) return result.scalar_one_or_none() - async def create(self, sub: str, role_id: int) -> User: + async def create(self, sub: str, role_id: int, username: str, full_name: str | None = None) -> User: """ Create a new user. Args: sub: The Keycloak subject ID role_id: The role ID to assign + username: The Keycloak preferred_username + full_name: The Keycloak name claim (optional) Returns: The created User """ - user = User(sub=sub, role_id=role_id) + user = User(sub=sub, role_id=role_id, username=username, full_name=full_name) self.session.add(user) await self.session.flush() await self.session.refresh(user) @@ -148,6 +150,25 @@ class UserRepository: await self.session.refresh(user) return user + async def update_profile(self, user: User, username: str, full_name: str | None = None) -> User: + """ + Update a user's profile info (username and full_name) from Keycloak. + Called on subsequent logins to sync changes from Keycloak. + + Args: + user: The user to update + username: The Keycloak preferred_username + full_name: The Keycloak name claim (optional) + + Returns: + The updated User + """ + user.username = username + user.full_name = full_name + await self.session.flush() + await self.session.refresh(user) + return user + async def get_or_create_default_role(self) -> Role: """ Get the default user role, creating it if it doesn't exist. @@ -164,13 +185,16 @@ class UserRepository: return role - async def get_or_create_user(self, sub: str) -> tuple[User, bool]: + async def get_or_create_user(self, sub: str, username: str, full_name: str | None = None) -> tuple[User, bool]: """ Get an existing user or create a new one (Just-in-Time Provisioning). This is the main method called during login. + Also updates username/full_name on subsequent logins to sync with Keycloak. Args: sub: The Keycloak subject ID + username: The Keycloak preferred_username + full_name: The Keycloak name claim (optional) Returns: Tuple of (User, created) where created is True if a new user was created @@ -180,6 +204,8 @@ class UserRepository: if user is not None: logger.debug(f"Found existing user with sub: {sub}") + # Update profile info on subsequent logins to sync with Keycloak + user = await self.update_profile(user, username, full_name) return user, False # User doesn't exist, create them with default role @@ -189,7 +215,7 @@ class UserRepository: default_role = await self.get_or_create_default_role() # Create the user - user = await self.create(sub=sub, role_id=default_role.id) + user = await self.create(sub=sub, role_id=default_role.id, username=username, full_name=full_name) - logger.info(f"Created new user with id: {user.id}, sub: {sub}") + logger.info(f"Created new user with id: {user.id}, sub: {sub}, username: {username}") return user, True diff --git a/backend/src/service.py b/backend/src/service.py index 508ee60..7c999ef 100644 --- a/backend/src/service.py +++ b/backend/src/service.py @@ -126,6 +126,7 @@ class UserService: ) -> tuple[int, bool]: """ Provision a user in the database on first login (JIT provisioning). + Also updates username/full_name on subsequent logins to sync with Keycloak. Args: token: The access token from Keycloak @@ -134,9 +135,11 @@ class UserService: Returns: Tuple of (user_id, is_new_user) """ - # Decode the token to get the 'sub' claim + # Decode the token to get user claims token_info = AuthService.decode_token(token) sub = token_info.get("sub") + username = token_info.get("preferred_username", "unknown") + full_name = token_info.get("name") # Keycloak uses 'name' claim for full name if not sub: raise HTTPException( @@ -144,13 +147,13 @@ class UserService: detail="Token does not contain 'sub' claim" ) - # Get or create the user + # Get or create the user (will also update profile on subsequent logins) user_repo = UserRepository(db) - user, created = await user_repo.get_or_create_user(sub) + user, created = await user_repo.get_or_create_user(sub, username, full_name) if created: - logger.info(f"New user provisioned: {sub} -> user_id: {user.id}") + logger.info(f"New user provisioned: {sub} -> user_id: {user.id}, username: {username}") else: - logger.debug(f"Existing user logged in: {sub} -> user_id: {user.id}") + logger.debug(f"Existing user logged in: {sub} -> user_id: {user.id}, username: {username}") return user.id, created diff --git a/frontend/src/pages/RequirementDetailPage.tsx b/frontend/src/pages/RequirementDetailPage.tsx index 243aae8..0537ff0 100644 --- a/frontend/src/pages/RequirementDetailPage.tsx +++ b/frontend/src/pages/RequirementDetailPage.tsx @@ -710,20 +710,30 @@ export default function RequirementDetailPage() { {/* Title */}

{requirement.req_name}

- {/* Group Badges */} + {/* Group Chips/Tags */}
{requirement.groups.length > 0 ? ( - requirement.groups.map(group => ( - - {group.group_name} - - )) + <> + {requirement.groups.slice(0, 3).map(group => ( + + {group.group_name} + + ))} + {requirement.groups.length > 3 && ( + g.group_name).join(', ')} + > + +{requirement.groups.length - 3} more + + )} + ) : ( - + No groups )} diff --git a/frontend/src/pages/RequirementsPage.tsx b/frontend/src/pages/RequirementsPage.tsx index 3762a07..c87a5b0 100644 --- a/frontend/src/pages/RequirementsPage.tsx +++ b/frontend/src/pages/RequirementsPage.tsx @@ -22,21 +22,6 @@ const getValidationStatusStyle = (status: string): { bgColor: string; textColor: } } -// Helper to lighten a hex color for backgrounds -function lightenColor(hex: string, percent: number): string { - const num = parseInt(hex.replace('#', ''), 16) - const amt = Math.round(2.55 * percent) - const R = (num >> 16) + amt - const G = (num >> 8 & 0x00FF) + amt - const B = (num & 0x0000FF) + amt - return '#' + ( - 0x1000000 + - (R < 255 ? (R < 1 ? 0 : R) : 255) * 0x10000 + - (G < 255 ? (G < 1 ? 0 : G) : 255) * 0x100 + - (B < 255 ? (B < 1 ? 0 : B) : 255) - ).toString(16).slice(1) -} - export default function RequirementsPage() { const { user, logout, isAuditor } = useAuth() const { currentProject, isLoading: projectLoading } = useProject() @@ -188,14 +173,6 @@ export default function RequirementsPage() { navigate(`/requirements/${id}`) } - // Get the primary group color for a requirement (first group or default) - const getRequirementColor = (req: Requirement): string => { - if (req.groups.length > 0) { - return req.groups[0].hex_color - } - return '#6B7280' // default gray - } - // Modal functions const openCreateModal = () => { setShowCreateModal(true) @@ -444,8 +421,6 @@ export default function RequirementsPage() { {/* Requirements List */}
{sortedRequirements.map((req) => { - const primaryColor = getRequirementColor(req) - const bgColor = lightenColor(primaryColor, 60) const tagLabel = req.tag.tag_code const priorityName = req.priority?.priority_name ?? 'None' const validationStatus = req.validation_status || 'Not Validated' @@ -455,27 +430,54 @@ export default function RequirementsPage() { return (
- {/* Colored tag section */} -
+ {/* Tag and name section */} +
{tagLabel} - {req.req_name}
+ {/* Group chips */} +
+
+ {req.groups.length > 0 ? ( + <> + {req.groups.slice(0, 2).map(group => ( + + {group.group_name} + + ))} + {req.groups.length > 2 && ( + g.group_name).join(', ')} + > + +{req.groups.length - 2} more + + )} + + ) : ( + + No groups + + )} +
+
+ {/* Validation status */} -
+
{validationStatus} {isStale && ( - + ⚠ Stale )} @@ -488,7 +490,7 @@ export default function RequirementsPage() {
{/* Priority and Version */} -
+

Priority: {priorityName}

Version: {req.version}