-
Notifications
You must be signed in to change notification settings - Fork 36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: pagination on accounts listing #1196
Changes from 1 commit
8f85007
2e8815f
60af097
9e560df
18330ce
994186e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,14 +5,22 @@ | |
from webargs.flaskparser import use_kwargs, use_args | ||
from flask_security import current_user, auth_required | ||
from flask_json import as_json | ||
from sqlalchemy import or_, select, func | ||
|
||
from marshmallow import fields | ||
import marshmallow.validate as validate | ||
from flask_sqlalchemy.pagination import SelectPagination | ||
|
||
|
||
from flexmeasures.auth.policy import user_has_admin_access | ||
from flexmeasures.auth.decorators import permission_required_for_context | ||
from flexmeasures.data.models.audit_log import AuditLog | ||
from flexmeasures.data.models.user import Account | ||
from flexmeasures.data.models.user import Account, User | ||
from flexmeasures.data.models.generic_assets import GenericAsset | ||
from flexmeasures.data.services.accounts import get_accounts, get_audit_log_records | ||
from flexmeasures.api.common.schemas.users import AccountIdField | ||
from flexmeasures.data.schemas.account import AccountSchema | ||
from flexmeasures.api.common.schemas.generic_assets import SearchFilterField | ||
from flexmeasures.utils.time_utils import server_now | ||
|
||
""" | ||
|
@@ -34,29 +42,63 @@ class AccountAPI(FlaskView): | |
decorators = [auth_required()] | ||
|
||
@route("", methods=["GET"]) | ||
@use_kwargs( | ||
{ | ||
"page": fields.Int( | ||
required=False, validate=validate.Range(min=1), default=1 | ||
), | ||
"per_page": fields.Int( | ||
required=False, validate=validate.Range(min=1), default=10 | ||
), | ||
"filter": SearchFilterField(required=False, default=None), | ||
}, | ||
location="query", | ||
) | ||
@as_json | ||
def index(self): | ||
def index( | ||
self, | ||
page: int | None = None, | ||
per_page: int | None = None, | ||
filter: list[str] | None = None, | ||
): | ||
"""API endpoint to list all accounts accessible to the current user. | ||
|
||
.. :quickref: Account; Download account list | ||
|
||
This endpoint returns all accessible accounts. | ||
Accessible accounts are your own account and accounts you are a consultant for, or all accounts for admins. | ||
|
||
The endpoint supports pagination of the asset list using the `page` and `per_page` query parameters. | ||
|
||
- If the `page` parameter is not provided, all assets are returned, without pagination information. The result will be a list of assets. | ||
- If a `page` parameter is provided, the response will be paginated, showing a specific number of assets per page as defined by `per_page` (default is 10). | ||
Comment on lines
+71
to
+74
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should speak of accounts rather than assets. |
||
- If a search 'filter' such as 'solar "ACME corp"' is provided, the response will filter out assets where each search term is either present in their name or account name. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This comment needs an update. We are not actually applying the search filter to asset names. |
||
The response schema for pagination is inspired by https://datatables.net/manual/server-side#Returned-data | ||
|
||
**Example response** | ||
|
||
An example of one account being returned: | ||
|
||
.. sourcecode:: json | ||
|
||
[ | ||
{ | ||
"data" : [ | ||
{ | ||
'id': 1, | ||
'name': 'Test Account' | ||
'account_roles': [1, 3], | ||
'consultancy_account_id': 2, | ||
'primary_color': '#1a3443' | ||
'secondary_color': '#f1a122' | ||
Comment on lines
+91
to
+92
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing commas. |
||
'logo_url': 'https://example.com/logo.png' | ||
} | ||
] | ||
], | ||
"num-records" : 1, | ||
"filtered-records" : 1 | ||
Comment on lines
+96
to
+97
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Redundant spaces. |
||
|
||
} | ||
|
||
If no pagination is requested, the response only consists of the list under the "data" key. | ||
|
||
:reqheader Authorization: The authentication token | ||
:reqheader Content-Type: application/json | ||
|
@@ -76,7 +118,48 @@ def index(self): | |
else [] | ||
) | ||
|
||
return accounts_schema.dump(accounts), 200 | ||
query = db.session.query(Account).filter( | ||
Account.id.in_([a.id for a in accounts]) | ||
) | ||
|
||
if filter: | ||
search_terms = filter[0].split(" ") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Actually, since the @nhoening this is an example of why I suggested refactoring. The pagination implementation for assets uses a better function for splitting, which I had moved to a schema so any pagination implementation using search terms would use the same. |
||
query = query.filter( | ||
or_(*[Account.name.ilike(f"%{term}%") for term in search_terms]) | ||
) | ||
|
||
if page: | ||
select_pagination: SelectPagination = db.paginate( | ||
query, per_page=per_page, page=page | ||
) | ||
|
||
accounts_reponse: list = [] | ||
for account in select_pagination.items: | ||
user_count_query = select(func.count(User.id)).where( | ||
User.account_id == account.id | ||
) | ||
asset_count_query = select(func.count(GenericAsset.id)).where( | ||
GenericAsset.account_id == account.id | ||
) | ||
user_count = db.session.execute(user_count_query).scalar() | ||
asset_count = db.session.execute(asset_count_query).scalar() | ||
accounts_reponse.append( | ||
{ | ||
**account_schema.dump(account), | ||
"user_count": user_count, | ||
"asset_count": asset_count, | ||
} | ||
) | ||
|
||
response = { | ||
"data": accounts_reponse, | ||
"num-records": select_pagination.total, | ||
"filtered-records": select_pagination.total, | ||
} | ||
else: | ||
response = accounts_schema.dump(query.all(), many=True) | ||
|
||
return response, 200 | ||
|
||
@route("/<id>", methods=["GET"]) | ||
@use_kwargs({"account": AccountIdField(data_key="id")}, location="path") | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This schema should move to a more generic module, as it doesn't only deal with assets.