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

Feature/fwf 4371 Backend support different filter types #2596

Open
wants to merge 11 commits into
base: develop
Choose a base branch
from
Open
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ Mark items as `Added`, `Changed`, `Fixed`, `Modified`, `Removed`, `Untested Fea
* Added a new column, is_draft, to the application table to identify draft entries.
* Added Alembic script to update existing active drafts by setting is_draft to true in the application table.
* Added the includeSubmissionsCount=true parameter to the form list endpoint to include the submissions count.
* Added columns filter_type, parent_filter_id to the filter table.
* Added script to migrate existing filters to TASK filter type.
* Added filter_type parameter to filter list `/filter/user` endpoint to filter by filter type.

## 7.0.0 - 2025-01-10

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
"""Add filter type and parent filter id to filter table

Revision ID: 92bf83135905
Revises: 524194732683
Create Date: 2025-02-21 14:44:55.823150

"""
from alembic import op
import sqlalchemy as sa
from sqlalchemy.dialects import postgresql

# revision identifiers, used by Alembic.
revision = '92bf83135905'
down_revision = '524194732683'
branch_labels = None
depends_on = None


def upgrade():
# ### commands auto generated by Alembic - please adjust! ###
# Create the enum type
enum_type = postgresql.ENUM('TASK', 'ATTRIBUTE', 'DATE', name='FilterType')
enum_type.create(op.get_bind(), checkfirst=True)
op.add_column('filter', sa.Column('filter_type', enum_type, nullable=True, server_default='TASK'))
# Update existing rows to set the default value
op.execute("UPDATE filter SET filter_type = 'TASK' WHERE filter_type IS NULL")
# Alter the column to set `nullable=False`
op.alter_column('filter', 'filter_type', nullable=False)
op.create_index(op.f('ix_filter_filter_type'), 'filter', ['filter_type'], unique=False)
op.add_column('filter', sa.Column('parent_filter_id', sa.Integer(), nullable=True))
# ### end Alembic commands ###


def downgrade():
# ### commands auto generated by Alembic - please adjust! ###
op.drop_index(op.f('ix_filter_filter_type'), table_name='filter')
op.drop_column('filter', 'filter_type')
op.execute("DROP TYPE \"FilterType\"")
op.drop_column('filter', 'parent_filter_id')
# ### end Alembic commands ###
2 changes: 1 addition & 1 deletion forms-flow-api/src/formsflow_api/models/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from .base_model import BaseModel
from .db import db, ma
from .draft import Draft
from .filter import Filter
from .filter import Filter, FilterType
from .form_history_logs import FormHistory
from .form_process_mapper import FormProcessMapper
from .process import Process, ProcessStatus, ProcessType
Expand Down
29 changes: 27 additions & 2 deletions forms-flow-api/src/formsflow_api/models/filter.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,28 @@

from __future__ import annotations

from enum import Enum, unique
from typing import List

from formsflow_api_utils.utils.enums import FilterStatus
from sqlalchemy import JSON, and_, asc, case, or_
from sqlalchemy.dialects.postgresql import ARRAY
from sqlalchemy.dialects.postgresql import ARRAY, ENUM

from formsflow_api.models.base_model import BaseModel
from formsflow_api.models.db import db

from .audit_mixin import AuditDateTimeMixin, AuditUserMixin


@unique
class FilterType(Enum):
"""Filter type enum."""

TASK = "TASK"
ATTRIBUTE = "ATTRIBUTE"
DATE = "DATE"


class Filter(AuditDateTimeMixin, AuditUserMixin, BaseModel, db.Model):
"""This class manages filter information."""

Expand All @@ -30,6 +40,13 @@ class Filter(AuditDateTimeMixin, AuditUserMixin, BaseModel, db.Model):
status = db.Column(db.String(10), nullable=True)
task_visible_attributes = db.Column(JSON, nullable=True)
order = db.Column(db.Integer, nullable=True, comment="Display order")
filter_type = db.Column(
ENUM(FilterType, name="FilterType"),
nullable=False,
default=FilterType.TASK,
index=True,
)
parent_filter_id = db.Column(db.Integer, nullable=True)

@classmethod
def find_all_active_filters(cls, tenant: str = None) -> List[Filter]:
Expand Down Expand Up @@ -67,23 +84,31 @@ def create_filter_from_dict(cls, filter_data: dict) -> Filter:
filter_obj.task_visible_attributes = filter_data.get(
"task_visible_attributes"
)
filter_obj.filter_type = filter_data.get("filter_type")
filter_obj.parent_filter_id = filter_data.get("parent_filter_id")
filter_obj.save()
return filter_obj
return None

@classmethod
def find_user_filters(
def find_user_filters( # pylint: disable=too-many-arguments, too-many-positional-arguments
cls,
roles: List[str] = None,
user: str = None,
tenant: str = None,
admin: bool = False,
filter_type: str = None,
parent_filter_id: int = None,
):
"""Find active filters of the user."""
query = cls._auth_query(
roles, user, tenant, admin, filter_empty_tenant_key=True
)
query = query.filter(Filter.status == str(FilterStatus.ACTIVE.value))
if filter_type:
query = query.filter(Filter.filter_type == filter_type)
if parent_filter_id:
query = query.filter(Filter.parent_filter_id == parent_filter_id)
order_by_user_first = case((Filter.created_by == user, 1), else_=2)
query = query.order_by(
order_by_user_first, Filter.order, Filter.created_by, asc(Filter.name)
Expand Down
19 changes: 15 additions & 4 deletions forms-flow-api/src/formsflow_api/resources/filter.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@
"users": fields.List(
fields.String(), description="Authorized Users to the filter"
),
"parentFilterId": fields.Integer(description="Parent filter id"),
"filterType": fields.String(description="Filter type"),
},
)
filter_response = API.inherit(
Expand Down Expand Up @@ -143,7 +145,9 @@ def post():
"showUndefinedVariable":false
},
"users": [],
"roles": ["/formsflow/formsflow-reviewer"]
"roles": ["/formsflow/formsflow-reviewer"],
"parentFilterId": null,
"filterType": "TASK"
}
```
"""
Expand All @@ -164,6 +168,13 @@ class UsersFilterList(Resource):
@auth.has_one_of_roles([MANAGE_ALL_FILTERS, VIEW_FILTERS])
@profiletime
@API.doc(
params={
"filterType": {
"in": "query",
"description": "Filter type - TASK/DATE/ATTRIBUTE",
"default": "TASK",
}
},
responses={
200: "OK:- Successful request.",
403: "FORBIDDEN:- Permission denied",
Expand All @@ -176,7 +187,7 @@ def get():

Get all active filters of current reviewer user for requests with ```reviewer permission```.
"""
response, status = FilterService.get_user_filters(), HTTPStatus.OK
response, status = FilterService.get_user_filters(request.args), HTTPStatus.OK
return response, status


