diff --git a/docs/sdk/notification.md b/docs/sdk/notification.md deleted file mode 100644 index 474ce1d14..000000000 --- a/docs/sdk/notification.md +++ /dev/null @@ -1,3 +0,0 @@ -# Notification module - -::: kili.presentation.client.notification.NotificationClientMethods diff --git a/mkdocs.yml b/mkdocs.yml index 40df8b9d2..d46c617f6 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -23,7 +23,6 @@ nav: - Label Utils: sdk/label_utils.md - Label Parsing: sdk/label_parsing.md - LLM: sdk/llm.md - - Notification: sdk/notification.md - Organization: sdk/organization.md - Plugins: sdk/plugins.md - Project: sdk/project.md diff --git a/src/kili/adapters/kili_api_gateway/notification/mappers.py b/src/kili/adapters/kili_api_gateway/notification/mappers.py index f019ab764..8e0941e1e 100644 --- a/src/kili/adapters/kili_api_gateway/notification/mappers.py +++ b/src/kili/adapters/kili_api_gateway/notification/mappers.py @@ -1,14 +1,8 @@ """Mappers for notification API calls.""" - -from kili.adapters.kili_api_gateway.user.mappers import user_where_mapper from kili.domain.notification import NotificationFilter def map_notification_filter(filters: NotificationFilter) -> dict: """Build the GraphQL NotificationWhere variable to be sent in an operation.""" - return { - "hasBeenSeen": filters.has_been_seen, - "id": filters.id, - "user": user_where_mapper(filters.user) if filters.user else None, - } + return {"id": filters.id} diff --git a/src/kili/adapters/kili_api_gateway/notification/operations_mixin.py b/src/kili/adapters/kili_api_gateway/notification/operations_mixin.py index 580a2a04b..a537af987 100644 --- a/src/kili/adapters/kili_api_gateway/notification/operations_mixin.py +++ b/src/kili/adapters/kili_api_gateway/notification/operations_mixin.py @@ -28,8 +28,3 @@ def list_notifications( return PaginatedGraphQLQuery(self.graphql_client).execute_query_from_paginated_call( query, where, options, "Retrieving notifications", GQL_COUNT_NOTIFICATIONS ) - - def count_notification(self, filters: NotificationFilter) -> int: - """Count notifications.""" - variables = {"where": map_notification_filter(filters=filters)} - return self.graphql_client.execute(GQL_COUNT_NOTIFICATIONS, variables)["data"] diff --git a/src/kili/client.py b/src/kili/client.py index 54d66a2c8..bfd16fa1b 100644 --- a/src/kili/client.py +++ b/src/kili/client.py @@ -15,7 +15,6 @@ from kili.core.graphql.graphql_client import GraphQLClient, GraphQLClientName from kili.entrypoints.mutations.asset import MutationsAsset from kili.entrypoints.mutations.issue import MutationsIssue -from kili.entrypoints.mutations.notification import MutationsNotification from kili.entrypoints.mutations.plugins import MutationsPlugins from kili.entrypoints.mutations.project import MutationsProject from kili.entrypoints.mutations.project_version import MutationsProjectVersion @@ -31,7 +30,6 @@ from kili.presentation.client.internal import InternalClientMethods from kili.presentation.client.issue import IssueClientMethods from kili.presentation.client.label import LabelClientMethods -from kili.presentation.client.notification import NotificationClientMethods from kili.presentation.client.organization import OrganizationClientMethods from kili.presentation.client.project import ProjectClientMethods from kili.presentation.client.project_workflow import ProjectWorkflowClientMethods @@ -63,7 +61,6 @@ def filter(self, record) -> bool: class Kili( # pylint: disable=too-many-ancestors,too-many-instance-attributes MutationsAsset, MutationsIssue, - MutationsNotification, MutationsPlugins, MutationsProject, MutationsProjectVersion, @@ -75,7 +72,6 @@ class Kili( # pylint: disable=too-many-ancestors,too-many-instance-attributes CloudStorageClientMethods, IssueClientMethods, LabelClientMethods, - NotificationClientMethods, OrganizationClientMethods, ProjectClientMethods, ProjectWorkflowClientMethods, diff --git a/src/kili/core/enums.py b/src/kili/core/enums.py index 7b5267d99..ff0d96bc2 100644 --- a/src/kili/core/enums.py +++ b/src/kili/core/enums.py @@ -18,13 +18,6 @@ ] -NotificationStatus = Literal[ - "FAILURE", - "PENDING", - "SUCCESS", -] - - OrganizationRole = Literal[ "ADMIN", "USER", diff --git a/src/kili/domain/notification.py b/src/kili/domain/notification.py index 717036bc3..350f900ea 100644 --- a/src/kili/domain/notification.py +++ b/src/kili/domain/notification.py @@ -1,10 +1,7 @@ """Notification domain.""" from dataclasses import dataclass -from typing import TYPE_CHECKING, NewType, Optional - -if TYPE_CHECKING: - from .user import UserFilter +from typing import NewType, Optional NotificationId = NewType("NotificationId", str) @@ -13,6 +10,4 @@ class NotificationFilter: """Notification filter.""" - has_been_seen: Optional[bool] id: Optional[NotificationId] - user: Optional["UserFilter"] diff --git a/src/kili/entrypoints/mutations/notification/__init__.py b/src/kili/entrypoints/mutations/notification/__init__.py deleted file mode 100644 index 87765758a..000000000 --- a/src/kili/entrypoints/mutations/notification/__init__.py +++ /dev/null @@ -1,74 +0,0 @@ -"""Notification mutations.""" -from typing import Optional, Union - -from typeguard import typechecked - -from kili.core.graphql.graphql_client import GraphQLClient -from kili.entrypoints.base import BaseOperationEntrypointMixin -from kili.utils.logcontext import for_all_methods, log_call - -from .queries import GQL_CREATE_NOTIFICATION, GQL_UPDATE_PROPERTIES_IN_NOTIFICATION - - -@for_all_methods(log_call, exclude=["__init__"]) -class MutationsNotification(BaseOperationEntrypointMixin): - """Set of Notification mutations.""" - - graphql_client: GraphQLClient - - @typechecked - def create_notification(self, message: str, status: str, url: str, user_id: str): - """Create a notification. - - This method is currently only active for Kili administrators. - - Args: - message : - status : - url : - user_id : - - Returns: - A result object which indicates if the mutation was successful, - or an error message. - """ - variables = { - "data": { - "message": message, - "progress": None, - "status": status, - "url": url, - "userID": user_id, - } - } - result = self.graphql_client.execute(GQL_CREATE_NOTIFICATION, variables) - return self.format_result("data", result) - - @typechecked - def update_properties_in_notification( - self, - notification_id: str, - has_been_seen: Union[bool, None], - status: str, - url: str, - progress: Optional[int] = None, - task_id: Optional[str] = None, - ): - """Modify a notification. - - This method is currently only active for Kili administrators. - - Returns: - A result object which indicates if the mutation was successful, - or an error message. - """ - variables = { - "id": notification_id, - "hasBeenSeen": has_been_seen, - "progress": progress, - "status": status, - "taskId": task_id, - "url": url, - } - result = self.graphql_client.execute(GQL_UPDATE_PROPERTIES_IN_NOTIFICATION, variables) - return self.format_result("data", result) diff --git a/src/kili/entrypoints/mutations/notification/fragments.py b/src/kili/entrypoints/mutations/notification/fragments.py deleted file mode 100644 index f27a85179..000000000 --- a/src/kili/entrypoints/mutations/notification/fragments.py +++ /dev/null @@ -1,5 +0,0 @@ -"""Fragments of notification mutations.""" - -NOTIFICATION_FRAGMENT = """ -id -""" diff --git a/src/kili/entrypoints/mutations/notification/queries.py b/src/kili/entrypoints/mutations/notification/queries.py deleted file mode 100644 index b9e169d4e..000000000 --- a/src/kili/entrypoints/mutations/notification/queries.py +++ /dev/null @@ -1,41 +0,0 @@ -"""Queries of notification mutations.""" - -from .fragments import NOTIFICATION_FRAGMENT - -GQL_CREATE_NOTIFICATION = f""" -mutation( - $data: NotificationData! -) {{ - data: createNotification( - data: $data - ) {{ - {NOTIFICATION_FRAGMENT} - }} -}} -""" - -GQL_UPDATE_PROPERTIES_IN_NOTIFICATION = f""" -mutation( - $id: ID! - $hasBeenSeen: Boolean - $progress: Int - $status: NotificationStatus - $taskId: ID - $url: String -) {{ - data: updatePropertiesInNotification( - where: {{ - id: $id - taskId: $taskId - }} - data: {{ - hasBeenSeen: $hasBeenSeen - progress: $progress - status: $status - url: $url - }} - ) {{ - {NOTIFICATION_FRAGMENT} - }} -}} -""" diff --git a/src/kili/presentation/client/notification.py b/src/kili/presentation/client/notification.py deleted file mode 100644 index 68b060ac5..000000000 --- a/src/kili/presentation/client/notification.py +++ /dev/null @@ -1,145 +0,0 @@ -"""Client presentation methods for notifications.""" - -from collections.abc import Generator, Iterable -from typing import Literal, Optional, overload - -from typeguard import typechecked - -from kili.adapters.kili_api_gateway.helpers.queries import QueryOptions -from kili.domain.notification import NotificationFilter, NotificationId -from kili.domain.types import ListOrTuple -from kili.domain.user import UserFilter, UserId -from kili.presentation.client.helpers.common_validators import ( - disable_tqdm_if_as_generator, - resolve_disable_tqdm, -) -from kili.use_cases.notification import NotificationUseCases -from kili.utils.logcontext import for_all_methods, log_call - -from .base import BaseClientMethods - - -@for_all_methods(log_call, exclude=["__init__"]) -class NotificationClientMethods(BaseClientMethods): - """Methods attached to the Kili client, to run actions on notifications.""" - - @overload - def notifications( - self, - fields: ListOrTuple[str] = ( - "createdAt", - "hasBeenSeen", - "id", - "message", - "status", - "userID", - ), - first: Optional[int] = None, - has_been_seen: Optional[bool] = None, - notification_id: Optional[str] = None, - skip: int = 0, - user_id: Optional[str] = None, - disable_tqdm: Optional[bool] = None, - *, - as_generator: Literal[True], - ) -> Generator[dict, None, None]: - ... - - @overload - def notifications( - self, - fields: ListOrTuple[str] = ( - "createdAt", - "hasBeenSeen", - "id", - "message", - "status", - "userID", - ), - first: Optional[int] = None, - has_been_seen: Optional[bool] = None, - notification_id: Optional[str] = None, - skip: int = 0, - user_id: Optional[str] = None, - disable_tqdm: Optional[bool] = None, - *, - as_generator: Literal[False] = False, - ) -> list[dict]: - ... - - @typechecked - def notifications( - self, - fields: ListOrTuple[str] = ( - "createdAt", - "hasBeenSeen", - "id", - "message", - "status", - "userID", - ), - first: Optional[int] = None, - has_been_seen: Optional[bool] = None, - notification_id: Optional[str] = None, - skip: int = 0, - user_id: Optional[str] = None, - disable_tqdm: Optional[bool] = None, - *, - as_generator: bool = False, - ) -> Iterable[dict]: - # pylint: disable=line-too-long - """Get a generator or a list of notifications respecting a set of criteria. - - Args: - fields: All the fields to request among the possible fields for the notifications - See [the documentation](https://api-docs.kili-technology.com/types/objects/notification) for all possible fields. - first: Number of notifications to query - has_been_seen: If the notifications returned should have been seen. - notification_id: If given, will return the notification which has this id - skip: Number of notifications to skip (they are ordered by their date of creation, - first to last). - user_id: If given, returns the notifications of a specific user - disable_tqdm: If `True`, the progress bar will be disabled - as_generator: If `True`, a generator on the notifications is returned. - - Returns: - An iterable of notifications. - """ - disable_tqdm = resolve_disable_tqdm(disable_tqdm, getattr(self, "disable_tqdm", None)) - disable_tqdm = disable_tqdm_if_as_generator(as_generator, disable_tqdm) - options = QueryOptions(disable_tqdm, first, skip) - filters = NotificationFilter( - has_been_seen=has_been_seen, - id=NotificationId(notification_id) if notification_id else None, - user=UserFilter(id=UserId(user_id)) if user_id else None, - ) - notifications_gen = NotificationUseCases(self.kili_api_gateway).list_notifications( - options=options, fields=fields, filters=filters - ) - if as_generator: - return notifications_gen - return list(notifications_gen) - - @typechecked - def count_notifications( - self, - has_been_seen: Optional[bool] = None, - user_id: Optional[str] = None, - notification_id: Optional[str] = None, - ) -> int: - """Count the number of notifications. - - Args: - has_been_seen: Filter on notifications that have been seen. - user_id: Filter on the notifications of a specific user. - notification_id: Filter on a specific notification. - - Returns: - The number of notifications with the parameters provided - """ - filters = NotificationFilter( - has_been_seen=has_been_seen, - id=NotificationId(notification_id) if notification_id else None, - user=UserFilter(id=UserId(user_id)) if user_id else None, - ) - return NotificationUseCases(self.kili_api_gateway).count_notifications(filters=filters) diff --git a/src/kili/services/asset_import/base.py b/src/kili/services/asset_import/base.py index 0d93fc750..5ad9b9232 100644 --- a/src/kili/services/asset_import/base.py +++ b/src/kili/services/asset_import/base.py @@ -31,6 +31,7 @@ ) from kili.core.helpers import T, format_result, get_mime_type, is_url from kili.core.utils.pagination import batcher +from kili.domain.notification import NotificationFilter, NotificationId from kili.domain.organization import OrganizationFilters from kili.domain.project import InputType, ProjectId from kili.domain.types import ListOrTuple @@ -139,7 +140,13 @@ def verify_batch_imported(self, notification_id: str) -> None: reraise=True, ): with attempt: - notification = self.kili.notifications(notification_id=notification_id)[0] + notification = list( + self.kili.kili_api_gateway.list_notifications( + filters=NotificationFilter(id=NotificationId(notification_id)), + fields=("status",), + options=QueryOptions(disable_tqdm=True, first=1, skip=0), + ) + )[0] if notification["status"] == "FAILURE": error_message = ( "Some assets were not imported. " diff --git a/src/kili/use_cases/notification/__init__.py b/src/kili/use_cases/notification/__init__.py deleted file mode 100644 index 95151a345..000000000 --- a/src/kili/use_cases/notification/__init__.py +++ /dev/null @@ -1,24 +0,0 @@ -"""Notification use cases.""" - -from collections.abc import Generator - -from kili.adapters.kili_api_gateway.helpers.queries import QueryOptions -from kili.domain.notification import NotificationFilter -from kili.domain.types import ListOrTuple -from kili.use_cases.base import BaseUseCases - - -class NotificationUseCases(BaseUseCases): - """Notification use cases.""" - - def list_notifications( - self, filters: NotificationFilter, fields: ListOrTuple[str], options: QueryOptions - ) -> Generator[dict, None, None]: - """List notifications.""" - return self._kili_api_gateway.list_notifications( - filters=filters, fields=fields, options=options - ) - - def count_notifications(self, filters: NotificationFilter) -> int: - """Count notifications.""" - return self._kili_api_gateway.count_notification(filters=filters) diff --git a/tests/integration/presentation/test_notification.py b/tests/integration/presentation/test_notification.py deleted file mode 100644 index c0ab81517..000000000 --- a/tests/integration/presentation/test_notification.py +++ /dev/null @@ -1,21 +0,0 @@ -import pytest_mock - -from kili.presentation.client.notification import NotificationClientMethods -from kili.use_cases.notification import NotificationUseCases - - -def test_given_client_when_fetching_notifications_it_works( - mocker: pytest_mock.MockerFixture, kili_api_gateway -): - mocker.patch.object( - NotificationUseCases, "list_notifications", return_value=(n for n in [{"id": "notif_id"}]) - ) - # Given - kili = NotificationClientMethods() - kili.kili_api_gateway = kili_api_gateway - - # When - notifs = kili.notifications() - - # Then - assert notifs == [{"id": "notif_id"}] diff --git a/tests/unit/services/asset_import/base.py b/tests/unit/services/asset_import/base.py index 5b70cc625..3f2074cf4 100644 --- a/tests/unit/services/asset_import/base.py +++ b/tests/unit/services/asset_import/base.py @@ -28,7 +28,9 @@ def setUp(self): ) self.kili = mocked_auth self.kili.kili_api_gateway.count_assets = MagicMock(return_value=1) - self.kili.notifications = MagicMock(return_value=[{"status": "SUCCESS"}]) + self.kili.kili_api_gateway.list_notifications = MagicMock( + return_value=[{"status": "SUCCESS"}] + ) self.kili.kili_api_gateway.list_assets = MagicMock(return_value=[]) self.kili.kili_api_gateway.list_organizations = MagicMock( return_value=organization_generator(upload_local_data=True) diff --git a/tests/unit/services/asset_import/test_verify_batch_imported.py b/tests/unit/services/asset_import/test_verify_batch_imported.py new file mode 100644 index 000000000..c2db612aa --- /dev/null +++ b/tests/unit/services/asset_import/test_verify_batch_imported.py @@ -0,0 +1,83 @@ +"""Tests for the asynchronous import verification poll. + +`verify_batch_imported` reads notifications through the API gateway rather than through a +client method. These tests exercise the real gateway and fake only the transport, so a +regression in the filter, the requested fields or the pagination options is caught here. +""" + +import re +from unittest.mock import MagicMock + +import pytest + +from kili.adapters.kili_api_gateway.kili_api_gateway import KiliAPIGateway +from kili.domain.project import ProjectId +from kili.services.asset_import.base import BaseBatchImporter, BatchParams, ProjectParams +from kili.services.asset_import.exceptions import BatchImportError + + +def build_importer(notification_status: str): + """Build an importer backed by a real gateway, recording every operation sent.""" + issued = [] + + def execute(query, variables=None, **_kwargs): + match = re.search(r"(?:query|mutation)\s+(\w+)", query) + operation = match.group(1) if match else query + issued.append((operation, query, variables)) + if operation == "countNotifications": + return {"data": 1} + return {"data": [{"status": notification_status}]} + + graphql_client = MagicMock() + graphql_client.execute = MagicMock(side_effect=execute) + kili = MagicMock() + kili.kili_api_gateway = KiliAPIGateway(graphql_client=graphql_client, http_client=MagicMock()) + importer = BaseBatchImporter( + kili, + ProjectParams(project_id=ProjectId("project_id"), input_type="VIDEO"), + BatchParams(is_asynchronous=True, is_hosted=False), + MagicMock(), + ) + return importer, issued + + +def test_given_a_successful_notification_when_verifying_then_it_filters_on_the_notification_id(): + # Given + importer, issued = build_importer("SUCCESS") + + # When + importer.verify_batch_imported("notification_id") + + # Then + operations = [operation for operation, _, _ in issued] + assert operations == ["countNotifications", "notifications"] + + _, notifications_query, notifications_variables = issued[1] + assert notifications_variables == { + "where": {"id": "notification_id"}, + "first": 1, + "skip": 0, + } + assert "status" in notifications_query + + +def test_given_a_successful_notification_when_verifying_then_it_only_requests_the_status_field(): + # Given + importer, issued = build_importer("SUCCESS") + + # When + importer.verify_batch_imported("notification_id") + + # Then + _, notifications_query, _ = issued[1] + requested_fields = re.findall(r"^\s*(\w+)\s*$", notifications_query, flags=re.MULTILINE) + assert requested_fields == ["status"] + + +def test_given_a_failed_notification_when_verifying_then_it_raises_a_batch_import_error(): + # Given + importer, _ = build_importer("FAILURE") + + # When / Then + with pytest.raises(BatchImportError): + importer.verify_batch_imported("notification_id")