TTAHUB-5344 and TTAHUB-5348 - Compliant Follow-up Reviews with TTA Support widget Table View#3681
TTAHUB-5344 and TTAHUB-5348 - Compliant Follow-up Reviews with TTA Support widget Table View#3681Andrew565 wants to merge 4 commits into
Conversation
|
|
|
|
There was a problem hiding this comment.
Pull request overview
This PR adds a new “Compliant Follow-up Reviews with TTA Support” widget to the Regional Dashboard Monitoring page, including a frontend table view and a new backend widget endpoint to supply the data.
Changes:
- Added a new backend monitoring widget (
compliantFollowUpReviewsWithTtaSupport) that computes monthly counts for follow-up reviews with/without TTA support. - Added a new frontend widget component intended to render the widget in a horizontal table view (with a menu toggle for tabular vs. non-tabular display).
- Wired the widget into the Regional Dashboard Monitoring page and registered it in the widget index.
Impact assessment:
- Benefits: Medium — introduces the requested widget/table view on the monitoring dashboard.
- Risks: High — current backend SQL appears to ignore filters and contains counting/aliasing issues; frontend has a missing component reference that can break rendering/build depending on configuration.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| src/widgets/monitoring/compliantFollowUpReviewsWithTtaSupport.ts | Adds backend widget logic + SQL query to compute monthly with/without-TTA review counts. |
| src/widgets/index.js | Registers the new backend widget for routing/lookup. |
| frontend/src/widgets/CompliantFollowUpReviewsWithTtaSupport.js | Adds the frontend widget container and horizontal table rendering. |
| frontend/src/widgets/CompliantFollowUpReviewsWithTtaSupport.css | Adds widget stylesheet (currently empty). |
| frontend/src/pages/RegionalDashboard/components/MonitoringReportDashboard.js | Adds the widget to the Monitoring dashboard layout. |
| SELECT | ||
| dr.review_name, | ||
| month_start, | ||
| bool_or(ar.id IS NOT NULL) has_tta | ||
| FROM "GrantDeliveredReviews" gdr |
| SELECT | ||
| month_start, | ||
| COUNT(review_name) total_reviews, | ||
| COUNT(review_name) FILTER (WHERE has_tta) with_tta, | ||
| COUNT(review_name) FILTER (WHERE NOT has_tta) without_tta | ||
| FROM review_set |
| LEFT JOIN "ActivityReports" ar | ||
| ON aro."activityReportId" = ar.id | ||
| AND ar."calculatedStatus" = 'approved' | ||
| AND ar."startDate" BETWEEN report_delivery_date AND complete_date | ||
| WHERE dr."deletedAt" IS NULL |
| FROM "GrantDeliveredReviews" gdr | ||
| JOIN "DeliveredReviews" dr | ||
| ON gdr."deliveredReviewId" = dr.id | ||
| JOIN months | ||
| ON dr.complete_date BETWEEN month_start AND month_start + INTERVAL '1 month' - INTERVAL '1 day' | ||
| LEFT JOIN "DeliveredReviewCitations" drc | ||
| ON dr.id = drc."deliveredReviewId" | ||
| LEFT JOIN "Citations" c |
| SELECT | ||
| month_start, | ||
| COUNT(review_name) total_reviews, |
| {showTabularData ? ( | ||
| <HorizontalTableWidget | ||
| headers={months} | ||
| data={tableData} | ||
| caption="Compliant Follow-up Reviews" | ||
| firstHeading="Follow-up reviews" | ||
| lastHeading="Totals" | ||
| showTotalColumn | ||
| stickyFirstColumn | ||
| stickyLastColumn | ||
| enableCheckboxes={false} | ||
| selectAllIdPrefix="compliant-follow-up-reviews" | ||
| hideFirstColumnBorder | ||
| footerData={false} | ||
| /> | ||
| ) : ( | ||
| <CompliantReviewsGrid data={data} /> | ||
| )} |
| const months = useMemo(() => { | ||
| if (!data || data.length === 0) return []; | ||
| return data.months; | ||
| }, [data]); |
| const showEmptyState = loading || !data || data.length === 0; | ||
|
|
| CompliantFollowUpReviewsWithTtaSupport.propTypes = { | ||
| filters: PropTypes.arrayOf(PropTypes.shape({})).isRequired, | ||
| }; | ||
|
|
||
| CompliantFollowUpReviewsWithTtaSupport.defaultProps = { | ||
| data: [], | ||
| }; |
| export function CompliantFollowUpReviewsWithTtaSupport({ loading, data }) { | ||
| const { setIsAppLoading } = useContext(AppLoadingContext); | ||
| const drawerTriggerRef = useRef(null); | ||
| const capture = useMediaCapture(EXPORT_NAME); | ||
| const [showTabularData, setShowTabularData] = useState(true); | ||
|
|
| // EXPORT_NAME | ||
| // ); | ||
|
|
||
| const menuItems = useWidgetMenuItems( |
There was a problem hiding this comment.
Codex said: This still exposes an Export table action even though exportRows is never passed. useWidgetMenuItems calls exportRows() whenever the widget is in table mode, so the export action will fail the first time a user clicks it. Either wire useWidgetExport here now, or remove the export menu path until the table export is implemented.
| <DrawerTriggerButton drawerTriggerRef={drawerTriggerRef}> | ||
| About this data | ||
| </DrawerTriggerButton> | ||
| </div> |
There was a problem hiding this comment.
Codex said: This empty-state logic is checking data.length, but this widget does not consume an array response. The backend returns an object with months and reviews, so data.length is undefined for the real success shape. That means an empty response can fall through and render a blank widget instead of NoResultsFound, and loading currently also routes through the empty-state branch. This should be based on the actual shape, for example !loading && data?.months?.length === 0 or an equivalent check on reviews.
| `WITH months AS ( | ||
| SELECT unnest(ARRAY[:monthStarts]::date[]) AS month_start | ||
| ), | ||
| review_set AS ( |
There was a problem hiding this comment.
Codex said: Grouping by review_name and then counting COUNT(review_name) is a data-integrity bug. If two different reviews share the same name in the same month, they collapse into one row here, and if review_name is NULL the row is excluded from the counts entirely because COUNT(column) ignores nulls. This should group on a stable review identifier such as dr.id or gdr.id and count COUNT(*) or COUNT(DISTINCT dr.id) instead.
| footerData={false} | ||
| /> | ||
| ) : ( | ||
| <CompliantReviewsGrid data={data} /> |
There was a problem hiding this comment.
Codex said: Toggling away from the default table view will throw here because CompliantReviewsGrid is neither imported nor defined in this file. Since the menu already exposes Display graph, this is a user-triggered runtime failure. If this PR is intentionally table-only, the graph toggle should be removed until the alternate view exists.
| }; | ||
| } | ||
|
|
||
| // Main query to get the count of follow-up reviews with and without TTA support for each month in the range |
There was a problem hiding this comment.
Codex said: This aggregation is not scoped to the same data set the widget title describes. The month range above is derived from filtered grant citations and approved reports, but this raw SQL never constrains gdr to the filtered grants, never applies the delivered-review filters, and never limits the TTA join to the scoped citations or approved reports. With dashboard filters active, this can over-count out-of-scope reviews and mark a review as with TTA because of unrelated data. A safer fix is to mirror the monitoringOverview scoping here: derive the scoped grant/review/report IDs first, then apply them explicitly inside this query.
| export function CompliantFollowUpReviewsWithTtaSupport({ loading, data }) { | ||
| const { setIsAppLoading } = useContext(AppLoadingContext); | ||
| const drawerTriggerRef = useRef(null); | ||
| const capture = useMediaCapture(EXPORT_NAME); |
There was a problem hiding this comment.
Codex said: The menu actions are wired as if screenshot/export support exists, but this hook call does not match the useMediaCapture contract. The hook expects a ref plus a title; here it only receives the title string, so the screenshot path will fail when clicked. If this widget is not ready to support screenshots and exports yet, the safer option is to suppress those menu items until the ref and export wiring are implemented.
Description of change
Table view of the Compliant Follow-up Reviews with TTA Support widget for the Regional Dashboard Monitoring page.
How to test
Jira Issue(s)
Checklists
Every PR
Before merge to main
Production Deploy
ready_for_reviewtransition triggers the Slack/Jira automation)elainaparrishis the authorized approver under normal circumstances)After merge/deploy