diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index eba0186fb867995aeed85aeb3f68b2b8f7a6840b..d7deccbe6dd8890a5e5bad7b545bc54b57912634 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -208,6 +208,7 @@ staging_trigger_unified-api_redeployment: branch: 'main' only: - main + resource_group: unified-api-staging production_deploy: extends: .deploy @@ -233,3 +234,4 @@ production_trigger_unified-api_redeployment: branch: 'production' only: - production + resource_group: unified-api-production diff --git a/.prettierrc b/.prettierrc new file mode 100644 index 0000000000000000000000000000000000000000..d5fabb4b358550c5742b1640249ba72cd8d164e7 --- /dev/null +++ b/.prettierrc @@ -0,0 +1,17 @@ +{ + "semi": false, + "singleQuote": true, + "tabWidth": 2, + "useTabs": false, + "printWidth": 120, + "importOrder": [ + "^@holi/core/helpers/__mocks__/(.*)$", + "<THIRD_PARTY_MODULES>", + "^@react-navigation/(.*)$", + "^@holi/(.*)$", + "^@holi-apps/(.*)$", + "^[./]" + ], + "importOrderSeparation": true, + "importOrderSortSpecifiers": true +} diff --git a/openbook_appointments/services.py b/openbook_appointments/services.py index 55c8a6978dc932d2c47143cfce1da23c3c6c865c..91e38fc4ced603b61c9940bcf8765e5e64cdd3a2 100644 --- a/openbook_appointments/services.py +++ b/openbook_appointments/services.py @@ -12,15 +12,22 @@ from openbook_common.utils.model_loaders import get_community_model, get_user_mo from openbook_communities.models import Community from openbook_notifications.services import NotificationsService from .checkers import AppointmentChecker +from .models import VideoCallLink from .repositories import AppointmentRepository, VideoCallLinkRepository, LocationDetailsRepository class AppointmentService: - def __init__(self): - self.repository = AppointmentRepository() - self.video_call_link_service = VideoCallLinkService() - self.location_service = LocationDetailsService() - self.checker = AppointmentChecker() + def __init__( + self, + repository: AppointmentRepository = None, + video_call_link_service: "VideoCallLinkService" = None, + location_service: "LocationDetailsService" = None, + checker: AppointmentChecker = None, + ): + self.repository = repository or AppointmentRepository() + self.video_call_link_service = video_call_link_service or VideoCallLinkService() + self.location_service = location_service or LocationDetailsService() + self.checker = checker or AppointmentChecker() def create_appointment( self, @@ -236,21 +243,34 @@ class AppointmentService: return recipients - def _update_video_call_link(self, appointment, video_call_info, video_call_link): - if not video_call_link and appointment.video_call_link: - self.video_call_link_service.delete_video_call_link(video_call_link_id=appointment.video_call_link.id) - return None - - if not appointment.video_call_link: - return self.video_call_link_service.create_video_call_link( - url=video_call_link, additional_info=video_call_info - ) - - return self.video_call_link_service.update_video_call_link( - video_call_link_id=appointment.video_call_link.id, - url=video_call_link, - additional_info=video_call_info, - ) + def _update_video_call_link(self, appointment, video_call_info, video_call_link) -> VideoCallLink | None: + # compare existing video call link vs. video call link from input + match (appointment.video_call_link, video_call_link): + case (None, None): + # both unset, do nothing + return None + + case (None, _): + # the appointment doesn't currently have one, user provides one + # -> create it + return self.video_call_link_service.create_video_call_link( + url=video_call_link, additional_info=video_call_info + ) + + case (_, None): + # the appointment has one, user provides None explicitly + # -> delete it + self.video_call_link_service.delete_video_call_link(video_call_link_id=appointment.video_call_link.id) + return None + + case (_, _): + # the appointment has one, user provides a different one + # -> update it + return self.video_call_link_service.update_video_call_link( + video_call_link_id=appointment.video_call_link.id, + url=video_call_link, + additional_info=video_call_info, + ) def _manage_location_details(self, appointment, location_details): appointment_location = appointment.location_details diff --git a/openbook_appointments/tests/helpers.py b/openbook_appointments/tests/helpers.py index e3649150633917a44c2863f8a558e12db768b67b..285db16692c1663f54707d4d93a04ddeb12248f4 100644 --- a/openbook_appointments/tests/helpers.py +++ b/openbook_appointments/tests/helpers.py @@ -1,8 +1,12 @@ from asgiref.sync import async_to_sync from benedict import benedict +from mixer.backend.django import mixer +from strawberry import UNSET from openbook.graphql_schema import schema from openbook_appointments.enums import StatusType +from openbook_appointments.models import Appointment, VideoCallLink +from openbook_auth.models import User def create_appointment(user, input): @@ -204,3 +208,12 @@ def update_input_data(appointment_id, input_data): data.pop("spaceId", None) data["appointmentId"] = appointment_id return data + + +def make_appointment( + video_call_link: VideoCallLink = UNSET, # abuse UNSET here, because we also want to be able to pass `None` explicitly + creator: User = UNSET, +) -> Appointment: + overrides = {k: v for k, v in locals().items() if v is not UNSET} + + return mixer.blend(Appointment, **overrides) diff --git a/openbook_appointments/tests/test_services.py b/openbook_appointments/tests/test_services.py new file mode 100644 index 0000000000000000000000000000000000000000..e8cc0773618dc22f44cf1a82f444e15a0bd8b312 --- /dev/null +++ b/openbook_appointments/tests/test_services.py @@ -0,0 +1,189 @@ +import unittest +from unittest.mock import Mock + +import pytest + +from openbook_appointments.checkers import AppointmentChecker +from openbook_appointments.models import VideoCallLink +from openbook_appointments.repositories import AppointmentRepository +from openbook_appointments.services import VideoCallLinkService, LocationDetailsService, AppointmentService +from openbook_appointments.tests.helpers import make_appointment +from openbook_common.tests.helpers import make_user + + +class AlwaysSucceedsAppointmentChecker(AppointmentChecker): + def check_can_create_appointment(self, user, invite_only, space_id): + return + + def check_can_query_invite_only_appointment(self, user, appointment): + return + + def check_can_delete_appointment(self, user, appointment): + return + + def check_can_update_appointment(self, user, appointment): + return + + +@pytest.mark.django_db +class AppointmentServiceTestCase(unittest.TestCase): + repo: AppointmentRepository | Mock + video_call_link_service: VideoCallLinkService | Mock + location_service: LocationDetailsService | Mock + checker: AlwaysSucceedsAppointmentChecker + + service: AppointmentService + + def setUp(self): + self.repo = Mock(spec=AppointmentRepository) + self.video_call_link_service = Mock(spec=VideoCallLinkService) + self.location_service = Mock(spec=LocationDetailsService) + self.checker = AlwaysSucceedsAppointmentChecker() + + self.service = AppointmentService(self.repo, self.video_call_link_service, self.location_service, self.checker) + + def test_updating_appointment_creates_new_video_call_link(self): + """ + Test that a new video call link is created if the appointment doesn't already + have one, and a new link is provided in the input. + """ + + # Given + user = make_user() + existing_appointment = make_appointment(video_call_link=None, creator=user) + + self.repo.get_appointment.return_value = existing_appointment + self.repo.update_appointment.return_value = existing_appointment + + new_video_call_link_url = "https://meet.holi.social/test" + new_video_call_info = "please join here" + + new_video_call_link = VideoCallLink( + url=new_video_call_link_url, + additional_info=new_video_call_info, + ) + + self.video_call_link_service.create_video_call_link.return_value = new_video_call_link + + # When + self.service.update_appointment( + appointment_id=existing_appointment.id, + user=user, + video_call_link=new_video_call_link_url, + video_call_info=new_video_call_info, + ) + + # Then + self.video_call_link_service.create_video_call_link.assert_called_once() + self.repo.update_appointment.assert_called_once() + + def test_updating_appointment_updates_video_call_link(self): + """ + Test that video call link is updated if appointment already has one, + and a new link is provided in the input. + """ + + # Given + user = make_user() + existing_video_call_link = VideoCallLink( + url="https://meet.holi.social/old-link", + ) + existing_video_call_link.save() + + existing_appointment = make_appointment(video_call_link=existing_video_call_link, creator=user) + + self.repo.get_appointment.return_value = existing_appointment + self.repo.update_appointment.return_value = existing_appointment + + new_video_call_link_url = "https://meet.holi.social/test" + new_video_call_info = "please join here" + + new_video_call_link = VideoCallLink( + url=new_video_call_link_url, + additional_info=new_video_call_info, + ) + + self.video_call_link_service.update_video_call_link.return_value = new_video_call_link + + # When + self.service.update_appointment( + appointment_id=existing_appointment.id, + user=user, + video_call_link=new_video_call_link_url, + video_call_info=new_video_call_info, + ) + + # Then + self.video_call_link_service.update_video_call_link.assert_called_once_with( + video_call_link_id=existing_video_call_link.id, + url=new_video_call_link_url, + additional_info=new_video_call_info, + ) + self.repo.update_appointment.assert_called_once() + + def test_updating_appointment_deletes_video_call_link(self): + # Given + user = make_user() + existing_video_call_link = VideoCallLink( + url="https://meet.holi.social/old-link", + ) + existing_video_call_link.save() + + existing_appointment = make_appointment(video_call_link=existing_video_call_link, creator=user) + + self.repo.get_appointment.return_value = existing_appointment + self.repo.update_appointment.return_value = existing_appointment + + new_video_call_link_url = None + new_video_call_info = None + + self.video_call_link_service.delete_video_call_link.return_value = 1 + + # When + self.service.update_appointment( + appointment_id=existing_appointment.id, + user=user, + video_call_link=new_video_call_link_url, + video_call_info=new_video_call_info, + ) + + # Then + self.video_call_link_service.delete_video_call_link.assert_called_once_with( + video_call_link_id=existing_video_call_link.id, + ) + + self.repo.update_appointment.assert_called_once() + self.assertIsNone(self.repo.update_appointment.call_args.kwargs["video_call_link"]) + + def test_updating_appointmnet_doesnt_touch_video_call_link(self): + """ + Test that nothing happens to the video call link if the appointment doesn't + have one, and none is provided in the input. + """ + + # Given + user = make_user() + existing_appointment = make_appointment(video_call_link=None, creator=user) + + self.repo.get_appointment.return_value = existing_appointment + self.repo.update_appointment.return_value = existing_appointment + + # When + self.service.update_appointment( + appointment_id=existing_appointment.id, + user=user, + video_call_link=None, + video_call_info=None, + ) + + # Then + self.video_call_link_service.create_video_call_link.assert_not_called() + self.video_call_link_service.delete_video_call_link.assert_not_called() + self.video_call_link_service.update_video_call_link.assert_not_called() + + self.repo.update_appointment.assert_called_once() + self.assertIsNone(self.repo.update_appointment.call_args.kwargs["video_call_link"]) + + +if __name__ == "__main__": + unittest.main() diff --git a/renovate.json b/renovate.json index 8af71a3159b0d6c820f2ea1bebc74b2646a611e6..3ae1faafc3916e4a27b3af156c71284e69f262d5 100644 --- a/renovate.json +++ b/renovate.json @@ -5,18 +5,43 @@ ], "packageRules": [ { - "matchUpdateTypes": ["patch", "pin", "digest"], + "matchUpdateTypes": [ + "patch", + "pin", + "digest" + ], "automerge": true }, { - "matchManagers": ["pip_requirements"], - "matchUpdateTypes": ["minor", "patch", "pin", "digest"], + "matchSourceUrls": [ + "https://github.com/hashicorp/terraform" + ], + "groupName": "terraform" + }, + { + "matchJsonata": [ + "$match(depName,/python$/)" + ], + "groupName": "python" + }, + { + "matchManagers": [ + "pip_requirements" + ], + "matchUpdateTypes": [ + "minor", + "patch", + "pin", + "digest" + ], "automerge": true }, { - "matchDepTypes": ["devDependencies"], + "matchDepTypes": [ + "devDependencies" + ], "automerge": true } - ], + ], "platformAutomerge": false -} +} \ No newline at end of file diff --git a/requirements.txt b/requirements.txt index 6dbc7b366d1204e7aab9857e51491dfa9e2c7bed..ee3f86d23a739705b0aa485b8560695639fbf10d 100644 --- a/requirements.txt +++ b/requirements.txt @@ -11,7 +11,7 @@ aiofiles~=24.1.0 aiohttp~=3.10.1 aiosignal~=1.3.1 annotated-types~=0.7.0 -anyio~=4.4.0 +anyio~=4.6.2.post1 appdirs~=1.4.4 ASGIMiddlewareStaticFile~=0.6.1 asgiref~=3.8.1 @@ -20,12 +20,12 @@ attrs~=24.2.0 Authlib~=1.3.1 bandit~=1.7.9 beautifulsoup4~=4.12.3 -black~=24.8.0 +black~=24.10.0 blurhash-python~=1.2.2 boto3~=1.34.154 botocore~=1.34.154 cachetools~=5.4.0 -certifi~=2024.7.4 +certifi~=2024.8.30 cffi~=1.17.0 charset-normalizer~=3.3.2 click~=8.1.3 @@ -125,7 +125,7 @@ python-benedict~=0.33.2 python-dateutil~=2.9.0.post0 python-dotenv~=1.0.1 python-fsutil~=0.14.1 -python-ipware~=2.0.2 +python-ipware~=3.0.0 python-magic~=0.4.27 python-slugify~=8.0.4 pytz~=2024.1 diff --git a/smoketest/main.js b/smoketest/main.js index 333b25289d837f87c2bbc3da95f10d8a1fced74d..eb881fdb4ae7e1adfae3d246348f48f4248b133f 100644 --- a/smoketest/main.js +++ b/smoketest/main.js @@ -1,44 +1,40 @@ -import http from "k6/http"; -import { check, sleep } from "k6"; -import exec from "k6/execution"; +import http from 'k6/http' +import { check } from 'k6'; -// This configuration only executes 1 test, enough for a smoketest +// You don't need to change anything in this section, it's k6 glue code. +// See the default function at the end of the file for defining your smoketest. +// This configuration only executes 1 test, enough for a smoketest. The smoketest will fail on any check failing. +const allChecksNeedToPassTreshold = { checks: [{ threshold: 'rate==1', abortOnFail: true }] } export const options = { vus: 1, iterations: 1, -}; + thresholds: allChecksNeedToPassTreshold, +} -export default () => { - // define the graphql request - const params = { - headers: { - "Content-Type": "application/json", - }, - }; - const query = `query{topics{totalResults}}`; - - // perform the graphql request - const response = http.post( - `${__ENV.BASE_URL}`, - JSON.stringify({ query }), - params, - ); +/** + * Performs a GraphQL query and checks the response using the provided function. Fails if any of the provided expectations are not met. + * @param {string} query The GraphQL query to perform + * @param {(response: http.Response) => Array<boolean>} checkFunction + * A function that takes the HTTP response as an argument and returns an array + * of boolean values, each indicating success or failure of a test. + */ +function forQuery(query, checkFunction) { + const response = http.post(`${__ENV.BASE_URL}`, JSON.stringify({ query }), { + headers: { 'Content-Type': 'application/json' }, + }) + checkFunction(response) +} - // define the tests - const testResults = [ +// Define your smoketest(s) here. +export default () => { + forQuery(`query{topics{totalResults}}`, (response) => { check(response, { - "is status 200": (r) => r.status === 200, - }), + 'is status 200': (r) => r.status === 200, + }) check(JSON.parse(response.body), { // there can be multiple tests here, e.g. //"contains topics object": (r) => typeof r.data.topics != null, - "contains totalResults count": (r) => - typeof r.data.topics.totalResults == "number", - }), - ]; - - // fail on any unmet expectations. We need to do this explicitly, because k6 is a load testing tool - if (testResults.includes(false)) { - exec.test.abort("smoke test failed"); - } -}; + 'contains totalResults count': (r) => typeof r.data.topics.totalResults == 'number', + }) + }) +}