Skip to content

Commit

Permalink
Address feedback
Browse files Browse the repository at this point in the history
Addressing the feedback.

There are few important changes:
- get_flags is now only returning non change requests
- added get_change_requests function which only returns change requests

Changed so the set the AssessmentRecord.workflow_status to CHANGE_REQUESTED
on POST change_request.

There is no endpoint on assessment_store to update the flag
so I had to make use of the model directly inside the data_services.py.

Determining the status of the Application no longer relies on
Change Requests flags, directly uses AssessmentRecord.workflow_status.

Flags still override the final status of the AssessmentRecord if they exist.
  • Loading branch information
albertkol committed Jan 8, 2025
1 parent 3f9b41e commit 5ed752c
Show file tree
Hide file tree
Showing 5 changed files with 104 additions and 37 deletions.
25 changes: 17 additions & 8 deletions assess/assessments/routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@
get_assessment_progress,
get_associated_tags_for_application,
get_bulk_accounts_dict,
get_change_requests,
get_comments,
get_flags,
get_fund,
Expand All @@ -120,8 +121,9 @@
get_tags_for_fund_round,
get_users_for_fund,
match_comment_to_theme,
submit_change_request,
submit_comment,
submit_flag,
update_assessment_record_status,
)
from assess.services.models.comment import CommentType
from assess.services.models.flag import FlagType
Expand All @@ -143,6 +145,7 @@
map_application_with_sub_criterias_and_themes,
order_entire_application_by_themes,
)
from assessment_store.db.models.assessment_record.enums import Status as WorkflowStatus
from common.blueprints import Blueprint
from config import Config

