From a698cf65aeaea92705992ece03cc18ab208efe3f Mon Sep 17 00:00:00 2001 From: Helmy Giacoman Date: Thu, 5 Sep 2024 19:27:41 +0200 Subject: [PATCH 01/18] Implement async service, module and update views NHUB-534 --- newsroom/exceptions.py | 31 +++++ newsroom/news_api/section_filters/__init__.py | 16 ++- newsroom/section_filters/__init__.py | 21 +-- newsroom/section_filters/model.py | 12 ++ newsroom/section_filters/module.py | 43 ++++++ newsroom/section_filters/service.py | 6 + newsroom/section_filters/views.py | 131 +++++++++++------- newsroom/users/views.py | 2 +- newsroom/utils.py | 45 +++++- newsroom/web/default_settings.py | 3 +- 10 files changed, 231 insertions(+), 79 deletions(-) create mode 100644 newsroom/section_filters/model.py create mode 100644 newsroom/section_filters/module.py create mode 100644 newsroom/section_filters/service.py diff --git a/newsroom/exceptions.py b/newsroom/exceptions.py index 875181279..9d2276dd6 100644 --- a/newsroom/exceptions.py +++ b/newsroom/exceptions.py @@ -1,3 +1,8 @@ +import json +from typing import Any +from werkzeug.exceptions import HTTPException + + class AuthorizationError(Exception): code: int message: str @@ -6,3 +11,29 @@ def __init__(self, code: int, message: str): super().__init__(self) self.code = code self.message = message + + +class ValidationException(HTTPException): + """ + Custom exception class to handle validation errors and return them in a JSON format. + + This exception is intended to be raised when there are validation errors in user input + during request processing. + + Attributes: + code (int): HTTP status code, defaults to 400 (Bad Request). + errors (Any): A collection of validation error details that will be returned in the response. + """ + + code = 400 + + def __init__(self, errors: Any, description=None): + super().__init__(description) + self.errors = errors + + def get_body(self, *args): + return json.dumps(self.errors) + + def get_headers(self, *args): + """Ensure that the content type is JSON""" + return [("Content-Type", "application/json")] diff --git a/newsroom/news_api/section_filters/__init__.py b/newsroom/news_api/section_filters/__init__.py index 608642e79..c99f99d8e 100644 --- a/newsroom/news_api/section_filters/__init__.py +++ b/newsroom/news_api/section_filters/__init__.py @@ -1,14 +1,16 @@ import superdesk -from newsroom.section_filters import SectionFiltersService, SectionFiltersResource + +# from newsroom.section_filters import SectionFiltersService, SectionFiltersResource def init_app(app): - superdesk.register_resource("section_filters", NewsAPISectionFilterResource, SectionFiltersService, _app=app) + # superdesk.register_resource("section_filters", NewsAPISectionFilterResource, SectionFiltersService, _app=app) + pass -class NewsAPISectionFilterResource(SectionFiltersResource): - """ - Overload the newsroom section filter resource so we can set it to be an internal resource - """ +# class NewsAPISectionFilterResource(SectionFiltersResource): +# """ +# Overload the newsroom section filter resource so we can set it to be an internal resource +# """ - internal_resource = True +# internal_resource = True diff --git a/newsroom/section_filters/__init__.py b/newsroom/section_filters/__init__.py index a3706a19a..8fe58404b 100644 --- a/newsroom/section_filters/__init__.py +++ b/newsroom/section_filters/__init__.py @@ -1,19 +1,6 @@ -from quart_babel import lazy_gettext -import superdesk -from superdesk.flask import Blueprint +from .model import SectionFilter +from .service import SectionFiltersService -from .section_filters import SectionFiltersResource, SectionFiltersService +from .module import module # noqa -blueprint = Blueprint("section_filters", __name__) - -from . import views # noqa - - -def init_app(app): - superdesk.register_resource("section_filters", SectionFiltersResource, SectionFiltersService, _app=app) - app.settings_app( - "section-filters", - lazy_gettext("Section Filters"), - weight=450, - data=views.get_settings_data, - ) +__all__ = ["SectionFilter", "SectionFiltersService"] diff --git a/newsroom/section_filters/model.py b/newsroom/section_filters/model.py new file mode 100644 index 000000000..e75191daf --- /dev/null +++ b/newsroom/section_filters/model.py @@ -0,0 +1,12 @@ +from typing import Optional +from newsroom.core.resources.model import NewshubResourceModel + + +class SectionFilter(NewshubResourceModel): + name: str + description: Optional[str] + sd_product_id: Optional[str] = None + query: Optional[str] = None + is_enabled: bool = True + filter_type: Optional[str] = "wire" + search_type: Optional[str] = "wire" diff --git a/newsroom/section_filters/module.py b/newsroom/section_filters/module.py new file mode 100644 index 000000000..4795074f7 --- /dev/null +++ b/newsroom/section_filters/module.py @@ -0,0 +1,43 @@ +from quart_babel import lazy_gettext + +from newsroom import MONGO_PREFIX +from newsroom.web.factory import NewsroomWebApp +from superdesk.core.module import Module +from superdesk.core.resources import ResourceConfig, MongoIndexOptions, MongoResourceConfig + +from .model import SectionFilter +from .service import SectionFiltersService +from .views import get_settings_data, section_filters_endpoints + +section_filters_config = ResourceConfig( + name="section_filters", + data_class=SectionFilter, + service=SectionFiltersService, + mongo=MongoResourceConfig( + prefix=MONGO_PREFIX, + indexes=MongoIndexOptions( + name="name", + keys=[("name", 1)], + unique=True, + collation={"locale": "en", "strength": 2}, + ), + ), + default_sort=[("name", 1)], +) + + +def init_module(app: NewsroomWebApp): + app.wsgi.settings_app( + "section-filters", + lazy_gettext("Section Filters"), + weight=450, + data=get_settings_data, + ) + + +module = Module( + name="newsroom.section_filters", + resources=[section_filters_config], + endpoints=[section_filters_endpoints], + init=init_module, +) diff --git a/newsroom/section_filters/service.py b/newsroom/section_filters/service.py new file mode 100644 index 000000000..b04e0731b --- /dev/null +++ b/newsroom/section_filters/service.py @@ -0,0 +1,6 @@ +from newsroom.core.resources.service import NewshubAsyncResourceService +from .model import SectionFilter + + +class SectionFiltersService(NewshubAsyncResourceService[SectionFilter]): + resource_name = "section_filters" diff --git a/newsroom/section_filters/views.py b/newsroom/section_filters/views.py index 0ffa07542..ecf03570d 100644 --- a/newsroom/section_filters/views.py +++ b/newsroom/section_filters/views.py @@ -1,86 +1,121 @@ import re +import json +from typing import Any, Dict, Optional from bson import ObjectId from quart_babel import gettext +from pydantic import BaseModel, field_validator -from superdesk.core import get_current_app -from superdesk.flask import jsonify, request -from superdesk import get_resource_service +from superdesk.core.web import EndpointGroup, Response, Request from newsroom.decorator import admin_only -from newsroom.section_filters import blueprint +from newsroom.core import get_current_wsgi_app from newsroom.utils import ( + create_or_abort, get_json_or_400, - get_entity_or_404, - set_original_creator, - set_version_creator, + success_response, ) -from newsroom.utils import query_resource +from .service import SectionFiltersService +from .model import SectionFilter -def get_settings_data(): +section_filters_endpoints = EndpointGroup("section_filters", __name__) + + +async def get_settings_data(): """Get the settings data for section filter :param context """ + all_filters = [obj async for obj in SectionFiltersService().get_all_raw()] + data = { - "section_filters": list(query_resource("section_filters")), - "sections": get_current_app().as_any().sections, + "section_filters": all_filters, + "sections": get_current_wsgi_app().sections, } + return data -@blueprint.route("/section_filters", methods=["GET"]) +def validate_section_filter(section_filter: dict) -> Response | None: + if not section_filter.get("name"): + return Response({"name": gettext("Name not found")}, 400, ()) + + +async def get_section_filter_or_abort(id: str) -> SectionFilter: + section_filter = await SectionFiltersService().find_by_id(id) + if section_filter is None: + abort(404) + + return section_filter + + +class IndexParams(BaseModel): + q: Optional[Dict[str, Any]] = None + + @field_validator("q", mode="before") + def parse_where(cls, value): + if isinstance(value, str): + return json.loads(value) + return value + + +@section_filters_endpoints.endpoint("/section_filters", methods=["GET"]) @admin_only -async def index(): - lookup = None - if request.args.get("q"): - lookup = request.args.get("q") - section_filters = list(query_resource("section_filters", lookup=lookup)) - return jsonify(section_filters), 200 +async def index(_a: None, params: IndexParams, _r: None): + cursor = await SectionFiltersService().search(lookup=params.q) + section_filters = await cursor.to_list_raw() + return success_response(section_filters) + + +class SearchParams(BaseModel): + q: str | None = None -@blueprint.route("/section_filters/search", methods=["GET"]) +@section_filters_endpoints.endpoint("/section_filters/search", methods=["GET"]) @admin_only -async def search(): +async def search(_a: None, params: SearchParams, _q: Request): lookup = None - if request.args.get("q"): - regex = re.compile(".*{}.*".format(request.args.get("q")), re.IGNORECASE) + if params.q: + regex = re.compile(".*{}.*".format(params.q), re.IGNORECASE) lookup = {"name": regex} - section_filters = list(query_resource("section_filters", lookup=lookup)) - return jsonify(section_filters), 200 - -def validate_section_filter(section_filter): - if not section_filter.get("name"): - return jsonify({"name": gettext("Name not found")}), 400 + cursor = await SectionFiltersService().search(lookup=lookup) + section_filters = await cursor.to_list_raw() + return success_response(section_filters) -@blueprint.route("/section_filters/new", methods=["POST"]) +@section_filters_endpoints.endpoint("/section_filters/new", methods=["POST"]) @admin_only async def create(): - section_filter = await get_json_or_400() + creation_data = await get_json_or_400() - validation = validate_section_filter(section_filter) + validation = validate_section_filter(creation_data) if validation: return validation section = next( - (s for s in get_current_app().as_any().sections if s["_id"] == section_filter.get("filter_type")), + (s for s in get_current_wsgi_app().sections if s["_id"] == creation_data.get("filter_type")), None, ) if section and section.get("search_type"): - section_filter["search_type"] = section["search_type"] + creation_data["search_type"] = section["search_type"] + creation_data["id"] = ObjectId() + + section_filter_id = await create_or_abort(SectionFiltersService, SectionFilter, creation_data) - set_original_creator(section_filter) - ids = get_resource_service("section_filters").post([section_filter]) - return jsonify({"success": True, "_id": ids[0]}), 201 + return success_response({"success": True, "_id": section_filter_id}, status_code=201) -@blueprint.route("/section_filters/", methods=["POST"]) +class DetailArgs(BaseModel): + id: str + + +@section_filters_endpoints.endpoint("/section_filters/", methods=["POST"]) @admin_only -async def edit(id): - get_entity_or_404(ObjectId(id), "section_filters") +async def edit(args: DetailArgs, _p: None, _q: None): + await get_section_filter_or_abort(args.id) + data = await get_json_or_400() updates = { "name": data.get("name"), @@ -95,15 +130,17 @@ async def edit(id): if validation: return validation - set_version_creator(updates) - get_resource_service("section_filters").patch(id=ObjectId(id), updates=updates) - return jsonify({"success": True}), 200 + await SectionFiltersService().update(args.id, updates) + return success_response({"success": True}) -@blueprint.route("/section_filters/", methods=["DELETE"]) + +@section_filters_endpoints.endpoint("/section_filters/", methods=["DELETE"]) @admin_only -async def delete(id): +async def delete(args: DetailArgs, _p: None, _r: None): """Deletes the section_filters by given id""" - get_entity_or_404(ObjectId(id), "section_filters") - get_resource_service("section_filters").delete_action({"_id": ObjectId(id)}) - return jsonify({"success": True}), 200 + + section_filter = await get_section_filter_or_abort(args.id) + await SectionFiltersService().delete(section_filter) + + return success_response({"success": True}) diff --git a/newsroom/users/views.py b/newsroom/users/views.py index 1a5361041..33507ba8c 100644 --- a/newsroom/users/views.py +++ b/newsroom/users/views.py @@ -12,7 +12,7 @@ from superdesk import get_resource_service from superdesk.core.web import Request, Response from superdesk.core import get_current_app, get_app_config -from superdesk.core.resources.cursor import SearchRequest +from superdesk.core.types import SearchRequest from superdesk.core.resources.fields import ObjectId as ObjectIdField from newsroom.user_roles import UserRole diff --git a/newsroom/utils.py b/newsroom/utils.py index 48113ef75..f1b6f8371 100644 --- a/newsroom/utils.py +++ b/newsroom/utils.py @@ -1,5 +1,6 @@ import pytz import regex +from newsroom.exceptions import ValidationException import superdesk from functools import reduce from pydantic import BaseModel, ValidationError @@ -23,6 +24,8 @@ from superdesk.json_utils import try_cast from superdesk.etree import parse_html from superdesk.text_utils import get_text +from superdesk.core.resources.model import ResourceModel +from superdesk.core.resources.service import AsyncResourceService from superdesk.core.resources.validators import get_field_errors_from_pydantic_validation_error from newsroom.flask import flash @@ -149,7 +152,7 @@ async def get_json_or_400_async(req: Request): return data -def success_response(data: Any, headers: Sequence = ()): +def success_response(data: Any, headers: Sequence = (), status_code: int = 200): """ Shortcut to return a `superdesk.core.web.Response` with a status_code 200 and empty headers by default @@ -157,19 +160,51 @@ def success_response(data: Any, headers: Sequence = ()): if isinstance(data, BaseModel): data = data.model_dump(by_alias=True, exclude_unset=True) - return Response(data, 200, headers) + return Response(data, status_code, headers) -def response_from_validation(error: ValidationError, code=400): +def parse_validation_error(error: ValidationError) -> dict[str, str]: """ - Constructs a Response object with pydantic model validation error. + Parses pydantic model validation error and returns a key -> value dict from it """ - errors = { + return { field: list(err.values())[0] for field, err in get_field_errors_from_pydantic_validation_error(error).items() } + + +def response_from_validation(error: ValidationError, code=400): + """ + Constructs a Response object with pydantic model validation error. + """ + errors = parse_validation_error(error) + return Response(errors, code, ()) +async def create_or_abort( + service_class: type[AsyncResourceService], model_class: type[ResourceModel], creation_data: dict +) -> int: + """ + Validate creation data using the provided model class. If valid, creates a new record + using the provided service class. Raises ValidationException on validation errors. + + Args: + service_class (type[AsyncResourceService]): The service class used to create the record. + model_class (type[ResourceModel]): The model class used for validation. + creation_data (dict): The data to be validated and used for creation. + + Raises: + ValidationException: If validation errors are encountered. + """ + try: + new_model = model_class.model_validate(creation_data) + ids = await service_class().create([new_model]) + return ids[0] + except ValidationError as err: + errors = parse_validation_error(err) + raise ValidationException(errors=errors) + + def get_type(type: Optional[str] = None) -> str: item_type = type or request.args.get("type", "wire") types: Dict[Union[str, None], str] = { diff --git a/newsroom/web/default_settings.py b/newsroom/web/default_settings.py index 1686eaf7e..ac7b834dc 100644 --- a/newsroom/web/default_settings.py +++ b/newsroom/web/default_settings.py @@ -123,7 +123,6 @@ "newsroom.topics", "newsroom.notifications", "newsroom.products", - "newsroom.section_filters", "newsroom.navigations", "newsroom.cards", "newsroom.reports", @@ -150,7 +149,6 @@ "newsroom.history", "newsroom.notifications", "newsroom.products", - "newsroom.section_filters", "newsroom.navigations", "newsroom.cards", "newsroom.reports", @@ -178,6 +176,7 @@ "newsroom.companies", "newsroom.assets", "newsroom.users", + "newsroom.section_filters", ] SITE_NAME = "Newshub" From 29e4c9c5b49840c74cabfa5bd2f03d0724894163 Mon Sep 17 00:00:00 2001 From: Helmy Giacoman Date: Thu, 5 Sep 2024 19:47:57 +0200 Subject: [PATCH 02/18] Fix section filters tests NHUB-534 --- newsroom/section_filters/views.py | 1 + tests/core/test_section_filters.py | 11 ++++++----- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/newsroom/section_filters/views.py b/newsroom/section_filters/views.py index ecf03570d..f554bb0a8 100644 --- a/newsroom/section_filters/views.py +++ b/newsroom/section_filters/views.py @@ -3,6 +3,7 @@ from typing import Any, Dict, Optional from bson import ObjectId +from quart import abort from quart_babel import gettext from pydantic import BaseModel, field_validator diff --git a/tests/core/test_section_filters.py b/tests/core/test_section_filters.py index b91a1e75f..70890f309 100644 --- a/tests/core/test_section_filters.py +++ b/tests/core/test_section_filters.py @@ -8,8 +8,8 @@ @fixture(autouse=True) async def init(app): - app.data.insert( - "section_filters", + collection = app.async_app.mongo.get_collection_async("section_filters") + collection.insert_many( [ { "_id": ObjectId("59b4c5c61d41c8d736852fbf"), @@ -91,7 +91,6 @@ async def test_delete_product(client): json={ "name": "Breaking", "description": "Breaking news", - "parents": "59b4c5c61d41c8d736852fbf", "is_enabled": True, "query": "bar", }, @@ -102,6 +101,7 @@ async def test_delete_product(client): response = await client.get("/section_filters") data = json.loads(await response.get_data()) + assert 1 == len(data) assert data[0]["name"] == "Breaking" @@ -109,9 +109,10 @@ async def test_delete_product(client): async def test_gets_all_products(client, app): await test_login_succeeds_for_admin(client) + collection = app.async_app.mongo.get_collection_async("section_filters") + for i in range(250): - app.data.insert( - "section_filters", + collection.insert_many( [ { "name": "Sport-%s" % i, From b9240ab979e9153001172bacbc0b54d2a93c20dd Mon Sep 17 00:00:00 2001 From: Helmy Giacoman Date: Fri, 6 Sep 2024 18:27:21 +0200 Subject: [PATCH 03/18] Set old section_filters app back The complexity of moving all the references to the new async service is too high to address in ticket NHUB-534. NHUB-534 --- newsroom/section_filters/__init__.py | 8 ++++++++ newsroom/web/default_settings.py | 1 + 2 files changed, 9 insertions(+) diff --git a/newsroom/section_filters/__init__.py b/newsroom/section_filters/__init__.py index 8fe58404b..9fc5cd592 100644 --- a/newsroom/section_filters/__init__.py +++ b/newsroom/section_filters/__init__.py @@ -4,3 +4,11 @@ from .module import module # noqa __all__ = ["SectionFilter", "SectionFiltersService"] + + +def init_app(app): + # TODO-Async: Remove this once agenda, news_api.news, search and wire are migrated to async + import superdesk + from .section_filters import SectionFiltersResource, SectionFiltersService + + superdesk.register_resource("section_filters", SectionFiltersResource, SectionFiltersService, _app=app) diff --git a/newsroom/web/default_settings.py b/newsroom/web/default_settings.py index ac7b834dc..254875cdd 100644 --- a/newsroom/web/default_settings.py +++ b/newsroom/web/default_settings.py @@ -151,6 +151,7 @@ "newsroom.products", "newsroom.navigations", "newsroom.cards", + "newsroom.section_filters", "newsroom.reports", "newsroom.public", "newsroom.agenda", From c5c760ee1714f93b636d18c2fe8a53e571af2680 Mon Sep 17 00:00:00 2001 From: Helmy Giacoman Date: Fri, 6 Sep 2024 19:00:38 +0200 Subject: [PATCH 04/18] Implement section filter methods in async service NHUB-534 --- newsroom/section_filters/service.py | 46 ++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/newsroom/section_filters/service.py b/newsroom/section_filters/service.py index b04e0731b..b5ca71604 100644 --- a/newsroom/section_filters/service.py +++ b/newsroom/section_filters/service.py @@ -1,6 +1,50 @@ +from superdesk.flask import g +from superdesk.core.resources import AsyncCacheableService + +from newsroom.search.service import query_string from newsroom.core.resources.service import NewshubAsyncResourceService + from .model import SectionFilter -class SectionFiltersService(NewshubAsyncResourceService[SectionFilter]): +class SectionFiltersService(NewshubAsyncResourceService[SectionFilter], AsyncCacheableService): resource_name = "section_filters" + cache_lookup = {"is_enabled": True} + + async def get_section_filters(self, filter_type) -> list: + """Get the list of section filter by filter type + + :param filter_type: Type of filter + """ + section_filters = await self.get_section_filters_dict() + return section_filters.get(filter_type) or [] + + async def get_section_filters_dict(self) -> dict[str, list]: + """Get the list of all section filters""" + if not getattr(g, "section_filters", None): + filters: dict[str, list] = {} + for f in await self.get_cached(): + if not filters.get(f.get("filter_type")): + filters[f.get("filter_type")] = [] + filters[f.get("filter_type")].append(f) + g.section_filters = filters + return g.section_filters + + async def apply_section_filter(self, query, product_type, filters=None): + """Get the list of base products for product type + + :param query: Dict of elasticsearch query + :param product_type: Type of product + :param filters: filters for each section + """ + if not filters: + section_filters = await self.get_section_filters(product_type) + else: + section_filters = filters.get(product_type) + + if not section_filters: + return + + for f in section_filters: + if f.get("query"): + query["bool"].setdefault("filter", []).append(query_string(f.get("query"))) From 37a9a277a47ec63b4e02dcff4c782a60fca71dd9 Mon Sep 17 00:00:00 2001 From: Helmy Giacoman Date: Fri, 6 Sep 2024 19:34:13 +0200 Subject: [PATCH 05/18] Move news_api section filters NHUB-534 --- newsroom/news_api/section_filters/__init__.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/newsroom/news_api/section_filters/__init__.py b/newsroom/news_api/section_filters/__init__.py index c99f99d8e..09805cb35 100644 --- a/newsroom/news_api/section_filters/__init__.py +++ b/newsroom/news_api/section_filters/__init__.py @@ -1,16 +1,16 @@ import superdesk -# from newsroom.section_filters import SectionFiltersService, SectionFiltersResource +from newsroom.section_filters.section_filters import SectionFiltersService, SectionFiltersResource +# TODO-ASYNC: remove this once news_api modules are migrate to async def init_app(app): - # superdesk.register_resource("section_filters", NewsAPISectionFilterResource, SectionFiltersService, _app=app) - pass + superdesk.register_resource("section_filters", NewsAPISectionFilterResource, SectionFiltersService, _app=app) -# class NewsAPISectionFilterResource(SectionFiltersResource): -# """ -# Overload the newsroom section filter resource so we can set it to be an internal resource -# """ +class NewsAPISectionFilterResource(SectionFiltersResource): + """ + Overload the newsroom section filter resource so we can set it to be an internal resource + """ -# internal_resource = True + internal_resource = True From b7545acb10155614f6a4ef7e16e42cb24379278d Mon Sep 17 00:00:00 2001 From: Helmy Giacoman Date: Fri, 6 Sep 2024 22:56:26 +0200 Subject: [PATCH 06/18] Fix lint --- newsroom/section_filters/views.py | 1 + 1 file changed, 1 insertion(+) diff --git a/newsroom/section_filters/views.py b/newsroom/section_filters/views.py index f554bb0a8..9f85b0bb2 100644 --- a/newsroom/section_filters/views.py +++ b/newsroom/section_filters/views.py @@ -41,6 +41,7 @@ async def get_settings_data(): def validate_section_filter(section_filter: dict) -> Response | None: if not section_filter.get("name"): return Response({"name": gettext("Name not found")}, 400, ()) + return None async def get_section_filter_or_abort(id: str) -> SectionFilter: From 4c549702671132b3a0dc74ca8bca261af91f78c8 Mon Sep 17 00:00:00 2001 From: Helmy Giacoman Date: Fri, 6 Sep 2024 22:56:41 +0200 Subject: [PATCH 07/18] Adjust requirements to check e2e tests NHUB-534 --- dev-requirements.txt | 2 +- requirements.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dev-requirements.txt b/dev-requirements.txt index 0b1490adc..930e72fbd 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -471,7 +471,7 @@ six==1.16.0 # python-dateutil sniffio==1.3.1 # via asgi-tools -superdesk-core @ git+https://github.com/superdesk/superdesk-core.git@async +superdesk-core @ git+https://github.com/superdesk/superdesk-core.git@hg/NHUB-534-cacheable-service # via -r requirements.txt superdesk-planning @ git+https://github.com/superdesk/superdesk-planning.git@async # via -r requirements.txt diff --git a/requirements.txt b/requirements.txt index 6b8a22913..0365f1eca 100644 --- a/requirements.txt +++ b/requirements.txt @@ -15,5 +15,5 @@ quart-rate-limiter>=0.10.0,<0.11 # Fix issue with xhtml2pdf not working with python-bidi>=0.5 python-bidi<0.5 -superdesk-core @ git+https://github.com/superdesk/superdesk-core.git@async +superdesk-core @ git+https://github.com/superdesk/superdesk-core.git@hg/NHUB-534-cacheable-service superdesk-planning @ git+https://github.com/superdesk/superdesk-planning.git@async From 524c11fe4b5373a78dbd27e6ee1c3e1d76489723 Mon Sep 17 00:00:00 2001 From: Helmy Giacoman Date: Mon, 9 Sep 2024 11:14:41 +0200 Subject: [PATCH 08/18] Fix small issue with index NHUB-534 --- e2e/package.json | 3 +++ newsroom/section_filters/module.py | 14 ++++++++------ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/e2e/package.json b/e2e/package.json index 4fd6e21bb..a4346db33 100644 --- a/e2e/package.json +++ b/e2e/package.json @@ -19,5 +19,8 @@ "build": "export NODE_ENV=production && webpack --progress --profile --colors", "start": "webpack-dev-server --progress --colors --content-base dist --host 0.0.0.0", "start-client-server": "http-server dist -p 8080 -s" + }, + "volta": { + "node": "14.21.3" } } diff --git a/newsroom/section_filters/module.py b/newsroom/section_filters/module.py index 4795074f7..ac65f2a19 100644 --- a/newsroom/section_filters/module.py +++ b/newsroom/section_filters/module.py @@ -15,12 +15,14 @@ service=SectionFiltersService, mongo=MongoResourceConfig( prefix=MONGO_PREFIX, - indexes=MongoIndexOptions( - name="name", - keys=[("name", 1)], - unique=True, - collation={"locale": "en", "strength": 2}, - ), + indexes=[ + MongoIndexOptions( + name="name", + keys=[("name", 1)], + unique=True, + collation={"locale": "en", "strength": 2}, + ) + ], ), default_sort=[("name", 1)], ) From b3cc6fbde9aa35373a54017d230598e39d90274d Mon Sep 17 00:00:00 2001 From: Helmy Giacoman Date: Mon, 9 Sep 2024 11:17:59 +0200 Subject: [PATCH 09/18] Update upload-artifact to v4 --- .github/workflows/e2e.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index ef1a819fa..4276dc40c 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -42,7 +42,7 @@ jobs: CYPRESS_SCREENSHOTS_FOLDER: /tmp/cypress - name: Upload screenshots if: ${{ failure() }} - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v4 with: name: screenshots-e2e path: /tmp/cypress/**/*.png From 3beb8e2fe9eed6057d340c79357eade4b218be3e Mon Sep 17 00:00:00 2001 From: Helmy Giacoman Date: Tue, 10 Sep 2024 14:19:00 +0200 Subject: [PATCH 10/18] Implement review feedback NHUB-534 --- newsroom/section_filters/model.py | 6 +++--- newsroom/section_filters/service.py | 12 +++++++----- newsroom/section_filters/views.py | 18 +++++++----------- requirements.txt | 2 +- 4 files changed, 18 insertions(+), 20 deletions(-) diff --git a/newsroom/section_filters/model.py b/newsroom/section_filters/model.py index e75191daf..d5c84328a 100644 --- a/newsroom/section_filters/model.py +++ b/newsroom/section_filters/model.py @@ -4,9 +4,9 @@ class SectionFilter(NewshubResourceModel): name: str - description: Optional[str] + description: str = "" sd_product_id: Optional[str] = None query: Optional[str] = None is_enabled: bool = True - filter_type: Optional[str] = "wire" - search_type: Optional[str] = "wire" + filter_type: str = "wire" + search_type: str = "wire" diff --git a/newsroom/section_filters/service.py b/newsroom/section_filters/service.py index b5ca71604..851be1dae 100644 --- a/newsroom/section_filters/service.py +++ b/newsroom/section_filters/service.py @@ -1,6 +1,7 @@ from superdesk.flask import g from superdesk.core.resources import AsyncCacheableService +from newsroom.search import BoolQuery from newsroom.search.service import query_string from newsroom.core.resources.service import NewshubAsyncResourceService @@ -11,7 +12,7 @@ class SectionFiltersService(NewshubAsyncResourceService[SectionFilter], AsyncCac resource_name = "section_filters" cache_lookup = {"is_enabled": True} - async def get_section_filters(self, filter_type) -> list: + async def get_section_filters(self, filter_type: str) -> list[dict]: """Get the list of section filter by filter type :param filter_type: Type of filter @@ -19,7 +20,7 @@ async def get_section_filters(self, filter_type) -> list: section_filters = await self.get_section_filters_dict() return section_filters.get(filter_type) or [] - async def get_section_filters_dict(self) -> dict[str, list]: + async def get_section_filters_dict(self) -> dict[str, list[dict]]: """Get the list of all section filters""" if not getattr(g, "section_filters", None): filters: dict[str, list] = {} @@ -30,7 +31,7 @@ async def get_section_filters_dict(self) -> dict[str, list]: g.section_filters = filters return g.section_filters - async def apply_section_filter(self, query, product_type, filters=None): + async def apply_section_filter(self, query: BoolQuery, product_type: str, filters=None): """Get the list of base products for product type :param query: Dict of elasticsearch query @@ -46,5 +47,6 @@ async def apply_section_filter(self, query, product_type, filters=None): return for f in section_filters: - if f.get("query"): - query["bool"].setdefault("filter", []).append(query_string(f.get("query"))) + filter_query = f.get("query") + if filter_query: + query["bool"].setdefault("filter", []).append(query_string(str(filter_query))) diff --git a/newsroom/section_filters/views.py b/newsroom/section_filters/views.py index 9f85b0bb2..a767993d5 100644 --- a/newsroom/section_filters/views.py +++ b/newsroom/section_filters/views.py @@ -11,11 +11,7 @@ from newsroom.decorator import admin_only from newsroom.core import get_current_wsgi_app -from newsroom.utils import ( - create_or_abort, - get_json_or_400, - success_response, -) +from newsroom.utils import create_or_abort, get_json_or_400 from .service import SectionFiltersService from .model import SectionFilter @@ -67,7 +63,7 @@ def parse_where(cls, value): async def index(_a: None, params: IndexParams, _r: None): cursor = await SectionFiltersService().search(lookup=params.q) section_filters = await cursor.to_list_raw() - return success_response(section_filters) + return Response(section_filters) class SearchParams(BaseModel): @@ -84,7 +80,7 @@ async def search(_a: None, params: SearchParams, _q: Request): cursor = await SectionFiltersService().search(lookup=lookup) section_filters = await cursor.to_list_raw() - return success_response(section_filters) + return Response(section_filters) @section_filters_endpoints.endpoint("/section_filters/new", methods=["POST"]) @@ -106,7 +102,7 @@ async def create(): section_filter_id = await create_or_abort(SectionFiltersService, SectionFilter, creation_data) - return success_response({"success": True, "_id": section_filter_id}, status_code=201) + return Response({"success": True, "_id": section_filter_id}, 201) class DetailArgs(BaseModel): @@ -134,10 +130,10 @@ async def edit(args: DetailArgs, _p: None, _q: None): await SectionFiltersService().update(args.id, updates) - return success_response({"success": True}) + return Response({"success": True}) -@section_filters_endpoints.endpoint("/section_filters/", methods=["DELETE"]) +@section_filters_endpoints.endpoint("/section_filters/", methods=["DELETE"]) @admin_only async def delete(args: DetailArgs, _p: None, _r: None): """Deletes the section_filters by given id""" @@ -145,4 +141,4 @@ async def delete(args: DetailArgs, _p: None, _r: None): section_filter = await get_section_filter_or_abort(args.id) await SectionFiltersService().delete(section_filter) - return success_response({"success": True}) + return Response({"success": True}) diff --git a/requirements.txt b/requirements.txt index 0365f1eca..1d4553806 100644 --- a/requirements.txt +++ b/requirements.txt @@ -15,5 +15,5 @@ quart-rate-limiter>=0.10.0,<0.11 # Fix issue with xhtml2pdf not working with python-bidi>=0.5 python-bidi<0.5 -superdesk-core @ git+https://github.com/superdesk/superdesk-core.git@hg/NHUB-534-cacheable-service +superdesk-core @ git+https://github.com/superdesk/superdesk-core.git@hg/minor-improvements superdesk-planning @ git+https://github.com/superdesk/superdesk-planning.git@async From c6d9c3ae83386d818ff93b4337ee213a70131981 Mon Sep 17 00:00:00 2001 From: Helmy Giacoman Date: Tue, 10 Sep 2024 14:36:35 +0200 Subject: [PATCH 11/18] Fix tests temporarily --- dev-requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-requirements.txt b/dev-requirements.txt index 930e72fbd..630381bbc 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -471,7 +471,7 @@ six==1.16.0 # python-dateutil sniffio==1.3.1 # via asgi-tools -superdesk-core @ git+https://github.com/superdesk/superdesk-core.git@hg/NHUB-534-cacheable-service +superdesk-core @ git+https://github.com/superdesk/superdesk-core.git@hg/minor-improvements # via -r requirements.txt superdesk-planning @ git+https://github.com/superdesk/superdesk-planning.git@async # via -r requirements.txt From 6db0badb273a48f69d5d262cfda0aa67260eb659 Mon Sep 17 00:00:00 2001 From: Helmy Giacoman Date: Tue, 10 Sep 2024 16:35:38 +0200 Subject: [PATCH 12/18] Improvements after PR review NHUB-534 --- newsroom/section_filters/views.py | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/newsroom/section_filters/views.py b/newsroom/section_filters/views.py index a767993d5..0d905b7b0 100644 --- a/newsroom/section_filters/views.py +++ b/newsroom/section_filters/views.py @@ -5,13 +5,13 @@ from bson import ObjectId from quart import abort from quart_babel import gettext -from pydantic import BaseModel, field_validator +from pydantic import BaseModel, ValidationError, field_validator from superdesk.core.web import EndpointGroup, Response, Request from newsroom.decorator import admin_only from newsroom.core import get_current_wsgi_app -from newsroom.utils import create_or_abort, get_json_or_400 +from newsroom.utils import create_or_abort, get_json_or_400, response_from_validation from .service import SectionFiltersService from .model import SectionFilter @@ -87,13 +87,10 @@ async def search(_a: None, params: SearchParams, _q: Request): @admin_only async def create(): creation_data = await get_json_or_400() - - validation = validate_section_filter(creation_data) - if validation: - return validation + app_sections = get_current_wsgi_app().sections section = next( - (s for s in get_current_wsgi_app().sections if s["_id"] == creation_data.get("filter_type")), + (s for s in app_sections if s["_id"] == creation_data.get("filter_type")), None, ) if section and section.get("search_type"): @@ -124,13 +121,11 @@ async def edit(args: DetailArgs, _p: None, _q: None): "filter_type": data.get("filter_type", "wire"), } - validation = validate_section_filter(updates) - if validation: - return validation - - await SectionFiltersService().update(args.id, updates) - - return Response({"success": True}) + try: + await SectionFiltersService().update(args.id, updates) + return Response({"success": True}) + except ValidationError as err: + return response_from_validation(err) @section_filters_endpoints.endpoint("/section_filters/", methods=["DELETE"]) From f26015e0c35aa3e0ade37a26e8ed023d8053f3ee Mon Sep 17 00:00:00 2001 From: Helmy Giacoman Date: Tue, 10 Sep 2024 16:36:47 +0200 Subject: [PATCH 13/18] Remove not used method NHUB-534 --- newsroom/section_filters/views.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/newsroom/section_filters/views.py b/newsroom/section_filters/views.py index 0d905b7b0..ca95d7f0a 100644 --- a/newsroom/section_filters/views.py +++ b/newsroom/section_filters/views.py @@ -4,7 +4,6 @@ from bson import ObjectId from quart import abort -from quart_babel import gettext from pydantic import BaseModel, ValidationError, field_validator from superdesk.core.web import EndpointGroup, Response, Request @@ -34,12 +33,6 @@ async def get_settings_data(): return data -def validate_section_filter(section_filter: dict) -> Response | None: - if not section_filter.get("name"): - return Response({"name": gettext("Name not found")}, 400, ()) - return None - - async def get_section_filter_or_abort(id: str) -> SectionFilter: section_filter = await SectionFiltersService().find_by_id(id) if section_filter is None: From 332deed39348562b4539144895dd455013f01a10 Mon Sep 17 00:00:00 2001 From: Helmy Giacoman Date: Wed, 11 Sep 2024 11:46:40 +0200 Subject: [PATCH 14/18] Implement validation error handler NHUB-534 --- newsroom/exceptions.py | 31 -------------------------- newsroom/factory/app.py | 12 ++++++++++- newsroom/section_filters/views.py | 18 ++++++---------- newsroom/utils.py | 36 ------------------------------- 4 files changed, 18 insertions(+), 79 deletions(-) diff --git a/newsroom/exceptions.py b/newsroom/exceptions.py index 9d2276dd6..875181279 100644 --- a/newsroom/exceptions.py +++ b/newsroom/exceptions.py @@ -1,8 +1,3 @@ -import json -from typing import Any -from werkzeug.exceptions import HTTPException - - class AuthorizationError(Exception): code: int message: str @@ -11,29 +6,3 @@ def __init__(self, code: int, message: str): super().__init__(self) self.code = code self.message = message - - -class ValidationException(HTTPException): - """ - Custom exception class to handle validation errors and return them in a JSON format. - - This exception is intended to be raised when there are validation errors in user input - during request processing. - - Attributes: - code (int): HTTP status code, defaults to 400 (Bad Request). - errors (Any): A collection of validation error details that will be returned in the response. - """ - - code = 400 - - def __init__(self, errors: Any, description=None): - super().__init__(description) - self.errors = errors - - def get_body(self, *args): - return json.dumps(self.errors) - - def get_headers(self, *args): - """Ensure that the content type is JSON""" - return [("Content-Type", "application/json")] diff --git a/newsroom/factory/app.py b/newsroom/factory/app.py index 85abbf705..f652ab4d4 100644 --- a/newsroom/factory/app.py +++ b/newsroom/factory/app.py @@ -14,6 +14,7 @@ from flask_mail import Mail from flask_caching import Cache from elasticapm.contrib.flask import ElasticAPM +from pydantic import ValidationError from superdesk.flask import jsonify, request, render_template, g from superdesk.storage import AmazonMediaStorage, SuperdeskGridFSMediaStorage @@ -31,7 +32,7 @@ import newsroom from newsroom.auth import SessionAuth from newsroom.exceptions import AuthorizationError -from newsroom.utils import is_json_request +from newsroom.utils import is_json_request, parse_validation_error from newsroom.gettext import setup_babel @@ -214,11 +215,20 @@ def superdesk_api_error(err): async def authorization_error(err: AuthorizationError): return await render_template("authorization_error.html", message=err.message), err.code + def handle_validation_error(error: ValidationError): + """ + Gets the ValidationException error raised from Core framework's models/services, parses and returns it + to the client in a valid json format. + """ + errors = parse_validation_error(error) + return jsonify(errors), 400 + self.register_error_handler(AssertionError, assertion_error) self.register_error_handler(404, render_404) self.register_error_handler(403, render_403) self.register_error_handler(SuperdeskApiError, superdesk_api_error) self.register_error_handler(AuthorizationError, authorization_error) + self.register_error_handler(ValidationError, handle_validation_error) def general_setting( self, diff --git a/newsroom/section_filters/views.py b/newsroom/section_filters/views.py index ca95d7f0a..435539164 100644 --- a/newsroom/section_filters/views.py +++ b/newsroom/section_filters/views.py @@ -2,15 +2,14 @@ import json from typing import Any, Dict, Optional -from bson import ObjectId from quart import abort -from pydantic import BaseModel, ValidationError, field_validator +from pydantic import BaseModel, field_validator from superdesk.core.web import EndpointGroup, Response, Request from newsroom.decorator import admin_only from newsroom.core import get_current_wsgi_app -from newsroom.utils import create_or_abort, get_json_or_400, response_from_validation +from newsroom.utils import get_json_or_400 from .service import SectionFiltersService from .model import SectionFilter @@ -81,6 +80,7 @@ async def search(_a: None, params: SearchParams, _q: Request): async def create(): creation_data = await get_json_or_400() app_sections = get_current_wsgi_app().sections + service = SectionFiltersService() section = next( (s for s in app_sections if s["_id"] == creation_data.get("filter_type")), @@ -88,10 +88,9 @@ async def create(): ) if section and section.get("search_type"): creation_data["search_type"] = section["search_type"] - creation_data["id"] = ObjectId() - - section_filter_id = await create_or_abort(SectionFiltersService, SectionFilter, creation_data) + creation_data["id"] = service.generate_id() + section_filter_id = await service.create([creation_data]) return Response({"success": True, "_id": section_filter_id}, 201) @@ -114,11 +113,8 @@ async def edit(args: DetailArgs, _p: None, _q: None): "filter_type": data.get("filter_type", "wire"), } - try: - await SectionFiltersService().update(args.id, updates) - return Response({"success": True}) - except ValidationError as err: - return response_from_validation(err) + await SectionFiltersService().update(args.id, updates) + return Response({"success": True}) @section_filters_endpoints.endpoint("/section_filters/", methods=["DELETE"]) diff --git a/newsroom/utils.py b/newsroom/utils.py index f1b6f8371..755d85b9d 100644 --- a/newsroom/utils.py +++ b/newsroom/utils.py @@ -1,6 +1,5 @@ import pytz import regex -from newsroom.exceptions import ValidationException import superdesk from functools import reduce from pydantic import BaseModel, ValidationError @@ -24,8 +23,6 @@ from superdesk.json_utils import try_cast from superdesk.etree import parse_html from superdesk.text_utils import get_text -from superdesk.core.resources.model import ResourceModel -from superdesk.core.resources.service import AsyncResourceService from superdesk.core.resources.validators import get_field_errors_from_pydantic_validation_error from newsroom.flask import flash @@ -172,39 +169,6 @@ def parse_validation_error(error: ValidationError) -> dict[str, str]: } -def response_from_validation(error: ValidationError, code=400): - """ - Constructs a Response object with pydantic model validation error. - """ - errors = parse_validation_error(error) - - return Response(errors, code, ()) - - -async def create_or_abort( - service_class: type[AsyncResourceService], model_class: type[ResourceModel], creation_data: dict -) -> int: - """ - Validate creation data using the provided model class. If valid, creates a new record - using the provided service class. Raises ValidationException on validation errors. - - Args: - service_class (type[AsyncResourceService]): The service class used to create the record. - model_class (type[ResourceModel]): The model class used for validation. - creation_data (dict): The data to be validated and used for creation. - - Raises: - ValidationException: If validation errors are encountered. - """ - try: - new_model = model_class.model_validate(creation_data) - ids = await service_class().create([new_model]) - return ids[0] - except ValidationError as err: - errors = parse_validation_error(err) - raise ValidationException(errors=errors) - - def get_type(type: Optional[str] = None) -> str: item_type = type or request.args.get("type", "wire") types: Dict[Union[str, None], str] = { From 46f73a582d3637fb28bd1c36d5b8909662e3552c Mon Sep 17 00:00:00 2001 From: Helmy Giacoman Date: Wed, 11 Sep 2024 11:47:12 +0200 Subject: [PATCH 15/18] Update references to `Response` --- newsroom/cards/views.py | 19 +++--------- newsroom/companies/views.py | 36 ++++++++------------- newsroom/oauth_clients/views.py | 12 +++---- newsroom/settings/views.py | 2 +- newsroom/users/views.py | 55 +++++++++++---------------------- 5 files changed, 43 insertions(+), 81 deletions(-) diff --git a/newsroom/cards/views.py b/newsroom/cards/views.py index 381752bf3..23b76992b 100644 --- a/newsroom/cards/views.py +++ b/newsroom/cards/views.py @@ -1,15 +1,13 @@ import re -from pydantic import BaseModel, ValidationError +from pydantic import BaseModel from superdesk.core import json from superdesk.core.web import Request, Response, EndpointGroup from superdesk.core.resources.fields import ObjectId as ObjectIdField from newsroom.decorator import admin_only, login_required -from newsroom.utils import response_from_validation -from .model import CardResourceModel from .service import CardsResourceService @@ -57,11 +55,7 @@ async def create(request: Request) -> Response: if not card_data.get("_id"): card_data["_id"] = service.generate_id() - try: - new_card = CardResourceModel.model_validate(card_data) - new_ids = await service.create([new_card]) - except ValidationError as error: - return response_from_validation(error) + new_ids = await service.create([card_data]) return Response({"success": True, "_id": new_ids[0]}, 201, ()) @@ -79,12 +73,9 @@ async def edit(args: CardItemUrlArgs, params: None, request: Request) -> Respons if not card_data: request.abort(400) - try: - await service.update(args.id, card_data, etag=request.get_header("If-Match")) - except ValidationError as error: - return response_from_validation(error) + await service.update(args.id, card_data, etag=request.get_header("If-Match")) - return Response({"success": True}, 200, ()) + return Response({"success": True}) @cards_endpoints.endpoint("/cards/", methods=["DELETE"]) @@ -99,4 +90,4 @@ async def delete(args: CardItemUrlArgs, params: None, request: Request) -> Respo await service.delete(original, etag=request.get_header("If-Match")) - return Response({"success": True}, 200, ()) + return Response({"success": True}) diff --git a/newsroom/companies/views.py b/newsroom/companies/views.py index cdf6e6c9e..94d272953 100644 --- a/newsroom/companies/views.py +++ b/newsroom/companies/views.py @@ -5,7 +5,7 @@ from datetime import datetime from quart_babel import gettext from werkzeug.exceptions import NotFound, BadRequest -from pydantic import BaseModel, ValidationError +from pydantic import BaseModel from superdesk.core import get_app_config, get_current_app from superdesk.core.web import Request, Response @@ -17,14 +17,13 @@ get_public_user_data, query_resource, get_json_or_400_async, - response_from_validation, success_response, ) from newsroom.ui_config_async import UiConfigResourceService from .module import company_endpoints, company_configs from .utils import get_users_by_company -from .companies_async import CompanyService, CompanyResource +from .companies_async import CompanyService def get_company_types_options(): @@ -69,7 +68,7 @@ async def search_companies(args: None, params: CompanySearchArgs, request: Reque lookup = {"name": regex} cursor = await CompanyService().search(lookup) companies = await cursor.to_list_raw() - return Response(companies, 200, ()) + return Response(companies) @company_endpoints.endpoint("/companies/new", methods=["POST"]) @@ -82,13 +81,8 @@ async def create_company(request: Request) -> Response: company_data = get_company_updates(company) company_data["_id"] = ObjectId() - try: - new_company = CompanyResource.model_validate(company_data) - ids = await CompanyService().create([new_company]) - except ValidationError as error: - return response_from_validation(error) - - return Response({"success": True, "_id": ids[0]}, 201, ()) + ids = await CompanyService().create([company_data]) + return Response({"success": True, "_id": ids[0]}, 201) def get_company_updates(data, original=None): @@ -144,18 +138,14 @@ async def edit_company(args: CompanyItemArgs, params: None, request: Request) -> if not original: raise NotFound(gettext("Company not found")) elif request.method == "GET": - return Response(original, 200, ()) + return Response(original) request_json = await get_json_or_400_async(request) if not isinstance(request_json, dict): return request.abort(400) updates = get_company_updates(request_json, original) - - try: - await service.update(args.company_id, updates) - except ValidationError as error: - return response_from_validation(error) + await service.update(args.company_id, updates) return success_response({"success": True}) @@ -174,14 +164,14 @@ async def delete_company(args: CompanyItemArgs, params: None, request: Request) try: await UsersService().delete_many(lookup={"company": args.company_id}) except BadRequest as er: - return Response({"error": str(er)}, 403, ()) + return Response({"error": str(er)}, 403) try: await service.delete(original) except Exception as e: - return Response({"error": str(e)}, 400, ()) + return Response({"error": str(e)}, 400) - return Response({"success": True}, 200, ()) + return Response({"success": True}) @company_endpoints.endpoint("/companies//users", methods=["GET"]) @@ -193,7 +183,7 @@ async def company_users(args: CompanyItemArgs, params: None, request: Request) - public_data = get_public_user_data(user.model_dump(by_alias=True)) users_list.append(public_data) - return Response(users_list, 200, ()) + return Response(users_list) @company_endpoints.endpoint("/companies//approve", methods=["POST"]) @@ -206,7 +196,7 @@ async def approve_company(args: CompanyItemArgs, params: None, request: Request) if not original: raise NotFound(gettext("Company not found")) elif original.is_approved: - return Response({"error": gettext("Company is already approved")}, 403, ()) + return Response({"error": gettext("Company is already approved")}, 403) # Activate this Company updates = { @@ -221,4 +211,4 @@ async def approve_company(args: CompanyItemArgs, params: None, request: Request) async for user in company_users: await UsersService().approve_user(user) - return Response({"success": True}, 200, ()) + return Response({"success": True}) diff --git a/newsroom/oauth_clients/views.py b/newsroom/oauth_clients/views.py index bad0326e7..7640c8da7 100644 --- a/newsroom/oauth_clients/views.py +++ b/newsroom/oauth_clients/views.py @@ -40,7 +40,7 @@ async def search(args: None, params: ClientSearchArgs, request: Request) -> Resp lookup = {"name": regex} cursor = await ClientService().search(lookup) data = await cursor.to_list_raw() - return Response(data, 200, ()) + return Response(data) @clients_endpoints.endpoint("/oauth_clients/new", methods=["POST"]) @@ -72,7 +72,7 @@ async def create(request: Request) -> Response: (), ) - return Response({"success": True, "_id": ids[0], "password": password}, 201, ()) + return Response({"success": True, "_id": ids[0], "password": password}, 201) @clients_endpoints.endpoint("/oauth_clients/", methods=["GET", "POST"]) @@ -86,7 +86,7 @@ async def edit(args: ClientArgs, params: None, request: Request) -> Response: if not original: return NotFound(gettext("Client not found")) elif request.method == "GET": - return Response(original, 200, ()) + return Response(original) request_json = await get_json_or_400_async(request) if not isinstance(request_json, dict): @@ -95,7 +95,7 @@ async def edit(args: ClientArgs, params: None, request: Request) -> Response: updates = {} updates["name"] = request_json.get("name") await service.update(args.client_id, updates) - return Response({"success": True}, 200, ()) + return Response({"success": True}) @clients_endpoints.endpoint("/oauth_clients/", methods=["DELETE"]) @@ -112,5 +112,5 @@ async def delete(args: ClientArgs, params: None, request: Request) -> Response: try: await service.delete(original) except Exception as e: - return Response({"error": str(e)}, 400, ()) - return Response({"success": True}, 200, ()) + return Response({"error": str(e)}, 400) + return Response({"success": True}) diff --git a/newsroom/settings/views.py b/newsroom/settings/views.py index bdb041979..be2ee6b70 100644 --- a/newsroom/settings/views.py +++ b/newsroom/settings/views.py @@ -56,7 +56,7 @@ async def update_values(request: Request): update_settings_document(updates) g.settings = None # reset cache on update - return Response(updates, 200, ()) + return Response(updates) def validate_general_settings(values): diff --git a/newsroom/users/views.py b/newsroom/users/views.py index 33507ba8c..034dca75d 100644 --- a/newsroom/users/views.py +++ b/newsroom/users/views.py @@ -3,7 +3,7 @@ from copy import deepcopy from typing import Any, Dict, Optional -from pydantic import BaseModel, ValidationError, field_validator +from pydantic import BaseModel, field_validator from bson import ObjectId from quart_babel import gettext @@ -43,7 +43,6 @@ get_json_or_400_async, query_resource, get_vocabulary, - response_from_validation, success_response, ) from newsroom.monitoring.views import get_monitoring_for_company @@ -172,7 +171,7 @@ async def search(args: None, params: SearchArgs, request: Request) -> Response: mongo_cursor = await UsersService().find(SearchRequest(where=lookup, sort=sort, max_results=250)) users = await mongo_cursor.to_list_raw() - return Response(users, 200, ()) + return Response(users) @users_endpoints.endpoint("/users/new", methods=["POST"]) @@ -182,15 +181,11 @@ async def create(request: Request): if await form.validate(): if not _is_email_address_valid(form.email.data): - return Response({"email": [gettext("Email address is already in use")]}, 400, ()) + return Response({"email": [gettext("Email address is already in use")]}, 400) creation_data = get_updates_from_form(form, on_create=True) creation_data["id"] = ObjectId() - - try: - new_user = UserResourceModel.model_validate(creation_data) - except ValidationError as error: - return response_from_validation(error) + new_user = UserResourceModel.model_validate(creation_data) if is_current_user_company_admin(): company_from_admin = await get_company_from_user_or_session() @@ -203,7 +198,7 @@ async def create(request: Request): elif form.company.data: new_user.company = ObjectId(form.company.data) elif new_user.user_type != "administrator": - return Response({"company": [gettext("Company is required for non administrators")]}, 400, ()) + return Response({"company": [gettext("Company is required for non administrators")]}, 400) new_user.receive_email = True new_user.receive_app_notifications = True @@ -215,18 +210,15 @@ async def create(request: Request): if auth_provider.features["verify_email"]: add_token_data(new_user) - try: - ids = await UsersService().create([new_user]) - except ValidationError as error: - return response_from_validation(error) + ids = await UsersService().create([new_user]) if auth_provider.features["verify_email"]: user_dict = new_user.model_dump(by_alias=True, exclude_unset=True) await send_token(user_dict, token_type="new_account", update_token=False) - return Response({"success": True, "_id": ids[0]}, 201, ()) + return Response({"success": True, "_id": ids[0]}, 201) - return Response(form.errors, 400, ()) + return Response(form.errors, 400) @users_endpoints.endpoint("/users//resend_invite", methods=["POST"]) @@ -246,7 +238,7 @@ async def resent_invite(args: RouteArguments, params: None, request: Request): if not user: return NotFound(gettext("User not found")) elif user.is_validated: - return Response({"is_validated": gettext("User is already validated")}, 400, ()) + return Response({"is_validated": gettext("User is already validated")}, 400) elif user_is_company_admin and (company is None or user.company != company.id): # Company admins can only resent invites for members of their company only await request.abort(403) @@ -294,10 +286,10 @@ async def edit(args: RouteArguments, params: None, request: Request): if await form.validate_on_submit(): if form.email.data != user.email and not _is_email_address_valid(form.email.data): - return Response({"email": [gettext("Email address is already in use")]}, 400, ()) + return Response({"email": [gettext("Email address is already in use")]}, 400) elif not user_is_company_admin and not form.company.data and form.user_type.data != "administrator": - return Response({"company": [gettext("Company is required for non administrators")]}, 400, ()) + return Response({"company": [gettext("Company is required for non administrators")]}, 400) updates = get_updates_from_form(form) @@ -320,14 +312,10 @@ async def edit(args: RouteArguments, params: None, request: Request): if field not in allowed_fields: updates.pop(field, None) - try: - await UsersService().update(args.user_id, updates) - except ValidationError as error: - return response_from_validation(error) - + await UsersService().update(args.user_id, updates) return success_response({"success": True}) - return Response(form.errors, 400, ()) + return Response(form.errors, 400) return success_response(user) @@ -372,14 +360,10 @@ async def edit_user_profile(args: RouteArguments, params: None, request: Request form = await UserForm.create_form(user=user) if await form.validate_on_submit(): updates = {key: val for key, val in form.data.items() if key in USER_PROFILE_UPDATES} - try: - await UsersService().update(args.user_id, updates) - except ValidationError as error: - return response_from_validation(error) - + await UsersService().update(args.user_id, updates) return success_response({"success": True}) - return Response(form.errors, 400, ()) + return Response(form.errors, 400) @users_endpoints.endpoint("/users//notification_schedules", methods=["POST"]) @@ -401,10 +385,7 @@ async def edit_user_notification_schedules(args: RouteArguments, params: None, r updates["notification_schedule"].update(data) - try: - await UsersService().update(args.user_id, updates) - except ValidationError as error: - return response_from_validation(error) + await UsersService().update(args.user_id, updates) return success_response({"success": True}) @@ -437,7 +418,7 @@ async def _resend_token(user_id, token_type): if await send_token(user.model_dump(by_alias=True), token_type): return success_response({"success": True}) - return Response({"message": "Token could not be sent"}, 400, ()) + return Response({"message": "Token could not be sent"}, 400) @users_endpoints.endpoint("/users/", methods=["DELETE"]) @@ -493,7 +474,7 @@ async def approve_user(args: RouteArguments, params: None, request: Request): return NotFound(gettext("User not found")) if user.is_approved: - return Response({"error": gettext("User is already approved")}, 403, ()) + return Response({"error": gettext("User is already approved")}, 403) await users_service.approve_user(user) return success_response({"success": True}) From 18d814de3672ea8a281ccbca47000c228aedcdae Mon Sep 17 00:00:00 2001 From: Helmy Giacoman Date: Thu, 12 Sep 2024 14:38:21 +0200 Subject: [PATCH 16/18] Use abort from request instead NHUB-534 --- newsroom/section_filters/views.py | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/newsroom/section_filters/views.py b/newsroom/section_filters/views.py index 435539164..4e680fb42 100644 --- a/newsroom/section_filters/views.py +++ b/newsroom/section_filters/views.py @@ -1,8 +1,6 @@ import re import json from typing import Any, Dict, Optional - -from quart import abort from pydantic import BaseModel, field_validator from superdesk.core.web import EndpointGroup, Response, Request @@ -32,10 +30,10 @@ async def get_settings_data(): return data -async def get_section_filter_or_abort(id: str) -> SectionFilter: +async def get_section_filter_or_abort(id: str, request: Request) -> SectionFilter: section_filter = await SectionFiltersService().find_by_id(id) if section_filter is None: - abort(404) + request.abort(404) return section_filter @@ -64,7 +62,7 @@ class SearchParams(BaseModel): @section_filters_endpoints.endpoint("/section_filters/search", methods=["GET"]) @admin_only -async def search(_a: None, params: SearchParams, _q: Request): +async def search(_a: None, params: SearchParams, _r: None): lookup = None if params.q: regex = re.compile(".*{}.*".format(params.q), re.IGNORECASE) @@ -91,7 +89,7 @@ async def create(): creation_data["id"] = service.generate_id() section_filter_id = await service.create([creation_data]) - return Response({"success": True, "_id": section_filter_id}, 201) + return Response({"success": True, "_id": section_filter_id[0]}, 201) class DetailArgs(BaseModel): @@ -100,8 +98,8 @@ class DetailArgs(BaseModel): @section_filters_endpoints.endpoint("/section_filters/", methods=["POST"]) @admin_only -async def edit(args: DetailArgs, _p: None, _q: None): - await get_section_filter_or_abort(args.id) +async def edit(args: DetailArgs, _p: None, request: Request): + await get_section_filter_or_abort(args.id, request) data = await get_json_or_400() updates = { @@ -119,10 +117,10 @@ async def edit(args: DetailArgs, _p: None, _q: None): @section_filters_endpoints.endpoint("/section_filters/", methods=["DELETE"]) @admin_only -async def delete(args: DetailArgs, _p: None, _r: None): +async def delete(args: DetailArgs, _p: None, request: Request): """Deletes the section_filters by given id""" - section_filter = await get_section_filter_or_abort(args.id) + section_filter = await get_section_filter_or_abort(args.id, request) await SectionFiltersService().delete(section_filter) return Response({"success": True}) From 9a2174c201a457afc45b031bf29b932661c24494 Mon Sep 17 00:00:00 2001 From: Helmy Giacoman Date: Thu, 12 Sep 2024 15:07:46 +0200 Subject: [PATCH 17/18] Update views.py --- newsroom/section_filters/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/newsroom/section_filters/views.py b/newsroom/section_filters/views.py index 4e680fb42..6a6856a42 100644 --- a/newsroom/section_filters/views.py +++ b/newsroom/section_filters/views.py @@ -33,7 +33,7 @@ async def get_settings_data(): async def get_section_filter_or_abort(id: str, request: Request) -> SectionFilter: section_filter = await SectionFiltersService().find_by_id(id) if section_filter is None: - request.abort(404) + await request.abort(404) return section_filter From b63198378570535ef7d5d9ce24b42deab46b4582 Mon Sep 17 00:00:00 2001 From: Helmy Giacoman Date: Fri, 13 Sep 2024 13:31:37 +0200 Subject: [PATCH 18/18] Fix requirements to right branch NHUB-534 --- dev-requirements.txt | 2 +- requirements.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dev-requirements.txt b/dev-requirements.txt index 630381bbc..0b1490adc 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -471,7 +471,7 @@ six==1.16.0 # python-dateutil sniffio==1.3.1 # via asgi-tools -superdesk-core @ git+https://github.com/superdesk/superdesk-core.git@hg/minor-improvements +superdesk-core @ git+https://github.com/superdesk/superdesk-core.git@async # via -r requirements.txt superdesk-planning @ git+https://github.com/superdesk/superdesk-planning.git@async # via -r requirements.txt diff --git a/requirements.txt b/requirements.txt index 1d4553806..6b8a22913 100644 --- a/requirements.txt +++ b/requirements.txt @@ -15,5 +15,5 @@ quart-rate-limiter>=0.10.0,<0.11 # Fix issue with xhtml2pdf not working with python-bidi>=0.5 python-bidi<0.5 -superdesk-core @ git+https://github.com/superdesk/superdesk-core.git@hg/minor-improvements +superdesk-core @ git+https://github.com/superdesk/superdesk-core.git@async superdesk-planning @ git+https://github.com/superdesk/superdesk-planning.git@async