Amaan taking over for Juhitha-Reddy Add PM dashboard for resource and certification management - #4517
Amaan taking over for Juhitha-Reddy Add PM dashboard for resource and certification management#4517Juhitha-Reddy wants to merge 22 commits into
Conversation
✅ Deploy Preview for highestgoodnetwork-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
4ca5841
rajanidi1999
left a comment
There was a problem hiding this comment.
Hi,
I have reviewed the PR locally and verified that it is working as expected. The Resource Management Dashboard loads correctly, and the filtering, search functionality, and action buttons (Approve/Deny) are functioning properly. The UI looks consistent and user-friendly, and overall the implementation is solid. Good job!
rithika-paii
left a comment
There was a problem hiding this comment.
Hi Juhitha,
I reviewed PR #4517 and tested the Resource Management Dashboard locally. The overall functionality works as expected, including tab navigation and rendering of the dashboard content. However, I found one UI issue in dark mode:
In dark mode, the text/icon for the active tab becomes difficult to see. For example, the active tab label is not clearly visible, whereas in light mode, both Certifications and Resource Requests are clearly readable. (See attached video)
PR.4517-.Changes.mov
|
|
Hi @iAbhi001 @akv-iu PR.4517.demo.mov |
|
Addressed the remaining issues. Resolved the remaining SonarCloud warnings by replacing the deprecated DOM removal method with Tested the Resource Management Dashboard and Reports page to verify the CSV download still works, the tab labels and styling display correctly in both light and dark modes, and the Reports page continues to render as expected after removing the duplicate import. Ready for re-review. |
iAbhi001
left a comment
There was a problem hiding this comment.
Hi @AmaanSyed09,
I re-tested PR #4517 locally on the feature/pm-resource-dashboard branch across Administrator, Manager, and Volunteer roles.
Testing & Verification Highlights:
- Search Functionality: Tested searching by Request ID (
REQ001,REQ) and Certification ID (CERT001,CERT). Exact matching, partial matching, and whitespace normalization (e.g.REQ001) all return expected records. Standard text searches also function correctly. - CSV Export: Clicked "Export Summary" on the Certifications tab and confirmed the downloaded CSV contains complete data rows along with the header columns.
- Dark Mode UI: Verified that active/inactive tab titles, table summary text ("Showing X of N..."), and hover states render clearly with proper contrast. Table headers no longer highlight on hover.
- Role-Based Access Control: Verified that privileged roles can access
/pm/dashboard/resourceswhile non-privileged accounts (Volunteer) are properly blocked and redirected.
Everything is working smoothly and all previous review feedback has been addressed. Approved!
DeepighaJ
left a comment
There was a problem hiding this comment.
- Tested the PR locally. The search issues were resolved now.
- The Resource Requests tab loads the mock data successfully, and the Status, Teacher ID, and Search filters work as expected.
- Approve/Deny actions correctly update the corresponding status badges, and the priority/status badges display the appropriate colors.
- The Certifications tab loads the mock data correctly, the Active, Expiring Soon, and Expired filters function as expected, and the CSV export generates a file containing the certification data.
- Dark mode styling is maintained across tables, text, and badges, and the layout is responsive on desktop, tablet, and mobile viewports.
- Verified that logging in as a non-privileged (Volunteer) user correctly blocks access. All tested functionality works as expected and previous behavior is preserved.
There was a problem hiding this comment.
The dashboard itself looks good, and I can see from the thread it's been tested thoroughly.
I am requesting changes for something outside it though: the :global change to reportsPage.module.css breaks the existing Reports page, since those class names are no longer exported and the styles[...] lookups return undefined. I have attached screenshots in a comment.
I have also flagged the Skills permission reuse and an unrelated mock data file.
| @media screen and (width <= 1200px) { | ||
| .category-data-container { | ||
| grid-template-columns: 1fr; | ||
| :global { |
There was a problem hiding this comment.
Wrapping this file in :global stops the class names being exported by the module, but Reports.jsx and ReportTableSearchPanel.jsx read them as styles['category-container'], styles['card-category-item'] etc - so those come back undefined and the Reports page loses its styling. I have attached comparison screenshots of /reports on development vs this branch.
This file also seems to be unrelated to the PM dashboard - maybe it should be in a separate PR.
Development Branch
PR Specific Branch
| component={PMResourceDashboard} | ||
| fallback | ||
| allowedRoles={[UserRole.Administrator, UserRole.Owner, UserRole.Manager]} | ||
| routePermissions={RoutePermissions.accessHgnSkillsDashboard} |
There was a problem hiding this comment.
accessHgnSkillsDashboard is also used by the HGN Skills routes. Since
ProtectedRoute treats allowedRoles and routePermissions as alternative ways to grant access, a non-PM user holding that permission can reach this dashboard too.
Is that intended? If this should be limited to the stated PM roles, it needs its own permission key and a test for that case.
| @@ -0,0 +1,53 @@ | |||
| // Helper function to generate dates relative to now | |||
There was a problem hiding this comment.
This is new mock data for ApplicationTimeChart, but that chart doesn't import it and it's unrelated to the resource/certification dashboard. Is it meant to be in this PR?
|
Hi @AdiDubbs Investigated the Reports page styling issue and confirmed it originated from changes introduced by the previous PR owner. The Reports stylesheet had been wrapped in a broad Verified the Reports page locally in both Light Mode and Dark Mode and confirmed the layout now matches the current I also investigated the remaining route permission comment and confirmed the reported behavior is part of the previous implementation and predates my takeover. I verified that there is currently no dedicated permission available for the PM Resource Dashboard. I'm not sure whether this should be addressed as part of this PR since it appears to require a separate permission-model decision, so no changes were made for this. Ready for re-review. |
There was a problem hiding this comment.
Thanks for the turnaround. I verified the Reports fixes: the broad :global wrapper is gone, the module class names export again, and the unused data.js file was removed.
I am still requesting changes for three dashboard issues: certification status can contradict the expiry text and make the Expired filter miss expired records; date-only values render a day early in timezones west of UTC; and the tab controls are mouse-only because they render as anchors without href.
On the route-permission point, I understand a dedicated permission needs a broader decision. Still, removing routePermissions is not a safe workaround: ProtectedRoute redirects only when that prop is present, while allowedRoles can grant access but cannot deny it. Without routePermissions, every authenticated user can reach the route. Please establish the intended permission model before merging, or update the PR description and test plan to state the current access behavior.
| certificationType: 'First Aid Training', | ||
| issueDate: '2024-06-15', | ||
| expiryDate: '2025-06-15', | ||
| status: 'active', |
There was a problem hiding this comment.
status is stored separately from expiryDate, while the expiry text is calculated from the date, so the two contradict each other. This record shows an ACTIVE badge while the same row says it expired, and the one expiring 2026-08-15 is also still marked active rather than expiring soon. Since the filter reads status, expired records get missed by the Expired filter. Please derive the status from the expiry date, or make the fixture data consistent.
| <td>{cert.certificationType}</td> | ||
| <td>{cert.certifyingBody}</td> | ||
| <td>{cert.trainingHours}h</td> | ||
| <td>{new Date(cert.issueDate).toLocaleDateString()}</td> |
There was a problem hiding this comment.
Date-only strings are parsed as UTC by new Date() and then formatted in local time, so in timezones west of UTC 2024-06-15 renders as 6/14/2024. The same applies to the expiry date below and the request date in ResourceRequestsTab.jsx. Please parse these as local calendar dates, or format the original string without a UTC conversion.
| <Col lg={12}> | ||
| <Nav tabs className={styles.tabNav}> | ||
| <NavItem> | ||
| <NavLink |
There was a problem hiding this comment.
These NavLinks have onClick but no href, and Reactstrap renders them as anchors. Anchors without an href are not keyboard-focusable, so the tabs cannot be reached or switched with the keyboard. Using tag="button" plus tab semantics would address this.
|
Addressed the remaining issues. Updated the Certifications tab to derive certification status from the expiry date so the status badge, filters, and expiry warning stay in sync. Fixed the date handling in the Certifications and Resource Requests tabs so date-only values display the correct calendar day across time zones. Also updated the Resource Requests and Certifications tabs to support keyboard navigation by rendering them as accessible button-based tab controls with proper tab semantics while preserving the existing behavior. Verified the certification status logic, date rendering, and keyboard accessibility in both Light Mode and Dark Mode. The remaining route permission concern has been raised with the team, and I am currently waiting for clarification on the intended permission model before making any related changes. Ready for re-review. |
|
Addressed the remaining route permission issue. Resolved the latest merge conflict first and verified the existing PM Resource Dashboard changes remained intact. Added a dedicated Verified the dashboard continues to load correctly with Owner access after the merge and permission update. Administrator, Owner, and Manager roles retain access, while other authenticated users now require the dedicated PM Resource Dashboard permission, and users without either are denied access. Ready for re-review. |
ShreyasB02
left a comment
There was a problem hiding this comment.
Testing Summary
I've thoroughly tested PR #4517 locally on the feature/pm-resource-dashboard branch across multiple devices and scenarios. Here's what has been verified and working:
Core Dashboard Functionality
- Dashboard loads correctly at
/pm/dashboard/resourcesfor Administrator, Owner, and Manager roles - Access control properly blocks Volunteer users from accessing the route
Resource Requests Tab
- Mock data loads and displays correctly
- Status filter works as expected across all filter values
- Teacher ID filter functions properly
- Search functionality works across all fields:
- Request ID searches (exact and partial):
REQ001,REQ002,REQ - Whitespace-normalized searches:
REQ001,REQ 001return expected results - Teacher name, resource type, and description searches all function correctly
- Request ID searches (exact and partial):
- Approve/Deny buttons update status badges with correct priority and status colors
Certifications Tab
- Mock data loads correctly
- All filters (Active, Expiring Soon, Expired) work as expected
- CSV export generates a file with complete certification data
- Certification status correctly derived from expiry date (no contradictions)
Date & Time Handling
- Issue dates and expiry dates display the correct calendar day in local timezone
- Date-only values render correctly (verified
2024-06-15displays as 6/15/2024, not 6/14)
Dark Mode Styling
- Tab titles are clearly visible in both active and inactive states
- "Showing X of N requests/certifications" summary text is readable
- All table text and badges render with proper contrast
- Table headers do not highlight on hover (only data rows highlight)
Keyboard Accessibility
- Tab navigation works fully with keyboard
- Resource Requests and Certifications tabs are accessible via Tab, Enter, and Space keys
Permissions
- Dashboard uses the dedicated
accessPMResourceDashboardpermission correctly - Administrator, Owner, and Manager roles retain access
- Users with only the
accessPMResourceDashboardpermission can access the dashboard - Users without the permission are denied access
Responsive Design
- Tested on iPad Pro, iPhone 14 Pro Max, and Samsung Galaxy S20 Ultra
- Layout is fully responsive and usable across all tested devices
Requested Changes
1. Improve Permission Denied UX
Currently, when a Volunteer-role-based user attempts to access /pm/dashboard/resources, they are silently redirected to the home page. This lacks clear user feedback.
Recommendation:
- Option A: Display a toast notification that clearly states "Permission Denied: You do not have access to the Resource Dashboard"
- Option B: Render a dedicated permission denied page with an explanation and a link to navigate back
The current silent redirect is confusing and doesn't communicate why the user cannot access the resource.
linlin-husky
left a comment
There was a problem hiding this comment.
Hi, I have reviewed the pr and have found the below area for improvement.
- Issue: As shown in the screenshot, the right edge of the table is cut off cleanly at the border, making it look like a static, truncated table rather than a scrollable container. Users might miss critical columns without an explicit hint.
- Suggestions:
- Visible Scrollbar / Scroll Shadow: Add a persistent scrollbar styling or an edge shadow/gradient indicator to signal horizontal overflow.
- Scroll Hint: Display a subtle swipe/scroll hint text or icon (e.g., "⇄ Scroll table horizontally to view more") on mobile screens so users know they can drag to access the action buttons.
|
Addressed the remaining issues. Improved the Resource Requests and Certifications table overflow experience with a synchronized top horizontal scrollbar, a right-edge fade indicator to make additional off-screen content clear, improved Dark Mode scrollbar visibility, and responsive right-edge spacing while preserving existing table functionality across screen sizes. Added clear permission-denied feedback for the PM Resource Dashboard. Unauthorized users are still redirected to the dashboard using the existing access model, but now receive a "Permission Denied: You do not have access to the Resource Dashboard." error notification. Added targeted tests covering allowed roles, dedicated permission access, denied access, route-specific redirect state, and toast behavior. Verified the responsive table updates in Light and Dark Mode. ProtectedRoute tests passed with 12 tests, Dashboard tests passed with 3 tests, ESLint passed, and git diff --check passed. Ready for re-review. |
|
Addressed the SonarCloud issues. Refactored the duplicated table overflow and scroll synchronization logic into a shared PM Dashboard hook to reduce new-code duplication. Updated the Resource Requests and Certifications tables with a persistent top horizontal scrollbar that stays synchronized with direct touchpad/swipe scrolling, supports drag and keyboard interaction, and preserves the existing right-edge fade and responsive behavior. Updated the Dark Mode tab hover background to meet the required contrast level. Verified both tables in Light and Dark Mode, including persistent scrollbar visibility, two-way scroll synchronization after page refresh, responsive behavior, and existing table functionality. ESLint, Stylelint, and git diff --check passed. |
|
























Description
This PR implements the Phase 4 – Resource Management Dashboard (Project Manager View), providing a unified interface for project managers to review, filter, and manage teacher resource requests and educator certifications. This includes new UI components, improved routing, filters, data export functionality, and accessibility updates.
Implements: Phase 4 – Resource Management Dashboard (Project Manager View)
Related PRs (if any):
This is a frontend-only PR.
Backend integration will follow in a separate PR once the API endpoints for resource requests and certifications are implemented.
Main changes explained:
How to test:
Screenshots or videos of changes:
Uploading Screen Recording 2025-11-29 at 1.07.43 PM.mov…
Note:
• Current implementation uses mock data. Backend API endpoints will be implemented in a future PR.
• The dashboard is keyboard-navigable and passes required accessibility checks.
• Compatible with Chrome, Firefox, and Safari.