From 7450325655238f1066107e1f86d1a4b3f88f64ba Mon Sep 17 00:00:00 2001 From: Christoph Ladurner Date: Mon, 23 Sep 2024 22:58:27 +0200 Subject: [PATCH 01/12] fix: add compatibility layer to move to flask>=3 * flask-sqlalchemy moved pagination. * this change has been added to have a smooth migration to flask>=3.0.0 without creating a hard cut and major versions release. --- invenio_banners/services/results.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/invenio_banners/services/results.py b/invenio_banners/services/results.py index 929ce93..f04d2d7 100644 --- a/invenio_banners/services/results.py +++ b/invenio_banners/services/results.py @@ -1,14 +1,22 @@ # -*- coding: utf-8 -*- # # Copyright (C) 2022-2023 CERN. +# Copyright (C) 2024 Graz University of Technology. # # Invenio-Banners is free software; you can redistribute it and/or modify it # under the terms of the MIT License; see LICENSE file for more details. """Service results.""" -from flask_sqlalchemy import Pagination + from invenio_records_resources.services.records.results import RecordItem, RecordList +try: + # flask_sqlalchemy<3.0.0 + from flask_sqlalchemy import Pagination +except ImportError: + # flask_sqlalchemy>=3.0.0 + from flask_sqlalchemy.pagination import Pagination + class BannerItem(RecordItem): """Single banner result.""" From 59217110cfdfe882161d77c3b2da776ce010d983 Mon Sep 17 00:00:00 2001 From: Christoph Ladurner Date: Wed, 4 Sep 2024 21:07:03 +0200 Subject: [PATCH 02/12] fix: SAWarning * SAWarning: "This declarative base already contains a class with the " "same class name and module name as %s.%s, and will " "be replaced in the string-lookup table." * if a model should not be versioned the __versioned__ should not be there at all * if it is there, it will be added to version_manager.pending_classes (which seems like a bug, because it is versioning set to false) * those pending_classes will be checked against declared classes in invenio_db.ext and since the banner model should not have a versioned table it is not in the declared_classes list. this starts then a try of the version_manager to create the missing tables which causes then a duplicate error which is shown in the SAWarning --- invenio_banners/records/models.py | 1 - 1 file changed, 1 deletion(-) diff --git a/invenio_banners/records/models.py b/invenio_banners/records/models.py index 0d05802..35761bd 100644 --- a/invenio_banners/records/models.py +++ b/invenio_banners/records/models.py @@ -24,7 +24,6 @@ class BannerModel(db.Model, Timestamp): """Defines a message to show to users.""" __tablename__ = "banners" - __versioned__ = {"versioning": False} id = db.Column(db.Integer, primary_key=True) From fef01c8339560c5ab5269bbe3b1ea3d831538b5f Mon Sep 17 00:00:00 2001 From: Christoph Ladurner Date: Thu, 26 Sep 2024 11:47:36 +0200 Subject: [PATCH 03/12] fix: SADeprecationWarning * SADeprecationWarning: Invoking or_() without arguments is deprecated, and will be disallowed in a future release. For an empty or_() construct, use 'or_(false(), *args)' or 'or_(False, *args)'. BannerModel.query.filter(or_(*filters)) * https://github.com/sqlalchemy/sqlalchemy/issues/5054 --- invenio_banners/records/models.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/invenio_banners/records/models.py b/invenio_banners/records/models.py index 35761bd..a33167a 100644 --- a/invenio_banners/records/models.py +++ b/invenio_banners/records/models.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- # # Copyright (C) 2020-2023 CERN. +# Copyright (C) 2024 Graz University of Technology. # # Invenio-Banners is free software; you can redistribute it and/or modify it # under the terms of the MIT License; see LICENSE file for more details. @@ -113,7 +114,7 @@ def get_active(cls, url_path): def search(cls, search_params, filters): """Filter banners accordingly to query params.""" banners = ( - BannerModel.query.filter(or_(*filters)) + BannerModel.query.filter(or_(False, *filters)) .order_by( search_params["sort_direction"](text(",".join(search_params["sort"]))) ) From 00f35b7b7ebda167a3865d596f16cb8e6bda1785 Mon Sep 17 00:00:00 2001 From: Christoph Ladurner Date: Mon, 23 Sep 2024 23:12:12 +0200 Subject: [PATCH 04/12] tests: move to reusable workflows --- .github/workflows/tests.yml | 51 ++++--------------------------------- 1 file changed, 5 insertions(+), 46 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 9017d79..ff96fec 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -15,55 +15,14 @@ on: branches: master schedule: # * is a special character in YAML so you have to quote this string - - cron: '0 3 * * 6' + - cron: "0 3 * * 6" workflow_dispatch: inputs: reason: - description: 'Reason' + description: "Reason" required: false - default: 'Manual trigger' + default: "Manual trigger" jobs: - Tests: - runs-on: ubuntu-20.04 - strategy: - matrix: - python-version: [3.8, 3.9] - requirements-level: [pypi] - db-service: [postgresql14] - search-service: [opensearch2] - - env: - DB: ${{ matrix.db-service }} - EXTRAS: tests,${{ matrix.search-service }} - steps: - - name: Checkout - uses: actions/checkout@v2 - - - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@v2 - with: - python-version: ${{ matrix.python-version }} - - - name: Generate dependencies - run: | - python -m pip install --upgrade pip setuptools py wheel requirements-builder - requirements-builder -e "$EXTRAS" --level=${{ matrix.requirements-level }} setup.py > .${{ matrix.requirements-level }}-${{ matrix.python-version }}-requirements.txt - - - name: Cache pip - uses: actions/cache@v2 - with: - path: ~/.cache/pip - key: ${{ runner.os }}-pip-${{ hashFiles('.${{ matrix.requirements-level }}-${{ matrix.python-version }}-requirements.txt') }} - - - name: Install dependencies - run: | - pip install -r .${{ matrix.requirements-level }}-${{ matrix.python-version }}-requirements.txt - pip install ".[$EXTRAS]" - pip freeze - docker --version - docker-compose --version - - - name: Run tests - run: | - ./run-tests.sh + Python: + uses: inveniosoftware/workflows/.github/workflows/tests-python.yml@master From a2d6c4ba398960979637fbf81b3529606031006a Mon Sep 17 00:00:00 2001 From: Christoph Ladurner Date: Mon, 23 Sep 2024 23:45:22 +0200 Subject: [PATCH 05/12] chore: fix black, isort --- invenio_banners/resources/errors.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/invenio_banners/resources/errors.py b/invenio_banners/resources/errors.py index b412fae..d231293 100644 --- a/invenio_banners/resources/errors.py +++ b/invenio_banners/resources/errors.py @@ -1,19 +1,19 @@ # -*- coding: utf-8 -*- # # Copyright (C) 2023 CERN. +# Copyright (C) 2024 Graz University of Technology. # # Invenio-Banners is free software; you can redistribute it and/or modify it # under the terms of the MIT License; see LICENSE file for more details. """Errors.""" -from flask_resources import HTTPJSONException, create_error_handler - -from ..services.errors import BannerNotExistsError import marshmallow as ma from flask_resources import HTTPJSONException, create_error_handler from invenio_records_resources.errors import validation_error_to_list_errors +from ..services.errors import BannerNotExistsError + class HTTPJSONValidationException(HTTPJSONException): """HTTP exception serializing to JSON and reflecting Marshmallow errors.""" @@ -25,7 +25,7 @@ def __init__(self, exception): super().__init__(code=400, errors=validation_error_to_list_errors(exception)) -class ErrorHandlersMixin(): +class ErrorHandlersMixin: """Mixin to define error handlers.""" error_handlers = { From acf12be6150583e7b3fdad16828d6231dc7b2852 Mon Sep 17 00:00:00 2001 From: Christoph Ladurner Date: Tue, 24 Sep 2024 12:52:32 +0200 Subject: [PATCH 06/12] tests: make it run again * not sure why it ever worked * the requirement for test_create_banner and test_disable_expired_after_create_action can't be fulfilled at the same time. * to create the banner1 in test_create_banner the enddate has to be in the future otherwise the disable_expired service method kicks in and sets to active == False * but in test_disable_expired_after_create_action banner1 should be in the past so that active expires by creating the banner2 over the resource endpoint. using BannerModel.create() doesn't expire old entries --- tests/resources/test_resources.py | 43 +++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 13 deletions(-) diff --git a/tests/resources/test_resources.py b/tests/resources/test_resources.py index ef05ae7..6ad9792 100644 --- a/tests/resources/test_resources.py +++ b/tests/resources/test_resources.py @@ -1,12 +1,13 @@ # -*- coding: utf-8 -*- # # Copyright (C) 2022 CERN. +# Copyright (C) 2024 Graz University of Technology. # # Invenio-Banners is free software; you can redistribute it and/or modify it # under the terms of the MIT License; see LICENSE file for more details. """Banner resource tests.""" -from datetime import date, datetime +from datetime import date, datetime, timedelta import pytest from invenio_records_resources.services.errors import PermissionDeniedError @@ -14,29 +15,45 @@ from invenio_banners.records import BannerModel banners = { + "banner0": { + "message": "banner0", + "url_path": "/banner0", + "category": "info", + "active": True, + "start_datetime": date(2022, 7, 20).strftime("%Y-%m-%d %H:%M:%S"), + "end_datetime": (datetime.utcnow() - timedelta(days=20)).strftime( + "%Y-%m-%d %H:%M:%S" + ), + }, "banner1": { "message": "banner1", "url_path": "/banner1", "category": "info", "active": True, - "start_datetime": date(2022, 7, 20), - "end_datetime": date(2023, 1, 29), + "start_datetime": date(2022, 7, 20).strftime("%Y-%m-%d %H:%M:%S"), + "end_datetime": (datetime.utcnow() + timedelta(days=20)).strftime( + "%Y-%m-%d %H:%M:%S" + ), }, "banner2": { "message": "banner2", "url_path": "/banner2", "category": "other", "active": False, - "start_datetime": date(2022, 12, 15), - "end_datetime": date(2023, 1, 5), + "start_datetime": date(2022, 12, 15).strftime("%Y-%m-%d %H:%M:%S"), + "end_datetime": (datetime.utcnow() + timedelta(days=10)).strftime( + "%Y-%m-%d %H:%M:%S" + ), }, "banner3": { "message": "banner3", "url_path": "/banner3", "category": "warning", "active": True, - "start_datetime": date(2023, 1, 20), - "end_datetime": date(2023, 2, 25), + "start_datetime": date(2023, 1, 20).strftime("%Y-%m-%d %H:%M:%S"), + "end_datetime": (datetime.utcnow() + timedelta(days=30)).strftime( + "%Y-%m-%d %H:%M:%S" + ), }, } @@ -108,15 +125,15 @@ def test_create_banner(client, admin, headers): def test_disable_expired_after_create_action(client, admin, headers): """Disable expired banners after a create a banner action.""" # create banner first - banner1 = BannerModel.create(banners["banner1"]) - assert banner1.active is True + banner0 = BannerModel.create(banners["banner0"]) + assert banner0.active is True banner_data = banners["banner2"] admin.login(client) _create_banner(client, banner_data, headers, 201).json - expired_banner = BannerModel.get(banner1.id) + expired_banner = BannerModel.get(banner0.id) assert expired_banner.active is False @@ -145,9 +162,9 @@ def test_update_banner(client, admin, headers): def test_disable_expired_after_update_action(client, admin, headers): """Disable expired banners after an update a banner action.""" # create banner first - banner1 = BannerModel.create(banners["banner1"]) + banner0 = BannerModel.create(banners["banner0"]) banner2 = BannerModel.create(banners["banner2"]) - assert banner1.active is True + assert banner0.active is True admin.login(client) @@ -160,7 +177,7 @@ def test_disable_expired_after_update_action(client, admin, headers): _update_banner(client, banner2.id, new_data, headers, 200).json - expired_banner = BannerModel.get(banner1.id) + expired_banner = BannerModel.get(banner0.id) assert expired_banner.active is False From 526f01a17b331c5d43f6511a56541fc59fbd82cf Mon Sep 17 00:00:00 2001 From: Christoph Ladurner Date: Tue, 1 Oct 2024 09:48:57 +0200 Subject: [PATCH 07/12] tests: remove SAWarning * SAWarning: nested transaction already deassociated from connection * the context manager does the commit automatically --- invenio_banners/records/models.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/invenio_banners/records/models.py b/invenio_banners/records/models.py index a33167a..ed69a77 100644 --- a/invenio_banners/records/models.py +++ b/invenio_banners/records/models.py @@ -51,6 +51,7 @@ def create(cls, data): """Create a new banner.""" _categories = [t[0] for t in current_app.config["BANNERS_CATEGORIES"]] assert data.get("category") in _categories + with db.session.begin_nested(): obj = cls( message=data.get("message"), @@ -62,7 +63,6 @@ def create(cls, data): ) db.session.add(obj) - db.session.commit() return obj @classmethod @@ -87,8 +87,6 @@ def delete(cls, banner): with db.session.begin_nested(): db.session.delete(banner) - db.session.commit() - @classmethod def get_active(cls, url_path): """Return active banners.""" From 5ca1b0c078c06462f463479c69324b8a2c633cfc Mon Sep 17 00:00:00 2001 From: Christoph Ladurner Date: Tue, 1 Oct 2024 09:56:17 +0200 Subject: [PATCH 08/12] chore: change .format() to f-string --- tests/resources/test_resources.py | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/tests/resources/test_resources.py b/tests/resources/test_resources.py index 6ad9792..a024faa 100644 --- a/tests/resources/test_resources.py +++ b/tests/resources/test_resources.py @@ -60,36 +60,28 @@ def _create_banner(client, data, headers, status_code=None): """Send POST request.""" - result = client.post( - "/banners/", - headers=headers, - json=data, - ) + result = client.post("/banners/", headers=headers, json=data) assert result.status_code == status_code return result def _update_banner(client, id, data, headers, status_code=None): """Send PUT request.""" - result = client.put( - "/banners/{0}".format(id), - headers=headers, - json=data, - ) + result = client.put(f"/banners/{id}", headers=headers, json=data) assert result.status_code == status_code return result def _delete_banner(client, id, headers, status_code=None): """Send DELETE request.""" - result = client.delete("/banners/{0}".format(id), headers=headers) + result = client.delete(f"/banners/{id}", headers=headers) assert result.status_code == status_code return result def _get_banner(client, id, status_code=None): """Send GET request.""" - result = client.get("/banners/{0}".format(id)) + result = client.get(f"/banners/{id}") assert result.status_code == status_code return result From 6492e27fcc4fd031a10dc120ee50486dfecfd200 Mon Sep 17 00:00:00 2001 From: Christoph Ladurner Date: Tue, 1 Oct 2024 09:56:35 +0200 Subject: [PATCH 09/12] global: apply new SQLAlchemy rules * change from model Model.query to db.session.query(Model) * without an update in pytest-invenio and without this commit it produces following errors: "sqlalchemy.exc.InvalidRequestError: Can't operate on closed transaction inside context manager. Please complete the context manager before emitting further commands." and 'sqlalchemy.exc.IntegrityError: (psycopg2.errors.UniqueViolation) duplicate key value violates unique constraint "uq_accounts_user_email"' * with the updated db fixture in pytest-inveniothis this commit fixes following TypeError: 'Session' object is not callable --- invenio_banners/records/models.py | 31 +++++++++++++++++-------------- tests/resources/test_resources.py | 3 ++- tests/services/test_services.py | 10 ++++++---- 3 files changed, 25 insertions(+), 19 deletions(-) diff --git a/invenio_banners/records/models.py b/invenio_banners/records/models.py index ed69a77..fdae11a 100644 --- a/invenio_banners/records/models.py +++ b/invenio_banners/records/models.py @@ -69,7 +69,7 @@ def create(cls, data): def update(cls, data, id): """Update an existing banner.""" with db.session.begin_nested(): - cls.query.filter_by(id=id).update(data) + db.session.query(cls).filter_by(id=id).update(data) db.session.commit() @@ -77,7 +77,7 @@ def update(cls, data, id): def get(cls, id): """Get banner by its id.""" try: - return cls.query.filter_by(id=id).one() + return db.session.query(cls).filter_by(id=id).one() except NoResultFound: raise BannerNotExistsError(id) @@ -93,7 +93,8 @@ def get_active(cls, url_path): now = datetime.utcnow() query = ( - cls.query.filter(cls.active.is_(True)) + db.session.query(cls) + .filter(cls.active.is_(True)) .filter(cls.start_datetime <= now) .filter((cls.end_datetime.is_(None)) | (now <= cls.end_datetime)) ) @@ -111,16 +112,17 @@ def get_active(cls, url_path): @classmethod def search(cls, search_params, filters): """Filter banners accordingly to query params.""" - banners = ( - BannerModel.query.filter(or_(False, *filters)) - .order_by( - search_params["sort_direction"](text(",".join(search_params["sort"]))) - ) - .paginate( - page=search_params["page"], - per_page=search_params["size"], - error_out=False, - ) + if filters == []: + filtered = db.session.query(BannerModel).filter() + else: + filtered = db.session.query(BannerModel).filter(or_(*filters)) + + banners = filtered.order_by( + search_params["sort_direction"](text(",".join(search_params["sort"]))) + ).paginate( + page=search_params["page"], + per_page=search_params["size"], + error_out=False, ) return banners @@ -131,7 +133,8 @@ def disable_expired(cls): now = datetime.utcnow() query = ( - cls.query.filter(cls.active.is_(True)) + db.session.query(cls) + .filter(cls.active.is_(True)) .filter(cls.end_datetime.isnot(None)) .filter(cls.end_datetime < now) ) diff --git a/tests/resources/test_resources.py b/tests/resources/test_resources.py index a024faa..e0f76e5 100644 --- a/tests/resources/test_resources.py +++ b/tests/resources/test_resources.py @@ -10,6 +10,7 @@ from datetime import date, datetime, timedelta import pytest +from invenio_db import db from invenio_records_resources.services.errors import PermissionDeniedError from invenio_banners.records import BannerModel @@ -209,7 +210,7 @@ def test_delete_banner(client, admin, headers): _delete_banner(client, banner.id, headers, 204) # check that it's not present in db - assert BannerModel.query.filter_by(id=banner.id).one_or_none() is None + assert db.session.query(BannerModel).filter_by(id=banner.id).one_or_none() is None def test_delete_is_forbidden(client, user, headers): diff --git a/tests/services/test_services.py b/tests/services/test_services.py index ceb5e25..acd0bfd 100644 --- a/tests/services/test_services.py +++ b/tests/services/test_services.py @@ -10,6 +10,7 @@ from datetime import datetime, timedelta import pytest +from invenio_db import db from invenio_records_resources.services.errors import PermissionDeniedError from invenio_banners.proxies import current_banners_service as service @@ -127,7 +128,7 @@ def test_delete_banner(app, superuser_identity): service.delete(superuser_identity, banner.id) # check that it's not present in db - assert BannerModel.query.filter_by(id=banner.id).one_or_none() is None + assert db.session.query(BannerModel).filter_by(id=banner.id).one_or_none() is None def test_delete_is_forbidden(app, simple_user_identity): @@ -207,11 +208,12 @@ def test_disable_expired_banners(app, superuser_identity): BannerModel.create(banners["expired"]) BannerModel.create(banners["active"]) - assert BannerModel.query.filter(BannerModel.active.is_(True)).count() == 2 - + assert ( + db.session.query(BannerModel).filter(BannerModel.active.is_(True)).count() == 2 + ) service.disable_expired(superuser_identity) - _banners = BannerModel.query.filter(BannerModel.active.is_(True)).all() + _banners = db.session.query(BannerModel).filter(BannerModel.active.is_(True)).all() assert len(_banners) == 1 assert _banners[0].message == "active" From eeb91b702a38df7e83b0391b821a4ce311acb681 Mon Sep 17 00:00:00 2001 From: Christoph Ladurner Date: Thu, 3 Oct 2024 08:49:43 +0200 Subject: [PATCH 10/12] fix: tests * test_banner_creation uses the active banner. the service method which is tested validates the data over the schema where start_datetime and end_datetime is a required field and it has to look like: '%Y-%m-%d %H:%M:%S' * the problem is that test_read_banner uses the same banner to test the read service. the change in "active" has the effect that the stored value in the database is not like "2024-07-10 12:04:03.348391" which it is due the strftime function is not used. db.DateTime could only bring back the date in form of a datetime object which is then used in isoformat if the value in the database contains the ".34383" part. * therefore the easiest solution is to use for test_read_banner the "other" banner * ATTENTION: be cautious during the migration from utcnow to now(timezone.utc) --- tests/services/test_services.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/services/test_services.py b/tests/services/test_services.py index acd0bfd..6cc9214 100644 --- a/tests/services/test_services.py +++ b/tests/services/test_services.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- # # Copyright (C) 2022 CERN. +# Copyright (C) 2024 Graz University of Technology. # # Invenio-Banners is free software; you can redistribute it and/or modify it # under the terms of the MIT License; see LICENSE file for more details. @@ -22,7 +23,10 @@ "message": "active", "url_path": "/active", "category": "info", - "end_datetime": datetime.utcnow() + timedelta(days=1), + "start_datetime": datetime.utcnow().strftime("%Y-%m-%d %H:%M:%S"), + "end_datetime": (datetime.utcnow() + timedelta(days=1)).strftime( + "%Y-%m-%d %H:%M:%S" + ), "active": True, }, "inactive": { @@ -151,7 +155,7 @@ def test_delete_non_existing_banner(app, superuser_identity): def test_read_banner(app, simple_user_identity): """Read a banner by id.""" # create banner first - banner = BannerModel.create(banners["active"]) + banner = BannerModel.create(banners["other"]) banner_result = service.read(simple_user_identity, banner.id) From 3df8f941e60e106c67f3f7a4a41512692b162763 Mon Sep 17 00:00:00 2001 From: Christoph Ladurner Date: Thu, 31 Oct 2024 21:38:18 +0100 Subject: [PATCH 11/12] refactor: use session.get where possible * recomended sqlalchemy syntax for sqlalchemy >= 2.0 --- invenio_banners/records/models.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/invenio_banners/records/models.py b/invenio_banners/records/models.py index fdae11a..05baa65 100644 --- a/invenio_banners/records/models.py +++ b/invenio_banners/records/models.py @@ -14,7 +14,6 @@ from flask import current_app from invenio_db import db from sqlalchemy import or_ -from sqlalchemy.orm.exc import NoResultFound from sqlalchemy.sql import text from sqlalchemy_utils.models import Timestamp @@ -69,6 +68,9 @@ def create(cls, data): def update(cls, data, id): """Update an existing banner.""" with db.session.begin_nested(): + # NOTE: + # with db.session.get(cls, id) the model itself would be + # returned and this classmethod would be called db.session.query(cls).filter_by(id=id).update(data) db.session.commit() @@ -76,10 +78,10 @@ def update(cls, data, id): @classmethod def get(cls, id): """Get banner by its id.""" - try: - return db.session.query(cls).filter_by(id=id).one() - except NoResultFound: - raise BannerNotExistsError(id) + if banner := db.session.get(cls, id): + return banner + + raise BannerNotExistsError(id) @classmethod def delete(cls, banner): From 67b9b3502c266ed16c9eb4d0e244d997e430c425 Mon Sep 17 00:00:00 2001 From: Christoph Ladurner Date: Thu, 31 Oct 2024 21:39:11 +0100 Subject: [PATCH 12/12] global: use uow instead of explicit commit --- invenio_banners/records/models.py | 4 ---- invenio_banners/services/service.py | 15 ++++++++++----- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/invenio_banners/records/models.py b/invenio_banners/records/models.py index 05baa65..c9238d1 100644 --- a/invenio_banners/records/models.py +++ b/invenio_banners/records/models.py @@ -73,8 +73,6 @@ def update(cls, data, id): # returned and this classmethod would be called db.session.query(cls).filter_by(id=id).update(data) - db.session.commit() - @classmethod def get(cls, id): """Get banner by its id.""" @@ -143,5 +141,3 @@ def disable_expired(cls): for old in query.all(): old.active = False - - db.session.commit() diff --git a/invenio_banners/services/service.py b/invenio_banners/services/service.py index ecfa336..3ed1993 100644 --- a/invenio_banners/services/service.py +++ b/invenio_banners/services/service.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- # # Copyright (C) 2022-2023 CERN. +# Copyright (C) 2024 Graz University of Technology. # # Invenio-Banners is free software; you can redistribute it and/or modify it # under the terms of the MIT License; see LICENSE file for more details. @@ -10,6 +11,7 @@ import distutils.util import arrow +from invenio_db.uow import unit_of_work from invenio_records_resources.services import RecordService from invenio_records_resources.services.base import LinksTemplate from invenio_records_resources.services.base.utils import map_search_params @@ -81,7 +83,8 @@ def search(self, identity, params): links_item_tpl=self.links_item_tpl, ) - def create(self, identity, data, raise_errors=True): + @unit_of_work() + def create(self, identity, data, raise_errors=True, uow=None): """Create a banner.""" self.require_permission(identity, "create") @@ -99,17 +102,18 @@ def create(self, identity, data, raise_errors=True): self, identity, banner, links_tpl=self.links_item_tpl, errors=errors ) - def delete(self, identity, id): + @unit_of_work() + def delete(self, identity, id, uow=None): """Delete a banner from database.""" self.require_permission(identity, "delete") banner = self.record_cls.get(id) - self.record_cls.delete(banner) return self.result_item(self, identity, banner, links_tpl=self.links_item_tpl) - def update(self, identity, id, data): + @unit_of_work() + def update(self, identity, id, data, uow=None): """Update a banner.""" self.require_permission(identity, "update") @@ -131,7 +135,8 @@ def update(self, identity, id, data): links_tpl=self.links_item_tpl, ) - def disable_expired(self, identity): + @unit_of_work() + def disable_expired(self, identity, uow=None): """Disable expired banners.""" self.require_permission(identity, "disable") self.record_cls.disable_expired()