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

[MDS-6263] Adjustments around Assigned status not behaving as aspected #3334

Merged
merged 8 commits into from
Dec 12, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,7 @@ export const AuthorizationsInvolved: FC<ProjectSummaryFormComponentProps> = ({ f

const systemFlag = useSelector(getSystemFlag);
const isCore = systemFlag === SystemFlagEnum.core;
const envFieldsDisabled = areAuthEnvFieldsDisabled(systemFlag, formValues?.status_code);
const envFieldsDisabled = areAuthEnvFieldsDisabled(systemFlag, formValues?.status_code, formValues?.confirmation_of_submission);

const handleChange = (e, code) => {
if (e.target.checked) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,18 @@ export const ProjectManagement: FC = () => {
component={RenderSelect}
data={projectLeadData}
/>
{isProjectLeadAssigned && (
<Paragraph>
<b>Warning:</b> Unassigning the project lead will set the Project Description status to
'Submitted' in MineSpace. Ensure the status is correct before proceeding.
</Paragraph>
)}
{!isNewProject && !isProjectLeadAssigned && (
<Alert
message="This project does not have a Project Lead"
description={<p>Please assign a Project Lead to this project.</p>}
message="Assign a Project Lead"
description={<p>Assigning a Project Lead will set the Project Description status
to 'Assigned' in Core and 'Submitted' in MineSpace. Please ensure the project
is set at the correct status before continuing.</p>}
type="warning"
showIcon
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,11 @@ export const ProjectSummaryForm: FC<ProjectSummaryFormProps> = ({
getProjectSummaryAuthorizationTypesArray
);
const formValues = useSelector(getFormValues(FORM.ADD_EDIT_PROJECT_SUMMARY)) as IProjectSummaryForm;
const { status_code } = formValues ?? {};
const { status_code, confirmation_of_submission } = formValues ?? {};

const fieldsDisabled = areFieldsDisabled(systemFlag, status_code);
const fieldsDisabled = areFieldsDisabled(systemFlag, status_code, confirmation_of_submission);
const docFieldsDisabled = areDocumentFieldsDisabled(systemFlag, status_code);
const authFieldsDisabled = areAuthFieldsDisabled(systemFlag, status_code);
const authFieldsDisabled = areAuthFieldsDisabled(systemFlag, status_code, confirmation_of_submission);

const handleTransformPayload = (valuesFromForm: any) => {
return formatProjectPayload(valuesFromForm, { projectSummaryAuthorizationTypesArray });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -360,13 +360,13 @@ exports[`Project Management renders properly 1`] = `
<div
class="ant-alert-message"
>
This project does not have a Project Lead
Assign a Project Lead
</div>
<div
class="ant-alert-description"
>
<p>
Please assign a Project Lead to this project.
Assigning a Project Lead will set the Project Description status to 'Assigned' in Core and 'Submitted' in MineSpace. Please ensure the project is set at the correct status before continuing.
</p>
</div>
</div>
Expand Down
12 changes: 6 additions & 6 deletions services/common/src/components/projects/projectUtils.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ const TEST_PARAMETERS = [
testFunction: areFieldsDisabled,
coreDisabledStatuses: ["WDN", "COM"],
coreEnabledStatuses: ["DFT", "SUB", "ASG", "UNR", "CHR", "OHD"],
msDisabledStatuses: ["SUB", "ASG", "UNR", "WDN", "OHD", "COM"],
msEnabledStatuses: ["DFT", "CHR"],
msDisabledStatuses: ["SUB", "UNR", "WDN", "OHD", "COM"],
msEnabledStatuses: ["DFT", "CHR", "ASG"],
Copy link
Collaborator

Choose a reason for hiding this comment

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

I do find it confusing that Assigned was added to enabled- like, generally they're still not supposed to be able to edit when it's in Assigned status, right? Is the status even assigned when there's no project lead assigned?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm, so is the flow that's causing issues basically:

  1. an MS/CORE user makes a draft project summary
  2. they assign a project lead
  3. the project gets put in Assigned status even though it's not submitted

I guess it's pretty wacky! I'm thinking that for the tests though, maybe pass a parameter to indicate that it has been submitted so that we know what to expect generally, and then add another test for the special case where it's assigned but not submitted?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I reverted the change to the arrays, and a new test has been added now.

},
{
label: "areDocumentFieldsDisabled",
Expand All @@ -25,16 +25,16 @@ const TEST_PARAMETERS = [
testFunction: areAuthFieldsDisabled,
coreDisabledStatuses: ["WDN", "COM", "CHR", "UNR"],
coreEnabledStatuses: ["DFT", "SUB", "ASG", "OHD"],
msDisabledStatuses: ["UNR", "WDN", "OHD", "COM", "SUB", "ASG", "CHR"],
msEnabledStatuses: ["DFT"],
msDisabledStatuses: ["UNR", "WDN", "OHD", "COM", "SUB", "CHR"],
msEnabledStatuses: ["DFT", "ASG"],
},
{
label: "areAuthEnvFieldsDisabled",
testFunction: areAuthEnvFieldsDisabled,
coreDisabledStatuses: ["WDN", "COM", "ASG", "UNR", "CHR", "OHD", "SUB"],
coreEnabledStatuses: ["DFT"],
msDisabledStatuses: ["UNR", "WDN", "OHD", "COM", "SUB", "ASG", "CHR"],
msEnabledStatuses: ["DFT"],
msDisabledStatuses: ["UNR", "WDN", "OHD", "COM", "SUB", "CHR"],
msEnabledStatuses: ["DFT", "ASG"],
}
];

Expand Down
17 changes: 6 additions & 11 deletions services/common/src/components/projects/projectUtils.ts
Original file line number Diff line number Diff line change
@@ -1,36 +1,32 @@
import { PROJECT_STATUS_CODES, SystemFlagEnum } from "@mds/common/constants/enums";
import { memoize } from "lodash";

export const areFieldsDisabled = memoize((systemFlag: SystemFlagEnum, projectSummaryStatusCode: string) => {
export const areFieldsDisabled = memoize((systemFlag: SystemFlagEnum, projectSummaryStatusCode: string, confirmationOfSubmission?: boolean) => {
// Return false (enabled) if status = "" => "Not Started"
const isStatusInEnum = (<any>Object).values(PROJECT_STATUS_CODES).includes(projectSummaryStatusCode);

if (!isStatusInEnum) return false;
const projectSummaryStatus = projectSummaryStatusCode as PROJECT_STATUS_CODES;

const disabledStatuses = [PROJECT_STATUS_CODES.WDN, PROJECT_STATUS_CODES.COM];

const enabledStatuses = systemFlag === SystemFlagEnum.core
? [PROJECT_STATUS_CODES.DFT, PROJECT_STATUS_CODES.ASG, PROJECT_STATUS_CODES.UNR, PROJECT_STATUS_CODES.CHR, PROJECT_STATUS_CODES.OHD, PROJECT_STATUS_CODES.SUB]
: [PROJECT_STATUS_CODES.DFT, PROJECT_STATUS_CODES.CHR];
: [PROJECT_STATUS_CODES.DFT, PROJECT_STATUS_CODES.CHR, ...(!confirmationOfSubmission ? [PROJECT_STATUS_CODES.ASG] : [])];

if (disabledStatuses.includes(projectSummaryStatus)) return true;
return !enabledStatuses.includes(projectSummaryStatus);

},
(systemFlag: SystemFlagEnum, projectSummaryStatusCode: string) => `${systemFlag}_${projectSummaryStatusCode}`);

export const areAuthFieldsDisabled = memoize((systemFlag: SystemFlagEnum, projectSummaryStatusCode: string) => {
const fieldsDisabled = areFieldsDisabled(systemFlag, projectSummaryStatusCode);
export const areAuthFieldsDisabled = memoize((systemFlag: SystemFlagEnum, projectSummaryStatusCode: string, confirmationOfSubmission?: boolean) => {
const fieldsDisabled = areFieldsDisabled(systemFlag, projectSummaryStatusCode, confirmationOfSubmission);
if (fieldsDisabled) return true;

const extraDisabledStatuses = [PROJECT_STATUS_CODES.CHR, PROJECT_STATUS_CODES.UNR];
const authDisabled = extraDisabledStatuses.includes(projectSummaryStatusCode as PROJECT_STATUS_CODES)
return authDisabled;
}, (systemFlag: SystemFlagEnum, projectSummaryStatusCode: string) => `${systemFlag}_${projectSummaryStatusCode}`);

export const areAuthEnvFieldsDisabled = memoize((systemFlag, projectSummaryStatusCode) => {
const authFieldsDisabled = areAuthFieldsDisabled(systemFlag, projectSummaryStatusCode);
export const areAuthEnvFieldsDisabled = memoize((systemFlag: SystemFlagEnum, projectSummaryStatusCode: string, confirmationOfSubmission?: boolean) => {
const authFieldsDisabled = areAuthFieldsDisabled(systemFlag, projectSummaryStatusCode, confirmationOfSubmission);
if (authFieldsDisabled) return true;

const extraDisabledStatuses = systemFlag === SystemFlagEnum.core
Expand All @@ -56,7 +52,6 @@ export const areDocumentFieldsDisabled = memoize((systemFlag: SystemFlagEnum, pr

if (disabledStatuses.includes(projectSummaryStatus)) return true;
return !enabledStatuses.includes(projectSummaryStatus);

},
(systemFlag: SystemFlagEnum, projectSummaryStatusCode: string) => `${systemFlag}_${projectSummaryStatusCode}`);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,4 +64,5 @@ export interface IProjectSummaryForm extends Omit<IProjectSummary, "authorizatio
legal_land_desc: string;
facility_pid_pin_crown_file_no: string;
zoning: boolean;
confirmation_of_submission?: boolean;
}
Original file line number Diff line number Diff line change
Expand Up @@ -1045,9 +1045,9 @@ def update(self,

# Update simple properties.
# If we assign a project lead update status to Assigned and vice versa Submitted.
if project_lead_party_guid and project.project_lead_party_guid is None:
if project_lead_party_guid and project.project_lead_party_guid is None and self.status_code == status_code:
self.status_code = "ASG"
elif project_lead_party_guid is None and project.project_lead_party_guid:
elif project_lead_party_guid is None and project.project_lead_party_guid and self.status_code == status_code:
self.status_code = "SUB"
else:
self.status_code = status_code
Expand Down Expand Up @@ -1327,7 +1327,7 @@ def send_project_summary_email(self, mine, message) -> None:
'COM': [PERM_RECL_EMAIL, project_lead_email]
}

send_ms_email = self.status_code != "DFT"
send_ms_email = self.status_code != "DFT" and self.status_code != "ASG"

emli_recipients = emli_emails.get(self.status_code)
cc = [MDS_EMAIL]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
from app.api.activity.utils import trigger_notification
from app.api.projects.project.project_util import notify_file_updates
from decimal import Decimal
from app.api.activity.models.activity_notification import ActivityRecipients

PAGE_DEFAULT = 1
PER_PAGE_DEFAULT = 25
Expand Down Expand Up @@ -206,7 +207,7 @@ def put(self, project_guid, project_summary_guid):
project = Project.find_by_project_guid(project_guid)
data = self.parser.parse_args()
is_historic = data.get('is_historic')

activity_recipients = ActivityRecipients.all_users
project_summary_validation = project_summary.validate_project_summary(data, is_historic)
if any(project_summary_validation[i] != [] for i in project_summary_validation):
current_app.logger.error(f'Project Summary schema validation failed with errors: {project_summary_validation}')
Expand Down Expand Up @@ -279,6 +280,7 @@ def put(self, project_guid, project_summary_guid):

if project_summary.status_code == 'ASG':
message = f'{project.project_title} for {project.mine_name} has been assigned'
activity_recipients = ActivityRecipients.core_users

if project_summary.status_code == 'CHR':
message = f'Changes have been requested by the ministry for {project.project_title} at {project.mine_name}'
Expand All @@ -294,12 +296,12 @@ def put(self, project_guid, project_summary_guid):

if project_summary.status_code == 'COM':
message = f'The status of the project description {project.project_title} for {project.mine_name} has been completed'

project_summary.send_project_summary_email(mine, message)
trigger_notification(message, ActivityType.major_mine_desc_submitted, project.mine, 'ProjectSummary',
project_summary.project_summary_guid, extra_data)


if message != '':
project_summary.send_project_summary_email(mine, message)
trigger_notification(message, ActivityType.major_mine_desc_submitted, project.mine, 'ProjectSummary',
project_summary.project_summary_guid, extra_data, None, activity_recipients)

# notify on document updates
if has_new_documents:
notify_file_updates(project, mine, project_summary.status_code)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,21 +40,10 @@ def test_sub_to_asg(mock_send_template_email, test_client, db_session, auth_head
"message": f'{updated_project_summary_title} for {project_summary.project.mine_name} has been assigned',
"core_project_summary_link": f'{Config.CORE_WEB_URL}/pre-applications/{project_summary.project.project_guid}/overview'
}

minespace_context = {
"mine": {
"mine_name": project_summary.mine_name,
"mine_no": project_summary.project.mine_no,
},
"message": f'{updated_project_summary_title} for {project_summary.project.mine_name} has been assigned',
"minespace_project_summary_link": f'{Config.MINESPACE_PROD_URL}/projects/{project_summary.project.project_guid}/overview',
"ema_auth_link": f'{Config.EMA_AUTH_LINK}',
}

# ARGS: subject, recipients, body, context, cc (ignore comparison with ANY)
emli_call = call(f'Project Description Notification for {project_summary.mine_name}', ANY, ANY, emli_context, cc=[MDS_EMAIL])
ms_call = call(f'Project Description Notification for {project_summary.mine_name}', ANY, ANY, minespace_context, cc=[MDS_EMAIL])

assert put_resp.status_code == 200
calls = [emli_call, ms_call]
calls = [emli_call]
mock_send_template_email.assert_has_calls(calls, True)
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ def test_delete_project_summary_bad_status_code(test_client, db_session, auth_he
def test_update_project_summary_assign_project_lead(test_client, db_session, auth_headers):
'''Assigning a project lead will change status code to ASG'''
project = ProjectFactory(project_summary=0)
project_summary = ProjectSummaryFactory(project=project)
project_summary = ProjectSummaryFactory(project=project, set_status_code='DFT')
party = PartyFactory(person=True)

data = {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -215,19 +215,15 @@ export const ProjectSummary: FC = () => {

if (!status_code || isNewProject) {
status_code = "DFT";
} else if (!newActiveTab) {
if (isCore) {
status_code = formValues.status_code;
} else {
status_code = "SUB";
}
} else if (!newActiveTab && status_code === "DFT") {
status_code = "SUB";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Noticed a sonarcloud issue- just some unused stuff that wants to be removed (isCore wants to be removed, and then probably related imports like the SystemFlagEnum and the selector)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This should be removed now.

is_historic = false;
if (amsFeatureEnabled) {
message = null;
}
}

if (isCore && !isNewProject) {
if (!isNewProject && newActiveTab) {
status_code = formValues.status_code;
}

Expand Down
Loading