Skip to content

[MDS-6526] AMS Application Files#3608

Merged
taraepp merged 4 commits intodevelopfrom
mds-6526-ams-application-files
Jul 31, 2025
Merged

[MDS-6526] AMS Application Files#3608
taraepp merged 4 commits intodevelopfrom
mds-6526-ams-application-files

Conversation

@taraepp
Copy link
Contributor

@taraepp taraepp commented Jul 22, 2025

Objective

  • Add AMS files to all documents page
  • file management

MDS-6526
Hierarchy was expanded to 3 levels, this is what "Major Mine Application" looks like now
image
And the ENV application files specifically
image

const [isEditMode] = useState(true);
const canEditMajorMineApplications = useAppSelector(userHasRole(USER_ROLES.role_edit_major_mine_applications));
const isUserProponent = useAppSelector(isProponent);
const [isEditMode] = useState(canEditMajorMineApplications || isUserProponent);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

turns out there wasn't really any role restrictions on the FE...


@api.doc(description='Request a document_manager_guid for uploading a document')
@requires_any_of([MINE_ADMIN, MINESPACE_PROPONENT, EDIT_PROJECT_SUMMARIES])
@requires_any_of([MINE_ADMIN, MINESPACE_PROPONENT, EDIT_MAJOR_MINE_APPLICATIONS])
Copy link
Contributor Author

Choose a reason for hiding this comment

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

had the wrong permission on the BE. Oops!

@@ -1,5 +1,6 @@
import React, { FC, useEffect, useState } from "react";
import { useDispatch, useSelector } from "react-redux";
import { useSelector } from "react-redux";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any reason we can't use the useAppSelector here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated 👍

const [isLoaded, setIsLoaded] = useState(true);
const canManageAmsFiles = canEditMajorMineApplications || isUserProponent;

const authsWithDocs =
Copy link
Collaborator

Choose a reason for hiding this comment

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

Might be a good idea to wrap these in useMemo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I checked how often they recalculate and it's only when data loads. Not worth it IMO.

Copy link
Collaborator

@matbusby-fw matbusby-fw left a comment

Choose a reason for hiding this comment

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

Left a couple thoughts on small changes that might be good.

@sonarqubecloud
Copy link

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

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud

@sonarqubecloud
Copy link

@sonarqubecloud
Copy link

@sonarqubecloud
Copy link

@taraepp taraepp added the 👍 Ready for review Pull request has been double checked by the author and is ready for comments and feedback. label Jul 30, 2025
@taraepp taraepp dismissed matbusby-fw’s stale review July 31, 2025 17:25

not adding memoization

@taraepp taraepp merged commit 589aea5 into develop Jul 31, 2025
20 checks passed
@taraepp taraepp deleted the mds-6526-ams-application-files branch July 31, 2025 17:25
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.

4 participants