Skip to content
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

Merged
merged 6 commits into from
Sep 27, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 88 additions & 5 deletions flexmeasures/api/v3_0/accounts.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

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.

from flexmeasures.utils.time_utils import server_now

"""
Expand All @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The 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.
Copy link
Contributor

Choose a reason for hiding this comment

The 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
Copy link
Contributor

Choose a reason for hiding this comment

The 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
Copy link
Contributor

Choose a reason for hiding this comment

The 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
Expand All @@ -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(" ")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This way of splitting does not work with search terms in quotes, such as the "ACME corp" in the docstring example.

Actually, since the SearchFilterField is used, the search string should already be split into a list of search terms, where each term may contain spaces. This line likely results in only applying the first search time, which is why it may have escaped developer testing.

@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")
Expand Down
2 changes: 1 addition & 1 deletion flexmeasures/api/v3_0/assets.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@
from flask_login import current_user
from flask_security import auth_required
from flask_json import as_json
from flask_sqlalchemy.pagination import SelectPagination

from marshmallow import fields
import marshmallow.validate as validate

from webargs.flaskparser import use_kwargs, use_args
from sqlalchemy import select, delete, func
from flask_sqlalchemy.pagination import SelectPagination
joshuaunity marked this conversation as resolved.
Show resolved Hide resolved

from flexmeasures.auth.decorators import permission_required_for_context
from flexmeasures.data import db
Expand Down
118 changes: 81 additions & 37 deletions flexmeasures/ui/templates/crud/accounts.html
Original file line number Diff line number Diff line change
Expand Up @@ -11,48 +11,92 @@
<h3>All Accounts
</h3>
<div class="table-responsive">
<table class="table table-striped paginate nav-on-click" title="View this account">
<thead>
<tr>
<th>Account</th>
<th>ID</th>
<th>Assets</th>
<th>Users</th>
<th>Roles</th>
<th class="d-none">URL</th>
</tr>
</thead>
<tbody>
{% for account in accounts %}
<tr>
<td>
{{ account.name }}
</td>
<td>
{{ account.id }}
</td>
<td>
{{ account.asset_count }}
</td>
<td>
{{ account.user_count }}
</td>
<td>
{{ account.account_roles | map(attribute='name') | join(", ") }}
</td>
<td class="d-none">
/accounts/{{ account.id }}
</td>
</tr>
{% endfor %}
</tbody>
</table>
</div>
<table class="table table-striped paginate nav-on-click" title="View data" id="accountsTable">
</table>
</div>
</div>
</div>
</div>
<div class="col-md-2"></div>
</div>
</div>

<script>
function Account(
id,
name,
assets,
users,
roles,
logo,
) {
this.id = id;
this.name = name;
this.assets = assets;
this.users = users;
this.roles = roles.map((role) => role).join(", ");
this.logo = logo ? `<img src="${logo}" alt="logo" style="width: 50px;">` : '';
this.url = `/accounts/${id}`;
}

$(document).ready(function () {
let unit = "";
// Initialize the DataTable
const table = $("#accountsTable").dataTable({
serverSide: true,
// make the table row vertically aligned with header
columns: [
{ data: "id", title: "ID" },
{ data: "name", title: "Name" },
{ data: "assets", title: "Assets" },
{ data: "users", title: "Users" },
{ data: "roles", title: "Roles" },
{ data: "logo", title: "logo" },
{ data: "url", title: "URL", className: "d-none" },
],

ajax: function (data, callback, settings) {
const basePath = window.location.origin;
let filter = data["search"]["value"];
let url = `${basePath}/api/v3_0/accounts?page=${
data["start"] / data["length"] + 1
}&per_page=${data["length"]}`;

if (filter.length > 0) {
url = `${url}&filter=${filter}`;
}

$.ajax({
type: "get",
url: url,
success: function (response, text) {
let clean_response = [];
response["data"].forEach((element) =>
clean_response.push(
new Account(
element["id"],
element["name"],
element["asset_count"],
element["user_count"],
element["account_roles"],
element["logo_url"],
)
)
);

callback({
data: clean_response,
recordsTotal: response["num-records"],
recordsFiltered: response["filtered-records"],
});
},
error: function (request, status, error) {
console.log("Error: ", error);
},
});
},
});
});
</script>

{% block paginate_tables_script %} {{ super() }} {% endblock %} {% endblock%}
Loading