Skip to content

Commit

Permalink
Validation - Changes according to review
Browse files Browse the repository at this point in the history
  • Loading branch information
Adrien Jézégou committed Jan 18, 2024
1 parent 570ea8c commit 5f97904
Show file tree
Hide file tree
Showing 12 changed files with 394 additions and 208 deletions.
182 changes: 100 additions & 82 deletions src/backend/partaj/core/api/referral_lite.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,27 @@ def list(self, request, *args, **kwargs):
set([unit_membership.role for unit_membership in unit_memberships])
)

units = list(
set([unit_membership.unit.id for unit_membership in unit_memberships])
)

if len(roles) > 1:
capture_message(
f"User {request.user.id} has been found with multiple roles",
"error",
)

role = None if len(roles) == 0 else roles[0]
if len(roles) == 0:
return Response(
{
"count": 0,
"next": None,
"previous": None,
"results": [],
}
)

role = roles[0]

# Set up the initial list of filters for all list queries
es_query_filters = [
Expand All @@ -69,14 +83,6 @@ def list(self, request, *args, **kwargs):
"should": [
{
"bool": {
"must": {
"terms": {
"units": [
str(unit_membership.unit.id)
for unit_membership in unit_memberships
]
}
},
"must_not": {
"term": {"state": str(models.ReferralState.DRAFT)}
},
Expand Down Expand Up @@ -131,18 +137,6 @@ def list(self, request, *args, **kwargs):
state = form.cleaned_data.get("state")
if len(state):
es_query_filters += [{"terms": {"state": state}}]
else:
# Make sure to exclude draft referrals whenever a list of acceptable states is not
# passed by the caller.
es_query_filters += [
{
"bool": {
"must_not": [
{"term": {"state": str(models.ReferralState.DRAFT)}}
]
}
}
]

due_date_after = form.cleaned_data.get("due_date_after")
if due_date_after:
Expand All @@ -157,26 +151,82 @@ def list(self, request, *args, **kwargs):
es_query_filters += [{"range": {"due_date": {"lt": due_date_before}}}]

task = form.cleaned_data.get("task")
if task:
# Check role and add a constant filter to :
# - All referral assigned to user for unit members
# - All referral assigned to user unit for granted users
if role == models.UnitMembershipRole.MEMBER:
es_query_filters += [
{
"bool": {
"must": {"term": {"assignees": request.user.id}},
}
}
]

if role in [
models.UnitMembershipRole.OWNER,
models.UnitMembershipRole.ADMIN,
models.UnitMembershipRole.SUPERADMIN,
]:
es_query_filters += [
{
"bool": {
"must": {"terms": {"units": units}},
}
}
]

if task == "process":
granted_unit_memberships = unit_memberships.filter(
role__in=[
models.UnitMembershipRole.OWNER,
models.UnitMembershipRole.ADMIN,
models.UnitMembershipRole.SUPERADMIN,
],
user=request.user,
).all()
es_query_filters += [
{
"bool": {
"should": [
{"term": {"assignees": request.user.id}},
"must": [
{
"terms": {
"units": [
str(granted_unit_membership.unit.id)
for granted_unit_membership in granted_unit_memberships
"bool": {
"must_not": [
{
"term": {
"state": models.ReferralState.RECEIVED
}
},
]
}
},
{
"bool": {
"must_not": [
{
"term": {
"events.verb": ReportEventVerb.REQUEST_VALIDATION,
}
},
{
"term": {
"events.receiver_unit_name": request.user.unit_name,
}
},
{
"term": {
"events.receiver_role": role,
}
},
]
}
},
{
"bool": {
"must_not": [
{
"term": {
"events.verb": ReportEventVerb.REQUEST_CHANGE,
}
},
{
"term": {
"last_author": request.user.id,
}
},
]
}
},
Expand All @@ -186,63 +236,20 @@ def list(self, request, *args, **kwargs):
]

elif task == "assign":
owner_unit_memberships = unit_memberships.filter(
role__in=[
models.UnitMembershipRole.OWNER,
],
user=request.user,
).all()
owner_units = units if role == models.UnitMembershipRole.OWNER else []

es_query_filters += [
{
"terms": {
"units": [
str(owner_unit_membership.unit.id)
for owner_unit_membership in owner_unit_memberships
]
}
},
{"terms": {"units": owner_units}},
{"term": {"state": models.ReferralState.RECEIVED}},
]
elif task == "validate":
granted_unit_memberships = unit_memberships.filter(
role__in=[
models.UnitMembershipRole.OWNER,
models.UnitMembershipRole.ADMIN,
models.UnitMembershipRole.SUPERADMIN,
],
user=request.user,
).all()

if role not in [
models.UnitMembershipRole.OWNER,
models.UnitMembershipRole.ADMIN,
models.UnitMembershipRole.SUPERADMIN,
]:
return Response(
{
"count": 0,
"next": None,
"previous": None,
"results": [],
}
)

es_query_filters += [
{
"bool": {
"should": [
{
"bool": {
"must": [
{
"terms": {
"units": [
str(granted_unit_membership.unit.id)
for granted_unit_membership in granted_unit_memberships
]
}
},
{
"term": {
"expected_validators": request.user.id
Expand Down Expand Up @@ -312,8 +319,19 @@ def list(self, request, *args, **kwargs):
}
},
{
"term": {
"events.sender_id": request.user.id,
"bool": {
"should": [
{
"term": {
"last_author": request.user.id,
}
},
{
"term": {
"events.sender_id": request.user.id,
}
},
]
}
},
]
Expand Down
26 changes: 22 additions & 4 deletions src/backend/tests/partaj/core/test_api_referrallite.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1248,6 +1248,10 @@ def test_list_referrals_for_user_by_themselves(self):
Users can get the list of referrals for themselves.
"""
user = factories.UserFactory()
unit = factories.UnitFactory()
factories.UnitMembershipFactory(
unit=unit, user=user, role=models.UnitMembershipRole.MEMBER
)
referrals = [
factories.ReferralFactory(
state=models.ReferralState.RECEIVED,
Expand All @@ -1265,6 +1269,9 @@ def test_list_referrals_for_user_by_themselves(self):
),
]

for referral in referrals:
referral.units.set([unit])

self.setup_elasticsearch()
response = self.client.get(
f"/api/referrallites/?user={user.id}",
Expand All @@ -1282,6 +1289,10 @@ def test_list_referrals_for_user_by_themselves_performance(self):
is not overwhelming and the duration stays in the acceptable range.
"""
user = factories.UserFactory()
unit = factories.UnitFactory()
factories.UnitMembershipFactory(
unit=unit, user=user, role=models.UnitMembershipRole.MEMBER
)
factories.ReferralFactory.create_batch(
100,
post__users=[user],
Expand All @@ -1297,7 +1308,7 @@ def test_list_referrals_for_user_by_themselves_performance(self):
self.setup_elasticsearch()

# Only one query at request time, for authentication
with self.assertNumQueries(2):
with self.assertNumQueries(3):
pre_response = perf_counter()
response = self.client.get(
f"/api/referrallites/?user={user_id}",
Expand All @@ -1317,9 +1328,12 @@ def test_list_referrals_for_more_than_one_user(self):
user = factories.UserFactory()
topic = factories.TopicFactory()
topic.unit.members.add(user)

requester_1 = factories.UserFactory()
requester_2 = factories.UserFactory()
unit = factories.UnitFactory()
factories.UnitMembershipFactory(
unit=unit, user=user, role=models.UnitMembershipRole.OWNER
)

referrals = [
factories.ReferralFactory(
Expand Down Expand Up @@ -1355,17 +1369,21 @@ def test_list_referrals_for_more_than_one_user(self):
),
]

for referral in referrals:
referral.units.set([unit])

self.setup_elasticsearch()
response = self.client.get(
(f"/api/referrallites/?user={requester_1.id},{requester_2.id}"),
HTTP_AUTHORIZATION=f"Token {Token.objects.get_or_create(user=user)[0]}",
)

self.assertEqual(response.status_code, 200)
self.assertEqual(response.json()["count"], 3)
self.assertEqual(response.json()["count"], 4)
self.assertEqual(response.json()["results"][0]["id"], referrals[3].id)
self.assertEqual(response.json()["results"][1]["id"], referrals[2].id)
self.assertEqual(response.json()["results"][2]["id"], referrals[0].id)
self.assertEqual(response.json()["results"][2]["id"], referrals[1].id)
self.assertEqual(response.json()["results"][3]["id"], referrals[0].id)

def test_list_referrals_for_nonexistent_user(self):
"""
Expand Down
Loading

0 comments on commit 5f97904

Please sign in to comment.