Expand All @@ -187,7 +198,7 @@ class FilterResourceById(Resource):
"""Resource for managing filter by id."""

@staticmethod
@auth.has_one_of_roles([MANAGE_ALL_FILTERS])
@auth.has_one_of_roles([MANAGE_ALL_FILTERS, VIEW_FILTERS])
@profiletime
@API.doc(
responses={
Expand All @@ -204,7 +215,7 @@ def get(filter_id: int):
Get filter details corresponding to a filter id for requests with ```REVIEWER_GROUP``` permission.
"""
filter_result = FilterService.get_filter_by_id(filter_id)
response, status = filter_schema.dump(filter_result), HTTPStatus.OK
response, status = filter_result, HTTPStatus.OK

return response, status

Expand Down
15 changes: 15 additions & 0 deletions forms-flow-api/src/formsflow_api/schemas/filter.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,18 @@ class Meta: # pylint: disable=too-few-public-methods
isMyTasksEnabled = fields.Bool(load_only=True)
isTasksForCurrentUserGroupsEnabled = fields.Bool(load_only=True)
order = fields.Int(data_key="order", allow_none=True)
filter_type = fields.Method(
"get_filter_type",
deserialize="load_filter_type",
data_key="filterType",
allow_none=True,
)
parent_filter_id = fields.Int(data_key="parentFilterId", allow_none=True)

def get_filter_type(self, obj):
"""This method is to get the filter type."""
return obj.filter_type.value

def load_filter_type(self, value):
"""This method is to load the filter type."""
return value.upper() if value else None
59 changes: 55 additions & 4 deletions forms-flow-api/src/formsflow_api/services/filter.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from formsflow_api_utils.utils.user_context import UserContext, user_context

from formsflow_api.constants import BusinessErrorCode
from formsflow_api.models import Filter, User
from formsflow_api.models import Filter, FilterType, User
from formsflow_api.schemas import FilterSchema

filter_schema = FilterSchema()
Expand Down Expand Up @@ -45,9 +45,38 @@ def create_filter(filter_payload, **kwargs):
filter_data = Filter.create_filter_from_dict(filter_payload)
return filter_schema.dump(filter_data)

@staticmethod
def get_attribute_filters(filter_type, user):
"""Get attribute filters & create lookup table with parent filter id."""
attribute_filters = {}
if filter_type == FilterType.TASK:
filters = Filter.find_user_filters(
roles=user.group_or_roles,
user=user.user_name,
tenant=user.tenant_key,
admin=ADMIN in user.roles,
filter_type=FilterType.ATTRIBUTE,
)
filters = filter_schema.dump(filters, many=True)
# Retain only the first filter for each parentFilterId
for f in filters:
parent_filter_id = f["parentFilterId"]
if parent_filter_id not in attribute_filters:
attribute_filters[parent_filter_id] = f
return attribute_filters

@staticmethod
def set_attribute_filters(filter_item, attribute_filters):
"""Set attribute filters for a filter item if it is of type TASK."""
if filter_item["filterType"] == FilterType.TASK.value:
matched_filter = attribute_filters.get(filter_item["id"])
filter_item["attributeFilters"] = (
[matched_filter] if matched_filter is not None else []
)

@staticmethod
@user_context
def get_user_filters(**kwargs):
def get_user_filters(request_args, **kwargs): # pylint: disable=too-many-locals
"""Get filters for the user."""
user: UserContext = kwargs["user"]
tenant_key = user.tenant_key
Expand Down Expand Up @@ -99,18 +128,26 @@ def get_user_filters(**kwargs):
},
)
filter_obj.save()

# If filter type is not provided, get all filters for type task and attribute
filter_type = (
request_args.get("filterType").upper()
if request_args.get("filterType")
else FilterType.TASK
)
filters = Filter.find_user_filters(
roles=user.group_or_roles,
user=user.user_name,
tenant=tenant_key,
admin=ADMIN in user.roles,
filter_type=filter_type,
)
filter_data = filter_schema.dump(filters, many=True)
default_variables = [
{"name": "applicationId", "label": "Submission Id"},
{"name": "formName", "label": "Form Name"},
]
attribute_filters = FilterService.get_attribute_filters(filter_type, user)

# User who created the filter or admin have edit permission.
for filter_item in filter_data:
filter_item["editPermission"] = (
Expand All @@ -121,6 +158,8 @@ def get_user_filters(**kwargs):
filter_item["variables"] += [
var for var in default_variables if var not in filter_item["variables"]
]
# Return attribute filters for task filters
FilterService.set_attribute_filters(filter_item, attribute_filters)
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this here ?
Can't we have 2 endpoints which I think will be easy to handle on the UI ?
Please note that there are other changes coming in to the picture on show/hide and re-ordering. So I feel it would be better to keep each filter as separate response.
Endpoints would be as simple as /filters/TASK
/filters/ATTRIBUTE?parenFilterId=<>

What do you think ?

Copy link
Contributor

Choose a reason for hiding this comment

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

The approach what auslin has followed is to quickly load the initial data when user comes to review screen.

  1. Suppose if we have more number of attribute filters and task filters, if we go with task api which will also have all attributes inside it , TIME TO GET THE RESPONSE WILL BE MORE and only after the response is received , the task list will be refreshed and shown.

  2. If we go with initially loading the TASK FILTER list , and ATTRIBUTE FILTER list after that , Task listing will have to wait for 2 APIs ( synchoronous ) to complete inorder to refresh and show.

Inorder to mitigate the above 2 issues, we thought of returning the TASK filter initially with either the first ATTRIBUTE filter alone in an array , or already selected 1 ATTRIBUTE filter as part of TASK filter and parallely we will call the full ATTRIBUTE filter endpoint so that user wont feel the delay in screen. By the time user plans to change the attibute filter, the ATTRIBUTE filter list will be available to frontend

We will definitely need attibute listing GET endpoint as a separate one so that if we change the TASK filter, updated attributes can be retrieved. Any other use cases like show/hide and re-ordering might need this endpoint.

@sumesh-aot Please share your thoughts
cc @auslin-aot

Copy link
Member

Choose a reason for hiding this comment

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

Understood.
Adding just one filter to the list is misrepresentation of the resource.

  • For 7.1.0 : we will only default the main filter
  • Keep the attribute filter selection as empty on first load
  • Let the user select the attribute filter based on their need
  • On change of task filter, save it as the default filter
  • Keep the main filter, attribute filter stored in state, so that if the user navigates on the same session and comes back they will see the last worked state

If we go with this, we wouldn't need the attributes filter to load initially right ?

Copy link
Member

Choose a reason for hiding this comment

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

Below are the agreed scope for 7.1.0

  • User can save TASK filter and ATTRIBUTES filter
  • Date filter cannot be saved
  • When a reviewer selects the TASK filter, we will save it as the default filter for the user
  • We won't save the ATTRIBUTES filter on the selection
  • Numbers displayed on the label would be the total number of tasks under TASK filter
  • By default only TASK filter will be displayed, user will need to select ATTRIBUTES filter and put in dates if needed
  • On same session, state will be maintained so that user can navigate and come back to the previous page

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the clarification @sumesh-aot We will separate task and attribute filter and do according to the requirements

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed attributeFilters from the response of filter list API

response = {"filters": filter_data}
# get user default filter
user_data = User.get_user_by_user_name(user_name=user.user_name)
Expand All @@ -141,7 +180,19 @@ def get_filter_by_id(filter_id, **kwargs):
admin=ADMIN in user.roles,
)
if filter_result:
return filter_result
response = filter_schema.dump(filter_result)
attribute_filters = Filter.find_user_filters(
roles=user.group_or_roles,
user=user.user_name,
tenant=tenant_key,
admin=ADMIN in user.roles,
filter_type=FilterType.ATTRIBUTE,
parent_filter_id=response["id"],
)
response["attributeFilters"] = filter_schema.dump(
attribute_filters, many=True
)
return response
raise BusinessException(BusinessErrorCode.FILTER_NOT_FOUND)

@staticmethod
Expand Down
62 changes: 62 additions & 0 deletions forms-flow-api/tests/unit/api/test_filter.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,3 +147,65 @@ def test_get_user_filters_by_order(app, client, session, jwt):
assert len(response.json.get("filters")) == 2
assert response.json.get("filters")[0].get("name") == "Reviewer Task"
assert response.json.get("filters")[1].get("name") == "Clerk Task"


def test_attribute_filter(app, client, session, jwt):
"""Test attribute filter with valid payload."""
token = get_token(jwt, role=CREATE_FILTERS, username="reviewer")
headers = {"Authorization": f"Bearer {token}", "content-type": "application/json"}
# Create task filter
response = client.post(
"/filter", headers=headers, json=get_filter_payload(name="Task filter1", roles=["formsflow-reviewer"])
)
assert response.status_code == 201
assert response.json.get("id") is not None
assert response.json.get("name") == "Task filter1"
assert response.json.get("filterType") == "TASK"
assert response.json.get("parentFilterId") is None

parent_filter_id = response.json.get("id")
# Create attribute filter for the task filter
response = client.post(
"/filter", headers=headers, json=get_filter_payload(name="Attribute filter1", roles=["formsflow-reviewer"], parent_filter_id=parent_filter_id, filter_type="ATTRIBUTE")
)
assert response.status_code == 201
assert response.json.get("id") is not None
assert response.json.get("name") == "Attribute filter1"
assert response.json.get("filterType") == "ATTRIBUTE"

token = get_token(jwt, role=VIEW_FILTERS, username="reviewer")
headers = {"Authorization": f"Bearer {token}", "content-type": "application/json"}
# Get task filters for the user
response = client.get("/filter/user", headers=headers)
assert response.status_code == 200
assert len(response.json.get("filters")) == 1
assert response.json.get("filters")[0].get("name") == "Task filter1"
assert response.json.get("filters")[0].get("attributeFilters")

# Get filter by id
response = client.get(f"/filter/{parent_filter_id}", headers=headers)
assert response.status_code == 200
assert response.json.get("name") == "Task filter1"
assert response.json.get("attributeFilters")


def test_date_filter(app, client, session, jwt):
"""Test date filter with valid payload."""
token = get_token(jwt, role=CREATE_FILTERS, username="reviewer")
headers = {"Authorization": f"Bearer {token}", "content-type": "application/json"}
# Create task filter
response = client.post(
"/filter", headers=headers, json=get_filter_payload(name="Date filter1", roles=["formsflow-reviewer"], filter_type="DATE")
)
assert response.status_code == 201
assert response.json.get("id") is not None
assert response.json.get("name") == "Date filter1"
assert response.json.get("filterType") == "DATE"

token = get_token(jwt, role=VIEW_FILTERS, username="reviewer")
headers = {"Authorization": f"Bearer {token}", "content-type": "application/json"}
# Get task filters for the user
response = client.get("/filter/user?filterType=DATE", headers=headers)
assert response.status_code == 200
assert len(response.json.get("filters")) == 1
assert response.json.get("filters")[0].get("name") == "Date filter1"
Loading
Loading