From 151226eafcc80b0de47139604b4426aee7f74656 Mon Sep 17 00:00:00 2001 From: Jonathan Rios Date: Tue, 6 Feb 2024 15:43:05 +0100 Subject: [PATCH] LITE-29268 User can delete or update a feed --- .../credentials/api/schemas.py | 2 +- connect_bi_reporter/feeds/api/schemas.py | 14 +- connect_bi_reporter/feeds/api/views.py | 49 ++++- connect_bi_reporter/feeds/errors.py | 4 +- connect_bi_reporter/feeds/services.py | 44 ++++- connect_bi_reporter/feeds/validator.py | 6 +- connect_bi_reporter/schemas.py | 79 ++++---- tests/feeds/api/test_views.py | 182 +++++++++++++++++- tests/test_schemas.py | 32 +++ 9 files changed, 361 insertions(+), 51 deletions(-) create mode 100644 tests/test_schemas.py diff --git a/connect_bi_reporter/credentials/api/schemas.py b/connect_bi_reporter/credentials/api/schemas.py index a969f0e..152fe1f 100644 --- a/connect_bi_reporter/credentials/api/schemas.py +++ b/connect_bi_reporter/credentials/api/schemas.py @@ -1,6 +1,6 @@ from typing import Optional -from connect_bi_reporter.schemas import Events, NonNullSchema, ReferenceSchema +from connect_extension_utils.api.schemas import Events, NonNullSchema, ReferenceSchema class CredentialCreateSchema(NonNullSchema): diff --git a/connect_bi_reporter/feeds/api/schemas.py b/connect_bi_reporter/feeds/api/schemas.py index 0ff7c2a..da8ee1f 100644 --- a/connect_bi_reporter/feeds/api/schemas.py +++ b/connect_bi_reporter/feeds/api/schemas.py @@ -1,6 +1,8 @@ from typing import Optional -from connect_bi_reporter.schemas import Events, NonNullSchema, ReferenceSchema +from connect_extension_utils.api.schemas import Events, NonNullSchema, ReferenceSchema + +from connect_bi_reporter.schemas import flatten class FeedCreateSchema(NonNullSchema): @@ -31,3 +33,13 @@ def map_to_feed_schema(feed): 'updated': {'at': feed.updated_at, 'by': {'id': feed.updated_by}}, }, ) + + +class FeedUpdateSchema(NonNullSchema): + credential: Optional[ReferenceSchema] + file_name: Optional[str] + description: Optional[str] + + def dict(self, *args, **kwargs): + result = super().dict(*args, **kwargs) + return flatten(result) diff --git a/connect_bi_reporter/feeds/api/views.py b/connect_bi_reporter/feeds/api/views.py index b65ae7a..53c9547 100644 --- a/connect_bi_reporter/feeds/api/views.py +++ b/connect_bi_reporter/feeds/api/views.py @@ -9,8 +9,19 @@ from connect_extension_utils.api.views import get_user_data_from_auth_token from connect_extension_utils.db.models import get_db, VerboseBaseSession -from connect_bi_reporter.feeds.api.schemas import FeedCreateSchema, FeedSchema, map_to_feed_schema -from connect_bi_reporter.feeds.services import create_feed, get_feed_or_404, get_feeds +from connect_bi_reporter.feeds.api.schemas import ( + FeedCreateSchema, + FeedSchema, + FeedUpdateSchema, + map_to_feed_schema, +) +from connect_bi_reporter.feeds.services import ( + create_feed, + delete_feed, + get_feed_or_404, + get_feeds, + update_feed, +) from connect_bi_reporter.feeds.validator import FeedValidator @@ -63,3 +74,37 @@ def create_feed( account_id = installation['owner']['id'] feed = create_feed(db, feed_schema, account_id, logged_user_data) return map_to_feed_schema(feed) + + @router.put( + '/feeds/{feed_id}', + summary='Update a Feed', + response_model=FeedSchema, + status_code=status.HTTP_200_OK, + ) + def update_feed( + self, + feed_id: str, + feed_schema: FeedUpdateSchema, + db: VerboseBaseSession = Depends(get_db), + installation: dict = Depends(get_installation), + logger: Logger = Depends(get_logger), + request: Request = None, + ): + logged_user_data = get_user_data_from_auth_token(request.headers['connect-auth']) + feed = update_feed( + db, feed_schema, installation, feed_id, logged_user_data, logger, + ) + return map_to_feed_schema(feed) + + @router.delete( + '/feeds/{feed_id}', + summary='Delete a Feed', + status_code=status.HTTP_204_NO_CONTENT, + ) + def delete_feed( + self, + feed_id: str, + db: VerboseBaseSession = Depends(get_db), + installation: dict = Depends(get_installation), + ): + delete_feed(db, installation, feed_id) diff --git a/connect_bi_reporter/feeds/errors.py b/connect_bi_reporter/feeds/errors.py index 5ea404c..b5cf399 100644 --- a/connect_bi_reporter/feeds/errors.py +++ b/connect_bi_reporter/feeds/errors.py @@ -6,5 +6,7 @@ class FeedError(ExtensionErrorBase): ERRORS = { 0: "Report schedule `{report_schedule}` not valid for feed creation: {reason}", - 1: "Credential `{credential_id}` not valid for feed creation.", + 1: "Can not {action} Feed, the Credential `{credential_id}` is not valid.", + 2: "Can not delete Feed `{feed_id}`, " + "is already related to Uploads `{uploads}`.", } diff --git a/connect_bi_reporter/feeds/services.py b/connect_bi_reporter/feeds/services.py index 320eae8..21e4227 100644 --- a/connect_bi_reporter/feeds/services.py +++ b/connect_bi_reporter/feeds/services.py @@ -1,9 +1,12 @@ +from logging import Logger from typing import Any, Dict from connect_extension_utils.api.views import get_object_or_404 +from connect_bi_reporter.feeds.api.schemas import FeedCreateSchema, FeedUpdateSchema +from connect_bi_reporter.feeds.errors import FeedError from connect_bi_reporter.feeds.models import Feed -from connect_bi_reporter.feeds.api.schemas import FeedCreateSchema +from connect_bi_reporter.feeds.validator import FeedValidator def create_feed(db, data: FeedCreateSchema, account_id: str, user: Dict[str, Any]): @@ -34,3 +37,42 @@ def get_feed_or_404(db, installation: Dict[str, Any], feed_id: str): def get_feeds(db, installation: Dict[str, Any]): return db.query(Feed).filter_by(account_id=installation['owner']['id']).all() + + +def update_feed( + db, + data: FeedUpdateSchema, + installation: Dict[str, Any], + feed_id: str, + user: Dict[str, str], + logger: Logger, +): + feed = get_feed_or_404(db, installation, feed_id) + update_dict = data.dict() + if update_dict: + if data.credential: + FeedValidator.validate_credential(db, None, installation, data, logger, action='update') + for k, v in update_dict.items(): + setattr(feed, k, v) + feed.updated_by = user['id'] + db.commit() + db.refresh(feed) + return feed + + +def delete_feed( + db, + installation: Dict[str, Any], + feed_id: str, +): + feed = get_feed_or_404(db, installation, feed_id) + related_uploads = feed.upload.all() + if related_uploads: + raise FeedError.RF_002( + format_kwargs={ + 'feed_id': feed.id, + 'uploads': ', '.join(upload.id for upload in related_uploads), + }, + ) + db.delete(feed) + db.commit() diff --git a/connect_bi_reporter/feeds/validator.py b/connect_bi_reporter/feeds/validator.py index e4023b4..0a0f0ef 100644 --- a/connect_bi_reporter/feeds/validator.py +++ b/connect_bi_reporter/feeds/validator.py @@ -54,11 +54,15 @@ def validate_credential(cls, *args, **kwargs): installation = args[2] feed_schema = args[3] logger = args[4] + action = kwargs.pop('action', 'create') cred = db.query(Credential).filter( Credential.account_id == installation['owner']['id'], Credential.id == feed_schema.credential.id, ).one_or_none() if not cred: - exc = FeedError.RF_001(format_kwargs={'credential_id': feed_schema.credential.id}) + exc = FeedError.RF_001(format_kwargs={ + 'credential_id': feed_schema.credential.id, + 'action': action, + }) logger.warning(exc.message) raise exc diff --git a/connect_bi_reporter/schemas.py b/connect_bi_reporter/schemas.py index 04b2fac..0d2fc20 100644 --- a/connect_bi_reporter/schemas.py +++ b/connect_bi_reporter/schemas.py @@ -1,46 +1,41 @@ -from datetime import datetime -from typing import Dict, Optional, Union +from typing import Any, Dict -from pydantic import BaseModel, root_validator -_By = Optional[Union[str, Dict[str, str]]] -Events = Dict[str, Dict[str, Union[datetime, _By]]] - - -def clean_empties_from_dict(data): +def flatten(d: Dict[Any, Any]): """ - Removes inplace all the fields that are None or empty dicts in data. - Returns param data, that was modified inplace. - If the param is not a dict, will return the param unmodified. - :param data: dict - :rtype: dict + Transform nested dict to flat. + :param data: dict + :rtype: dict + + ``` + >>> d = { + 'greatgrandparent': { + 'grandparent': { + 'parent': { + 'child1': 'test', + 'child2': 'other_test', + }, + }, + }, + } + >>> flatten(d) + { + 'greatgrandparent_grandparent_parent_child1': 'test', + 'greatgrandparent_grandparent_parent_child2': 'other_test', + } + + ``` """ - if not isinstance(data, dict): - return data - - for key in list(data.keys()): - value = data[key] - if isinstance(value, dict): - clean_empties_from_dict(value) - value = data[key] - if not value: - del data[key] - return data - - -class NonNullSchema(BaseModel): - def dict(self, *args, **kwargs): - kwargs['exclude_none'] = True - return super().dict(*args, **kwargs) - - @root_validator(pre=True) - def validate_events(cls, values): - events = values.get('events') - if events: - values['events'] = clean_empties_from_dict(events) - return values - - -class ReferenceSchema(NonNullSchema): - id: str - name: Optional[str] + out = {} + for key, val in d.items(): + if isinstance(val, dict): + val = [val] + if isinstance(val, list): + for subdict in val: + deeper = flatten(subdict).items() + out.update( + {key + '_' + key2: val2 for key2, val2 in deeper}, + ) + else: + out[key] = val + return out diff --git a/tests/feeds/api/test_views.py b/tests/feeds/api/test_views.py index e66196d..fe9d2ab 100644 --- a/tests/feeds/api/test_views.py +++ b/tests/feeds/api/test_views.py @@ -1,5 +1,6 @@ from connect.client import ClientError import pytest +from connect_extension_utils.api.views import get_user_data_from_auth_token def test_create_feed( @@ -121,9 +122,11 @@ def test_create_feed_fail_credential_not_found( response_data = response.json() assert response_data == { 'error_code': 'RF_001', - 'errors': ['Credential `CRED-001` not valid for feed creation.'], + 'errors': ['Can not create Feed, the Credential `CRED-001` is not valid.'], } - assert caplog.records[0].message == 'Credential `CRED-001` not valid for feed creation.' + assert caplog.records[0].message == ( + 'Can not create Feed, the Credential `CRED-001` is not valid.' + ) def test_list_feeds(installation, feed_factory, api_client, connect_auth_header): @@ -210,3 +213,178 @@ def test_get_feed_404(installation, feed_factory, api_client, connect_auth_heade ) assert response.status_code == 404 + + +def test_update_feed( + installation, + feed_factory, + credential_factory, + api_client, + connect_auth_header, +): + cred = credential_factory(account_id=installation['owner']['id']) + feed = feed_factory(account_id=installation['owner']['id'], credential_id=cred.id) + updated_at = feed.updated_at + + body = { + 'credential': {'id': feed.credential_id}, + 'file_name': 'The file name', + } + response = api_client.put( + f'/api/feeds/{feed.id}', + installation=installation, + headers={'connect-auth': connect_auth_header}, + json=body, + ) + user = get_user_data_from_auth_token(connect_auth_header) + + assert response.status_code == 200 + + response_data = response.json() + assert response_data['id'] == feed.id + assert response_data['id'].startswith(feed_factory._meta.model.PREFIX) + assert response_data['file_name'] == body['file_name'] + assert response_data['description'] == feed.description + assert response_data['owner']['id'] == installation['owner']['id'] + events = response_data['events'] + assert events['created']['at'] is not None + assert events['created']['by'] is not None + assert feed.updated_at > updated_at + assert events['updated']['by']['id'] == user['id'] + + +def test_update_feed_404(installation, api_client, connect_auth_header): + body = { + 'credential': {'id': 'test'}, + 'file_name': 'The file name', + } + response = api_client.put( + '/api/feeds/NOT-FOUND', + installation=installation, + headers={'connect-auth': connect_auth_header}, + json=body, + ) + + assert response.status_code == 404 + response_data = response.json() + assert response_data['error_code'] == 'NFND_000' + assert response_data['errors'][0] == 'Object `NOT-FOUND` not found.' + + +def test_update_feed_nothing_to_update( + installation, + api_client, + connect_auth_header, + credential_factory, + feed_factory, +): + cred = credential_factory(account_id=installation['owner']['id']) + feed = feed_factory(account_id=installation['owner']['id'], credential_id=cred.id) + updated_at = feed.updated_at + body = {} + response = api_client.put( + f'/api/feeds/{feed.id}', + installation=installation, + headers={'connect-auth': connect_auth_header}, + json=body, + ) + + assert response.status_code == 200 + + response_data = response.json() + assert response_data['id'] is not None + assert response_data['file_name'] == feed.file_name + assert response_data['owner']['id'] == installation['owner']['id'] + events = response_data['events'] + assert events['created']['at'] is not None + assert events['created']['by'] is not None + assert events['updated']['at'] == feed.updated_at.isoformat() + assert feed.updated_at == updated_at + assert events['updated']['by'] == {'id': feed.updated_by} + + +def test_update_feed_invalid_credential( + installation, + api_client, + connect_auth_header, + credential_factory, + feed_factory, +): + cred = credential_factory() + feed = feed_factory(account_id=installation['owner']['id']) + body = { + 'credential': {'id': cred.id}, + 'file_name': 'The file name', + } + response = api_client.put( + f'/api/feeds/{feed.id}', + installation=installation, + headers={'connect-auth': connect_auth_header}, + json=body, + ) + assert response.status_code == 400 + response_data = response.json() + assert response_data['error_code'] == 'RF_001' + assert response_data['errors'][0] == ( + f'Can not update Feed, the Credential `{cred.id}` is not valid.' + ) + + +def test_delete_feed( + dbsession, + installation, + api_client, + connect_auth_header, + feed_factory, +): + feed = feed_factory(account_id=installation['owner']['id']) + response = api_client.delete( + f'/api/feeds/{feed.id}', + installation=installation, + headers={'connect-auth': connect_auth_header}, + ) + assert response.status_code == 204 + assert not dbsession.query(feed_factory._meta.model).all() + + +def test_delete_feed_404( + installation, + api_client, + connect_auth_header, +): + response = api_client.delete( + '/api/feeds/NOT-FOUND', + installation=installation, + headers={'connect-auth': connect_auth_header}, + ) + + assert response.status_code == 404 + response_data = response.json() + assert response_data['error_code'] == 'NFND_000' + assert response_data['errors'][0] == 'Object `NOT-FOUND` not found.' + + +def test_delete_feed_already_in_use( + dbsession, + installation, + api_client, + connect_auth_header, + feed_factory, + upload_factory, +): + feed = feed_factory(account_id=installation['owner']['id']) + upload_factory(feed_id=feed.id) + + response = api_client.delete( + f'/api/feeds/{feed.id}', + installation=installation, + headers={'connect-auth': connect_auth_header}, + ) + assert response.status_code == 400 + response_data = response.json() + assert response_data['error_code'] == 'RF_002' + assert response_data['errors'][0] == ( + f'Can not delete Feed `{feed.id}`, ' + f'is already related to Uploads `{", ".join(feed.id for feed in feed.upload.all())}`.' + ) + assert dbsession.query(feed_factory._meta.model).count() == 1 diff --git a/tests/test_schemas.py b/tests/test_schemas.py new file mode 100644 index 0000000..ca52091 --- /dev/null +++ b/tests/test_schemas.py @@ -0,0 +1,32 @@ +import pytest + +from connect_bi_reporter.schemas import flatten + + +@pytest.mark.parametrize( + 'test_case,expected', + ( + ( + { + 'greatgrandparent': { + 'grandparent': { + 'parent': { + 'child1': 'test', + 'child2': 'other_test', + }, + }, + }, + }, + { + 'greatgrandparent_grandparent_parent_child1': 'test', + 'greatgrandparent_grandparent_parent_child2': 'other_test', + }, + ), + ( + {'parent': {'child': 'test'}, 'mother': 'other'}, + {'parent_child': 'test', 'mother': 'other'}, + ), + ), +) +def test_flatten(test_case, expected): + assert flatten(test_case) == expected