From f578b2fa00f1fdb5d241f200740bb298dd8865fd Mon Sep 17 00:00:00 2001 From: Zineb El Bachiri <100568984+gozineb@users.noreply.github.com> Date: Fri, 24 Nov 2023 10:25:02 +0100 Subject: [PATCH] refactor: onboarding module (#1702) # Description Please include a summary of the changes and the related issue. Please also include relevant motivation and context. ## Checklist before requesting a review Please delete options that are not relevant. - [ ] My code follows the style guidelines of this project - [ ] I have performed a self-review of my code - [ ] I have commented hard-to-understand areas - [ ] I have ideally added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] Any dependent changes have been merged ## Screenshots (if appropriate): --- backend/celery_worker.py | 7 ++- backend/main.py | 2 +- backend/models/databases/repository.py | 16 ------ backend/models/databases/supabase/__init__.py | 1 - backend/models/databases/supabase/supabase.py | 3 -- backend/modules/onboarding/__init__.py | 0 .../modules/onboarding/controller/__init__.py | 0 .../controller}/onboarding_routes.py | 15 +++--- backend/modules/onboarding/dto/__init__.py | 1 + backend/modules/onboarding/dto/inputs.py | 16 ++++++ backend/modules/onboarding/entity/__init__.py | 1 + .../modules/onboarding/entity/onboarding.py | 10 ++++ .../modules/onboarding/repository/__init__.py | 0 .../onboarding/repository/onboardings.py} | 43 ++++------------ .../repository/onboardings_interface.py | 43 ++++++++++++++++ .../modules/onboarding/service/__init__.py | 1 + .../onboarding/service/onboarding_service.py | 50 +++++++++++++++++++ .../onboarding/create_user_onboarding.py | 15 ------ .../onboarding/get_user_onboarding.py | 18 ------- .../remove_onboarding_more_than_x_days.py | 10 ---- .../onboarding/update_user_onboarding.py | 21 -------- backend/tests/test_onboarding.py | 8 ++- 22 files changed, 148 insertions(+), 133 deletions(-) create mode 100644 backend/modules/onboarding/__init__.py create mode 100644 backend/modules/onboarding/controller/__init__.py rename backend/{routes => modules/onboarding/controller}/onboarding_routes.py (69%) create mode 100644 backend/modules/onboarding/dto/__init__.py create mode 100644 backend/modules/onboarding/dto/inputs.py create mode 100644 backend/modules/onboarding/entity/__init__.py create mode 100644 backend/modules/onboarding/entity/onboarding.py create mode 100644 backend/modules/onboarding/repository/__init__.py rename backend/{models/databases/supabase/onboarding.py => modules/onboarding/repository/onboardings.py} (69%) create mode 100644 backend/modules/onboarding/repository/onboardings_interface.py create mode 100644 backend/modules/onboarding/service/__init__.py create mode 100644 backend/modules/onboarding/service/onboarding_service.py delete mode 100644 backend/repository/onboarding/create_user_onboarding.py delete mode 100644 backend/repository/onboarding/get_user_onboarding.py delete mode 100644 backend/repository/onboarding/remove_onboarding_more_than_x_days.py delete mode 100644 backend/repository/onboarding/update_user_onboarding.py diff --git a/backend/celery_worker.py b/backend/celery_worker.py index 2dd74d8b2..911bfb59f 100644 --- a/backend/celery_worker.py +++ b/backend/celery_worker.py @@ -9,18 +9,17 @@ from models.databases.supabase.notifications import NotificationUpdatablePropert from models.files import File from models.notifications import NotificationsStatusEnum from models.settings import get_supabase_client +from modules.onboarding.service.onboarding_service import OnboardingService from packages.files.crawl.crawler import CrawlWebsite from packages.files.parsers.github import process_github from packages.files.processors import filter_file from repository.brain.update_brain_last_update_time import update_brain_last_update_time from repository.notification.update_notification import update_notification_by_id -from repository.onboarding.remove_onboarding_more_than_x_days import ( - remove_onboarding_more_than_x_days, -) CELERY_BROKER_URL = os.getenv("CELERY_BROKER_URL", "") CELEBRY_BROKER_QUEUE_NAME = os.getenv("CELEBRY_BROKER_QUEUE_NAME", "quivr") +onboardingService = OnboardingService() if CELERY_BROKER_URL.startswith("sqs"): broker_transport_options = { @@ -162,7 +161,7 @@ def process_crawl_and_notify( @celery.task def remove_onboarding_more_than_x_days_task(): - remove_onboarding_more_than_x_days(7) + onboardingService.remove_onboarding_more_than_x_days(7) celery.conf.beat_schedule = { diff --git a/backend/main.py b/backend/main.py index d9b9639b6..d6177b28c 100644 --- a/backend/main.py +++ b/backend/main.py @@ -13,6 +13,7 @@ from fastapi import FastAPI, HTTPException from fastapi.responses import JSONResponse from logger import get_logger from middlewares.cors import add_cors_middleware +from modules.onboarding.controller.onboarding_routes import onboarding_router from modules.prompt.controller.prompt_routes import prompt_router from modules.user.controller.user_controller import user_router from routes.api_key_routes import api_key_router @@ -24,7 +25,6 @@ from routes.explore_routes import explore_router from routes.knowledge_routes import knowledge_router from routes.misc_routes import misc_router from routes.notification_routes import notification_router -from routes.onboarding_routes import onboarding_router from routes.subscription_routes import subscription_router from routes.upload_routes import upload_router diff --git a/backend/models/databases/repository.py b/backend/models/databases/repository.py index 7695b2b0a..cea48eb42 100644 --- a/backend/models/databases/repository.py +++ b/backend/models/databases/repository.py @@ -244,22 +244,6 @@ class Repository(ABC): def get_all_knowledge_in_brain(self, brain_id: UUID): pass - @abstractmethod - def get_user_onboarding(self, user_id: UUID): - pass - - @abstractmethod - def update_user_onboarding(self, user_id: UUID, onboarding): - pass - - @abstractmethod - def remove_user_onboarding(self, user_id: UUID): - pass - - @abstractmethod - def remove_onboarding_more_than_x_days(self, days: int): - pass - @abstractmethod def get_api_brain_definition(self, brain_id: UUID): pass diff --git a/backend/models/databases/supabase/__init__.py b/backend/models/databases/supabase/__init__.py index 297529031..ff053aab0 100644 --- a/backend/models/databases/supabase/__init__.py +++ b/backend/models/databases/supabase/__init__.py @@ -7,6 +7,5 @@ from models.databases.supabase.chats import Chats from models.databases.supabase.files import File from models.databases.supabase.knowledge import Knowledges from models.databases.supabase.notifications import Notifications -from models.databases.supabase.onboarding import Onboarding from models.databases.supabase.user_usage import UserUsage from models.databases.supabase.vectors import Vector diff --git a/backend/models/databases/supabase/supabase.py b/backend/models/databases/supabase/supabase.py index 136fe2300..578fcda63 100644 --- a/backend/models/databases/supabase/supabase.py +++ b/backend/models/databases/supabase/supabase.py @@ -8,7 +8,6 @@ from models.databases.supabase import ( File, Knowledges, Notifications, - Onboarding, UserUsage, Vector, ) @@ -24,7 +23,6 @@ class SupabaseDB( ApiKeyHandler, Chats, Vector, - Onboarding, Notifications, Knowledges, ApiBrainDefinitions, @@ -40,5 +38,4 @@ class SupabaseDB( Vector.__init__(self, supabase_client) Notifications.__init__(self, supabase_client) Knowledges.__init__(self, supabase_client) - Onboarding.__init__(self, supabase_client) ApiBrainDefinitions.__init__(self, supabase_client) diff --git a/backend/modules/onboarding/__init__.py b/backend/modules/onboarding/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/backend/modules/onboarding/controller/__init__.py b/backend/modules/onboarding/controller/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/backend/routes/onboarding_routes.py b/backend/modules/onboarding/controller/onboarding_routes.py similarity index 69% rename from backend/routes/onboarding_routes.py rename to backend/modules/onboarding/controller/onboarding_routes.py index 4220b34f1..39d6dc5c4 100644 --- a/backend/routes/onboarding_routes.py +++ b/backend/modules/onboarding/controller/onboarding_routes.py @@ -3,16 +3,15 @@ from middlewares.auth import ( # Assuming you have a get_current_user function AuthBearer, get_current_user, ) -from models.databases.supabase.onboarding import ( - OnboardingStates, - OnboardingUpdatableProperties, -) +from modules.onboarding.dto.inputs import OnboardingUpdatableProperties +from modules.onboarding.entity.onboarding import OnboardingStates +from modules.onboarding.service.onboarding_service import OnboardingService from modules.user.entity.user_identity import UserIdentity -from repository.onboarding.get_user_onboarding import get_user_onboarding -from repository.onboarding.update_user_onboarding import update_user_onboarding onboarding_router = APIRouter() +onboardingService = OnboardingService() + @onboarding_router.get( "/onboarding", @@ -26,7 +25,7 @@ async def get_user_onboarding_handler( Get user onboarding information for the current user """ - return get_user_onboarding(current_user.id) + return onboardingService.get_user_onboarding(current_user.id) @onboarding_router.put( @@ -42,4 +41,4 @@ async def update_user_onboarding_handler( Update user onboarding information for the current user """ - return update_user_onboarding(current_user.id, onboarding) + return onboardingService.update_user_onboarding(current_user.id, onboarding) diff --git a/backend/modules/onboarding/dto/__init__.py b/backend/modules/onboarding/dto/__init__.py new file mode 100644 index 000000000..797b0e7df --- /dev/null +++ b/backend/modules/onboarding/dto/__init__.py @@ -0,0 +1 @@ +from .inputs import OnboardingUpdatableProperties diff --git a/backend/modules/onboarding/dto/inputs.py b/backend/modules/onboarding/dto/inputs.py new file mode 100644 index 000000000..8bf76fdbc --- /dev/null +++ b/backend/modules/onboarding/dto/inputs.py @@ -0,0 +1,16 @@ +from typing import Optional + +from pydantic import BaseModel + + +class OnboardingUpdatableProperties(BaseModel): + + """Properties that can be received on onboarding update""" + + onboarding_a: Optional[bool] + onboarding_b1: Optional[bool] + onboarding_b2: Optional[bool] + onboarding_b3: Optional[bool] + + class Config: + extra = "forbid" diff --git a/backend/modules/onboarding/entity/__init__.py b/backend/modules/onboarding/entity/__init__.py new file mode 100644 index 000000000..b22b64072 --- /dev/null +++ b/backend/modules/onboarding/entity/__init__.py @@ -0,0 +1 @@ +from .onboarding import OnboardingStates diff --git a/backend/modules/onboarding/entity/onboarding.py b/backend/modules/onboarding/entity/onboarding.py new file mode 100644 index 000000000..713f3f415 --- /dev/null +++ b/backend/modules/onboarding/entity/onboarding.py @@ -0,0 +1,10 @@ +from pydantic import BaseModel + + +class OnboardingStates(BaseModel): + """Response when getting onboarding""" + + onboarding_a: bool + onboarding_b1: bool + onboarding_b2: bool + onboarding_b3: bool diff --git a/backend/modules/onboarding/repository/__init__.py b/backend/modules/onboarding/repository/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/backend/models/databases/supabase/onboarding.py b/backend/modules/onboarding/repository/onboardings.py similarity index 69% rename from backend/models/databases/supabase/onboarding.py rename to backend/modules/onboarding/repository/onboardings.py index 4837cee96..f91844dbf 100644 --- a/backend/models/databases/supabase/onboarding.py +++ b/backend/modules/onboarding/repository/onboardings.py @@ -1,39 +1,16 @@ from datetime import datetime, timedelta -from typing import Optional -from uuid import UUID from fastapi import HTTPException -from models.databases.repository import Repository -from pydantic import BaseModel +from modules.onboarding.entity.onboarding import OnboardingStates + +from .onboardings_interface import OnboardingInterface -class OnboardingUpdatableProperties(BaseModel): - - """Properties that can be received on onboarding update""" - - onboarding_a: Optional[bool] - onboarding_b1: Optional[bool] - onboarding_b2: Optional[bool] - onboarding_b3: Optional[bool] - - class Config: - extra = "forbid" - - -class OnboardingStates(BaseModel): - """Response when getting onboarding""" - - onboarding_a: bool - onboarding_b1: bool - onboarding_b2: bool - onboarding_b3: bool - - -class Onboarding(Repository): +class Onboarding(OnboardingInterface): def __init__(self, supabase_client): self.db = supabase_client - def get_user_onboarding(self, user_id: UUID) -> OnboardingStates | None: + def get_user_onboarding(self, user_id): """ Get user onboarding information by user_id """ @@ -55,9 +32,7 @@ class Onboarding(Repository): return OnboardingStates(**onboarding_data[0]) - def update_user_onboarding( - self, user_id: UUID, onboarding: OnboardingUpdatableProperties - ) -> OnboardingStates: + def update_user_onboarding(self, user_id, onboarding): """Update user onboarding information by user_id""" update_data = { key: value for key, value in onboarding.dict().items() if value is not None @@ -76,7 +51,7 @@ class Onboarding(Repository): return OnboardingStates(**response[0]) - def remove_user_onboarding(self, user_id: UUID) -> OnboardingStates | None: + def remove_user_onboarding(self, user_id): """ Remove user onboarding information by user_id """ @@ -92,7 +67,7 @@ class Onboarding(Repository): return OnboardingStates(**onboarding_data[0]) - def create_user_onboarding(self, user_id: UUID) -> OnboardingStates: + def create_user_onboarding(self, user_id): """ Create user onboarding information by user_id """ @@ -110,7 +85,7 @@ class Onboarding(Repository): return OnboardingStates(**onboarding_data[0]) - def remove_onboarding_more_than_x_days(self, days: int): + def remove_onboarding_more_than_x_days(self, days): """ Remove onboarding if it is older than x days """ diff --git a/backend/modules/onboarding/repository/onboardings_interface.py b/backend/modules/onboarding/repository/onboardings_interface.py new file mode 100644 index 000000000..a3a972925 --- /dev/null +++ b/backend/modules/onboarding/repository/onboardings_interface.py @@ -0,0 +1,43 @@ +from abc import ABC, abstractmethod +from uuid import UUID + +from modules.onboarding.dto.inputs import OnboardingUpdatableProperties +from modules.onboarding.entity.onboarding import OnboardingStates + + +class OnboardingInterface(ABC): + @abstractmethod + def get_user_onboarding(self, user_id: UUID) -> OnboardingStates | None: + """ + Get user onboarding information by user_id + """ + pass + + @abstractmethod + def update_user_onboarding( + self, user_id: UUID, onboarding: OnboardingUpdatableProperties + ) -> OnboardingStates: + """Update user onboarding information by user_id""" + pass + + @abstractmethod + def remove_user_onboarding(self, user_id: UUID) -> OnboardingStates | None: + """ + Remove user onboarding information by user_id + """ + pass + + @abstractmethod + def create_user_onboarding(self, user_id: UUID) -> OnboardingStates: + """ + Create user onboarding information by user_id + """ + pass + + @abstractmethod + def remove_onboarding_more_than_x_days(self, days: int): + """ + Remove onboarding if it is older than x days + """ + """Update a prompt by id""" + pass diff --git a/backend/modules/onboarding/service/__init__.py b/backend/modules/onboarding/service/__init__.py new file mode 100644 index 000000000..6a1859546 --- /dev/null +++ b/backend/modules/onboarding/service/__init__.py @@ -0,0 +1 @@ +from .onboarding_service import OnboardingService \ No newline at end of file diff --git a/backend/modules/onboarding/service/onboarding_service.py b/backend/modules/onboarding/service/onboarding_service.py new file mode 100644 index 000000000..53bfc7c05 --- /dev/null +++ b/backend/modules/onboarding/service/onboarding_service.py @@ -0,0 +1,50 @@ +from uuid import UUID + +from models.settings import get_supabase_client +from modules.onboarding.dto.inputs import OnboardingUpdatableProperties +from modules.onboarding.entity.onboarding import OnboardingStates +from modules.onboarding.repository.onboardings import Onboarding + + +class OnboardingService: + repository: Onboarding + + def __init__(self): + supabase_client = get_supabase_client() + self.repository = Onboarding(supabase_client) + + def create_user_onboarding(self, user_id: UUID) -> OnboardingStates: + """Update user onboarding information by user_id""" + + return self.repository.create_user_onboarding(user_id) + + def get_user_onboarding(self, user_id: UUID) -> OnboardingStates | None: + """ + Get a user's onboarding status + + Args: + user_id (UUID): The id of the user + + Returns: + Onboardings: The user's onboarding status + """ + return self.repository.get_user_onboarding(user_id) + + def remove_onboarding_more_than_x_days(self, days: int): + """ + Remove onboarding if it is older than x days + """ + + self.repository.remove_onboarding_more_than_x_days(days) + + def update_user_onboarding( + self, user_id: UUID, onboarding: OnboardingUpdatableProperties + ) -> OnboardingStates: + """Update user onboarding information by user_id""" + + updated_onboarding = self.repository.update_user_onboarding(user_id, onboarding) + + if all(not value for value in updated_onboarding.dict().values()): + self.repository.remove_user_onboarding(user_id) + + return updated_onboarding diff --git a/backend/repository/onboarding/create_user_onboarding.py b/backend/repository/onboarding/create_user_onboarding.py deleted file mode 100644 index 7ddb1e2bf..000000000 --- a/backend/repository/onboarding/create_user_onboarding.py +++ /dev/null @@ -1,15 +0,0 @@ -from uuid import UUID - -from models.databases.supabase.onboarding import ( - OnboardingStates, -) -from models.settings import get_supabase_db - - -def create_user_onboarding(user_id: UUID) -> OnboardingStates: - """Update user onboarding information by user_id""" - - supabase_db = get_supabase_db() - created_user_onboarding = supabase_db.create_user_onboarding(user_id) - - return created_user_onboarding diff --git a/backend/repository/onboarding/get_user_onboarding.py b/backend/repository/onboarding/get_user_onboarding.py deleted file mode 100644 index 411ec8992..000000000 --- a/backend/repository/onboarding/get_user_onboarding.py +++ /dev/null @@ -1,18 +0,0 @@ -from uuid import UUID - -from models.databases.supabase.onboarding import OnboardingStates -from models.settings import get_supabase_db - - -def get_user_onboarding(user_id: UUID) -> OnboardingStates | None: - """ - Get a user's onboarding status - - Args: - user_id (UUID): The id of the user - - Returns: - Onboardings: The user's onboarding status - """ - supabase_db = get_supabase_db() - return supabase_db.get_user_onboarding(user_id) diff --git a/backend/repository/onboarding/remove_onboarding_more_than_x_days.py b/backend/repository/onboarding/remove_onboarding_more_than_x_days.py deleted file mode 100644 index f65b8378c..000000000 --- a/backend/repository/onboarding/remove_onboarding_more_than_x_days.py +++ /dev/null @@ -1,10 +0,0 @@ -from models.settings import get_supabase_db - - -def remove_onboarding_more_than_x_days(days: int): - """ - Remove onboarding if it is older than x days - """ - supabase_db = get_supabase_db() - - supabase_db.remove_onboarding_more_than_x_days(days) diff --git a/backend/repository/onboarding/update_user_onboarding.py b/backend/repository/onboarding/update_user_onboarding.py deleted file mode 100644 index 672caa8c8..000000000 --- a/backend/repository/onboarding/update_user_onboarding.py +++ /dev/null @@ -1,21 +0,0 @@ -from uuid import UUID - -from models.databases.supabase.onboarding import ( - OnboardingStates, - OnboardingUpdatableProperties, -) -from models.settings import get_supabase_db - - -def update_user_onboarding( - user_id: UUID, onboarding: OnboardingUpdatableProperties -) -> OnboardingStates: - """Update user onboarding information by user_id""" - - supabase_db = get_supabase_db() - updated_onboarding = supabase_db.update_user_onboarding(user_id, onboarding) - - if all(not value for value in updated_onboarding.dict().values()): - supabase_db.remove_user_onboarding(user_id) - - return updated_onboarding diff --git a/backend/tests/test_onboarding.py b/backend/tests/test_onboarding.py index 4cfdd5306..6aecbb3ea 100644 --- a/backend/tests/test_onboarding.py +++ b/backend/tests/test_onboarding.py @@ -1,4 +1,6 @@ -from repository.onboarding.create_user_onboarding import create_user_onboarding +from modules.onboarding.service.onboarding_service import OnboardingService + +onboardingService = OnboardingService() def test_remove_onboarding(client, api_key): @@ -19,7 +21,9 @@ def test_remove_onboarding(client, api_key): def test_create_onboarding(client, api_key): response = client.get("/user", headers={"Authorization": "Bearer " + api_key}) - create_user_onboarding_response = create_user_onboarding(response.json().get("id")) + create_user_onboarding_response = onboardingService.create_user_onboarding( + response.json().get("id") + ) assert create_user_onboarding_response == { "onboarding_a": True, "onboarding_b1": True,