Expand Down Expand Up @@ -1174,15 +1177,16 @@ def display_sub_criteria(

state = get_state_for_tasklist_banner(application_id)
flags_list = get_flags(application_id)
change_requests_list = get_change_requests(application_id)

user_id_list = []
change_requests = []
for flag_data in flags_list:
if sub_criteria_id not in flag_data.sections_to_flag or not flag_data.is_change_request:
for change_request in change_requests_list:
if sub_criteria_id not in change_request.sections_to_flag:
continue

change_requests.append(flag_data)
for flag_item in flag_data.updates:
change_requests.append(change_request)
for flag_item in change_request.updates:
if flag_item["user_id"] not in user_id_list:
user_id_list.append(flag_item["user_id"])

Expand Down Expand Up @@ -1249,7 +1253,7 @@ def display_sub_criteria(
"fund": get_fund(sub_criteria.fund_id),
"application_id": application_id,
"comments": theme_matched_comments,
"flags_list": change_requests,
"change_requests": change_requests,
"accounts_list": accounts_list,
"is_flaggable": False, # Flag button is disabled in sub-criteria page,
"display_comment_box": add_comment_argument,
Expand Down Expand Up @@ -1293,7 +1297,7 @@ def request_changes(application_id, sub_criteria_id, theme_id):
)

if request.method == "POST" and form.validate_on_submit():
submit_flag(
submit_change_request(
application_id=application_id,
flag_type=FlagType.RAISED.name,
user_id=g.account_id,
Expand All @@ -1303,6 +1307,11 @@ def request_changes(application_id, sub_criteria_id, theme_id):
is_change_request=True,
)

update_assessment_record_status(
application_id=application_id,
status=WorkflowStatus.CHANGE_REQUESTED,
)

# update apply.status apply.forms[].status and each form.question[].status

return redirect(
Expand Down Expand Up @@ -1594,7 +1603,7 @@ def application(application_id):
application_id=application_id,
accounts_list=accounts_list,
teams_flag_stats=teams_flag_stats,
flags_list=[flag for flag in flags_list if not flag.is_change_request],
flags_list=flags_list,
is_flaggable=is_flaggable(flag_status),
is_qa_complete=state.is_qa_complete,
qa_complete=qa_complete,
Expand Down
4 changes: 2 additions & 2 deletions assess/assessments/templates/assessments/sub_criteria.html
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ <h2 class="govuk-error-summary__title">
{{ navbar(application_id, sub_criteria, current_theme.id) }}
</div>
<div class="theme govuk-grid-column-two-thirds">
{% for flag in flags_list %}
{{ assessment_change_request(flag, questions, accounts_list[flag.latest_user_id], application_id) }}
{% for change_request in change_requests %}
{{ assessment_change_request(change_request, questions, accounts_list[change_request.latest_user_id], application_id) }}
{% endfor %}
<h3 class="govuk-heading-m response-title">Applicant's response</h3>
{{ theme(answers_meta)}}
Expand Down
75 changes: 61 additions & 14 deletions assess/services/data_services.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import requests
from flask import abort, current_app
from sqlalchemy import select

from assess.scoring.models.score import Score
from assess.services.models.application import Application
Expand All @@ -19,8 +20,11 @@
from assess.themes.deprecated_theme_mapper import (
map_application_with_sub_criteria_themes_fields,
)
from assessment_store.db.models.assessment_record.assessment_records import AssessmentRecord
from assessment_store.db.models.assessment_record.enums import Status as WorkflowStatus
from common.locale_selector.get_lang import get_lang
from config import Config
from db import db


def get_data(endpoint: str, payload: Dict = None):
Expand Down Expand Up @@ -529,9 +533,19 @@ def get_flag(flag_id: str) -> Optional[Flag]:


def get_flags(application_id: str) -> List[Flag]:
flag = get_data(Config.ASSESSMENT_FLAGS_ENDPOINT.format(application_id=application_id))
if flag:
return Flag.from_list(flag)
flags_data = get_data(Config.ASSESSMENT_FLAGS_ENDPOINT.format(application_id=application_id))

if flags_data:
return [flag for flag in Flag.from_list(flags_data) if not flag.is_change_request]
else:
return []


def get_change_requests(application_id: str) -> List[Flag]:
flags_data = get_data(Config.ASSESSMENT_FLAGS_ENDPOINT.format(application_id=application_id))

if flags_data:
return [flag for flag in Flag.from_list(flags_data) if not flag.is_change_request]
else:
return []

Expand All @@ -549,8 +563,6 @@ def submit_flag(
section: str = None,
allocation: str = None,
flag_id: str = None,
field_ids: str = "",
is_change_request: bool = False,
) -> Flag | None:
"""Submits a new flag to the assessment store for an application.
Returns Flag if a flag is created
Expand All @@ -561,7 +573,7 @@ def submit_flag(
:param justification: The justification for raising the flag
:param section: The assessment section the flag has been raised for.
"""

flag_type = FlagType[flag_type]
if flag_id:
flag = requests.put(
Config.ASSESSMENT_FLAGS_POST_ENDPOINT,
Expand All @@ -570,9 +582,7 @@ def submit_flag(
"justification": justification,
"user_id": user_id,
"allocation": allocation,
"status": FlagType[flag_type].value,
"is_change_request": is_change_request,
"field_ids": field_ids,
"status": flag_type.value,
},
)
else:
Expand All @@ -584,17 +594,54 @@ def submit_flag(
"sections_to_flag": section,
"user_id": user_id,
"allocation": allocation,
"status": FlagType[flag_type].value,
"is_change_request": is_change_request,
"field_ids": field_ids,
"status": flag_type.value,
},
)

if flag:
flag_json = flag.json()
return Flag.from_dict(flag_json)

return None

def submit_change_request(
application_id: str,
flag_type: str,
user_id: str,
justification: str,
section: str | None = None,
field_ids: str = "",
is_change_request: bool = False,
) -> Flag:
flag = requests.post(
Config.ASSESSMENT_FLAGS_POST_ENDPOINT,
json={
"application_id": application_id,
"justification": justification,
"sections_to_flag": section,
"user_id": user_id,
"allocation": None,
"status": FlagType[flag_type].value,
"is_change_request": is_change_request,
"field_ids": field_ids,
},
)

flag_json = flag.json()

return Flag.from_dict(flag_json)


def update_assessment_record_status(application_id: str, status: WorkflowStatus):
assessment_record = db.session.scalar(
select(AssessmentRecord).where(
AssessmentRecord.application_id == application_id,
)
)

if not assessment_record:
return

assessment_record.workflow_status = status
db.session.commit()


def get_all_uploaded_documents_theme_answers(
Expand Down
27 changes: 15 additions & 12 deletions assess/shared/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,21 +77,23 @@ def determine_display_status(workflow_status: str, Flags: List[Flag], is_qa_comp
return display_status


def determine_flag_status(Flags: List[Flag]) -> str:
flag_status = ""
flags_list = [(Flag.from_dict(flag) if isinstance(flag, dict) else flag) for flag in Flags] if Flags else []
all_latest_status = [flag.latest_status for flag in flags_list]
def determine_flag_status(flags: list) -> str:
if not flags:
return ""

flags_list = []
all_latest_status = []

for flag in flags:
converted_flag = Flag.from_dict(flag) if isinstance(flag, dict) else flag

has_flag_with_raised_status = False
for flag in flags_list:
if flag.is_change_request:
if flag.latest_status.name == FlagType.RAISED.name:
has_flag_with_raised_status = True
break
if converted_flag.is_change_request:
continue

if has_flag_with_raised_status:
return "Change requested"
flags_list.append(converted_flag)
all_latest_status.append(converted_flag.latest_status)

flag_status = ""
if FlagType.STOPPED in all_latest_status:
flag_status = "Stopped"
elif all_latest_status.count(FlagType.RAISED) > 1:
Expand All @@ -100,6 +102,7 @@ def determine_flag_status(Flags: List[Flag]) -> str:
for flag in flags_list:
if flag.latest_status == FlagType.RAISED:
flag_status = ("Flagged for " + flag.latest_allocation) if flag.latest_allocation else "Flagged"

return flag_status


Expand Down
10 changes: 9 additions & 1 deletion tests/assess_tests/test_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,14 @@ def test_determine_display_status():


def test_determine_display_status_for_change_requests():
assert (
determine_display_status(
workflow_status="CHANGE_REQUESTED",
Flags=RAISED_FLAG,
is_qa_complete=False,
)
== "Flagged for TEAM_1"
)
assert (
determine_display_status(
workflow_status="CHANGE_RECEIVED",
Expand All @@ -164,7 +172,7 @@ def test_determine_display_status_for_change_requests():
)
assert (
determine_display_status(
workflow_status="NOT_STARTED",
workflow_status="CHANGE_REQUESTED",
Flags=RAISED_CHANGE_REQUEST,
is_qa_complete=False,
)
Expand Down

0 comments on commit 5ed752c

Please sign in to comment.