FWF-4761 [poc] Replace Recharts with Chart.js and update GraphQL #2905

leodube-aot · 2025-08-05T12:32:50Z

User description

Issue Tracking

JIRA: https://aottech.atlassian.net/browse/FWF-4761
Issue Type: POC

Changes

Added chart.js and react-chartjs-2:
- Implemented several different charts.
- Can switch between charts using a dropdown.
Updated GraphQL API:
- Implemented get_form and get_forms queries, including tenant auth.
- Implemented metrics queries.

Screenshots (if applicable)

Notes

Checklist

Updated changelog
Added meaningful title for pull request

PR Type

Enhancement

Description

Switch metrics fetch to GraphQL POST requests
Replace Recharts with Chart.js charts and dropdown
Add get_forms and metrics GraphQL resolvers
Refactor services: merge WebAPI and Formio data

Diagram Walkthrough

flowchart LR
  A["Dashboard UI"] -- "POST GraphQL query" --> B["GraphQL Resolvers"]
  B -- "invoke FormService/MetricService" --> C["Service Layer"]
  C -- "Beanie & SQLAlchemy" --> D["Formio (MongoDB) & WebAPI (Postgres)"]

File Walkthrough

	Relevant files

github-actions · 2025-08-05T12:34:43Z

PR Reviewer Guide 🔍

