Skip to content

[MDS-6460] Minespace NoW application stage status info#3527

Merged
asinn134 merged 8 commits intodevelopfrom
mds-6460-ms-now-application-status-page
May 9, 2025
Merged

[MDS-6460] Minespace NoW application stage status info#3527
asinn134 merged 8 commits intodevelopfrom
mds-6460-ms-now-application-status-page

Conversation

@asinn134
Copy link
Collaborator

@asinn134 asinn134 commented May 8, 2025

Objective

MDS-6460

  • In Minespace added page for user to view status of NoW application stages
  • Put status page behind new feature flag
  • Removed the extra text being shown in the CRR and PRR Core page
  • Moved the noticeOfWorkMocks file from Core directory to the Common directory
  • Also moved the noticeOfWorkActionCreator.spec file to the Common directory because the noticeOfWorkActionCreator file only exists there and not in Core. Also did so to fix test coverage issue.

image

@asinn134 asinn134 added the 👍 Ready for review Pull request has been double checked by the author and is ready for comments and feedback. label May 8, 2025
@sggerard sggerard requested a review from Copilot May 8, 2025 16:48
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements the Minespace NoW application stage status information, adding support for tracking and returning application progress details via new endpoints and updated UI state management.

  • Added a new field (application_progress) in the API response model for NoW applications.
  • Introduced new resources and updated action types/reducers for proponent-specific application details.
  • Updated mocks, interfaces, constants, and API endpoints to reflect the new application progress functionality.

Reviewed Changes

Copilot reviewed 38 out of 38 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
services/core-api/app/api/now_applications/response_models.py Adds the application_progress field to the response model.
services/core-api/app/api/now_applications/resources/now_application_proponent_resource.py Introduces a new resource for proponent application details including progress data.
services/core-api/app/api/now_applications/resources/now_application_list_proponent_resource.py Updates list resource to include application progress for each application.
services/core-api/app/api/now_applications/namespace.py Registers the new proponent resource under the proper route.
services/core-api/app/api/now_applications/models/now_application_progress.py Adds a helper method to retrieve progress data by application id.
services/common/src/utils/featureFlag.ts Introduces a new feature flag for application details view.
services/common/src/tests/mocks/dataMocks.tsx Updates mocks to include application_progress data.
services/common/src/redux/* Updates selectors, reducers, actions, and action creators to align with the new progress functionality.
services/common/src/interfaces/* Adds interfaces for application stages and progress.
services/common/src/constants/* Updates string constants, badge types, action types, and API endpoints to support the new feature.


@classmethod
def find_by_id(cls, now_application_id):
return cls.query.filter_by(now_application_id=now_application_id).all()
Copy link

Copilot AI May 8, 2025

Choose a reason for hiding this comment

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

The find_by_id method returns a list even when no records are found, so the 'if now_application_progress is None' check in the resource code will never be true. Consider returning a single record or updating the check to verify an empty list instead.

Suggested change
return cls.query.filter_by(now_application_id=now_application_id).all()
return cls.query.filter_by(now_application_id=now_application_id).first()

Copilot uses AI. Check for mistakes.
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 kept the .all() and removed the if now_application_progress is None

return response;
})
.catch(() => dispatch(error(NetworkReducerTypes.GET_PROPONENT_NOTICE_OF_WORK_APPLICATION)))
.finally(() => dispatch(hideLoading));
Copy link

Copilot AI May 8, 2025

Choose a reason for hiding this comment

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

The hideLoading action is dispatched without being invoked. Change it to dispatch(hideLoading()) to correctly invoke the action creator.

Suggested change
.finally(() => dispatch(hideLoading));
.finally(() => dispatch(hideLoading()));

Copilot uses AI. Check for mistakes.
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 is fixed now

noticeOfWorkStage: {
"In Progress": "warning",
Complete: "success",
"Not started": "default",
Copy link

Copilot AI May 8, 2025

Choose a reason for hiding this comment

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

There is a case inconsistency with the status string; the constant is defined as 'Not Started' elsewhere. Use consistent casing to avoid mismatches in status mapping.

Suggested change
"Not started": "default",
"Not Started": "default",

Copilot uses AI. Check for mistakes.
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 is fixed too now

initialValues={stateParams}
mineReportType={mine_reports_type}
/>
{mine_reports_type}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Extra text that Sam saw in CRR and PRR page

import * as API from "@/constants/API";
import * as MOCK from "@mds/common/tests/mocks/dataMocks";
import * as NOW_MOCK from "@/tests/mocks/noticeOfWorkMocks";
import * as NOW_MOCK from "@mds/common/tests/mocks/noticeOfWorkMock";
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There are around 50ish files in this PR that just changes the import path of the noticeOfWorkMock

@@ -0,0 +1,733 @@
import MockAdapter from "axios-mock-adapter";
Copy link
Contributor

Choose a reason for hiding this comment

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

is this a brand new file? How many TS errors come up if it's renamed to .ts? If it's manageable I'd be in favour of making it TS.

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 is now changed to .ts, it ended up being just a few small changes.

class TestApplicationProponentListResource:
"""GET /mines/<string:mine_guid>/now-applications"""

def test_get_proponent_now_application_list_success(self, test_client, db_session, auth_headers):
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm thinking an error scenario would be good to add for both resources tests.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Check for non existent guids and error scenario test have been added now

hashRoute: (nowApplicationGuid, activeTab = "overview", link) =>
`/notice-of-work/${nowApplicationGuid}/${activeTab}/${link}`,
component: NoticeOfWorkPage,
helpKey: "View-Proponent-Notice-Of-Work",
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd take out "Proponent" from here, just because it shows up in the help title and is kind of redundant on MS.

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 has been removed from the help key now.

}));
const columns: ColumnsType<any> = [
{
title: <b>Stage</b>,
Copy link
Contributor

Choose a reason for hiding this comment

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

I was wondering why these are styled individually but then I looked at the mockups!
Hmm, just looking at some of Denise's other mockups, they also have bold table headers, and I think it's actually a nice change.
In the goal of keeping styling somewhat consistent, I'd recommend taking the <b>s out and adding font-weight to the .ant-table-thead>tr>th selector. I'm seeing that the line-height, colour, and border also changed, and so did the little divider between each column... Might even be out of scope.

Copy link
Collaborator Author

@asinn134 asinn134 May 9, 2025

Choose a reason for hiding this comment

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

That makes a lot of sense, thanks! I removed the <b> and added the font-weight style to that particular class. I left the other styling alone for now.

@sonarqubecloud
Copy link

sonarqubecloud bot commented May 9, 2025

Quality Gate Passed Quality Gate passed for 'bcgov-sonarcloud_mds_minespace-web'

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
86.9% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@sonarqubecloud
Copy link

sonarqubecloud bot commented May 9, 2025

Quality Gate Failed Quality Gate failed for 'bcgov-sonarcloud_mds_common'

Failed conditions
51.3% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

]).then(() => setIsLoaded(true));
};

const handleTabChange = (newActiveTab) => {
Copy link
Collaborator Author

@asinn134 asinn134 May 9, 2025

Choose a reason for hiding this comment

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

Removed the switch in the handleTabChange function here

@sonarqubecloud
Copy link

sonarqubecloud bot commented May 9, 2025

@sonarqubecloud
Copy link

sonarqubecloud bot commented May 9, 2025

@asinn134 asinn134 requested a review from taraepp May 9, 2025 19:12
Copy link
Contributor

@taraepp taraepp left a comment

Choose a reason for hiding this comment

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

🎨

dataIndex: "project",
render: (text, record) => {
return (
<div title="">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a reason for the empty title here?

@asinn134 asinn134 merged commit 80586b8 into develop May 9, 2025
19 of 20 checks passed
@asinn134 asinn134 deleted the mds-6460-ms-now-application-status-page branch May 9, 2025 23:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

👍 Ready for review Pull request has been double checked by the author and is ready for comments and feedback.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants