From dc55efd0147514f2717e7570975129f7a317b1ea Mon Sep 17 00:00:00 2001 From: MarkLark86 Date: Tue, 12 Nov 2024 16:42:05 +1100 Subject: [PATCH] [NHUB-583] fix(celery): Await when sending task to celery (#1157) * [NHUB-583] fix(celery): Await when sending task to celery * enable celery tests, fix issues found * add types to get_aggregations --- newsroom/agenda/agenda_service.py | 2 +- newsroom/agenda/notifications.py | 116 +++++++++++++------------- newsroom/email.py | 12 +-- newsroom/push/agenda_manager.py | 2 +- newsroom/push/notifications.py | 4 +- newsroom/push/publishing.py | 2 +- newsroom/push/views.py | 6 +- newsroom/reports/content_activity.py | 23 +++-- newsroom/tests/markers.py | 8 +- newsroom/types/history.py | 6 +- newsroom/types/wire.py | 2 + newsroom/wire/filters.py | 8 +- tests/core/test_agenda_events_only.py | 15 ++-- tests/core/test_auth.py | 1 - tests/core/test_emails.py | 10 ++- tests/core/test_push.py | 63 ++++++-------- tests/core/test_push_events.py | 116 ++++++++++---------------- tests/core/test_reports.py | 40 +++++---- tests/utils.py | 2 +- 19 files changed, 205 insertions(+), 233 deletions(-) diff --git a/newsroom/agenda/agenda_service.py b/newsroom/agenda/agenda_service.py index 7f72eb171..1a6a562e4 100644 --- a/newsroom/agenda/agenda_service.py +++ b/newsroom/agenda/agenda_service.py @@ -301,7 +301,7 @@ async def enhance_coverages(self, coverages: list[dict[str, Any]]): wire_items = await WireSearchServiceAsync().get_items_by_id(text_delivery_ids) if await wire_items.count(): async for item in wire_items: - coverage = [c for c in completed_coverages if c.get("delivery_id") == item.get("_id")][0] + coverage = [c for c in completed_coverages if c.get("delivery_id") == item.id][0] coverage["publish_time"] = item.publish_schedule or item.firstpublished async def set_delivery(self, wire_item: dict[str, Any]) -> list[dict[str, Any]]: diff --git a/newsroom/agenda/notifications.py b/newsroom/agenda/notifications.py index 63492facc..bc868bc87 100644 --- a/newsroom/agenda/notifications.py +++ b/newsroom/agenda/notifications.py @@ -138,72 +138,68 @@ async def notify_agenda_update( != (agenda.get("dates") or {}).get("end").replace(tzinfo=None) ) - if agenda.get("state") and agenda.get("state") != original_agenda.get("state"): - state_changed = agenda.get("state") in notify_states - - if state_changed: - _fill_all_coverages( - agenda, - original_agenda, - coverage_updates, - cancelled=False if agenda.get("state") == WORKFLOW_STATE.SCHEDULED else True, - use_original_agenda=True, - ) + if agenda.get("state") and agenda.get("state") != original_agenda.get("state"): + state_changed = agenda.get("state") in notify_states + + if state_changed: + _fill_all_coverages( + agenda, + original_agenda, + coverage_updates, + cancelled=False if agenda.get("state") == WORKFLOW_STATE.SCHEDULED else True, + use_original_agenda=True, + ) + else: + if time_updated: + _fill_all_coverages(agenda, original_agenda, coverage_updates) else: - if time_updated: - _fill_all_coverages(agenda, original_agenda, coverage_updates) - else: - for coverage in agenda.get("coverages") or []: - existing_coverage = next( + for coverage in agenda.get("coverages") or []: + existing_coverage = next( + ( + c + for c in original_agenda.get("coverages") or [] + if c["coverage_id"] == coverage["coverage_id"] + ), + None, + ) + detailed_coverage = _get_detailed_coverage(agenda, original_agenda, coverage) + if detailed_coverage: + if not existing_coverage: + if coverage["workflow_status"] != WORKFLOW_STATE.CANCELLED: + coverage_updates["modified_coverages"].append(detailed_coverage) + elif coverage.get("workflow_status") == WORKFLOW_STATE.CANCELLED and existing_coverage.get( + "workflow_status" + ) != coverage.get("workflow_status"): + coverage_updates["cancelled_coverages"].append(detailed_coverage) + elif ( + ( + coverage.get("delivery_state") != existing_coverage.get("delivery_state") + and coverage.get("delivery_state") == "published" + ) + or ( + coverage.get("workflow_status") != existing_coverage.get("workflow_status") + and coverage.get("workflow_status") == "completed" + ) + or (existing_coverage.get("scheduled") != coverage.get("scheduled")) + ): + coverage_updates["modified_coverages"].append(detailed_coverage) + only_new_coverages = False + elif detailed_coverage["coverage_id"] != (coverage_updated or {}).get("coverage_id"): + coverage_updates["unaltered_coverages"].append(detailed_coverage) + + # Check for removed coverages - show it as cancelled + if item and item.get("type") == "planning": + for original_cov in original_agenda.get("coverages") or []: + updated_cov = next( ( c - for c in original_agenda.get("coverages") or [] - if c["coverage_id"] == coverage["coverage_id"] + for c in (agenda.get("coverages") or []) + if c.get("coverage_id") == original_cov.get("coverage_id") ), None, ) - detailed_coverage = _get_detailed_coverage(agenda, original_agenda, coverage) - if detailed_coverage: - if not existing_coverage: - if coverage["workflow_status"] != WORKFLOW_STATE.CANCELLED: - coverage_updates["modified_coverages"].append(detailed_coverage) - elif coverage.get( - "workflow_status" - ) == WORKFLOW_STATE.CANCELLED and existing_coverage.get( - "workflow_status" - ) != coverage.get( - "workflow_status" - ): - coverage_updates["cancelled_coverages"].append(detailed_coverage) - elif ( - ( - coverage.get("delivery_state") != existing_coverage.get("delivery_state") - and coverage.get("delivery_state") == "published" - ) - or ( - coverage.get("workflow_status") != existing_coverage.get("workflow_status") - and coverage.get("workflow_status") == "completed" - ) - or (existing_coverage.get("scheduled") != coverage.get("scheduled")) - ): - coverage_updates["modified_coverages"].append(detailed_coverage) - only_new_coverages = False - elif detailed_coverage["coverage_id"] != (coverage_updated or {}).get("coverage_id"): - coverage_updates["unaltered_coverages"].append(detailed_coverage) - - # Check for removed coverages - show it as cancelled - if item and item.get("type") == "planning": - for original_cov in original_agenda.get("coverages") or []: - updated_cov = next( - ( - c - for c in (agenda.get("coverages") or []) - if c.get("coverage_id") == original_cov.get("coverage_id") - ), - None, - ) - if not updated_cov: - coverage_updates["cancelled_coverages"].append(original_cov) + if not updated_cov: + coverage_updates["cancelled_coverages"].append(original_cov) if len(coverage_updates["modified_coverages"]) > 0 or len(coverage_updates["cancelled_coverages"]) > 0: coverage_modified = True diff --git a/newsroom/email.py b/newsroom/email.py index 477405324..73513fcec 100644 --- a/newsroom/email.py +++ b/newsroom/email.py @@ -111,7 +111,9 @@ def _send_email(to, subject, text_body, html_body=None, sender=None, sender_name return app.mail.send(msg) -def send_email(to, subject, text_body, html_body=None, sender=None, sender_name=None, attachments_info=None, cc=None): +async def send_email( + to, subject, text_body, html_body=None, sender=None, sender_name=None, attachments_info=None, cc=None +): """ Sends the email :param to: List of recipients @@ -132,7 +134,7 @@ def send_email(to, subject, text_body, html_body=None, sender=None, sender_name= "sender_name": sender_name or get_app_config("EMAIL_DEFAULT_SENDER_NAME"), "attachments_info": attachments_info, } - _send_email.apply_async(kwargs=kwargs) + await _send_email.apply_async(kwargs=kwargs) async def send_new_signup_email(company: Company, user: User, is_new_company: bool): @@ -286,7 +288,7 @@ async def _send_localized_email( text_body = await render_template(text_template, **template_kwargs) html_body = await render_template(html_template, **template_kwargs) - send_email( + await send_email( to=to, cc=cc, subject=subject, @@ -518,7 +520,7 @@ async def _send_wire_killed_notification_email(user: UserResourceModel, item: di subject = gettext("Kill/Takedown notice") text_body = to_text(await formatter.format_item(item)) - send_email(to=recipients, subject=subject, text_body=text_body) + await send_email(to=recipients, subject=subject, text_body=text_body) async def _send_agenda_killed_notification_email(user: UserResourceModel, item: dict[str, Any]) -> None: @@ -527,7 +529,7 @@ async def _send_agenda_killed_notification_email(user: UserResourceModel, item: subject = gettext("%(section)s cancelled notice", section=get_app_config("AGENDA_SECTION")) text_body = to_text(await formatter.format_item(item, item_type="agenda")) - send_email(to=recipients, subject=subject, text_body=text_body) + await send_email(to=recipients, subject=subject, text_body=text_body) def to_text(output: Union[str, bytes]) -> str: diff --git a/newsroom/push/agenda_manager.py b/newsroom/push/agenda_manager.py index 95b076a33..4fb00e9b5 100644 --- a/newsroom/push/agenda_manager.py +++ b/newsroom/push/agenda_manager.py @@ -201,4 +201,4 @@ async def set_item_reference(self, coverage): "coverage_id": coverage.get("coverage_id"), }, ) - notify_new_wire_item.delay(item.id, check_topics=False) + await notify_new_wire_item.delay(item.id, check_topics=False) diff --git a/newsroom/push/notifications.py b/newsroom/push/notifications.py index f8ae4baa5..256e75c65 100644 --- a/newsroom/push/notifications.py +++ b/newsroom/push/notifications.py @@ -240,7 +240,7 @@ async def _send_notification(section: str, users_ids: set[ObjectId]): dict( user=user_id, item=item["_id"], - resource=item.get("type"), + resource=item.get("type") or item.get("_type"), action="history_match", data=None, ) @@ -291,7 +291,7 @@ async def notify_agenda_topic_matches( users_dict: dict[str, User] = {str(user.id): user.to_dict() for user in users.values()} topic_matches |= set( [ - topic + topic["_id"] for topic in await get_agenda_notification_topics_for_query_by_id(item, users_dict) if topic.get("_id") not in topic_matches ] diff --git a/newsroom/push/publishing.py b/newsroom/push/publishing.py index b0c44f058..dfeafdc18 100644 --- a/newsroom/push/publishing.py +++ b/newsroom/push/publishing.py @@ -91,7 +91,7 @@ async def publish_item(self, doc: Item, original: Item): if doc.get("coverage_id"): agenda_items = await AgendaItemService().set_delivery(doc) if agenda_items: - [notify_new_agenda_item.delay(item["_id"], check_topics=False) for item in agenda_items] + [await notify_new_agenda_item.delay(item["_id"], check_topics=False) for item in agenda_items] except Exception as ex: logger.info("Failed to notify new wire item for Agenda watches") logger.exception(ex) diff --git a/newsroom/push/views.py b/newsroom/push/views.py index 712cbbb07..a707db050 100644 --- a/newsroom/push/views.py +++ b/newsroom/push/views.py @@ -37,7 +37,7 @@ async def handle_publish_event(item): orig = await AgendaItemService().find_by_id(item["guid"]) event_id = await publisher.publish_event(item, orig.to_dict() if orig else None) - notify_new_agenda_item.delay(event_id, check_topics=True, is_new=orig is None) + await notify_new_agenda_item.delay(event_id, check_topics=True, is_new=orig is None) async def handle_publish_planning(item): @@ -49,7 +49,7 @@ async def handle_publish_planning(item): # Prefer parent Event when sending notificaitons _id = event_id or plan_id - notify_new_agenda_item.delay(_id, check_topics=True, is_new=orig is None) + await notify_new_agenda_item.delay(_id, check_topics=True, is_new=orig is None) async def handle_publish_text_item(item): @@ -57,7 +57,7 @@ async def handle_publish_text_item(item): item["_id"] = await publisher.publish_item(item, orig.to_dict() if orig else None) if not item.get("nextversion"): - notify_new_wire_item.delay( + await notify_new_wire_item.delay( item["_id"], check_topics=orig is None or get_app_config("WIRE_NOTIFICATIONS_ON_CORRECTIONS") ) diff --git a/newsroom/reports/content_activity.py b/newsroom/reports/content_activity.py index e64cf81c0..41f2328c5 100644 --- a/newsroom/reports/content_activity.py +++ b/newsroom/reports/content_activity.py @@ -1,4 +1,4 @@ -from typing import Any, cast +from typing import Any, cast, TypedDict from copy import deepcopy from quart_babel import gettext @@ -15,6 +15,7 @@ from newsroom.wire import WireItemService from newsroom.agenda.filters import get_date_filters, apply_item_type_filter as apply_agenda_type_filter from newsroom.agenda import AgendaItemService +from newsroom.history_async import HistoryService from newsroom.utils import query_resource, MAX_TERMS_SIZE @@ -23,7 +24,7 @@ async def get_query_source(args: dict[str, Any], source: dict[str, Any]) -> dict[str, Any]: - search_request = NewshubSearchRequest[BaseSearchRequestArgs](section=args["section"]) + search_request = NewshubSearchRequest[BaseSearchRequestArgs](section=SectionEnum(args["section"])) query = search_request.search.query if args.get("genre"): @@ -84,16 +85,22 @@ async def get_items(args): while True: cursor = await service.search(source) - items = await cursor.to_list_raw() + items = await cursor.to_list() if not len(items): break source["from"] += CHUNK_SIZE - yield items + yield [item.to_dict() for item in items] -def get_aggregations(args, ids): +class ItemAggregation(TypedDict): + total: int + actions: dict[str, int] + companies: list[str] + + +async def get_aggregations(args: dict[str, Any], ids: list[str]) -> dict[str, ItemAggregation]: """Get action and company aggregations for the items provided""" if not args.get("section"): @@ -122,8 +129,8 @@ def get_aggregations(args, ids): }, } - results = get_resource_service("history").fetch_history(source) - aggs = (results.get("hits") or {}).get("aggregations") or {} + results = cast(ElasticsearchResourceCursorAsync, await HistoryService().search(source)) + aggs = (results.hits or {}).get("aggregations") or {} buckets = (aggs.get("items") or {}).get("buckets") or [] return { @@ -302,7 +309,7 @@ async def get_content_activity_report(): async for items in get_items(args): item_ids = [item.get("_id") for item in items] - aggs = get_aggregations(args, item_ids) + aggs = await get_aggregations(args, item_ids) for item in items: item_id = item["_id"] diff --git a/newsroom/tests/markers.py b/newsroom/tests/markers.py index a798a12ec..58549dbe1 100644 --- a/newsroom/tests/markers.py +++ b/newsroom/tests/markers.py @@ -5,16 +5,12 @@ # Sets ``app.config["FORCE_ENABLE_GOOGLE_OAUTH"]`` to True enable_google_login = mark.enable_google_login - # Adds ``newsroom.auth.saml`` to ``app.config["INSTALLED_APPS"]`` # This registers SAML views to the auth blueprint enable_saml = mark.enable_saml - -# Skips the test, due to known issue with async changes -# Change this to `requires_async_celery = mark.requires_async_celery` to run these tests -requires_async_celery = mark.skip(reason="Requires celery to support async tasks") - +# Mark tests that require celery +requires_async_celery = mark.requires_async_celery skip_auto_wire_items = mark.skip_auto_wire_items skip_auto_agenda_items = mark.skip_auto_agenda_items diff --git a/newsroom/types/history.py b/newsroom/types/history.py index d12eb14c0..4eb245e09 100644 --- a/newsroom/types/history.py +++ b/newsroom/types/history.py @@ -1,7 +1,7 @@ from typing import Annotated, Any from datetime import datetime -from superdesk.core.resources.fields import ObjectId as ObjectIdField, Keyword +from superdesk.core.resources.fields import ObjectId as ObjectIdField, Keyword, keyword_mapping from superdesk.core.resources.validators import validate_data_relation_async from newsroom.core.resources.model import NewshubResourceModel @@ -10,8 +10,8 @@ class HistoryResourceModel(NewshubResourceModel): action: Keyword versioncreated: datetime - user: Annotated[ObjectIdField | None, validate_data_relation_async("users")] = None - company: Annotated[ObjectIdField | None, validate_data_relation_async("companies")] = None + user: Annotated[ObjectIdField | None, keyword_mapping(), validate_data_relation_async("users")] = None + company: Annotated[ObjectIdField | None, keyword_mapping(), validate_data_relation_async("companies")] = None item: Keyword version: str section: Keyword diff --git a/newsroom/types/wire.py b/newsroom/types/wire.py index 729e7b3ff..53584b497 100644 --- a/newsroom/types/wire.py +++ b/newsroom/types/wire.py @@ -1,4 +1,5 @@ from typing import Annotated +from datetime import datetime from pydantic import Field @@ -14,6 +15,7 @@ class PublishedProduct: class WireItem(ContentAPIItem): products: Annotated[list[PublishedProduct], Field(default_factory=list)] + publish_schedule: datetime | None = None bookmarks: Annotated[list[fields.ObjectId], fields.keyword_mapping(), Field(default_factory=list)] downloads: Annotated[list[fields.ObjectId], fields.keyword_mapping(), Field(default_factory=list)] diff --git a/newsroom/wire/filters.py b/newsroom/wire/filters.py index 19224b28b..4b0d882d8 100644 --- a/newsroom/wire/filters.py +++ b/newsroom/wire/filters.py @@ -89,7 +89,13 @@ def apply_item_type_filter(request: NewshubSearchRequest) -> None: """Applies item type filter(s) based on the request args""" request.search.query.must_not.append({"term": {"type": "composite"}}) - if not request.args.ignore_latest: + try: + ignore_latest = request.args.ignore_latest + except AttributeError: + # This can happen when ``BaseSearchRequestArgs`` is used for search args + ignore_latest = False + + if not ignore_latest: request.search.query.must_not.append({"constant_score": {"filter": {"exists": {"field": "nextversion"}}}}) diff --git a/tests/core/test_agenda_events_only.py b/tests/core/test_agenda_events_only.py index efd1517fa..23ba2b88d 100644 --- a/tests/core/test_agenda_events_only.py +++ b/tests/core/test_agenda_events_only.py @@ -153,7 +153,8 @@ async def test_watched_event_sends_notification_for_event_update(client, app, mo await login_public(client) - await post_json(client, "/agenda_watch", {"items": [event["guid"]]}) + response = await post_json(client, "/agenda_watch", {"items": [event["guid"]]}) + assert response.status_code == 200, await response.get_data(as_text=True) # update comes in event["state"] = "rescheduled" @@ -163,12 +164,11 @@ async def test_watched_event_sends_notification_for_event_update(client, app, mo "tz": "Australia/Sydney", } - push_mock = mocker.patch("newsroom.notifications.push_notification") + push_mock = mocker.patch("newsroom.notifications.utils.push_notification") with app.mail.record_messages() as outbox: await post_json(client, "/push", event) notifications = await get_user_notifications(PUBLIC_USER_ID) - # TODO-ASYNC: len(outbox) is 0 assert len(outbox) == 1 assert "Subject: Prime minister press conference - updated" in str(outbox[0]) @@ -198,12 +198,11 @@ async def test_watched_event_sends_notification_for_unpost_event(client, app, mo event["pubstatus"] = "cancelled" event["state"] = "cancelled" - push_mock = mocker.patch("newsroom.notifications.push_notification") + push_mock = mocker.patch("newsroom.notifications.utils.push_notification") with app.mail.record_messages() as outbox: await post_json(client, "/push", event) notifications = await get_user_notifications(PUBLIC_USER_ID) - # TODO-ASYNC: len(outbox) is 0 assert len(outbox) == 1 assert "Subject: Prime minister press conference - updated" in str(outbox[0]) @@ -230,7 +229,7 @@ async def test_watched_event_sends_notification_for_added_planning(client, app, # planning comes in planning = deepcopy(test_planning) - push_mock = mocker.patch("newsroom.notifications.push_notification") + push_mock = mocker.patch("newsroom.notifications.utils.push_notification") with app.mail.record_messages() as outbox: await post_json(client, "/push", planning) notifications = await get_user_notifications(PUBLIC_USER_ID) @@ -256,7 +255,7 @@ async def test_watched_event_sends_notification_for_cancelled_planning(client, a planning["pubstatus"] = "cancelled" planning["state"] = "cancelled" - push_mock = mocker.patch("newsroom.notifications.push_notification") + push_mock = mocker.patch("newsroom.notifications.utils.push_notification") with app.mail.record_messages() as outbox: await post_json(client, "/push", planning) notifications = await get_user_notifications(PUBLIC_USER_ID) @@ -300,7 +299,7 @@ async def test_watched_event_sends_notification_for_added_coverage(client, app, } ) - push_mock = mocker.patch("newsroom.notifications.push_notification") + push_mock = mocker.patch("newsroom.notifications.utils.push_notification") with app.mail.record_messages() as outbox: await post_json(client, "/push", planning) notifications = await get_user_notifications(PUBLIC_USER_ID) diff --git a/tests/core/test_auth.py b/tests/core/test_auth.py index 4139f9883..94bd46a55 100644 --- a/tests/core/test_auth.py +++ b/tests/core/test_auth.py @@ -173,7 +173,6 @@ async def test_login_fails_for_not_approved_user(app, client): assert "Account has not been approved" in await response.get_data(as_text=True) -# TODO-ASYNC-AUTH: This one fails async def test_login_fails_for_many_times_gets_limited(client, app): for i in range(1, 100): response = await client.post( diff --git a/tests/core/test_emails.py b/tests/core/test_emails.py index feb0655ea..8d989f1b0 100644 --- a/tests/core/test_emails.py +++ b/tests/core/test_emails.py @@ -15,7 +15,7 @@ from unittest import mock from datetime import datetime -from newsroom.types import User +from newsroom.types import User, UserResourceModel, UserRole from newsroom.email import send_user_email from tests.fixtures import agenda_items from newsroom.tests import markers @@ -187,7 +187,7 @@ async def test_email_avoid_long_lines(client, app, mocker): async with app.app_context(): html = "

foo

" * 10000 text = "a" * 500 + " " + "b" * 500 + " " + "c" * 500 + "d" - send_email(html_body=html, text_body=text, to="to", subject="subject") + await send_email(html_body=html, text_body=text, to="to", subject="subject") assert len(sub.mock_calls) call = sub.mock_calls[0] check_lines_length(call.kwargs["kwargs"]["html_body"]) @@ -252,9 +252,11 @@ async def test_send_user_email(app): @markers.requires_async_celery async def test_item_killed_notification_email(app): - user = User( + user = UserResourceModel( + first_name="Foo", + last_name="Bar", email="foo@example.com", - user_type="user", + user_type=UserRole.PUBLIC, ) item = { diff --git a/tests/core/test_push.py b/tests/core/test_push.py index cb829d7c4..ac537fd8d 100644 --- a/tests/core/test_push.py +++ b/tests/core/test_push.py @@ -13,6 +13,7 @@ from newsroom.utils import get_company_dict_async, get_user_dict_async from newsroom.wire import WireSearchServiceAsync from newsroom.notifications import NotificationsService +from newsroom.history_async import HistoryService from newsroom.tests.fixtures import TEST_USER_ID # noqa - Fix cyclic import when running single test file from newsroom.tests import markers @@ -282,6 +283,7 @@ async def test_notify_topic_matches_for_new_item(client, app, mocker): { "email": "foo2@bar.com", "first_name": "Foo", + "last_name": "Bar", "is_enabled": True, "receive_email": True, "user_type": "administrator", @@ -322,14 +324,13 @@ async def test_notify_topic_matches_for_new_item(client, app, mocker): key = b"something random" app.config["PUSH_KEY"] = key - push_mock = mocker.patch("newsroom.push.push_notification") + push_mock = mocker.patch("newsroom.push.notifications.push_notification") data = {"guid": "foo", "type": "text", "headline": "this is a test"} headers = get_signature_headers(json.dumps(data), key) resp = await client.post("/push", json=data, headers=headers) assert 200 == resp.status_code - # TODO-ASYNC: TypeError: 'NoneType' object is not subscriptable assert push_mock.call_args[1]["item"]["_id"] == "foo" assert len(push_mock.call_args[1]["topics"]) == 1 @@ -357,6 +358,7 @@ async def test_notify_user_matches_for_new_item_in_history(client, app, mocker): user = { "email": "foo2@bar.com", "first_name": "Foo", + "last_name": "Bar", "is_enabled": True, "receive_email": True, "receive_app_notifications": True, @@ -366,17 +368,11 @@ async def test_notify_user_matches_for_new_item_in_history(client, app, mocker): user_ids = await create_entries_for("auth_user", [user]) user["_id"] = user_ids[0] - # TODO-ASYNC-AGENDA: Replace this with proper function call - app.data.insert( - "history", - docs=[ - { - "version": "1", - "_id": "bar", - } - ], + await HistoryService().create_history_record( + docs=[{"_id": "bar", "version": "1"}], action="download", - user=user, + user_id=user_ids[0], + company_id=company_ids[0], section="wire", ) @@ -384,12 +380,11 @@ async def test_notify_user_matches_for_new_item_in_history(client, app, mocker): key = b"something random" app.config["PUSH_KEY"] = key data = {"guid": "bar", "type": "text", "headline": "this is a test"} - push_mock = mocker.patch("newsroom.notifications.push_notification") + push_mock = mocker.patch("newsroom.notifications.utils.push_notification") headers = get_signature_headers(json.dumps(data), key) resp = await client.post("/push", json=data, headers=headers) assert 200 == resp.status_code - # TODO-ASYNC: TypeError: 'NoneType' object is not subscriptable assert push_mock.call_args[0][0] == "new_notifications" assert str(user_ids[0]) in push_mock.call_args[1]["counts"].keys() @@ -443,6 +438,7 @@ async def test_notify_user_matches_for_killed_item_in_history(client, app, mocke user = { "email": "foo2@bar.com", "first_name": "Foo", + "last_name": "Bar", "is_enabled": True, "receive_email": False, # should still get email "receive_app_notifications": True, @@ -452,17 +448,12 @@ async def test_notify_user_matches_for_killed_item_in_history(client, app, mocke user_ids = await create_entries_for("auth_user", [user]) user["_id"] = user_ids[0] - # TODO-ASYNC-AGENDA: Replace this with proper function call - app.data.insert( - "history", - docs=[ - { - "version": "1", - "_id": "bar", - } - ], + await HistoryService().create_history_record( + docs=[{"_id": "bar", "version": "1"}], action="download", - user=user, + user_id=user_ids[0], + company_id=company_ids[0], + section="wire", ) key = b"something random" @@ -476,14 +467,13 @@ async def test_notify_user_matches_for_killed_item_in_history(client, app, mocke "body_html": "Killed story", "pubstatus": "canceled", } - push_mock = mocker.patch("newsroom.notifications.push_notification") + push_mock = mocker.patch("newsroom.notifications.utils.push_notification") headers = get_signature_headers(json.dumps(data), key) with app.mail.record_messages() as outbox: resp = await client.post("/push", json=data, headers=headers) assert 200 == resp.status_code - # TODO-ASYNC: TypeError: 'NoneType' object is not subscriptable assert push_mock.call_args[0][0] == "new_notifications" assert str(user_ids[0]) in push_mock.call_args[1]["counts"].keys() assert len(outbox) == 1 @@ -500,6 +490,7 @@ async def test_notify_user_matches_for_new_item_in_bookmarks(client, app, mocker user = { "email": "foo2@bar.com", "first_name": "Foo", + "last_name": "Bar", "is_enabled": True, "is_approved": True, "receive_email": True, @@ -532,7 +523,7 @@ async def test_notify_user_matches_for_new_item_in_bookmarks(client, app, mocker "_id": "bar", "headline": "testing", "service": [{"code": "a", "name": "Service A"}], - "products": [{"code": 1, "name": "product-1"}], + "products": [{"code": "product-1", "name": "product-1"}], } ], ) @@ -552,12 +543,11 @@ async def test_notify_user_matches_for_new_item_in_bookmarks(client, app, mocker key = b"something random" app.config["PUSH_KEY"] = key data = {"guid": "bar", "type": "text", "headline": "this is a test"} - push_mock = mocker.patch("newsroom.notifications.push_notification") + push_mock = mocker.patch("newsroom.notifications.utils.push_notification") headers = get_signature_headers(json.dumps(data), key) resp = await client.post("/push", json=data, headers=headers) assert 200 == resp.status_code - # TODO-ASYNC: TypeError: 'NoneType' object is not subscriptable assert push_mock.call_args[0][0] == "new_notifications" assert str(user_ids[0]) in push_mock.call_args[1]["counts"].keys() @@ -573,11 +563,10 @@ async def test_notify_user_matches_for_new_item_in_bookmarks(client, app, mocker @markers.requires_async_celery async def test_do_not_notify_disabled_user(client, app, mocker): - await create_entries_for( + company_ids = await create_entries_for( "companies", [ { - "_id": 1, "name": "Press 2 co.", "is_enabled": True, } @@ -590,9 +579,10 @@ async def test_do_not_notify_disabled_user(client, app, mocker): { "email": "foo2@bar.com", "first_name": "Foo", + "last_name": "Bar", "is_enabled": True, "receive_email": True, - "company": 1, + "company": company_ids[0], } ], ) @@ -602,9 +592,9 @@ async def test_do_not_notify_disabled_user(client, app, mocker): session["user"] = user resp = await client.post( "users/%s/topics" % user, - json={"label": "bar", "query": "test", "notifications": True}, + json={"label": "bar", "topic_type": "wire", "query": "test", "notifications": True}, ) - assert 201 == resp.status_code + assert 201 == resp.status_code, await resp.get_data(as_text=True) # disable user user = await find_one_by_id("users", user_ids[0]) @@ -615,11 +605,10 @@ async def test_do_not_notify_disabled_user(client, app, mocker): key = b"something random" app.config["PUSH_KEY"] = key data = {"guid": "foo", "type": "text", "headline": "this is a test"} - push_mock = mocker.patch("newsroom.push.push_notification") + push_mock = mocker.patch("newsroom.push.notifications.push_notification") headers = get_signature_headers(json.dumps(data), key) resp = await client.post("/push", json=data, headers=headers) assert 200 == resp.status_code - # TODO-ASYNC: TypeError: 'NoneType' object is not subscriptable assert push_mock.call_args[1]["_items"][0]["_id"] == "foo" @@ -687,6 +676,7 @@ async def test_send_notification_emails(client, app): { "email": "foo2@bar.com", "first_name": "Foo", + "last_name": "Bar", "is_enabled": True, "receive_email": True, "user_type": "administrator", @@ -735,7 +725,6 @@ async def test_send_notification_emails(client, app): resp = await client.post("/push", json=data, headers=headers) assert 200 == resp.status_code - # TODO-ASYNC: len(outbox) is 0 assert len(outbox) == 1 assert "http://localhost:5050/wire?item=foo" in outbox[0].body diff --git a/tests/core/test_push_events.py b/tests/core/test_push_events.py index be453ad81..d742cc619 100644 --- a/tests/core/test_push_events.py +++ b/tests/core/test_push_events.py @@ -8,7 +8,6 @@ from quart import json from quart.datastructures import FileStorage -from superdesk import get_resource_service import newsroom.signals @@ -16,6 +15,7 @@ ADMIN_USER_ID, ) # noqa - Fix cyclic import when running single test file from newsroom.notifications import get_user_notifications +from newsroom.history_async import HistoryService from newsroom.tests import markers from .test_push import get_signature_headers @@ -435,6 +435,7 @@ async def test_notify_topic_matches_for_new_event_item(client, app, mocker): { "email": "foo2@bar.com", "first_name": "Foo", + "last_name": "Bar", "is_enabled": True, "receive_email": True, "user_type": "administrator", @@ -459,12 +460,11 @@ async def test_notify_topic_matches_for_new_event_item(client, app, mocker): key = b"something random" app.config["PUSH_KEY"] = key event["dates"]["start"] = "2018-05-29T04:00:00+0000" - push_mock = mocker.patch("newsroom.push.push_agenda_item_notification") + push_mock = mocker.patch("newsroom.push.notifications.push_agenda_item_notification") headers = get_signature_headers(json.dumps(event), key) resp = await client.post("/push", json=event, headers=headers) assert 200 == resp.status_code - # TODO-ASYNC: TypeError: 'NoneType' object is not subscriptable assert push_mock.call_args[1]["item"]["_id"] == "foo" assert len(push_mock.call_args[1]["topics"]) == 1 @@ -481,6 +481,7 @@ async def test_notify_topic_matches_for_new_planning_item(client, app, mocker): { "email": "foo2@bar.com", "first_name": "Foo", + "last_name": "Bar", "is_enabled": True, "receive_email": True, "user_type": "administrator", @@ -509,12 +510,11 @@ async def test_notify_topic_matches_for_new_planning_item(client, app, mocker): planning["guid"] = "bar2" planning["event_item"] = "foo" data = json.dumps(planning) - push_mock = mocker.patch("newsroom.push.push_agenda_item_notification") + push_mock = mocker.patch("newsroom.push.notifications.push_agenda_item_notification") headers = get_signature_headers(data, key) resp = await client.post("/push", json=planning, headers=headers) assert 200 == resp.status_code - # TODO-ASYNC: TypeError: 'NoneType' object is not subscriptable assert push_mock.call_args[1]["item"]["_id"] == "foo" assert len(push_mock.call_args[1]["topics"]) == 1 @@ -534,6 +534,7 @@ async def test_notify_topic_matches_for_ad_hoc_planning_item(client, app, mocker { "email": "foo2@bar.com", "first_name": "Foo", + "last_name": "Bar", "is_enabled": True, "receive_email": True, "user_type": "administrator", @@ -560,12 +561,11 @@ async def test_notify_topic_matches_for_ad_hoc_planning_item(client, app, mocker # resend the planning item data = json.dumps(planning) - push_mock = mocker.patch("newsroom.push.push_agenda_item_notification") + push_mock = mocker.patch("newsroom.push.notifications.push_agenda_item_notification") headers = get_signature_headers(data, key) resp = await client.post("/push", json=planning, headers=headers) assert 200 == resp.status_code - # TODO-ASYNC: TypeError: 'NoneType' object is not subscriptable assert push_mock.call_args[1]["item"]["_id"] == "bar3" assert len(push_mock.call_args[1]["topics"]) == 1 @@ -586,6 +586,7 @@ async def test_notify_user_matches_for_ad_hoc_agenda_in_history(client, app, moc user = { "email": "foo2@bar.com", "first_name": "Foo", + "last_name": "Bar", "is_enabled": True, "receive_email": True, "receive_app_notifications": True, @@ -595,17 +596,12 @@ async def test_notify_user_matches_for_ad_hoc_agenda_in_history(client, app, moc user_ids = await create_entries_for("auth_user", [user]) user["_id"] = user_ids[0] - # TODO-ASYNC-AGENDA: Replace this with the proper function call - app.data.insert( - "history", - docs=[ - { - "version": "1", - "_id": "bar3", - } - ], + await HistoryService().create_history_record( + docs=[{"_id": "bar3", "version": "1"}], action="download", - user=user, + user_id=user_ids[0], + company_id=company_ids[0], + section="wire", ) # remove event link from planning item @@ -617,16 +613,15 @@ async def test_notify_user_matches_for_ad_hoc_agenda_in_history(client, app, moc app.config["PUSH_KEY"] = key data = json.dumps(planning) - push_mock = mocker.patch("newsroom.notifications.push_notification") + push_mock = mocker.patch("newsroom.notifications.utils.push_notification") headers = get_signature_headers(data, key) resp = await client.post("/push", json=planning, headers=headers) assert 200 == resp.status_code - # TODO-ASYNC: TypeError: 'NoneType' object is not subscriptable assert push_mock.call_args[0][0] == "new_notifications" assert str(user_ids[0]) in push_mock.call_args[1]["counts"].keys() - notification = get_resource_service("notifications").find_one(req=None, user=user_ids[0]) + notification = (await get_user_notifications(user_ids[0]))[0] assert notification["action"] == "history_match" assert notification["item"] == "bar3" assert notification["resource"] == "agenda" @@ -649,6 +644,7 @@ async def test_notify_user_matches_for_new_agenda_in_history(client, app, mocker user = { "email": "foo2@bar.com", "first_name": "Foo", + "last_name": "Bar", "is_enabled": True, "receive_email": True, "receive_app_notifications": True, @@ -658,33 +654,27 @@ async def test_notify_user_matches_for_new_agenda_in_history(client, app, mocker user_ids = await create_entries_for("auth_user", [user]) user["_id"] = user_ids[0] - # TODO-ASYNC-AGENDA: Replace this with the proper function call - app.data.insert( - "history", - docs=[ - { - "version": "1", - "_id": "foo", - } - ], + await HistoryService().create_history_record( + docs=[{"_id": "foo", "version": "1"}], action="download", - user=user, + user_id=user_ids[0], + company_id=company_ids[0], + section="wire", ) key = b"something random" app.config["PUSH_KEY"] = key event = deepcopy(test_event) data = json.dumps(event) - push_mock = mocker.patch("newsroom.notifications.push_notification") + push_mock = mocker.patch("newsroom.notifications.utils.push_notification") headers = get_signature_headers(data, key) resp = await client.post("/push", json=event, headers=headers) assert 200 == resp.status_code - # TODO-ASYNC: TypeError: 'NoneType' object is not subscriptable assert push_mock.call_args[0][0] == "new_notifications" assert str(user_ids[0]) in push_mock.call_args[1]["counts"].keys() - notification = get_resource_service("notifications").find_one(req=None, user=user_ids[0]) + notification = (await get_user_notifications(user_ids[0]))[0] assert notification["action"] == "history_match" assert notification["item"] == "foo" assert notification["resource"] == "agenda" @@ -710,6 +700,7 @@ async def test_notify_user_matches_for_new_planning_in_history(client, app, mock user = { "email": "foo2@bar.com", "first_name": "Foo", + "last_name": "Bar", "is_enabled": True, "receive_email": True, "receive_app_notifications": True, @@ -719,17 +710,12 @@ async def test_notify_user_matches_for_new_planning_in_history(client, app, mock user_ids = await create_entries_for("auth_user", [user]) user["_id"] = user_ids[0] - # TODO-ASYNC-AGENDA: Replace this with the proper function call - app.data.insert( - "history", - docs=[ - { - "version": "1", - "_id": "foo", - } - ], + await HistoryService().create_history_record( + docs=[{"_id": "foo", "version": "1"}], action="download", - user=user, + user_id=user_ids[0], + company_id=company_ids[0], + section="wire", ) key = b"something random" @@ -739,16 +725,15 @@ async def test_notify_user_matches_for_new_planning_in_history(client, app, mock planning["guid"] = "bar2" planning["event_item"] = "foo" data = json.dumps(planning) - push_mock = mocker.patch("newsroom.notifications.push_notification") + push_mock = mocker.patch("newsroom.notifications.utils.push_notification") headers = get_signature_headers(data, key) resp = await client.post("/push", json=planning, headers=headers) assert 200 == resp.status_code - # TODO-ASYNC: TypeError: 'NoneType' object is not subscriptable assert push_mock.call_args[0][0] == "new_notifications" assert str(user_ids[0]) in push_mock.call_args[1]["counts"].keys() - notification = get_resource_service("notifications").find_one(req=None, user=user_ids[0]) + notification = (await get_user_notifications(user_ids[0]))[0] assert notification["action"] == "history_match" assert notification["item"] == "foo" assert notification["resource"] == "agenda" @@ -774,6 +759,7 @@ async def test_notify_user_matches_for_killed_item_in_history(client, app, mocke user = { "email": "foo2@bar.com", "first_name": "Foo", + "last_name": "Bar", "is_enabled": True, "receive_email": False, # should still get email "receive_app_notifications": True, @@ -783,17 +769,12 @@ async def test_notify_user_matches_for_killed_item_in_history(client, app, mocke user_ids = await create_entries_for("auth_user", [user]) user["_id"] = user_ids[0] - # TODO-ASYNC-AGENDA: Replace this with the proper function call - app.data.insert( - "history", - docs=[ - { - "version": "1", - "_id": "foo", - } - ], + await HistoryService().create_history_record( + docs=[{"_id": "foo", "version": "1"}], action="download", - user=user, + user_id=user_ids[0], + company_id=company_ids[0], + section="wire", ) key = b"something random" @@ -801,18 +782,18 @@ async def test_notify_user_matches_for_killed_item_in_history(client, app, mocke event["pubstatus"] = "cancelled" event["state"] = "cancelled" data = json.dumps(event) - push_mock = mocker.patch("newsroom.notifications.push_notification") + push_mock = mocker.patch("newsroom.notifications.utils.push_notification") headers = get_signature_headers(data, key) with app.mail.record_messages() as outbox: resp = await client.post("/push", json=event, headers=headers) assert 200 == resp.status_code - # TODO-ASYNC: TypeError: 'NoneType' object is not subscriptable assert push_mock.call_args[0][0] == "new_notifications" assert str(user_ids[0]) in push_mock.call_args[1]["counts"].keys() assert len(outbox) == 1 - notification = get_resource_service("notifications").find_one(req=None, user=user_ids[0]) + + notification = (await get_user_notifications(user_ids[0]))[0] assert notification["action"] == "history_match" assert notification["item"] == "foo" assert notification["resource"] == "agenda" @@ -908,12 +889,11 @@ async def test_watched_event_sends_notification_for_event_update(client, app, mo "tz": "Australia/Sydney", } - push_mock = mocker.patch("newsroom.notifications.push_notification") + push_mock = mocker.patch("newsroom.notifications.utils.push_notification") with app.mail.record_messages() as outbox: await post_json(client, "/push", event) notifications = await get_user_notifications(user_id) - # TODO-ASYNC: len(outbox) is 0 assert len(outbox) == 1 assert "Subject: Prime minister press conference - updated" in str(outbox[0]) assert "The event you have been following has been rescheduled" in str(outbox[0]) @@ -943,12 +923,11 @@ async def test_watched_event_sends_notification_for_unpost_event(client, app, mo event["pubstatus"] = "cancelled" event["state"] = "cancelled" - push_mock = mocker.patch("newsroom.notifications.push_notification") + push_mock = mocker.patch("newsroom.notifications.utils.push_notification") with app.mail.record_messages() as outbox: await post_json(client, "/push", event) notifications = await get_user_notifications(user_id) - # TODO-ASYNC: len(outbox) is 0 assert len(outbox) == 1 assert "Subject: Prime minister press conference - Coverage updated" in str(outbox[0]) assert "The event you have been following has been cancelled" in str(outbox[0]) @@ -975,12 +954,11 @@ async def test_watched_event_sends_notification_for_added_planning(client, app, # planning comes in planning = deepcopy(test_planning) - push_mock = mocker.patch("newsroom.notifications.push_notification") + push_mock = mocker.patch("newsroom.notifications.utils.push_notification") with app.mail.record_messages() as outbox: await post_json(client, "/push", planning) notifications = await get_user_notifications(user_id) - # TODO-ASYNC: len(outbox) is 0 assert len(outbox) == 1 assert "Subject: Prime minister press conference - Coverage updated" in str(outbox[0]) assert "The event you have been following has new coverage(s)" in str(outbox[0]) @@ -1012,12 +990,11 @@ async def test_watched_event_sends_notification_for_cancelled_planning(client, a planning["pubstatus"] = "cancelled" planning["state"] = "cancelled" - push_mock = mocker.patch("newsroom.notifications.push_notification") + push_mock = mocker.patch("newsroom.notifications.utils.push_notification") with app.mail.record_messages() as outbox: await post_json(client, "/push", planning) notifications = await get_user_notifications(user_id) - # TODO-ASYNC: len(outbox) is 0 assert len(outbox) == 1 assert "Subject: Prime minister press conference - Coverage updated" in str(outbox[0]) assert "! Text coverage 'Vivid Text Explainer' has been cancelled.\r\nNote: ed note here" in str(outbox[0]) @@ -1055,7 +1032,7 @@ async def test_watched_event_sends_notification_for_added_coverage(client, app, "ednote": "ed note here", "scheduled": "2018-05-29T10:51:52+0000", }, - "coverage_status": { + "news_coverage_status": { "name": "coverage intended", "label": "Planned", "qcode": "ncostat:int", @@ -1066,12 +1043,11 @@ async def test_watched_event_sends_notification_for_added_coverage(client, app, } ) - push_mock = mocker.patch("newsroom.notifications.push_notification") + push_mock = mocker.patch("newsroom.notifications.utils.push_notification") with app.mail.record_messages() as outbox: await post_json(client, "/push", planning) notifications = await get_user_notifications(user_id) - # TODO-ASYNC: len(outbox) is 0 assert len(outbox) == 1 assert "Subject: Prime minister press conference - Coverage updated" in str(outbox[0]) assert "! Video coverage 'Vivid planning item' due" in str(outbox[0]) diff --git a/tests/core/test_reports.py b/tests/core/test_reports.py index 317220658..a10b9ec3c 100644 --- a/tests/core/test_reports.py +++ b/tests/core/test_reports.py @@ -207,24 +207,22 @@ async def test_companies(client): assert report["results"][2]["name"] == "Paper Co." -# TODO-ASYNC :- Needs async reports Resource - -# async def test_content_activity_csv(client): -# today = datetime.now().date() -# resp = await client.get( -# "reports/export/content-activity?export=true&date_from={}&date_to={}".format( -# today.isoformat(), today.isoformat() -# ) -# ) -# assert 200 == resp.status_code - -# report = await resp.get_data(as_text=True) -# lines = report.splitlines() -# assert len(lines) > 1 - -# fields = lines[0].split(",") -# assert "Headline" == fields[1] - -# values = lines[1].split(",") -# assert "Amazon Is Opening More Bookstores" == values[1] -# assert "0" == values[-1] +async def test_content_activity_csv(client): + today = datetime.now().date() + resp = await client.get( + "reports/export/content-activity?export=true&date_from={}&date_to={}".format( + today.isoformat(), today.isoformat() + ) + ) + assert 200 == resp.status_code + + report = await resp.get_data(as_text=True) + lines = report.splitlines() + assert len(lines) > 1 + + fields = lines[0].split(",") + assert "Headline" == fields[1] + + values = lines[1].split(",") + assert "Amazon Is Opening More Bookstores" == values[1] + assert "0" == values[-1] diff --git a/tests/utils.py b/tests/utils.py index 9ebfb5185..14ca25086 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -69,7 +69,7 @@ def get_admin_user_id(app): return (get_resource_service("users").find_one(req=None, _id=ADMIN_USER_ID) or {}).get("_id") -def mock_send_email( +async def mock_send_email( to, subject, text_body, cc=None, html_body=None, sender=None, sender_name=None, attachments_info=[] ): if sender is None: