From cfc33f418be9b7be58b8e33e2939fd0125f5f710 Mon Sep 17 00:00:00 2001 From: Hamed Valiollahi Bayeki Date: Mon, 9 Dec 2024 15:45:01 -0800 Subject: [PATCH] feat: ensure alternate email is saved in user profile --- .../versions/2024-12-09-22-33_cd8698fe40e6.py | 34 +++++++++++++++++++ backend/lcfs/db/models/user/UserProfile.py | 3 -- backend/lcfs/tests/user/test_user_repo.py | 14 ++++---- backend/lcfs/tests/user/test_user_views.py | 7 ++-- backend/lcfs/tests/user/user_payloads.py | 1 - backend/lcfs/web/api/user/repo.py | 5 ++- backend/lcfs/web/api/user/schema.py | 5 ++- backend/lcfs/web/api/user/services.py | 9 ++--- backend/lcfs/web/api/user/views.py | 27 +++++---------- frontend/src/constants/routes/apiRoutes.js | 2 +- .../components/BCeIDNotificationSettings.jsx | 2 +- .../components/NotificationSettingsForm.jsx | 19 +++++------ 12 files changed, 69 insertions(+), 59 deletions(-) create mode 100644 backend/lcfs/db/migrations/versions/2024-12-09-22-33_cd8698fe40e6.py diff --git a/backend/lcfs/db/migrations/versions/2024-12-09-22-33_cd8698fe40e6.py b/backend/lcfs/db/migrations/versions/2024-12-09-22-33_cd8698fe40e6.py new file mode 100644 index 000000000..d812b2a94 --- /dev/null +++ b/backend/lcfs/db/migrations/versions/2024-12-09-22-33_cd8698fe40e6.py @@ -0,0 +1,34 @@ +"""Remove notifications email from user_profile + +Revision ID: cd8698fe40e6 +Revises: 26ab15f8ab18 +Create Date: 2024-12-09 22:33:29.554360 + +""" + +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision = "cd8698fe40e6" +down_revision = "26ab15f8ab18" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + # Remove notifications_email column from user_profile table + op.drop_column("user_profile", "notifications_email") + + +def downgrade() -> None: + # Add notifications_email column to user_profile table + op.add_column( + "user_profile", + sa.Column( + "notifications_email", + sa.String(length=255), + nullable=True, + comment="Email address used for notifications", + ), + ) diff --git a/backend/lcfs/db/models/user/UserProfile.py b/backend/lcfs/db/models/user/UserProfile.py index 681d19aa0..7fecca2f8 100644 --- a/backend/lcfs/db/models/user/UserProfile.py +++ b/backend/lcfs/db/models/user/UserProfile.py @@ -26,9 +26,6 @@ class UserProfile(BaseModel, Auditable): String(150), unique=True, nullable=False, comment="keycloak Username" ) email = Column(String(255), nullable=True, comment="Primary email address") - notifications_email = Column( - String(255), nullable=True, comment="Email address used for notifications" - ) title = Column(String(100), nullable=True, comment="Professional Title") phone = Column(String(50), nullable=True, comment="Primary phone number") mobile_phone = Column(String(50), nullable=True, comment="Mobile phone number") diff --git a/backend/lcfs/tests/user/test_user_repo.py b/backend/lcfs/tests/user/test_user_repo.py index 56cb75403..a96788933 100644 --- a/backend/lcfs/tests/user/test_user_repo.py +++ b/backend/lcfs/tests/user/test_user_repo.py @@ -1,11 +1,10 @@ -from unittest.mock import AsyncMock, Mock +from unittest.mock import Mock import pytest from lcfs.db.models import UserProfile, UserLoginHistory from lcfs.web.api.user.repo import UserRepository from lcfs.tests.user.user_payloads import user_orm_model -from lcfs.web.exception.exceptions import DataNotFoundException @pytest.fixture @@ -50,14 +49,13 @@ async def test_create_login_history(dbsession, user_repo): @pytest.mark.anyio -async def test_update_notifications_email_success(dbsession, user_repo): +async def test_update_email_success(dbsession, user_repo): # Arrange: Create a user in the database user = UserProfile( keycloak_user_id="user_id_1", keycloak_email="user1@domain.com", keycloak_username="username1", email="user1@domain.com", - notifications_email=None, title="Developer", phone="1234567890", mobile_phone="0987654321", @@ -70,10 +68,10 @@ async def test_update_notifications_email_success(dbsession, user_repo): await dbsession.commit() await dbsession.refresh(user) - # Act: Update the notifications email - updated_user = await user_repo.update_notifications_email( + # Act: Update the email + updated_user = await user_repo.update_email( user_profile_id=1, email="new_email@domain.com" ) - # Assert: Check if the notifications email was updated - assert updated_user.notifications_email == "new_email@domain.com" + # Assert: Check if the email was updated + assert updated_user.email == "new_email@domain.com" diff --git a/backend/lcfs/tests/user/test_user_views.py b/backend/lcfs/tests/user/test_user_views.py index 75a228c0c..1c68b120d 100644 --- a/backend/lcfs/tests/user/test_user_views.py +++ b/backend/lcfs/tests/user/test_user_views.py @@ -468,19 +468,18 @@ async def test_track_logged_in_success(client: AsyncClient, fastapi_app, set_moc @pytest.mark.anyio -async def test_update_notifications_email_success( +async def test_update_email_success( client: AsyncClient, fastapi_app, set_mock_user, - add_models, ): set_mock_user(fastapi_app, [RoleEnum.GOVERNMENT]) # Prepare request data - request_data = {"notifications_email": "new_email@domain.com"} + request_data = {"email": "new_email@domain.com"} # Act: Send POST request to the endpoint - url = fastapi_app.url_path_for("update_notifications_email") + url = fastapi_app.url_path_for("update_email") response = await client.post(url, json=request_data) # Assert: Check response status and content diff --git a/backend/lcfs/tests/user/user_payloads.py b/backend/lcfs/tests/user/user_payloads.py index 3491cfd5e..38b9c5acc 100644 --- a/backend/lcfs/tests/user/user_payloads.py +++ b/backend/lcfs/tests/user/user_payloads.py @@ -6,7 +6,6 @@ keycloak_email="email@domain.com", keycloak_username="username", email="email@domain.com", - notifications_email=None, title="Developer", phone="1234567890", mobile_phone="0987654321", diff --git a/backend/lcfs/web/api/user/repo.py b/backend/lcfs/web/api/user/repo.py index c1906c34c..f55e0eab4 100644 --- a/backend/lcfs/web/api/user/repo.py +++ b/backend/lcfs/web/api/user/repo.py @@ -669,7 +669,7 @@ async def create_login_history(self, user: UserProfile): self.db.add(login_history) @repo_handler - async def update_notifications_email( + async def update_email( self, user_profile_id: int, email: str ) -> UserProfile: # Fetch the user profile @@ -679,8 +679,7 @@ async def update_notifications_email( result = await self.db.execute(query) user_profile = result.scalar_one_or_none() - # Update the notifications_email field - user_profile.notifications_email = email + user_profile.email = email # Flush and refresh without committing await self.db.flush() diff --git a/backend/lcfs/web/api/user/schema.py b/backend/lcfs/web/api/user/schema.py index 6cf34fe46..e773133b1 100644 --- a/backend/lcfs/web/api/user/schema.py +++ b/backend/lcfs/web/api/user/schema.py @@ -40,7 +40,6 @@ class UserBaseSchema(BaseSchema): keycloak_username: str keycloak_email: EmailStr email: Optional[EmailStr] = None - notifications_email: Optional[EmailStr] = None title: Optional[str] = None phone: Optional[str] = None first_name: Optional[str] = None @@ -97,5 +96,5 @@ class UserLoginHistoryResponseSchema(BaseSchema): pagination: PaginationResponseSchema -class UpdateNotificationsEmailSchema(BaseSchema): - notifications_email: EmailStr +class UpdateEmailSchema(BaseSchema): + email: EmailStr diff --git a/backend/lcfs/web/api/user/services.py b/backend/lcfs/web/api/user/services.py index 48bad17de..bd539f6bb 100644 --- a/backend/lcfs/web/api/user/services.py +++ b/backend/lcfs/web/api/user/services.py @@ -334,15 +334,12 @@ async def track_user_login(self, user: UserProfile): await self.repo.create_login_history(user) @service_handler - async def update_notifications_email(self, user_id: int, email: str): + async def update_email(self, user_id: int, email: str): try: - # Update the notifications_email field of the user - return await self.repo.update_notifications_email(user_id, email) - # Return the updated user - return UserBaseSchema.model_validate(user) + return await self.repo.update_email(user_id, email) except DataNotFoundException as e: logger.error(f"User not found: {e}") raise HTTPException(status_code=404, detail=str(e)) except Exception as e: - logger.error(f"Error updating notifications email: {e}") + logger.error(f"Error updating email: {e}") raise HTTPException(status_code=500, detail="Internal Server Error") diff --git a/backend/lcfs/web/api/user/views.py b/backend/lcfs/web/api/user/views.py index ff1a69aca..fab92a37f 100644 --- a/backend/lcfs/web/api/user/views.py +++ b/backend/lcfs/web/api/user/views.py @@ -1,16 +1,7 @@ from typing import List import structlog -from fastapi import ( - APIRouter, - Body, - status, - Request, - Response, - Depends, - Query, - HTTPException, -) +from fastapi import APIRouter, Body, status, Request, Response, Depends, Query from fastapi.responses import StreamingResponse from lcfs.db import dependencies @@ -23,7 +14,7 @@ UserLoginHistoryResponseSchema, UsersSchema, UserActivitiesResponseSchema, - UpdateNotificationsEmailSchema, + UpdateEmailSchema, ) from lcfs.web.api.user.services import UserServices from lcfs.web.core.decorators import view_handler @@ -255,18 +246,18 @@ async def get_all_user_login_history( @router.post( - "/update-notifications-email", - response_model=UpdateNotificationsEmailSchema, + "/update-email", + response_model=UpdateEmailSchema, status_code=status.HTTP_200_OK, ) @view_handler(["*"]) -async def update_notifications_email( +async def update_email( request: Request, - email_data: UpdateNotificationsEmailSchema = Body(...), + email_data: UpdateEmailSchema = Body(...), service: UserServices = Depends(), ): user_id = request.user.user_profile_id - email = email_data.notifications_email + email = email_data.email - user = await service.update_notifications_email(user_id, email) - return user + user = await service.update_email(user_id, email) + return UpdateEmailSchema(email=user.email) diff --git a/frontend/src/constants/routes/apiRoutes.js b/frontend/src/constants/routes/apiRoutes.js index f9c089581..3cee90b71 100644 --- a/frontend/src/constants/routes/apiRoutes.js +++ b/frontend/src/constants/routes/apiRoutes.js @@ -75,5 +75,5 @@ export const apiRoutes = { getNotificationsCount: '/notifications/count', getNotificationSubscriptions: '/notifications/subscriptions', saveNotificationSubscriptions: '/notifications/subscriptions/save', - updateNotificationsEmail: '/users/update-notifications-email' + updateNotificationsEmail: '/users/update-email' } diff --git a/frontend/src/views/Notifications/NotificationMenu/components/BCeIDNotificationSettings.jsx b/frontend/src/views/Notifications/NotificationMenu/components/BCeIDNotificationSettings.jsx index 74af3935c..d96cd56e9 100644 --- a/frontend/src/views/Notifications/NotificationMenu/components/BCeIDNotificationSettings.jsx +++ b/frontend/src/views/Notifications/NotificationMenu/components/BCeIDNotificationSettings.jsx @@ -29,7 +29,7 @@ const BCeIDNotificationSettings = () => { ) } diff --git a/frontend/src/views/Notifications/NotificationMenu/components/NotificationSettingsForm.jsx b/frontend/src/views/Notifications/NotificationMenu/components/NotificationSettingsForm.jsx index 6fd482744..5a7e0bd93 100644 --- a/frontend/src/views/Notifications/NotificationMenu/components/NotificationSettingsForm.jsx +++ b/frontend/src/views/Notifications/NotificationMenu/components/NotificationSettingsForm.jsx @@ -25,8 +25,6 @@ import { notificationTypes, notificationChannels } from '@/constants/notificationTypes' -import { useNavigate } from 'react-router-dom' -import { ROUTES } from '@/constants/routes' import MailIcon from '@mui/icons-material/Mail' import NotificationsIcon from '@mui/icons-material/Notifications' import BCButton from '@/components/BCButton' @@ -40,19 +38,18 @@ const NotificationSettingsForm = ({ initialEmail }) => { const { t } = useTranslation(['notifications']) - const navigate = useNavigate() const [isFormLoading, setIsFormLoading] = useState(false) const [message, setMessage] = useState('') const { data: subscriptionsData, isLoading: isSubscriptionsLoading } = useNotificationSubscriptions() const createSubscription = useCreateSubscription() const deleteSubscription = useDeleteSubscription() - const updateNotificationsEmail = useUpdateNotificationsEmail() + const updateEmail = useUpdateNotificationsEmail() // Validation schema const validationSchema = Yup.object().shape({ ...(showEmailField && { - notificationEmail: Yup.string() + email: Yup.string() .email(t('errors.invalidEmail')) .required(t('errors.emailRequired')) }) @@ -75,7 +72,7 @@ const NotificationSettingsForm = ({ }) } if (showEmailField && initialEmail) { - setValue('notificationEmail', initialEmail) + setValue('email', initialEmail) } }, [subscriptionsData, showEmailField, initialEmail, setValue]) @@ -122,8 +119,8 @@ const NotificationSettingsForm = ({ try { if (showEmailField) { // BCeID user, save the email address - await updateNotificationsEmail.mutateAsync({ - notifications_email: data.notificationEmail + await updateEmail.mutateAsync({ + email: data.email }) setMessage(t('messages.emailSaved')) } @@ -423,11 +420,11 @@ const NotificationSettingsForm = ({ > {showEmailField && ( - - {t('notificationsEmail')}: + + {t('email')}: (