chore(inao-cem): Refactor custom components and fetch tax data #17195
Conversation
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
warning [email protected]: This version is no longer supported. Please see https://eslint.org/version-support for other options. WalkthroughThe changes in this pull request primarily involve modifications to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #17195 +/- ##
=======================================
Coverage 35.76% 35.77%
=======================================
Files 6931 6931
Lines 147977 148023 +46
Branches 42173 42181 +8
=======================================
+ Hits 52931 52959 +28
- Misses 95046 95064 +18
Flags with carried forward coverage won't be shown. Click here to find out more. see 12 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 13 Total Test Services: 0 Failed, 12 Passed Test ServicesThis report shows up to 10 services
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Outside diff range and nitpick comments (19)
libs/application/templates/inao/financial-statement-cemetery/src/forms/applicationForm/cemeteryKeyNumbersSection/equityAndLiabilitySubSection.ts (3)
18-21: LGTM! Good reorganization of utility functions.The relocation of financial calculation functions to a dedicated
sumsmodule improves code organization and maintainability.Consider using barrel exports (index.ts) to simplify these deep relative imports:
// libs/application/templates/inao/financial-statement-cemetery/src/utils/index.ts export * from './sums';This would allow shorter imports like:
import { sumTotalEquity } from '../../../utils'
Line range hint
23-166: Consider enhancing type safety for form values.The component structure is well-organized and reusable. However, to improve type safety and maintainability:
Consider defining explicit TypeScript interfaces for the form values:
interface EquityAndLiabilityFormData { fixedAssetsTotal: number; currentAssets: number; longTerm: number; shortTerm: number; equityAtTheBeginningOfTheYear: number; revaluationDueToPriceChanges: number; reevaluateOther: number; }This would provide better type checking and autocompletion when accessing form values in the utility functions.
Line range hint
1-21: Improve import readability while maintaining tree-shaking benefits.While the current import structure supports effective tree-shaking:
- Consider using a more descriptive alias than 'm' for messages:
import { messages } from '../../../lib/messages'
- Consider grouping related builder imports:
import { // Field builders buildTextField, buildDisplayField, // Section builders buildSubSection, buildMultiField, // Helper builders buildAlertMessageField, buildDescriptionField, } from '@island.is/application/core'libs/application/templates/inao/financial-statement-cemetery/src/utils/helpers.ts (1)
Line range hint
25-37: Typo in function name: 'isCemetryUnderFinancialLimit' should be 'isCemeteryUnderFinancialLimit'The function name contains a typo; 'Cemetry' should be corrected to 'Cemetery' to maintain consistency and avoid confusion.
Apply this diff to correct the function name:
-export const isCemetryUnderFinancialLimit = (answers: FormValue) => { +export const isCemeteryUnderFinancialLimit = (answers: FormValue) => {libs/application/templates/inao/financial-statement-cemetery/src/utils/currency.ts (3)
1-7: Ensure proper handling of decimal separators incurrencyStringToNumberThe current implementation removes commas, periods, and spaces, which might inadvertently strip decimal separators. Consider handling decimal separators explicitly to accurately parse values with decimals.
Suggested change:
-export const currencyStringToNumber = (str: string) => { - if (!str) { - return str - } - const cleanString = str.replace(/[,\s]+|[.\s]+/g, '') - return parseInt(cleanString, 10) +export const currencyStringToNumber = (str: string) => { + if (!str) { + return str + } + const normalizedStr = str.replace(/\s+/g, '') + const cleanString = normalizedStr.replace(',', '.') + return parseFloat(cleanString) }
9-12: Handle decimal values properly informatCurrencyIf
answerincludes decimal values, the current formatting may not display them correctly. Consider formatting decimal values appropriately to ensure correct display.Suggested change:
-export const formatCurrency = (answer?: string) => { - if (!answer) return '0. kr' - return answer.replace(/\B(?=(\d{3})+(?!\d))/g, '.') + ' kr.' +export const formatCurrency = (answer?: string) => { + if (!answer) return '0 kr.' + const number = parseFloat(answer) + return number.toLocaleString('is-IS', { style: 'currency', currency: 'ISK' }) }
14-16: Improve input validation inisPositiveNumberInStringIf
inputis not a valid number,Number(input)returnsNaN, and the function returnsfalse. Consider explicitly checking forNaNto handle invalid inputs appropriately.Suggested change:
export const isPositiveNumberInString = (input: string) => { - return Number(input) >= 0 + const num = Number(input) + return !isNaN(num) && num >= 0 }libs/application/templates/inao/financial-statement-cemetery/src/forms/applicationForm/cemeteryFinancialStatementSection/index.ts (2)
3-3: Fix typo in imported function nameThe helper function name contains a typo:
isCemetryUnderFinancialLimitshould beisCemeteryUnderFinancialLimit.
7-9: Add type annotation for condition parameterThe
answersparameter in the condition function lacks type annotation. Consider adding appropriate type information for better type safety and code maintainability.- condition: (answers) => { + condition: (answers: FormValue) => { return !isCemetryUnderFinancialLimit(answers) },libs/application/templates/inao/financial-statement-cemetery/src/forms/applicationForm/cemeteryCaretakerSection/index.ts (3)
8-8: Fix typo in imported function nameThe helper function name contains a typo:
isCemetryUnderFinancialLimitshould beisCemeteryUnderFinancialLimit.
11-14: Fix typo in section name and add type annotationThe section name contains a typo ("Careteker") and the condition parameter lacks type annotation.
-export const cemeteryCaretekerSection = buildSection({ +export const cemeteryCaretakerSection = buildSection({ - condition: (answers) => { + condition: (answers: FormValue) => { return isCemetryUnderFinancialLimit(answers) },
23-28: Consider extracting component name to a constantThe component name 'CemeteryCaretaker' is used as a string literal. Consider extracting it to a constant to prevent typos and enable easier refactoring.
+const CEMETERY_CARETAKER_COMPONENT = 'CemeteryCaretaker' as const; buildCustomField({ id: 'cemeteryCaretaker', title: m.cemeteryBoardmembers, - component: 'CemeteryCaretaker', + component: CEMETERY_CARETAKER_COMPONENT, childInputIds: Object.values(CEMETERYCARETAKER), }),libs/application/templates/inao/financial-statement-cemetery/src/types/types.ts (2)
50-54: Consider using type composition for similar structuresThe
CareTakertype has identical structure toBoardMember. Consider using type composition to avoid duplication and maintain consistency.-export type CareTaker = { - nationalId: string - name: string - role: string -} +export type CareTaker = BoardMemberAlso, consider renaming to
Caretakerto follow consistent naming conventions (camelCase for variables/types).
56-64: Enhance type safety for TaxInfo typesConsider making the types more specific:
- Add literal type for
__typename- Consider using an enum for
keyvalues if they are predefined+export enum TaxInfoKey { + // Add your specific keys here + KEY_1 = 1, + KEY_2 = 2, +} export type TaxInfoItem = { - __typename: string - key: number + __typename: 'TaxInfoItem' + key: TaxInfoKey value: string }libs/application/templates/inao/financial-statement-cemetery/src/fields/CemeteryOverview/AboutOverview.tsx (1)
30-32: Consider using formatNationalId consistentlyThe
formatNationalIdfunction is imported but not used for formatting the national ID.- value={nationalId ? nationalId : '-'} + value={nationalId ? formatNationalId(nationalId) : '-'}libs/application/templates/inao/financial-statement-cemetery/src/utils/overviewUtils.ts (1)
146-171: Add TypeScript interface for better type safetyThe function would benefit from a clearly defined return type interface.
interface AboutOverview { fullName: string; nationalId: string; powerOfAttorneyName?: string; powerOfAttorneyNationalId?: string; email: string; phoneNumber: string; } export const getAboutOverviewNumbers = (answers: FormValue): AboutOverview => { // existing implementation }libs/application/templates/inao/financial-statement-cemetery/src/lib/dataSchema.ts (1)
Line range hint
201-219: Fix incorrect age validation syntaxThere's a syntax error in the age validation that will cause it to always return false.
- .refine((val) => { - return ( - val ? kennitala.info(val).age < 18 : false, - { - params: m.nationalIdAgeError, - } - ) - }), + .refine( + (val) => val ? kennitala.info(val).age >= 18 : false, + { + params: m.nationalIdAgeError, + } + ),libs/application/templates/inao/financial-statement-cemetery/src/fields/CemeteryOverview/index.tsx (2)
33-63: Consider adding explicit TypeScript types for financial data.While the refactoring to use
getOverviewNumbersimproves maintainability, consider adding explicit types for the destructured financial values to enhance type safety and documentation.Example:
interface FinancialOverviewData { careIncome: string; burialRevenue: string; // ... other financial fields cemeteryCaretakers?: Array<{ name: string; nationalId: string; role: string; }>; } const { careIncome, burialRevenue, // ... }: FinancialOverviewData = getOverviewNumbers(application.answers);
69-69: Consider using React Context for sharing application data.The current pattern of passing
application.answersdirectly to child components could lead to prop drilling. Consider using React Context to make the application data available to all components that need it.Example implementation:
// Create a context const ApplicationContext = React.createContext<Application | undefined>(undefined); // Wrap the component tree export const CemeteryOverview = ({ application }: FieldBaseProps) => { return ( <ApplicationContext.Provider value={application}> <Box marginBottom={2}> <AboutOverview /> <CapitalNumberOverview /> {/* ... */} </Box> </ApplicationContext.Provider> ); }; // In child components const AboutOverview = () => { const application = useContext(ApplicationContext); // Use application.answers directly };Also applies to: 149-149
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (24)
.github/CODEOWNERS(1 hunks)libs/application/templates/inao/financial-statement-cemetery/src/dataProviders/index.ts(2 hunks)libs/application/templates/inao/financial-statement-cemetery/src/fields/CemeteryIncomeLimit/index.tsx(0 hunks)libs/application/templates/inao/financial-statement-cemetery/src/fields/CemeteryOverview/AboutOverview.tsx(1 hunks)libs/application/templates/inao/financial-statement-cemetery/src/fields/CemeteryOverview/CapitalNumbersOverview.tsx(2 hunks)libs/application/templates/inao/financial-statement-cemetery/src/fields/CemeteryOverview/index.tsx(4 hunks)libs/application/templates/inao/financial-statement-cemetery/src/fields/FetchDataBasedOnSelectedYear/index.tsx(1 hunks)libs/application/templates/inao/financial-statement-cemetery/src/fields/PowerOfAttorney/index.tsx(0 hunks)libs/application/templates/inao/financial-statement-cemetery/src/fields/index.ts(1 hunks)libs/application/templates/inao/financial-statement-cemetery/src/forms/applicationForm/cemeteryCaretakerSection/caretakerMultiField.ts(0 hunks)libs/application/templates/inao/financial-statement-cemetery/src/forms/applicationForm/cemeteryCaretakerSection/index.ts(1 hunks)libs/application/templates/inao/financial-statement-cemetery/src/forms/applicationForm/cemeteryFinancialStatementSection/index.ts(1 hunks)libs/application/templates/inao/financial-statement-cemetery/src/forms/applicationForm/cemeteryKeyNumbersSection/capitalNumberSubSection.ts(1 hunks)libs/application/templates/inao/financial-statement-cemetery/src/forms/applicationForm/cemeteryKeyNumbersSection/equityAndLiabilitySubSection.ts(1 hunks)libs/application/templates/inao/financial-statement-cemetery/src/forms/applicationForm/cemeteryKeyNumbersSection/opperatingCostSubSection.ts(2 hunks)libs/application/templates/inao/financial-statement-cemetery/src/forms/applicationForm/clientInfoSection/index.ts(2 hunks)libs/application/templates/inao/financial-statement-cemetery/src/hooks/useTotals.ts(0 hunks)libs/application/templates/inao/financial-statement-cemetery/src/lib/dataSchema.ts(1 hunks)libs/application/templates/inao/financial-statement-cemetery/src/types/types.ts(1 hunks)libs/application/templates/inao/financial-statement-cemetery/src/utils/constants.ts(1 hunks)libs/application/templates/inao/financial-statement-cemetery/src/utils/currency.ts(1 hunks)libs/application/templates/inao/financial-statement-cemetery/src/utils/helpers.ts(3 hunks)libs/application/templates/inao/financial-statement-cemetery/src/utils/overviewUtils.ts(1 hunks)libs/application/templates/inao/financial-statement-cemetery/src/utils/sums.ts(1 hunks)
💤 Files with no reviewable changes (4)
- libs/application/templates/inao/financial-statement-cemetery/src/fields/CemeteryIncomeLimit/index.tsx
- libs/application/templates/inao/financial-statement-cemetery/src/forms/applicationForm/cemeteryCaretakerSection/caretakerMultiField.ts
- libs/application/templates/inao/financial-statement-cemetery/src/fields/PowerOfAttorney/index.tsx
- libs/application/templates/inao/financial-statement-cemetery/src/hooks/useTotals.ts
✅ Files skipped from review due to trivial changes (1)
- libs/application/templates/inao/financial-statement-cemetery/src/forms/applicationForm/cemeteryKeyNumbersSection/capitalNumberSubSection.ts
🧰 Additional context used
📓 Path-based instructions (18)
libs/application/templates/inao/financial-statement-cemetery/src/utils/constants.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/inao/financial-statement-cemetery/src/fields/index.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/inao/financial-statement-cemetery/src/dataProviders/index.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/inao/financial-statement-cemetery/src/forms/applicationForm/cemeteryFinancialStatementSection/index.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/inao/financial-statement-cemetery/src/lib/dataSchema.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/inao/financial-statement-cemetery/src/forms/applicationForm/cemeteryKeyNumbersSection/opperatingCostSubSection.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/inao/financial-statement-cemetery/src/types/types.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/inao/financial-statement-cemetery/src/fields/CemeteryOverview/index.tsx (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/inao/financial-statement-cemetery/src/forms/applicationForm/cemeteryKeyNumbersSection/equityAndLiabilitySubSection.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/inao/financial-statement-cemetery/src/fields/CemeteryOverview/CapitalNumbersOverview.tsx (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/inao/financial-statement-cemetery/src/fields/CemeteryOverview/AboutOverview.tsx (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/inao/financial-statement-cemetery/src/forms/applicationForm/cemeteryCaretakerSection/index.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/inao/financial-statement-cemetery/src/utils/currency.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/inao/financial-statement-cemetery/src/forms/applicationForm/clientInfoSection/index.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/inao/financial-statement-cemetery/src/utils/overviewUtils.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/inao/financial-statement-cemetery/src/fields/FetchDataBasedOnSelectedYear/index.tsx (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/inao/financial-statement-cemetery/src/utils/helpers.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/inao/financial-statement-cemetery/src/utils/sums.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🔇 Additional comments (18)
libs/application/templates/inao/financial-statement-cemetery/src/utils/helpers.ts (2)
76-109: Function getCareIncomeAndBurialRevenueAndGrant implemented correctly
The function properly retrieves and returns the specified tax information.
112-134: Function getTaxInfoFromAnswers implemented correctly
The function correctly extracts tax information from the provided answers.
libs/application/templates/inao/financial-statement-cemetery/src/fields/index.ts (1)
3-3: Exporting FetchDataBasedOnSelectedYear is appropriate
The new component FetchDataBasedOnSelectedYear is correctly exported.
libs/application/templates/inao/financial-statement-cemetery/src/dataProviders/index.ts (1)
16-21: Configuration of IdentityApiProvider is appropriate
The IdentityApi is correctly configured with includeActorInfo: true and exported as IdentityApiProvider.
libs/application/templates/inao/financial-statement-cemetery/src/fields/CemeteryOverview/CapitalNumbersOverview.tsx (2)
5-11: LGTM! Type safety improvements
Good refactoring to use the common FormValue type and extract the prop type definition. This improves type safety and maintainability.
15-16: Verify the utility function implementation
The refactoring to use getCapitalNumbersOverviewNumbers looks good, but let's verify its implementation.
Also applies to: 26-37
✅ Verification successful
Implementation verified successfully
The getCapitalNumbersOverviewNumbers utility function is properly implemented in overviewUtils.ts. It correctly:
- Takes form answers as input
- Uses
getValueViaPathto safely access nested values from the answers object - Uses
formatCurrencyfrom UI components to format the currency values - Returns an object with properly formatted
capitalIncome,capitalCost, andtotalCapitalvalues
Both helper functions (getValueViaPath and formatCurrency) are well-established utilities used throughout the codebase, with formatCurrency specifically handling currency formatting by adding thousand separators and the 'kr.' suffix.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for the utility function implementation
ast-grep --pattern 'export const getCapitalNumbersOverviewNumbers = ($params) => {
$$$
}'
Length of output: 93
Script:
#!/bin/bash
# Let's try a broader search pattern to find the function implementation
rg -l "getCapitalNumbersOverviewNumbers"
# Then let's look at the content of files that contain this function
rg "getCapitalNumbersOverviewNumbers" -A 10
Length of output: 4787
Script:
#!/bin/bash
# Let's verify the helper functions used within getCapitalNumbersOverviewNumbers
rg "formatCurrency|getValueViaPath" -A 5
Length of output: 68760
libs/application/templates/inao/financial-statement-cemetery/src/fields/CemeteryOverview/AboutOverview.tsx (1)
9-21: LGTM! Clean data extraction
Good refactoring to use the utility function for data extraction. The destructuring makes the code more readable.
libs/application/templates/inao/financial-statement-cemetery/src/fields/FetchDataBasedOnSelectedYear/index.tsx (1)
37-43:
Add missing dependencies to useEffect
The useEffect dependency array is missing dependencies that could lead to stale closures.
- }, [data, setValue])
+ }, [data, setValue, CEMETERYOPERATIONIDS.incomeLimit])Likely invalid or redundant comment.
libs/application/templates/inao/financial-statement-cemetery/src/utils/constants.ts (1)
29-29: Verify the tax info type code usage
The addition of DONATIONS_TO_CEMETERYFUND = 334 to TaxInfoTypes looks good, but let's ensure consistent usage across the codebase.
✅ Verification successful
Let me analyze the usage of the new tax info type code by searching for references to DONATIONS_TO_CEMETERYFUND in TypeScript files.
Let me check how this enum value is being used in the financial statements client to ensure consistent usage.
Tax info type code 334 is consistently used for cemetery fund donations
The code 334 for DONATIONS_TO_CEMETERYFUND is consistently used across the codebase:
- Defined as an enum value in constants.ts
- Used in helpers.ts for tax info type filtering
- Properly mapped in the financial statements client service
- Documented in types with the correct Icelandic description "Framlög til kirkjugarðasjóðs"
- Verified in test specifications
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the usage of the new tax info type code
# Search for direct usage of the number 334 to ensure it's not hardcoded elsewhere
rg -n "334" --type ts
# Search for references to DONATIONS_TO_CEMETERYFUND
rg "DONATIONS_TO_CEMETERYFUND" --type ts
Length of output: 84435
Script:
#!/bin/bash
# Search for references to DONATIONS_TO_CEMETERYFUND in TypeScript files
rg -A 3 "DONATIONS_TO_CEMETERYFUND" --type ts
Length of output: 871
Script:
#!/bin/bash
# Search for references to the value 334 in financial statements related files
rg -A 3 "334" --type ts libs/clients/financial-statements-inao/
Length of output: 1716
libs/application/templates/inao/financial-statement-cemetery/src/forms/applicationForm/clientInfoSection/index.ts (3)
62-72: LGTM: Improved type-safe data access for power of attorney national ID
The implementation uses getValueViaPath for safer access to nested properties, reducing the risk of runtime errors.
74-83: LGTM: Improved type-safe data access for power of attorney name
The implementation uses getValueViaPath for safer access to nested properties, reducing the risk of runtime errors.
90-104: LGTM: Enhanced contact information field implementations
The changes improve type safety and consistency by:
- Using
getValueViaPathfor safer property access - Using specialized
buildPhoneFieldfor phone number input
libs/application/templates/inao/financial-statement-cemetery/src/forms/applicationForm/cemeteryKeyNumbersSection/opperatingCostSubSection.ts (2)
15-15: LGTM: Improved module organization
Moving sum-related functions to a dedicated utility file improves code organization and maintainability.
27-29: LGTM: Enhanced component naming and purpose
The changes improve clarity by:
- Using a more descriptive ID 'fetchDataBasedOnYear'
- Using a component name that better reflects its functionality
✅ Verification successful
Component change introduces expanded functionality
The new FetchDataBasedOnSelectedYear component is a significant enhancement over the old CemeteryIncomeLimit component:
- It fetches both financial limits and tax information based on the selected year
- Adds functionality to populate care income, burial revenue, grants, and donations
- Maintains backward compatibility by including the original income limit functionality
- Uses updated GraphQL queries and constants
The change is more than just a rename - it's a functional expansion that better serves the form's requirements.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the new component exists and the old one is removed
# Check if the new component exists
fd "FetchDataBasedOnSelectedYear" --type f --extension tsx --extension ts
# Check if the old component is fully removed
fd "CemeteryIncomeLimit" --type f --extension tsx --extension ts
Length of output: 138
Script:
#!/bin/bash
# Let's try a broader search using ripgrep to find any references to these components
rg "FetchDataBasedOnSelectedYear" -l
rg "CemeteryIncomeLimit" -l
# Also search for any similar component names that might be related
rg "FetchData.*Year" -l
Length of output: 1013
Script:
#!/bin/bash
# Let's check the content of both components to verify the functionality
cat libs/application/templates/inao/financial-statement-cemetery/src/fields/FetchDataBasedOnSelectedYear/index.tsx
cat libs/application/templates/financial-statements-inao/src/fields/CemetryIncomeLimit/index.tsx
Length of output: 4220
libs/application/templates/inao/financial-statement-cemetery/src/utils/overviewUtils.ts (1)
173-189: LGTM! Well-structured and consistent formatting
The function maintains good separation of concerns and ensures consistent currency formatting.
libs/application/templates/inao/financial-statement-cemetery/src/lib/dataSchema.ts (1)
6-8: LGTM! Import reorganization looks good
The movement of isPositiveNumberInString to the currency utils makes logical sense.
.github/CODEOWNERS (1)
241-247: LGTM! Ownership assignments are consistent.
The new entries follow the established pattern and correctly assign ownership of inao-related templates and modules to the @island-is/norda team.
libs/application/templates/inao/financial-statement-cemetery/src/fields/CemeteryOverview/index.tsx (1)
23-25: LGTM! Well-structured utility imports.
The new utility imports improve code modularity and follow good tree-shaking practices by importing specific functions.
...emplates/inao/financial-statement-cemetery/src/fields/FetchDataBasedOnSelectedYear/index.tsx
Show resolved
Hide resolved
...emplates/inao/financial-statement-cemetery/src/fields/FetchDataBasedOnSelectedYear/index.tsx
Outdated
Show resolved
Hide resolved
libs/application/templates/inao/financial-statement-cemetery/src/utils/overviewUtils.ts
Show resolved
Hide resolved
libs/application/templates/inao/financial-statement-cemetery/src/utils/sums.ts
Show resolved
Hide resolved
libs/application/templates/inao/financial-statement-cemetery/src/utils/sums.ts
Outdated
Show resolved
Hide resolved
...pplication/templates/inao/financial-statement-cemetery/src/fields/CemeteryOverview/index.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (2)
libs/application/templates/inao/financial-statement-cemetery/src/fields/CemeteryOverview/index.tsx (1)
30-63: Consider breaking down the overview numbers into logical groupsWhile the current implementation successfully consolidates the data extraction logic, the large number of destructured values (30+) could be organized into logical groups for better maintainability.
Consider grouping related values into separate objects:
const { income: { careIncome, burialRevenue, /* ... */ }, expenses: { payroll, funeralCost, /* ... */ }, assets: { fixedAssetsTotal, currentAssets, /* ... */ }, liabilities: { longTerm, shortTerm, /* ... */ }, equity: { equityAtTheBeginningOfTheYear, /* ... */ }, metadata: { email, fileName, /* ... */ } } = getOverviewNumbers(application.answers)libs/application/templates/inao/financial-statement-cemetery/src/utils/sums.ts (1)
11-225: Specify explicit return types for utility functionsTo enhance type safety and maintainability, consider specifying explicit return types for all utility functions. This practice improves code readability and helps prevent potential type-related issues.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
.github/CODEOWNERS(1 hunks)libs/application/templates/inao/financial-statement-cemetery/src/fields/CemeteryOverview/index.tsx(4 hunks)libs/application/templates/inao/financial-statement-cemetery/src/utils/sums.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
libs/application/templates/inao/financial-statement-cemetery/src/fields/CemeteryOverview/index.tsx (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/inao/financial-statement-cemetery/src/utils/sums.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🔇 Additional comments (5)
libs/application/templates/inao/financial-statement-cemetery/src/fields/CemeteryOverview/index.tsx (3)
23-25: LGTM: Well-structured utility imports
The new utility imports follow good modularization practices and maintain proper TypeScript usage, enhancing reusability across the application.
69-69: LGTM: Consistent prop passing pattern
The direct passing of application.answers to child components maintains consistency and eliminates unnecessary intermediate variables.
Also applies to: 149-149
229-231: Simplify the conditional rendering logic
The current condition has redundant checks. The length check is unnecessary when already checking if cemeteryCaretakers exists.
Apply this simplification:
- {cemeteryUnderFinancialLimit &&
- cemeteryCaretakers &&
- cemeteryCaretakers.length > 0 ? (
+ {cemeteryUnderFinancialLimit && cemeteryCaretakers?.length ? (libs/application/templates/inao/financial-statement-cemetery/src/utils/sums.ts (1)
11-225: Extract a reusable utility function to reduce code duplication
There is duplicated code in summing numerical values from strings across multiple functions. Consider creating a reusable utility function that handles the conversion and summation, which will reduce code duplication and enhance maintainability.
.github/CODEOWNERS (1)
241-248: Ownership assignments look appropriate
The new CODEOWNERS entries for the inao templates correctly assign ownership to @island-is/norda, ensuring proper responsibility for these modules.
libs/application/templates/inao/financial-statement-cemetery/src/utils/sums.ts
Show resolved
Hide resolved
libs/application/templates/inao/financial-statement-cemetery/src/utils/sums.ts
Show resolved
Hide resolved
libs/application/templates/inao/financial-statement-cemetery/src/utils/sums.ts
Show resolved
Hide resolved
libs/application/templates/inao/financial-statement-cemetery/src/utils/sums.ts
Show resolved
Hide resolved
What
Remove custom components that are no longer needed
Make logic to fetch tax data better
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Removals