diff --git a/documentation/changelog.rst b/documentation/changelog.rst index 032d64508..d0b61220a 100644 --- a/documentation/changelog.rst +++ b/documentation/changelog.rst @@ -14,7 +14,7 @@ New features * The data chart on the asset page splits up its color-coded sensor legend when showing more than 7 sensors, becoming a legend per subplot [see `PR #1176 `_ and `PR #1193 `_] * Speed up loading the users page, by making the pagination backend-based and adding support for that in the API [see `PR #1160 `_] * X-axis labels in CLI plots show datetime values in a readable and informative format [see `PR #1172 `_] -* Enhanced API for listing sensors: Added filtering and pagination on sensor index endpoint and created new endpoint to get all sensors under an asset [see `PR #1191 `_ ] +* Enhanced API for listing sensors: Added filtering and pagination on sensor index endpoint and created new endpoint to get all sensors under an asset [see `PR #1191 `_ and `PR #1219 `_] * Speed up loading the accounts page,by making the pagination backend-based and adding support for that in the API [see `PR #1196 `_] * Speed up loading the account detail page by by switching to server-side pagination for assets, replacing client-side pagination [see `PR #1202 `_] * Simplify and Globalize toasts in the flexmeasures project [see `PR #1207 _`] diff --git a/flexmeasures/api/v3_0/sensors.py b/flexmeasures/api/v3_0/sensors.py index 75d027e5c..cba456b38 100644 --- a/flexmeasures/api/v3_0/sensors.py +++ b/flexmeasures/api/v3_0/sensors.py @@ -3,10 +3,11 @@ import isodate from datetime import datetime, timedelta +from werkzeug.exceptions import Unauthorized from flask import current_app, url_for from flask_classful import FlaskView, route from flask_json import as_json -from flask_security import auth_required +from flask_security import auth_required, current_user from marshmallow import fields, ValidationError import marshmallow.validate as validate from rq.job import Job, NoSuchJobError @@ -39,6 +40,7 @@ from flexmeasures.data.models.time_series import Sensor, TimedBelief from flexmeasures.data.queries.utils import simplify_index from flexmeasures.data.schemas.sensors import SensorSchema, SensorIdField +from flexmeasures.data.schemas import AssetIdField from flexmeasures.api.common.schemas.search import SearchFilterField from flexmeasures.api.common.schemas.sensors import UnitField from flexmeasures.data.schemas.times import AwareDateTimeField, PlanningDurationField @@ -66,27 +68,30 @@ class SensorAPI(FlaskView): @route("", methods=["GET"]) @use_kwargs( { - "account": AccountIdField( - data_key="account_id", load_default=AccountIdField.load_current + "account": AccountIdField(data_key="account_id", required=False), + "asset": AssetIdField(data_key="asset_id", required=False), + "include_consultancy_clients": fields.Boolean( + required=False, load_default=False ), - "all_accessible": fields.Boolean(required=False, missing=False), + "include_public_assets": fields.Boolean(required=False, load_default=False), "page": fields.Int( - required=False, validate=validate.Range(min=1), default=1 + required=False, validate=validate.Range(min=1), load_default=None ), "per_page": fields.Int( - required=False, validate=validate.Range(min=1), default=10 + required=False, validate=validate.Range(min=1), load_default=10 ), - "filter": SearchFilterField(required=False, default=None), - "unit": UnitField(required=False, default=None), + "filter": SearchFilterField(required=False, load_default=None), + "unit": UnitField(required=False, load_default=None), }, location="query", ) - @permission_required_for_context("read", ctx_arg_name="account") @as_json def index( self, - account: Account, - all_accessible: bool = False, + account: Account | None = None, + asset: GenericAsset | None = None, + include_consultancy_clients: bool = False, + include_public_assets: bool = False, page: int | None = None, per_page: int | None = None, filter: list[str] | None = None, @@ -97,11 +102,28 @@ def index( .. :quickref: Sensor; Download sensor list This endpoint returns all accessible sensors. - Accessible sensors are sensors in the same account as the current user. - Alternatively, you can use the `all_accessible` query parameter to list sensors from all assets that the `current_user` has read access to, as well as all public assets. The default value is `false`. + By default, "accessible sensors" means all sensors in the same account as the current user (if they have read permission to the account). + + You can also specify an `account` (an ID parameter), if the user has read access to that account. In this case, all assets under the + specified account will be retrieved, and the sensors associated with these assets will be returned. + + Alternatively, you can filter by asset hierarchy by providing the `asset` parameter (ID). When this is set, all sensors on the specified + asset and its sub-assets are retrieved, provided the user has read access to the asset. + + NOTE: You can't set both account and asset at the same time, you can only have one set. The only exception is if the asset being specified is + part of the account that was set, then we allow to see sensors under that asset but then ignore the account (account = None). + + Finally, you can use the `include_consultancy_clients` parameter to include sensors from accounts for which the current user account is a consultant. + This is only possible if the user has the role of a consultant. Only admins can use this endpoint to fetch sensors from a different account (by using the `account_id` query parameter). + The `filter` parameter allows you to search for sensors by name or account name. + The `unit` parameter allows you to filter by unit. + + For the pagination of the sensor list, you can use the `page` and `per_page` query parameters, the `page` parameter is used to trigger + pagination, and the `per_page` parameter is used to specify the number of records per page. The default value for `page` is 1 and for `per_page` is 10. + **Example response** An example of one sensor being returned: @@ -135,19 +157,58 @@ def index( :status 403: INVALID_SENDER :status 422: UNPROCESSABLE_ENTITY """ - if isinstance(account, list): - accounts = account - else: - accounts: list = [account] if account else [] - account_ids: list = [acc.id for acc in accounts] + if account is None and asset is None: + if current_user.is_anonymous: + raise Unauthorized + account = current_user.account + + if account is not None and asset is not None: + if asset.account_id != account.id: + return { + "message": "Please provide either an account or an asset ID, not both" + }, 422 + else: + account = None + + if asset is not None: + check_access(asset, "read") + + asset_tree = ( + db.session.query(GenericAsset.id, GenericAsset.parent_asset_id) + .filter(GenericAsset.id == asset.id) + .cte(name="asset_tree", recursive=True) + ) + + recursive_part = db.session.query( + GenericAsset.id, GenericAsset.parent_asset_id + ).join(asset_tree, GenericAsset.parent_asset_id == asset_tree.c.id) + + asset_tree = asset_tree.union(recursive_part) + + child_assets = db.session.query(asset_tree).all() + + filter_statement = GenericAsset.id.in_( + [asset.id] + [a.id for a in child_assets] + ) + elif account is not None: + check_access(account, "read") + + account_ids: list = [account.id] - filter_statement = GenericAsset.account_id.in_(account_ids) + if include_consultancy_clients: + if current_user.has_role("consultant"): + consultancy_accounts = ( + db.session.query(Account) + .filter(Account.consultancy_account_id == account.id) + .all() + ) + account_ids.extend([acc.id for acc in consultancy_accounts]) + + filter_statement = GenericAsset.account_id.in_(account_ids) + else: + filter_statement = None - if all_accessible is not None: - consultancy_account_ids: list = [ - acc.consultancy_account_id for acc in accounts - ] - account_ids.extend(consultancy_account_ids) + if include_public_assets: filter_statement = or_( filter_statement, GenericAsset.account_id.is_(None), @@ -156,7 +217,7 @@ def index( sensor_query = ( select(Sensor) .join(GenericAsset, Sensor.generic_asset_id == GenericAsset.id) - .join(Account, GenericAsset.owner) + .outerjoin(Account, GenericAsset.owner) .filter(filter_statement) ) @@ -167,6 +228,7 @@ def index( or_( Sensor.name.ilike(f"%{term}%"), Account.name.ilike(f"%{term}%"), + GenericAsset.name.ilike(f"%{term}%"), ) for term in filter ) diff --git a/flexmeasures/api/v3_0/tests/test_sensors_api.py b/flexmeasures/api/v3_0/tests/test_sensors_api.py index 7ca9da878..2163ed12f 100644 --- a/flexmeasures/api/v3_0/tests/test_sensors_api.py +++ b/flexmeasures/api/v3_0/tests/test_sensors_api.py @@ -20,7 +20,7 @@ @pytest.mark.parametrize( - "requesting_user, search_by, search_value, exp_sensor_name, exp_num_results, all_accessible, use_pagination", + "requesting_user, search_by, search_value, exp_sensor_name, exp_num_results, include_consultancy_clients, use_pagination, expected_status_code, filter_account_id, filter_asset_id, asset_id_of_of_first_sensor_result", [ ( "test_supplier_user_4@seita.nl", @@ -30,6 +30,23 @@ 2, True, False, + 200, + None, + 5, + None, + ), + ( + "test_prosumer_user@seita.nl", + None, + None, + "power", + 2, + False, + False, + 200, + None, + 7, + 8, # We test that the endpoint returns the sensor on a battery asset (ID: 8) while we filter for the building asset (ID: 7) that includes it ), ( "test_supplier_user_4@seita.nl", @@ -39,6 +56,49 @@ 1, True, False, + 200, + None, + 5, + None, + ), + ( + "test_supplier_user_4@seita.nl", + None, + None, + None, + None, + None, + None, + 422, # Error expected due to both asset_id and account_id being provided + 1, + 5, + None, + ), + ( + "test_dummy_account_admin@seita.nl", + None, + None, + None, + None, + None, + None, + 403, # Error expected as the user lacks access to the specified asset + None, + 5, + None, + ), + ( + "test_supplier_user_4@seita.nl", + None, + None, + None, + None, + None, + None, + 403, # Error expected as the user lacks access to the specified account + 1, + None, + None, ), ( "test_supplier_user_4@seita.nl", @@ -48,6 +108,10 @@ 3, True, True, + 200, + None, + 5, + None, ), ( "test_supplier_user_4@seita.nl", @@ -57,6 +121,10 @@ 1, False, False, + 200, + None, + 5, + None, ), ], indirect=["requesting_user"], @@ -64,19 +132,34 @@ def test_fetch_sensors( client, setup_api_test_data, + add_battery_assets, requesting_user, search_by, search_value, exp_sensor_name, exp_num_results, - all_accessible, + include_consultancy_clients, use_pagination, + expected_status_code, + filter_account_id, + filter_asset_id, + asset_id_of_of_first_sensor_result, ): """ Retrieve all sensors. Our user here is admin, so is allowed to see all sensors. Pagination is tested only in passing, we should test filtering and page > 1 + + The `filter_asset_id` specifies the asset_id to filter for. + + The `asset_id_of_of_first_sensor_result` specifies the asset_id of the first sensor + in the result list. This sensors is expected to be from a child asset of the asset + specified in `filter_asset_id`. + + The `filter_account_id` specifies the account_id to filter for. + + `check_errors` is used to test the error handling of the endpoint. """ query = {search_by: search_value} @@ -88,8 +171,14 @@ def test_fetch_sensors( elif search_by == "filter": query["filter"] = search_value - if all_accessible: - query["all_accessible"] = True + if include_consultancy_clients: + query["include_consultancy_clients"] = True + + if filter_account_id: + query["account_id"] = filter_account_id + + if filter_asset_id: + query["asset_id"] = filter_asset_id response = client.get( url_for("SensorAPI:index"), @@ -97,21 +186,30 @@ def test_fetch_sensors( ) print("Server responded with:\n%s" % response.json) - assert response.status_code == 200 - if use_pagination: - assert isinstance(response.json["data"][0], dict) - assert is_valid_unit(response.json["data"][0]["unit"]) - assert response.json["num-records"] == exp_num_results - assert response.json["filtered-records"] == exp_num_results - else: - assert isinstance(response.json, list) - assert is_valid_unit(response.json[0]["unit"]) - assert response.json[0]["name"] == exp_sensor_name - assert len(response.json) == exp_num_results - - if search_by == "unit": - assert response.json[0]["unit"] == search_value + assert response.status_code == expected_status_code + if expected_status_code == 200: + if use_pagination: + assert isinstance(response.json["data"][0], dict) + assert is_valid_unit(response.json["data"][0]["unit"]) + assert response.json["num-records"] == exp_num_results + assert response.json["filtered-records"] == exp_num_results + else: + assert isinstance(response.json, list) + assert is_valid_unit(response.json[0]["unit"]) + assert response.json[0]["name"] == exp_sensor_name + assert len(response.json) == exp_num_results + + if asset_id_of_of_first_sensor_result is not None: + assert ( + response.json[0]["generic_asset_id"] + == asset_id_of_of_first_sensor_result + ) + elif filter_asset_id: + assert response.json[0]["generic_asset_id"] == filter_asset_id + + if search_by == "unit": + assert response.json[0]["unit"] == search_value @pytest.mark.parametrize( @@ -176,10 +274,12 @@ def test_post_a_sensor(client, setup_api_test_data, requesting_user, db): assert response.status_code == 201 assert response.json["name"] == "power" assert response.json["event_resolution"] == "PT1H" + assert response.json["generic_asset_id"] == post_data["generic_asset_id"] sensor: Sensor = db.session.execute( - select(Sensor).filter_by(name="power") + select(Sensor).filter_by(name="power", unit="kWh") ).scalar_one_or_none() + assert sensor is not None assert sensor.unit == "kWh" assert sensor.attributes["capacity_in_mw"] == 0.0074