(Review updated until commit `344636c`)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 No relevant tests
🔒 Security concerns GraphQL injection: Direct interpolation of variables into GraphQL query strings can lead to injection vulnerabilities. Switch to using GraphQL variables and parameterized queries to sanitize inputs.
⚡ Recommended focus areas for review GraphQL Injection GraphQL queries are constructed via string interpolation of user-provided values without using variables. This can lead to injection or malformed queries. Consider using GraphQL variables or proper escaping. const query = ` query Query { getForms( formName: "${formName}" fromDate: "${fromDate}" limit: ${limit} pageNo: ${pageNo} orderBy: "${searchBy}" toDate: "${toDate}" ) { items { id parentFormId totalSubmissions type version status title } totalCount } } `; RequestService.httpPOSTRequest(url, { query: query }, null, true, headers) Argument Misplacement In the `getMetricsSubmissionStatus` query, `orderBy` and `toDate` parameters reference incorrect variables (`toDate` uses `searchBy` and vice versa). Confirm the correct mapping of parameters to avoid logic errors. const query = ` query Query { getMetricsSubmissionStatus( formId: "${id}" fromDate: "${fromDate}" orderBy: "${toDate}" toDate: "${searchBy}" ) { Missing Default Case The `renderChart` switch statement lacks a default branch to handle unexpected `selectedChartValue` values, potentially causing `undefined` renderings. Add a default case or validate the selected value. const renderChart = () => { switch (selectedChartValue) { case 'pie': return ( <Pie data={chartData} options={chartOptions} /> ); case 'v-bar': return ( <Bar data={chartData} options={chartOptions} /> ); case 'doughnut': return ( <Doughnut data={chartData} options={chartOptions} /> ); case 'polar-area': return ( <PolarArea data={chartData} options={chartOptions} /> ); case 'radar': return ( <Radar data={chartData} options={chartOptions} /> ); } };

github-actions · 2025-08-05T12:35:56Z

PR Code Suggestions ✨

Latest suggestions up to 344636c

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Fix swapped GraphQL arguments The `orderBy` and `toDate` arguments have been swapped—`orderBy` should use the sort field and `toDate` the end date. Swap those two interpolations to match the intended parameters. forms-flow-web/src/apiManager/services/metricsServices.js [117-124] const query = ` query Query { getMetricsSubmissionStatus( formId: "${id}" fromDate: "${fromDate}" - orderBy: "${toDate}" - toDate: "${searchBy}" + orderBy: "${searchBy}" + toDate: "${toDate}" ) { count metric } } `; Suggestion importance[1-10]: 8 __ Why: The GraphQL parameters `orderBy` and `toDate` were swapped, causing incorrect query behavior; swapping them fixes this critical bug.	Medium
Security	Switch to GraphQL variables Interpolating user‐controlled values directly into the GraphQL query risks injection and encoding issues. Use GraphQL variables for all dynamic inputs instead of string interpolation. forms-flow-web/src/apiManager/services/metricsServices.js [37-45] const query = ` - query Query { + query Query( + $formName: String, $fromDate: String, $toDate: String, + $limit: Int!, $pageNo: Int!, $orderBy: String! + ) { getForms( - formName: "${formName}" - fromDate: "${fromDate}" - limit: ${limit} - pageNo: ${pageNo} - orderBy: "${searchBy}" - toDate: "${toDate}" + formName: $formName, + fromDate: $fromDate, + toDate: $toDate, + limit: $limit, + pageNo: $pageNo, + orderBy: $orderBy ) { items { /* ... */ } totalCount } } `; +const variables = { formName, fromDate, toDate, limit, pageNo, orderBy: searchBy }; +RequestService.httpPOSTRequest(url, { query, variables }, null, true, headers) Suggestion importance[1-10]: 7 __ Why: Interpolating dynamic values into the query risks injection and encoding issues; using GraphQL variables improves security and robustness.	Medium

Previous suggestions

Suggestions up to commit 344636c

Category	Suggestion	Impact
Possible issue	Correct swapped GraphQL parameters The arguments `orderBy` and `toDate` are swapped in the GraphQL query. Swap them so that `orderBy` uses `searchBy` and `toDate` uses `toDate` to send correct parameters. forms-flow-web/src/apiManager/services/metricsServices.js [117-124] const query = ` query Query { getMetricsSubmissionStatus( formId: "${id}" fromDate: "${fromDate}" - orderBy: "${toDate}" - toDate: "${searchBy}" + orderBy: "${searchBy}" + toDate: "${toDate}" ) { count metric } } `; Suggestion importance[1-10]: 9 __ Why: The query parameters `orderBy` and `toDate` are indeed swapped, causing incorrect filtering; fixing this corrects a critical bug in the GraphQL request.	High
General	Conditionally include formName filter Avoid interpolating an undefined `formName` into the query, which can lead to invalid filters. Build the argument string conditionally so you only include `formName` when it’s defined. forms-flow-web/src/apiManager/services/metricsServices.js [37-46] +const formNameArg = formName ? `formName: "${formName}"` : ""; const query = ` query Query { getForms( - formName: "${formName}" + ${formNameArg} fromDate: "${fromDate}" limit: ${limit} pageNo: ${pageNo} orderBy: "${searchBy}" toDate: "${toDate}" ) { - items { - id - parentFormId - totalSubmissions - type - version - status - title - } + items { /.../ } totalCount } } `; Suggestion importance[1-10]: 5 __ Why: Building the `formName` argument only when defined prevents sending `undefined`, but it’s a minor enhancement without functional impact.	Low
General	Add controlled value to select Make the select fully controlled by adding its `value` prop bound to state, so it correctly reflects the current `selectedChartValue`. forms-flow-web/src/components/Dashboard/StatusChart.js [155-160] <FormControl as="select" + value={selectedChartValue} onChange={(e) => setSelectedChartValue(e.target.value)} className="form-select p-1" title={t("Choose any")} aria-label="Select chart type" > Suggestion importance[1-10]: 5 __ Why: Adding the `value` prop makes the select a controlled component and improves UI consistency, though it’s a stylistic enhancement.	Low

Suggestions up to commit 7618299

Category	Suggestion	Impact
Possible issue	Fix mismatched GraphQL arguments The GraphQL arguments are mismatched: `orderBy` is using `toDate` and `toDate` is using `searchBy`. Swap them so that `orderBy` corresponds to the sort field and `toDate` to the end date. forms-flow-web/src/apiManager/services/metricsServices.js [117-129] const query = ` query Query { getMetricsSubmissionStatus( formId: "${id}" fromDate: "${fromDate}" - orderBy: "${toDate}" - toDate: "${searchBy}" + orderBy: "${searchBy}" + toDate: "${toDate}" ) { count metric } } `; Suggestion importance[1-10]: 8 __ Why: The GraphQL query arguments for `orderBy` and `toDate` are swapped in `getMetricsSubmissionStatus`, causing incorrect data fetching. Fixing them ensures correct metrics submission queries.	Medium
General	Control chart type selection Make the select a controlled component by binding its value to state. This ensures the dropdown always reflects the current `selectedChartValue`. forms-flow-web/src/components/Dashboard/StatusChart.js [155-161] <FormControl as="select" + value={selectedChartValue} onChange={(e) => setSelectedChartValue(e.target.value)} className="form-select p-1" title={t("Choose any")} aria-label="Select chart type" > Suggestion importance[1-10]: 6 __ Why: Adding `value={selectedChartValue}` ensures the `<FormControl>` reflects the current state, improving consistency in chart selection control.	Low
General	Use map for data arrays Use `map` to build `chartLabels` and `chartDataset` in a more concise, functional style. This avoids side-effecting in a `.map` call. forms-flow-web/src/components/Dashboard/StatusChart.js [73-78] -let chartLabels = []; -let chartDataset = []; -submissionsStatusList.map((metric) => { - chartLabels.push(metric.metric); - chartDataset.push(metric.count); -}); +const chartLabels = submissionsStatusList.map((m) => m.metric); +const chartDataset = submissionsStatusList.map((m) => m.count); Suggestion importance[1-10]: 4 __ Why: Replacing manual pushes in `.map` with functional `.map` calls eliminates side effects and results in cleaner, more declarative code.	Low

leodube-aot · 2025-08-05T13:33:23Z

forms-flow-data-layer/src/db/formio_db.py


 from src.config.envs import ENVS
-from src.models.formio import FormModel, SubmissionsModel  # Import your MongoDB models
+from src.models.formio import FormModel, SubmissionModel  # Import your MongoDB models


Updated model name to maintain a consistent naming convention.

sonarqubecloud · 2025-08-05T13:35:45Z

Quality Gate passed

Issues
12 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

leodube-aot · 2025-08-05T13:36:04Z

forms-flow-data-layer/src/models/webapi/__init__.py

 from src.models.webapi.base import BaseModel
 from src.models.webapi.constants import WebApiTables
-from src.models.webapi.formprocess_mapper import FormProcessMapper
+from src.models.webapi.form_process_mapper import FormProcessMapper


Renamed file to maintain a consistent naming convention.

github-actions · 2025-08-05T13:36:21Z

Persistent review updated to latest commit 344636c

github-actions · 2025-08-05T13:40:41Z

Persistent review updated to latest commit 344636c

arun-s-aot · 2025-08-05T16:30:08Z

COnverting to draft so that we look into this in detail after Current 7.2 release

cc @vinaayakh-aot @auslin-aot

leodube-aot added 15 commits August 4, 2025 21:41

Add Chart.js and some initial GraphQL configuration

9ddabbc

Add form name param to graphql submission query

476edc4

Fix typo

3ee5934

Update GraphQl forms queries to pull from multple db sources

48b5d78

Add comments to added form graphql query

9dfedc5

Rename SubmissionModel to Submission

ef1529f

Refactor and implement application class methods

2ee489a

Implement get_submission and remove base service

22792ec

Implement get_submission query

20ae99f

Fix typo

6189594

Add metric endpoint

354bfa0

Update metricsService

9c95701

Undo unneccessary changes to submission graphql queries

9882419

Fix rebase issues

6ef433b

Apply tenant checks on form graphql query

7618299

github-actions bot added the Review effort 4/5 label Aug 5, 2025

leodube-aot commented Aug 5, 2025

View reviewed changes

Final cleanup

344636c

leodube-aot commented Aug 5, 2025

View reviewed changes

leodube-aot marked this pull request as ready for review August 5, 2025 13:39

leodube-aot requested review from a team as code owners August 5, 2025 13:39

arun-s-aot marked this pull request as draft August 5, 2025 16:30

arun-s-aot added the DoNotMerge label Aug 6, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

FWF-4761 [poc] Replace Recharts with Chart.js and update GraphQL #2905

FWF-4761 [poc] Replace Recharts with Chart.js and update GraphQL #2905

Uh oh!

leodube-aot commented Aug 5, 2025 •

edited by github-actions bot

Loading

Uh oh!

github-actions bot commented Aug 5, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Aug 5, 2025 •

edited

Loading

Uh oh!

leodube-aot Aug 5, 2025

Uh oh!

sonarqubecloud bot commented Aug 5, 2025

Uh oh!

leodube-aot Aug 5, 2025

Uh oh!

github-actions bot commented Aug 5, 2025

Uh oh!

github-actions bot commented Aug 5, 2025

Uh oh!

arun-s-aot commented Aug 5, 2025

Uh oh!

Uh oh!

FWF-4761 [poc] Replace Recharts with Chart.js and update GraphQL #2905

Are you sure you want to change the base?

FWF-4761 [poc] Replace Recharts with Chart.js and update GraphQL #2905

Uh oh!

Conversation

leodube-aot commented Aug 5, 2025 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

User description

Issue Tracking

Changes

Screenshots (if applicable)

Notes

Checklist

PR Type

Description

Diagram Walkthrough

File Walkthrough

Uh oh!

github-actions bot commented Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Reviewer Guide 🔍

(Review updated until commit 344636c)

Uh oh!

github-actions bot commented Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Code Suggestions ✨

Previous suggestions

Uh oh!

leodube-aot Aug 5, 2025

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Aug 5, 2025

Quality Gate passed

Uh oh!

leodube-aot Aug 5, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Aug 5, 2025

Uh oh!

github-actions bot commented Aug 5, 2025

Uh oh!

arun-s-aot commented Aug 5, 2025

Uh oh!

Uh oh!

leodube-aot commented Aug 5, 2025 •

edited by github-actions bot

Loading

github-actions bot commented Aug 5, 2025 •

edited

Loading

(Review updated until commit `344636c`)

github-actions bot commented Aug 5, 2025 •

edited

Loading