From 5ed752c63c318165ee1e33a8ab685c0b754e2f14 Mon Sep 17 00:00:00 2001 From: Albert Artur Kolozsvari Date: Tue, 7 Jan 2025 23:54:57 +0000 Subject: [PATCH] Address feedback 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. --- assess/assessments/routes.py | 25 +++++-- .../templates/assessments/sub_criteria.html | 4 +- assess/services/data_services.py | 75 +++++++++++++++---- assess/shared/helpers.py | 27 ++++--- tests/assess_tests/test_helpers.py | 10 ++- 5 files changed, 104 insertions(+), 37 deletions(-) diff --git a/assess/assessments/routes.py b/assess/assessments/routes.py index ed1363176..bb61bd857 100644 --- a/assess/assessments/routes.py +++ b/assess/assessments/routes.py @@ -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, @@ -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 @@ -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 @@ -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"]) @@ -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, @@ -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, @@ -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( @@ -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, diff --git a/assess/assessments/templates/assessments/sub_criteria.html b/assess/assessments/templates/assessments/sub_criteria.html index 1e89ab307..31acaaf8c 100644 --- a/assess/assessments/templates/assessments/sub_criteria.html +++ b/assess/assessments/templates/assessments/sub_criteria.html @@ -60,8 +60,8 @@

{{ navbar(application_id, sub_criteria, current_theme.id) }}
- {% 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 %}

Applicant's response

{{ theme(answers_meta)}} diff --git a/assess/services/data_services.py b/assess/services/data_services.py index 374d01698..0c1ba7d42 100644 --- a/assess/services/data_services.py +++ b/assess/services/data_services.py @@ -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 @@ -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): @@ -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 [] @@ -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 @@ -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, @@ -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: @@ -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( diff --git a/assess/shared/helpers.py b/assess/shared/helpers.py index edfeec6c2..10989d289 100644 --- a/assess/shared/helpers.py +++ b/assess/shared/helpers.py @@ -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: @@ -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 diff --git a/tests/assess_tests/test_helpers.py b/tests/assess_tests/test_helpers.py index f863ebed0..6925ba9e6 100644 --- a/tests/assess_tests/test_helpers.py +++ b/tests/assess_tests/test_helpers.py @@ -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", @@ -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, )