feat: implement visual YoY yield history line chart in analytics dashboard#71
feat: implement visual YoY yield history line chart in analytics dashboard#71onkar0127 wants to merge 2 commits into
Conversation
|
@onkar0127 is attempting to deploy a commit to the karan3431's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
🎉 Thanks for your contribution, @onkar0127! Please make sure CI passes and the checklist in the PR template is complete. A maintainer will review this soon. — The AgroNavis team |
📝 WalkthroughWalkthroughAdds a year-over-year yield trend line chart to ChangesYoY Yield Trend Chart
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
frontend/src/components/__tests__/AnalyticsDashboard.test.tsx (1)
49-128: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAdd a regression test for mixed-unit same-year records.
Current tests validate casing and same-year summation, but not the mixed-unit case. Add one test asserting the intended behavior (normalize, split, or reject) so invalid cross-unit totals don’t regress.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/__tests__/AnalyticsDashboard.test.tsx` around lines 49 - 128, Add a new test case to the 'AnalyticsDashboard YoY Chart' describe block that validates the intended behavior for mixed-unit same-year records. This test should modify the mockYields to include records for the same crop type in the same year but with different units (for example, add a rice record for 2024 with a different unit than quintal), render the AnalyticsDashboard component, and then assert the correct behavior—whether mixed units are normalized together, split into separate entries, or rejected. This ensures that the application doesn't accidentally create invalid cross-unit totals when processing yield data with inconsistent units.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/components/AnalyticsDashboard.tsx`:
- Around line 275-285: The select element used for crop selection lacks an
accessible name for assistive technologies. Add either an associated label
element or an aria-label attribute to the select that renders the availableCrops
map to provide a clear, descriptive name that screen readers can announce to
users.
- Around line 120-127: The groupedByYear object structure does not account for
mixed units within the same year. Currently, line 124 stores only the first unit
encountered for each year while line 126 aggregates all quantities without unit
validation, causing invalid totals when a crop/year combination has mixed units.
Modify the grouping logic to either track quantities separately by both year and
unit (change the grouping key to include unit information), or implement unit
normalization to convert all quantities to a standard unit before aggregation.
Ensure that the unit stored in groupedByYear accurately represents the units of
all quantities being summed for that year.
In `@frontend/src/styles/AnalyticsDashboard.module.css`:
- Line 486: The box-shadow declarations in AnalyticsDashboard.module.css are
using the older rgba() color function syntax which triggers stylelint warnings.
Convert all rgba() functions to modern rgb() syntax with slash notation at lines
486, 519, and 533. Change the format from rgba(0, 0, 0, 0.06) to rgb(0 0 0 / 6%)
by replacing commas with spaces in the color values, and convert the decimal
alpha value (0.06) to a percentage (6%) after the forward slash.
---
Nitpick comments:
In `@frontend/src/components/__tests__/AnalyticsDashboard.test.tsx`:
- Around line 49-128: Add a new test case to the 'AnalyticsDashboard YoY Chart'
describe block that validates the intended behavior for mixed-unit same-year
records. This test should modify the mockYields to include records for the same
crop type in the same year but with different units (for example, add a rice
record for 2024 with a different unit than quintal), render the
AnalyticsDashboard component, and then assert the correct behavior—whether mixed
units are normalized together, split into separate entries, or rejected. This
ensures that the application doesn't accidentally create invalid cross-unit
totals when processing yield data with inconsistent units.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 75b3954d-0887-47ea-b13a-860f5dd46a88
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
frontend/package.jsonfrontend/src/components/AnalyticsDashboard.tsxfrontend/src/components/__tests__/AnalyticsDashboard.test.tsxfrontend/src/styles/AnalyticsDashboard.module.css
| const groupedByYear: Record<number, { quantity: number; unit: string }> = {}; | ||
| filtered.forEach(r => { | ||
| const yr = r.year; | ||
| if (!groupedByYear[yr]) { | ||
| groupedByYear[yr] = { quantity: 0, unit: r.unit || 'quintal' }; | ||
| } | ||
| groupedByYear[yr].quantity += r.quantity || 0; | ||
| }); |
There was a problem hiding this comment.
Prevent mixed-unit totals in yearly aggregation.
Line 124 stores a single unit per year, while Line 126 sums all quantity values regardless of each record’s unit. If a crop/year has mixed units (e.g., kg + quintal), the YoY total becomes invalid.
Suggested direction
-const groupedByYear: Record<number, { quantity: number; unit: string }> = {};
+const groupedByYear: Record<number, { quantity: number; unit: string }> = {};
filtered.forEach(r => {
const yr = r.year;
- if (!groupedByYear[yr]) {
- groupedByYear[yr] = { quantity: 0, unit: r.unit || 'quintal' };
- }
- groupedByYear[yr].quantity += r.quantity || 0;
+ const unit = (r.unit || 'quintal').trim().toLowerCase();
+ if (!groupedByYear[yr]) {
+ groupedByYear[yr] = { quantity: 0, unit };
+ }
+ if (groupedByYear[yr].unit !== unit) {
+ // Decide one policy: normalize units before sum OR split series OR reject mixed-unit buckets.
+ return;
+ }
+ groupedByYear[yr].quantity += r.quantity || 0;
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const groupedByYear: Record<number, { quantity: number; unit: string }> = {}; | |
| filtered.forEach(r => { | |
| const yr = r.year; | |
| if (!groupedByYear[yr]) { | |
| groupedByYear[yr] = { quantity: 0, unit: r.unit || 'quintal' }; | |
| } | |
| groupedByYear[yr].quantity += r.quantity || 0; | |
| }); | |
| const groupedByYear: Record<number, { quantity: number; unit: string }> = {}; | |
| filtered.forEach(r => { | |
| const yr = r.year; | |
| const unit = (r.unit || 'quintal').trim().toLowerCase(); | |
| if (!groupedByYear[yr]) { | |
| groupedByYear[yr] = { quantity: 0, unit }; | |
| } | |
| if (groupedByYear[yr].unit !== unit) { | |
| // Decide one policy: normalize units before sum OR split series OR reject mixed-unit buckets. | |
| return; | |
| } | |
| groupedByYear[yr].quantity += r.quantity || 0; | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/components/AnalyticsDashboard.tsx` around lines 120 - 127, The
groupedByYear object structure does not account for mixed units within the same
year. Currently, line 124 stores only the first unit encountered for each year
while line 126 aggregates all quantities without unit validation, causing
invalid totals when a crop/year combination has mixed units. Modify the grouping
logic to either track quantities separately by both year and unit (change the
grouping key to include unit information), or implement unit normalization to
convert all quantities to a standard unit before aggregation. Ensure that the
unit stored in groupedByYear accurately represents the units of all quantities
being summed for that year.
| <select | ||
| className={styles.cropSelect} | ||
| value={selectedCrop} | ||
| onChange={(e) => setSelectedCrop(e.target.value)} | ||
| > | ||
| {availableCrops.map((crop) => ( | ||
| <option key={crop} value={crop}> | ||
| {crop} | ||
| </option> | ||
| ))} | ||
| </select> |
There was a problem hiding this comment.
Add an accessible name for the crop combobox.
Line 275 renders a <select> without a label or aria-label, so assistive tech won’t announce its purpose clearly.
Quick fix
<select
className={styles.cropSelect}
+ aria-label="Select crop for YoY yield trend"
value={selectedCrop}
onChange={(e) => setSelectedCrop(e.target.value)}
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <select | |
| className={styles.cropSelect} | |
| value={selectedCrop} | |
| onChange={(e) => setSelectedCrop(e.target.value)} | |
| > | |
| {availableCrops.map((crop) => ( | |
| <option key={crop} value={crop}> | |
| {crop} | |
| </option> | |
| ))} | |
| </select> | |
| <select | |
| className={styles.cropSelect} | |
| aria-label="Select crop for YoY yield trend" | |
| value={selectedCrop} | |
| onChange={(e) => setSelectedCrop(e.target.value)} | |
| > | |
| {availableCrops.map((crop) => ( | |
| <option key={crop} value={crop}> | |
| {crop} | |
| </option> | |
| ))} | |
| </select> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/components/AnalyticsDashboard.tsx` around lines 275 - 285, The
select element used for crop selection lacks an accessible name for assistive
technologies. Add either an associated label element or an aria-label attribute
to the select that renders the availableCrops map to provide a clear,
descriptive name that screen readers can announce to users.
| background: #ffffff; | ||
| border-radius: 16px; | ||
| padding: 20px; | ||
| box-shadow: 0 1px 6px rgba(0, 0, 0, 0.06); |
There was a problem hiding this comment.
Use modern color function syntax for lint compliance.
Stylelint flags these declarations for color-function-notation / alpha-value-notation. Converting them to modern rgb(... / ...%) avoids lint failures.
Proposed lint-safe update
- box-shadow: 0 1px 6px rgba(0, 0, 0, 0.06);
+ box-shadow: 0 1px 6px rgb(0 0 0 / 6%);
- box-shadow: 0 0 0 3px rgba(16, 185, 129, 0.1);
+ box-shadow: 0 0 0 3px rgb(16 185 129 / 10%);
- box-shadow: 0 4px 12px rgba(0, 0, 0, 0.08);
+ box-shadow: 0 4px 12px rgb(0 0 0 / 8%);Also applies to: 519-519, 533-533
🧰 Tools
🪛 Stylelint (17.13.0)
[error] 486-486: Expected "0.06" to be "6%" (alpha-value-notation)
(alpha-value-notation)
[error] 486-486: Expected "rgba" to be "rgb" (color-function-alias-notation)
(color-function-alias-notation)
[error] 486-486: Expected modern color-function notation (color-function-notation)
(color-function-notation)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/styles/AnalyticsDashboard.module.css` at line 486, The
box-shadow declarations in AnalyticsDashboard.module.css are using the older
rgba() color function syntax which triggers stylelint warnings. Convert all
rgba() functions to modern rgb() syntax with slash notation at lines 486, 519,
and 533. Change the format from rgba(0, 0, 0, 0.06) to rgb(0 0 0 / 6%) by
replacing commas with spaces in the color values, and convert the decimal alpha
value (0.06) to a percentage (6%) after the forward slash.
Source: Linters/SAST tools
fixes #46
Description
📝 Summary of Changes
This PR implements a visual Year-over-Year (YoY) Yield History Line Chart inside the Analytics Dashboard using the
rechartslibrary. This allows farmers to track their crop yields dynamically across different years.Dependency Installation:
rechartslocally in thefrontendworkspace to enable visual charting capabilities.Data Selection & Processing Logic (AnalyticsDashboard.tsx):
ResponsiveContainer,LineChart,Line,XAxis,YAxis,CartesianGrid, andTooltip.CustomTooltipdisplaying year, total yield quantity, and crop unit.Styling & Accessibility (AnalyticsDashboard.module.css):
[data-theme='high-contrast']to override layouts, colors, and borders in High Contrast Mode to maintain high visibility in direct sunlight.Integration Testing (AnalyticsDashboard.test.tsx):
🧪 Verification & Testing Results
npx tsc --noEmitcompiled successfully with 0 errors and 0 warnings.npm run testpassed all test suites:AnalyticsDashboard.test.tsx(PASS - 3 tests verifying chart card, select options, YoY summation, and crop filtering)LanguageSwitcher.test.tsx(PASS - 1 test)DailyTaskReminders.test.tsx(PASS - 2 tests)geoUtils.test.ts(PASS - 7 tests)http://localhost:3000with no startup problems.Summary by CodeRabbit
New Features
Tests