feat(reports/publisher_reports): add filter for statistics on publisher records page #4660
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThis PR adds group-based filtering to the reporting system by introducing an optional Changes
Sequence DiagramsequenceDiagram
participant User
participant YearDetails
participant SelectPublishers
participant SelectPeriod
participant useYearDetails
participant ReportHooks as usePublishers/useAuxiliaryPioneers/<br/>etc.
participant PersonHooks as usePersons/<br/>useReportYearly/<br/>useReportMonthly
User->>SelectPublishers: selects group
SelectPublishers->>YearDetails: onChange(selectedPublishers)
YearDetails->>useYearDetails: handleChangeSelectedPublishers
useYearDetails->>useYearDetails: update selectedPublishers state
User->>SelectPeriod: selects month/service-year
SelectPeriod->>YearDetails: onChange(selectedMonth)
YearDetails->>useYearDetails: handleChangeSelectedMonth
useYearDetails->>useYearDetails: update selectedMonth state
useYearDetails->>useYearDetails: compute wholeYear = (selectedMonth === 'service-year')
useYearDetails->>YearDetails: return selectedPublishers, selectedMonth, wholeYear
YearDetails->>ReportHooks: pass publishersGroup + month/year/wholeYear
ReportHooks->>PersonHooks: call with publishersGroup parameter
PersonHooks->>PersonHooks: filter persons by fieldGroups matching publishersGroup
PersonHooks-->>ReportHooks: return filtered persons list
ReportHooks-->>YearDetails: compute reports with filtered data
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes The changes follow a consistent pattern of propagating a Possibly related PRs
Suggested labels
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/features/reports/publisher_records/years_stats/year_details/index.tsx (1)
40-63: Remove commented-out code.Commented-out code should not be committed to version control. Git history preserves the old implementation if needed. Keeping dead code reduces maintainability and creates confusion.
Apply this diff to remove the commented block:
- {/* <Tooltip - title={t('tr_wholeYearIsSelected')} - delaySpeed={'fast'} - show={wholeYear} - followCursor - sx={{ flex: 1 }} - > - <MonthSelector - year={year} - value={month} - onChange={handleMonthChange} - readOnly={wholeYear} - sx={{ - flex: 1, - }} - /> - </Tooltip> - <Box> - <SwitchWithLabel - label={t('tr_wholeYearSetting')} - checked={wholeYear} - onChange={handleToggleWholeYear} - /> - </Box> */} <SelectPublishers
🧹 Nitpick comments (3)
src/features/reports/publisher_records/years_stats/year_details/useYearDetails.tsx (1)
1-2: Remove unused imports.Line 2 imports
useEffectanduseState, but onlyuseStateis used. TheuseEffectimport is unused, triggering the eslint-disable comment on line 1. Line 4 also redundantly importsuseCallbackwhich is already available from the React import.Apply this diff:
-/* eslint-disable @typescript-eslint/no-unused-vars */ -import { useEffect, useState } from 'react'; +import { useState, useCallback } from 'react'; import { YearDetailsProps } from './index.types'; -import { useCallback } from 'react';src/features/reports/publisher_records/years_stats/year_details/select_period/index.types.ts (1)
1-5: Strengthen type definitions for better type safety.The
unknowntypes foronChangeandvalueare too loose. Based on the component implementation inindex.tsx, these should be more specific:
valueshould bestring(holds either 'service-year' or a month in 'YYYY/MM' format)onChangeshould acceptSelectChangeEvent<string>from MUIApply this diff to improve type safety:
+import { SelectChangeEvent } from '@mui/material'; + export type SelectPeriodProps = { - onChange: (e: unknown) => void; - value: unknown; + onChange: (e: SelectChangeEvent<string>) => void; + value: string; year: string; };src/features/reports/publisher_records/years_stats/year_details/select_period/useSelectPeriod.tsx (1)
3-11: Add proper return type and consider error handling.The return type uses
any[]which bypasses type safety. Based on thebuildServiceYearsListimplementation, months should be typed as{ value: string; label: string }[].Additionally, when the year is not found, an empty array is silently returned. Consider whether this is the desired behavior or if a warning/fallback would be more appropriate.
Apply this diff to add proper typing:
-const useSelectPeriod = ({ year }: { year: string }) => { +const useSelectPeriod = ({ year }: { year: string }): { months: { value: string; label: string }[] } => { const serviceYears = buildServiceYearsList(); const yearObj = serviceYears.find((y) => y.year === year); const months = yearObj ? yearObj.months : []; return { months, }; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/locales/en/general.jsonis excluded by!**/*.json
📒 Files selected for processing (19)
src/features/persons/hooks/usePersons.tsx(2 hunks)src/features/reports/hooks/useReportMonthly.tsx(1 hunks)src/features/reports/hooks/useReportYearly.tsx(1 hunks)src/features/reports/publisher_records/years_stats/auxiliary_pioneers/index.types.ts(1 hunks)src/features/reports/publisher_records/years_stats/auxiliary_pioneers/useAuxiliaryPioneers.tsx(1 hunks)src/features/reports/publisher_records/years_stats/fulltime_servants/index.types.ts(1 hunks)src/features/reports/publisher_records/years_stats/fulltime_servants/useFulltimeServants.tsx(1 hunks)src/features/reports/publisher_records/years_stats/publishers/index.types.ts(1 hunks)src/features/reports/publisher_records/years_stats/publishers/usePublishers.tsx(3 hunks)src/features/reports/publisher_records/years_stats/total_statistics/index.types.ts(1 hunks)src/features/reports/publisher_records/years_stats/total_statistics/useTotalStatistics.tsx(2 hunks)src/features/reports/publisher_records/years_stats/year_details/index.tsx(3 hunks)src/features/reports/publisher_records/years_stats/year_details/select_period/index.tsx(1 hunks)src/features/reports/publisher_records/years_stats/year_details/select_period/index.types.ts(1 hunks)src/features/reports/publisher_records/years_stats/year_details/select_period/useSelectPeriod.tsx(1 hunks)src/features/reports/publisher_records/years_stats/year_details/select_publishers/index.tsx(1 hunks)src/features/reports/publisher_records/years_stats/year_details/select_publishers/index.types.ts(1 hunks)src/features/reports/publisher_records/years_stats/year_details/select_publishers/useSelectPublishers.tsx(1 hunks)src/features/reports/publisher_records/years_stats/year_details/useYearDetails.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (11)
src/features/reports/publisher_records/years_stats/publishers/usePublishers.tsx (1)
src/features/reports/publisher_records/years_stats/publishers/index.types.ts (1)
PublishersProps(1-6)
src/features/reports/publisher_records/years_stats/fulltime_servants/useFulltimeServants.tsx (1)
src/features/reports/publisher_records/years_stats/fulltime_servants/index.types.ts (1)
FulltimeServantsProps(1-6)
src/features/persons/hooks/usePersons.tsx (2)
src/states/field_service_groups.ts (1)
fieldWithLanguageGroupsState(24-62)src/states/persons.ts (1)
personsActiveState(25-34)
src/features/reports/hooks/useReportMonthly.tsx (3)
src/states/field_service_groups.ts (1)
fieldWithLanguageGroupsState(24-62)src/states/field_service_reports.ts (1)
congFieldServiceReportsState(14-22)src/states/persons.ts (1)
personsState(17-17)
src/features/reports/publisher_records/years_stats/year_details/select_period/useSelectPeriod.tsx (1)
src/utils/date.ts (1)
buildServiceYearsList(233-292)
src/features/reports/publisher_records/years_stats/total_statistics/useTotalStatistics.tsx (1)
src/features/reports/publisher_records/years_stats/total_statistics/index.types.ts (1)
TotalStatisticsProps(1-4)
src/features/reports/publisher_records/years_stats/year_details/select_publishers/useSelectPublishers.tsx (1)
src/states/field_service_groups.ts (1)
fieldWithLanguageGroupsState(24-62)
src/features/reports/publisher_records/years_stats/auxiliary_pioneers/useAuxiliaryPioneers.tsx (1)
src/features/reports/publisher_records/years_stats/auxiliary_pioneers/index.types.ts (1)
AuxiliaryPioneersProps(1-6)
src/features/reports/publisher_records/years_stats/year_details/select_period/index.tsx (1)
src/features/reports/publisher_records/years_stats/year_details/select_period/index.types.ts (1)
SelectPeriodProps(1-5)
src/features/reports/hooks/useReportYearly.tsx (3)
src/states/field_service_groups.ts (1)
fieldWithLanguageGroupsState(24-62)src/states/field_service_reports.ts (1)
congFieldServiceReportsState(14-22)src/states/persons.ts (1)
personsState(17-17)
src/features/reports/publisher_records/years_stats/year_details/select_publishers/index.tsx (1)
src/features/reports/publisher_records/years_stats/year_details/select_publishers/index.types.ts (1)
SelectPublishersProps(1-4)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: SonarCloud
- GitHub Check: Code QL
- GitHub Check: Summary
🔇 Additional comments (12)
src/features/reports/publisher_records/years_stats/fulltime_servants/index.types.ts (1)
5-5: LGTM!The addition of
publishersGroupis consistent with the group-filtering feature and follows the same pattern applied to other Props types in this PR.src/features/reports/publisher_records/years_stats/publishers/index.types.ts (1)
5-5: LGTM!Consistent addition of
publishersGroupproperty to support group-based filtering.src/features/reports/publisher_records/years_stats/auxiliary_pioneers/index.types.ts (1)
5-5: LGTM!Consistent with the group-filtering pattern applied throughout this PR.
src/features/reports/publisher_records/years_stats/fulltime_servants/useFulltimeServants.tsx (1)
14-22: LGTM!The
publishersGroupparameter is correctly destructured and passed to the dependent hooks (usePersons,useReportYearly,useReportMonthly), enabling group-scoped data retrieval.src/features/reports/publisher_records/years_stats/total_statistics/index.types.ts (1)
3-3: LGTM!Addition of
publishersGroupaligns with the group-filtering feature being introduced.src/features/reports/publisher_records/years_stats/total_statistics/useTotalStatistics.tsx (1)
11-18: LGTM! Clean parameter threading.The addition of
publishersGroupparameter and its propagation tousePersonsis implemented correctly, enabling group-based filtering for statistics.src/features/reports/publisher_records/years_stats/year_details/select_period/index.tsx (1)
9-38: LGTM! Well-structured component.The component properly integrates the custom hook, translation, and MUI components to render period selection UI. The static "service-year" option followed by dynamic month options provides clear structure.
src/features/reports/publisher_records/years_stats/year_details/select_publishers/index.tsx (1)
9-38: LGTM! Consistent and clean implementation.The component follows the same well-structured pattern as
SelectPeriod, with proper hook integration, translations, and rendering of both "all" and group-specific options.src/features/reports/publisher_records/years_stats/year_details/select_publishers/useSelectPublishers.tsx (1)
5-21: LGTM! Clean data transformation.The hook properly transforms field service groups into a simple structure for the UI, with appropriate fallback naming using translation keys and 1-based indexing.
src/features/reports/publisher_records/years_stats/publishers/usePublishers.tsx (1)
10-22: Clean implementation of group-based filtering.The addition of
publishersGroupparameter and its propagation to all dependent hooks (usePersons,useReportYearly,useReportMonthly) is consistent and correctly implemented.src/features/reports/publisher_records/years_stats/auxiliary_pioneers/useAuxiliaryPioneers.tsx (1)
10-22: LGTM!The implementation follows the same clean pattern as
usePublishers.tsx, correctly propagatingpublishersGroupto all dependent hooks.src/features/reports/publisher_records/years_stats/year_details/index.tsx (1)
75-96: Correct propagation of group-based filtering.The updated component props correctly pass
selectedMonth,selectedPublishers, and the derivedpublishersGroupto all child statistics components.
| const findedGroup = fieldGroups.find((g) => g.group_id === group); | ||
|
|
||
| return allPersons.filter((person) => | ||
| findedGroup.group_data.members.some( | ||
| (personInGroup) => personInGroup.person_uid === person.person_uid | ||
| ) | ||
| ); |
There was a problem hiding this comment.
Null pointer exception risk when group is not found.
If no group matches the provided group ID, findedGroup will be undefined, causing a runtime error on line 33 when accessing findedGroup.group_data.members.
Apply this diff to add a guard:
const findedGroup = fieldGroups.find((g) => g.group_id === group);
+
+ if (!findedGroup) {
+ return [];
+ }
return allPersons.filter((person) =>
findedGroup.group_data.members.some(
(personInGroup) => personInGroup.person_uid === person.person_uid
)
);📝 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 findedGroup = fieldGroups.find((g) => g.group_id === group); | |
| return allPersons.filter((person) => | |
| findedGroup.group_data.members.some( | |
| (personInGroup) => personInGroup.person_uid === person.person_uid | |
| ) | |
| ); | |
| const findedGroup = fieldGroups.find((g) => g.group_id === group); | |
| if (!findedGroup) { | |
| return []; | |
| } | |
| return allPersons.filter((person) => | |
| findedGroup.group_data.members.some( | |
| (personInGroup) => personInGroup.person_uid === person.person_uid | |
| ) | |
| ); |
🤖 Prompt for AI Agents
In src/features/persons/hooks/usePersons.tsx around lines 30 to 36, the code
assumes fieldGroups.find(...) always returns a group and directly accesses
findedGroup.group_data.members which can throw if findedGroup is undefined;
update the logic to first check that findedGroup exists and that
findedGroup.group_data?.members is an array, and if not return an empty array
(or appropriate fallback) before filtering allPersons, otherwise proceed to
filter using the members list.
| return allPersons; | ||
| } | ||
|
|
||
| const findedGroup = fieldGroups.find((g) => g.group_id === group); |
There was a problem hiding this comment.
Fix typo: findedGroup should be foundGroup.
"Finded" is not correct English; the past tense of "find" is "found".
Apply this diff:
- const findedGroup = fieldGroups.find((g) => g.group_id === group);
+ const foundGroup = fieldGroups.find((g) => g.group_id === group);
- if (!findedGroup) {
+ if (!foundGroup) {
return [];
}
return allPersons.filter((person) =>
- findedGroup.group_data.members.some(
+ foundGroup.group_data.members.some(
(personInGroup) => personInGroup.person_uid === person.person_uid
)
);📝 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 findedGroup = fieldGroups.find((g) => g.group_id === group); | |
| const foundGroup = fieldGroups.find((g) => g.group_id === group); | |
| if (!foundGroup) { | |
| return []; | |
| } | |
| return allPersons.filter((person) => | |
| foundGroup.group_data.members.some( | |
| (personInGroup) => personInGroup.person_uid === person.person_uid | |
| ) | |
| ); |
🤖 Prompt for AI Agents
In src/features/persons/hooks/usePersons.tsx around line 30, rename the variable
findedGroup to foundGroup to fix the typo; update its declaration and all
subsequent references in this file (including any returns, uses in JSX or logic)
to use foundGroup, preserving the same value and types so behavior is unchanged.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| const persons = useMemo(() => { | ||
| if (!group || group === 'all') { | ||
| return allPersons; | ||
| } | ||
|
|
||
| const findedGroup = fieldGroups.find((g) => g.group_id === group); | ||
|
|
||
| return allPersons.filter((person) => | ||
| findedGroup.group_data.members.some( | ||
| (personInGroup) => personInGroup.person_uid === person.person_uid | ||
| ) | ||
| ); | ||
| }, [allPersons, fieldGroups, group]); |
There was a problem hiding this comment.
CRITICAL: Handle undefined group to prevent runtime error.
If the specified group ID is not found in fieldGroups, findedGroup will be undefined, causing a runtime error when accessing findedGroup.group_data.members on line 28.
Additionally, "findedGroup" is a typo—the correct past participle is "foundGroup" or "found".
Apply this diff to add null safety:
const persons = useMemo(() => {
if (!group || group === 'all') {
return allPersons;
}
- const findedGroup = fieldGroups.find((g) => g.group_id === group);
+ const foundGroup = fieldGroups.find((g) => g.group_id === group);
+
+ if (!foundGroup) {
+ console.warn(`Group with id "${group}" not found`);
+ return [];
+ }
return allPersons.filter((person) =>
- findedGroup.group_data.members.some(
+ foundGroup.group_data.members.some(
(personInGroup) => personInGroup.person_uid === person.person_uid
)
);
}, [allPersons, fieldGroups, group]);📝 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 persons = useMemo(() => { | |
| if (!group || group === 'all') { | |
| return allPersons; | |
| } | |
| const findedGroup = fieldGroups.find((g) => g.group_id === group); | |
| return allPersons.filter((person) => | |
| findedGroup.group_data.members.some( | |
| (personInGroup) => personInGroup.person_uid === person.person_uid | |
| ) | |
| ); | |
| }, [allPersons, fieldGroups, group]); | |
| const persons = useMemo(() => { | |
| if (!group || group === 'all') { | |
| return allPersons; | |
| } | |
| const foundGroup = fieldGroups.find((g) => g.group_id === group); | |
| if (!foundGroup) { | |
| console.warn(`Group with id "${group}" not found`); | |
| return []; | |
| } | |
| return allPersons.filter((person) => | |
| foundGroup.group_data.members.some( | |
| (personInGroup) => personInGroup.person_uid === person.person_uid | |
| ) | |
| ); | |
| }, [allPersons, fieldGroups, group]); |
🤖 Prompt for AI Agents
In src/features/reports/hooks/useReportMonthly.tsx around lines 20 to 32, the
code assumes fieldGroups.find(...) always returns a group and uses
findedGroup.group_data.members which will throw if undefined and also uses a
typo "findedGroup"; fix by renaming to foundGroup, guard against not found (if
!foundGroup return [] or fallback to allPersons as appropriate), and update the
filter to only access foundGroup.group_data.members when foundGroup exists
(e.g., return allPersons when group is not found or handle an empty members
array) so there is no runtime access of properties on undefined.
| const persons = useMemo(() => { | ||
| if (!group || group === 'all') { | ||
| return allPersons; | ||
| } | ||
|
|
||
| const findedGroup = fieldGroups.find((g) => g.group_id === group); | ||
|
|
||
| return allPersons.filter((person) => | ||
| findedGroup.group_data.members.some( | ||
| (personInGroup) => personInGroup.person_uid === person.person_uid | ||
| ) | ||
| ); | ||
| }, [allPersons, fieldGroups, group]); |
There was a problem hiding this comment.
CRITICAL: Handle undefined group to prevent runtime error.
This has the same critical issue as useReportMonthly.tsx: if the group ID is not found, findedGroup will be undefined, causing a runtime error when accessing findedGroup.group_data.members on line 24.
Also fix the typo: "findedGroup" → "foundGroup".
Apply this diff:
const persons = useMemo(() => {
if (!group || group === 'all') {
return allPersons;
}
- const findedGroup = fieldGroups.find((g) => g.group_id === group);
+ const foundGroup = fieldGroups.find((g) => g.group_id === group);
+
+ if (!foundGroup) {
+ console.warn(`Group with id "${group}" not found`);
+ return [];
+ }
return allPersons.filter((person) =>
- findedGroup.group_data.members.some(
+ foundGroup.group_data.members.some(
(personInGroup) => personInGroup.person_uid === person.person_uid
)
);
}, [allPersons, fieldGroups, group]);📝 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 persons = useMemo(() => { | |
| if (!group || group === 'all') { | |
| return allPersons; | |
| } | |
| const findedGroup = fieldGroups.find((g) => g.group_id === group); | |
| return allPersons.filter((person) => | |
| findedGroup.group_data.members.some( | |
| (personInGroup) => personInGroup.person_uid === person.person_uid | |
| ) | |
| ); | |
| }, [allPersons, fieldGroups, group]); | |
| const persons = useMemo(() => { | |
| if (!group || group === 'all') { | |
| return allPersons; | |
| } | |
| const foundGroup = fieldGroups.find((g) => g.group_id === group); | |
| if (!foundGroup) { | |
| console.warn(`Group with id "${group}" not found`); | |
| return []; | |
| } | |
| return allPersons.filter((person) => | |
| foundGroup.group_data.members.some( | |
| (personInGroup) => personInGroup.person_uid === person.person_uid | |
| ) | |
| ); | |
| }, [allPersons, fieldGroups, group]); |
🤖 Prompt for AI Agents
In src/features/reports/hooks/useReportYearly.tsx around lines 16 to 28, the
code assumes the group lookup always succeeds and uses
findedGroup.group_data.members causing a runtime error if the group ID isn't
found; rename findedGroup to foundGroup, check whether foundGroup is defined
before accessing group_data.members (e.g., if not found return allPersons or an
empty array), and use safe access (optional chaining) when filtering to prevent
accessing properties on undefined.
| @@ -1,3 +1,4 @@ | |||
| /* eslint-disable @typescript-eslint/no-unused-vars */ | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove the eslint-disable directive.
The @typescript-eslint/no-unused-vars disable directive masks potential code quality issues. Address the underlying unused variables or imports instead of suppressing the warning.
🤖 Prompt for AI Agents
In src/features/reports/publisher_records/years_stats/year_details/index.tsx at
lines 1 to 1, remove the top-line "/* eslint-disable
@typescript-eslint/no-unused-vars */" and address the underlying unused
variables/imports: run the linter or TypeScript compiler to list unused
identifiers, then either delete unused imports/variables, rename intentionally
unused parameters to _ or _name, or use the proper exported/used value; finally
re-run tests/lint to confirm the warning is gone and only intentional unused
identifiers remain.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| import Select from '@components/select'; | ||
| import MenuItem from '@components/menuitem'; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify if Select and MenuItem imports are needed.
These imports appear unused in the visible code. If SelectPublishers and SelectPeriod are self-contained components that don't require these to be imported here, remove them.
Run the following script to check if these imports are used:
🏁 Script executed:
#!/bin/bash
# Description: Check if Select and MenuItem from the imports are used in this file
# Search for usage of Select and MenuItem components in the file
rg -n --type=tsx -A 2 -B 2 '\bSelect\b|\bMenuItem\b' src/features/reports/publisher_records/years_stats/year_details/index.tsxLength of output: 27
---
🏁 Script executed:
```shell
#!/bin/bash
# View the file content to check for Select and MenuItem usage
echo "=== File content ==="
cat src/features/reports/publisher_records/years_stats/year_details/index.tsx
echo -e "\n=== Searching for Select usage ==="
rg -n 'Select' src/features/reports/publisher_records/years_stats/year_details/index.tsx
echo -e "\n=== Searching for MenuItem usage ==="
rg -n 'MenuItem' src/features/reports/publisher_records/years_stats/year_details/index.tsx
Length of output: 3566
Remove unused Select and MenuItem imports
These imports at lines 13-14 are not referenced anywhere in the file. The component uses SelectPublishers and SelectPeriod instead. Remove:
import Select from '@components/select';
import MenuItem from '@components/menuitem';
🤖 Prompt for AI Agents
In src/features/reports/publisher_records/years_stats/year_details/index.tsx
around lines 13 to 14, remove the unused imports `import Select from
'@components/select';` and `import MenuItem from '@components/menuitem';` since
the file uses SelectPublishers and SelectPeriod instead; delete those two import
lines to eliminate unused imports and update any import ordering if necessary.
There was a problem hiding this comment.
@FussuChalice: if not needed, then these can be removed.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| export type SelectPublishersProps = { | ||
| onChange: (e: unknown) => void; | ||
| value: unknown; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Replace unknown types with specific types for better type safety.
Both onChange and value are typed as unknown, which bypasses TypeScript's type checking. Based on the context, value should be string (matching publishersGroup across other components), and onChange should accept a React change event.
Apply this diff:
export type SelectPublishersProps = {
- onChange: (e: unknown) => void;
- value: unknown;
+ onChange: (e: React.ChangeEvent<HTMLSelectElement>) => void;
+ value: string;
};🤖 Prompt for AI Agents
In
src/features/reports/publisher_records/years_stats/year_details/select_publishers/index.types.ts
lines 1-4, replace the loose unknown types: change value to string and type
onChange as a React change event handler (e.g. (e:
React.ChangeEvent<HTMLSelectElement>) => void). Add the necessary React import
(or import { ChangeEvent } from 'react') if not already present so the file
compiles.
| const useYearDetails = ({ year }: YearDetailsProps) => { | ||
| const [wholeYear, setWholeYear] = useState(true); | ||
| const [month, setMonth] = useState(''); | ||
| const [selectedPublishers, setSelectedPublshers] = useState('all'); |
There was a problem hiding this comment.
Fix typo in state setter name.
setSelectedPublshers is missing the letter 'i' and should be setSelectedPublishers. This typo appears in both the state declaration (line 7) and the handler (line 11).
Apply this diff:
- const [selectedPublishers, setSelectedPublshers] = useState('all');
+ const [selectedPublishers, setSelectedPublishers] = useState('all'); const handleChangeSelectedPublishers = useCallback((e) => {
- setSelectedPublshers(e.target.value);
+ setSelectedPublishers(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.
| const [selectedPublishers, setSelectedPublshers] = useState('all'); | |
| const [selectedPublishers, setSelectedPublishers] = useState('all'); |
🤖 Prompt for AI Agents
In
src/features/reports/publisher_records/years_stats/year_details/useYearDetails.tsx
around lines 7 and 11, the state setter is misspelled as setSelectedPublshers;
rename it to setSelectedPublishers in the useState declaration and update every
reference (including the handler on line 11) to the corrected name to keep the
state variable and setter consistent.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| return allPersons; | ||
| } | ||
|
|
||
| const findedGroup = fieldGroups.find((g) => g.group_id === group); |
| @@ -1,3 +1,4 @@ | |||
| /* eslint-disable @typescript-eslint/no-unused-vars */ | |||
| import Select from '@components/select'; | ||
| import MenuItem from '@components/menuitem'; |
There was a problem hiding this comment.
@FussuChalice: if not needed, then these can be removed.
| justifyContent="space-between" | ||
| > | ||
| <Tooltip | ||
| {/* <Tooltip |
There was a problem hiding this comment.
@FussuChalice: if no longer needed, this can be removed too.
| const useYearDetails = ({ year }: YearDetailsProps) => { | ||
| const [wholeYear, setWholeYear] = useState(true); | ||
| const [month, setMonth] = useState(''); | ||
| const [selectedPublishers, setSelectedPublshers] = useState('all'); |
|
@FussuChalice: does this replace the #4193 entirely? |
2 participants
Description
Added a filter for groups and periods to the statistics on the publisher records page.
Thanks to Igor Selin, from whom I took a few lines of code.
These are the fixes for draft #4193.
Fixes # (issue)
Type of change
Checklist: