fix(ojoi): Input validation and bugfixes #17124
Conversation
WalkthroughThe changes in this pull request primarily involve modifications to several components and hooks related to the Official Journal of Iceland application. Key updates include adjustments to the handling of date parameters, the addition of new properties for input controls, and enhancements to the rendering logic of various components. Notably, the types for date fields have been changed from Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (7)
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 ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #17124 +/- ##
==========================================
- Coverage 35.73% 35.70% -0.03%
==========================================
Files 6926 6924 -2
Lines 147693 147610 -83
Branches 42074 42033 -41
==========================================
- Hits 52775 52707 -68
+ Misses 94918 94903 -15 Flags with carried forward coverage won't be shown. Click here to find out more.
... and 23 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (22)
libs/application/templates/official-journal-of-iceland/src/lib/messages/attachments.ts (1)
Line range hint
1-99: Consider extracting common message patterns into constants.Several messages use similar patterns (e.g., 'Bæta við viðauka'). Consider extracting these into constants to improve maintainability and ensure consistency across the codebase.
Example refactor:
const COMMON_MESSAGES = { ADD_ATTACHMENT: 'Bæta við viðauka', } as const; export const attachments = { // ... other messages ... buttons: defineMessages({ asAttachment: { id: 'ojoi.application:attachments.buttons.additionType.asAttachment', defaultMessage: COMMON_MESSAGES.ADD_ATTACHMENT, description: 'Label of the button to upload attachments', }, addAddition: { id: 'ojoi.application:attachments.buttons.addAddition', defaultMessage: COMMON_MESSAGES.ADD_ATTACHMENT, description: 'Label of the button to add an addition', }, // ... other buttons ... }), // ... rest of the file }libs/application/templates/official-journal-of-iceland/src/fields/Publishing.tsx (1)
Line range hint
52-77: Consider optimizing category state updatesThe category update logic could be optimized to reduce unnecessary state updates.
Consider this optimization:
const onCategoryChange = (value?: string) => { - setIsUpdatingCategory(true) if (!value) { - setIsUpdatingCategory(false) return } + setIsUpdatingCategory(true) const currentAnswers = structuredClone(currentApplication.answers) const selectedCategories = currentAnswers.advert?.categories || [] - const newCategories = selectedCategories.includes(value) - ? selectedCategories.filter((c) => c !== value) - : [...selectedCategories, value] + const newCategories = new Set(selectedCategories) + if (newCategories.has(value)) { + newCategories.delete(value) + } else { + newCategories.add(value) + } const updatedAnswers = set( currentAnswers, InputFields.advert.categories, - newCategories, + Array.from(newCategories), ) updateApplication(updatedAnswers, () => { setIsUpdatingCategory(false) }) }libs/island-ui/core/src/lib/Header/Header.css.ts (1)
24-31: LGTM! Consider adding scrollbar styling for consistency.The changes effectively address the header title overflow issue by implementing a scrollable container with responsive max heights. The implementation follows good mobile-first practices and uses type-safe CSS-in-TS.
Consider adding custom scrollbar styling for better visual consistency:
export const infoDescription = style({ fontWeight: 300, lineHeight: 1.5, fontSize: 14, maxHeight: 40, position: 'relative', overflow: 'auto', + '&::-webkit-scrollbar': { + width: '4px', + }, + '&::-webkit-scrollbar-thumb': { + backgroundColor: theme.color.dark200, + borderRadius: '2px', + }, ...themeUtils.responsiveStyle({ md: { fontSize: 18, maxHeight: 66, }, }), })libs/island-ui/core/src/lib/Header/Header.tsx (1)
Line range hint
11-63: Consider planning the removal of deprecated props.The component includes multiple deprecated props that should be removed in a future update. Since this is a core UI component, consider:
- Creating a migration guide for users of deprecated props
- Planning a major version bump when removing these props
- Adding console warnings for deprecated prop usage
apps/web/components/OfficialJournalOfIceland/OJOIAdvertCard.tsx (1)
69-69: LGTM! Consider adding type narrowing for better type safetyThe improved condition prevents rendering empty category containers. For additional type safety, consider narrowing the type:
-{categories && categories.length > 0 && ( +{Array.isArray(categories) && categories.length > 0 && (apps/web/screens/OfficialJournalOfIceland/OJOISearch.tsx (1)
175-176: LGTM! Consider adding type validationThe change to use string dates in the refetch call is consistent with the GraphQL input requirements. However, we could enhance type safety.
Consider adding runtime type validation:
refetch({ input: { department: [searchValues.deild], category: [searchValues.malaflokkur], involvedParty: [searchValues.stofnun], type: [searchValues.tegund], - dateFrom: searchValues.dagsFra, - dateTo: searchValues.dagsTil, + dateFrom: searchValues.dagsFra?.match(/^\d{4}-\d{2}-\d{2}$/) ? searchValues.dagsFra : undefined, + dateTo: searchValues.dagsTil?.match(/^\d{4}-\d{2}-\d{2}$/) ? searchValues.dagsTil : undefined, search: searchValues.q, page: searchValues.sida, pageSize: searchValues.staerd, }, })libs/application/templates/official-journal-of-iceland/src/fields/Attachments.tsx (2)
20-24: Consider memoizing application valueThe
useApplicationhook is called on every render. Consider memoizing the options object to prevent unnecessary re-renders.- const { updateApplication, application: currentApplication } = useApplication( - { - applicationId: application.id, - }, - ) + const options = useMemo(() => ({ applicationId: application.id }), [application.id]) + const { updateApplication, application: currentApplication } = useApplication(options)
30-42: Simplify state update logicThe state update logic can be made more concise while maintaining the same functionality.
const handleChange = () => { - const current = asDocument setAsDocument((prev) => !prev) - setValue(InputFields.misc.asDocument, !current) + const newValue = !asDocument + setValue(InputFields.misc.asDocument, newValue) updateApplication({ ...currentApplication.answers, misc: { ...currentApplication.answers.misc, - asDocument: !current, + asDocument: newValue, }, }) }libs/application/templates/official-journal-of-iceland/src/components/additions/Additions.tsx (2)
36-38: Consider memoizing initial state valueThe initial state calculation could be memoized to prevent unnecessary recalculations on re-renders.
- const [asRoman, setAsRoman] = useState<boolean>( - application.answers.misc?.asRoman ?? false, - ) + const initialAsRoman = useMemo( + () => application.answers.misc?.asRoman ?? false, + [application.answers.misc?.asRoman] + ) + const [asRoman, setAsRoman] = useState<boolean>(initialAsRoman)
Line range hint
113-132: Add validation for maximum additionsThe maximum additions limit is only enforced through UI button disable. Consider adding validation to prevent programmatic additions beyond the limit.
const onAddAddition = () => { const currentAnswers = structuredClone(currentApplication.answers) let currentAdditions = getValueViaPath( currentAnswers, InputFields.advert.additions, ) if (!isAddition(currentAdditions)) { currentAdditions = [] } // TS not inferring the type after the check above if (isAddition(currentAdditions)) { + if (currentAdditions.length >= MAXIMUM_ADDITIONS_COUNT) { + console.error('Maximum additions limit reached') + return + } const newAddition = getAddition(additions.length + 1, asRoman) // ... rest of the function } }libs/application/templates/official-journal-of-iceland/src/lib/dataSchema.ts (1)
81-82: Add JSDoc comments to document the purpose of new propertiesThe new boolean properties look well-structured and maintain type safety through Zod schema. However, their purpose isn't immediately clear from the naming alone.
Consider adding JSDoc comments to improve maintainability:
const miscSchema = z .object({ signatureType: z.string().optional(), selectedTemplate: z.string().optional(), + /** Controls whether the content should be rendered as a document */ asDocument: z.boolean().optional(), + /** Controls whether to use Roman numerals for formatting */ asRoman: z.boolean().optional(), }) .partial()libs/application/templates/official-journal-of-iceland/src/components/signatures/Member.tsx (1)
Line range hint
1-28: Consider enhancing input validationThe component could benefit from additional validation:
- Input pattern for valid name characters
- Minimum length requirement
- Proper handling of special characters
type Props = { name: string label: string defaultValue?: string + minLength?: number + pattern?: string onChange: ( e: React.ChangeEvent<HTMLInputElement | HTMLTextAreaElement>, ) => void } export const SignatureMember = ({ name, label, defaultValue, + minLength = 2, + pattern = "^[\\p{L}\\s'-]+$", onChange, }: Props) => { return ( <Input name={name} label={label} size="sm" backgroundColor="blue" defaultValue={defaultValue} maxLength={100} + minLength={minLength} + pattern={pattern} onChange={onChange} /> ) }libs/application/templates/official-journal-of-iceland/src/components/input/OJOIInputController.tsx (2)
18-18: Consider adding input validation type safetyThe maxLength prop could benefit from runtime validation to ensure positive integers.
type Props = { name: string label: string | MessageDescriptor placeholder?: string | MessageDescriptor defaultValue?: string loading?: boolean applicationId: string disabled?: boolean textarea?: boolean - maxLength?: number + maxLength?: number & { __brand: 'PositiveInteger' } onChange?: (value: string) => void } +const isPositiveInteger = (value: number): value is number & { __brand: 'PositiveInteger' } => { + return Number.isInteger(value) && value > 0 +}Also applies to: 31-31
Line range hint
46-51: Consider memoizing handleChange for performanceThe handleChange function is recreated on every render. Consider memoizing it with useCallback.
- const handleChange = (value: string) => { + const handleChange = useCallback((value: string) => { const currentAnswers = structuredClone(application.answers) const newAnswers = set(currentAnswers, name, value) return newAnswers - } + }, [application.answers, name])libs/application/templates/official-journal-of-iceland/src/fields/Advert.tsx (2)
Line range hint
28-52: Consider optimizing department change handlingThe department change handler could be optimized:
- Add error handling for the lazy types query
- Debounce the department change handler
- Add loading state for type changes
+ const debouncedHandleDepartmentChange = useCallback( + debounce((value: string) => { + useLazyTypes({ + params: { + department: value, + pageSize: 100, + }, + }).catch((error) => { + console.error('Failed to fetch types:', error); + // Add error handling UI feedback here + }); + }, 300), + [useLazyTypes] + );
Line range hint
76-83: Add error handling for HTML preview generationThe title preview generation lacks error handling and could fail silently.
- const titlePreview = getAdvertMarkup({ + const titlePreview = try { + getAdvertMarkup({ type: currentApplication.answers.advert?.typeName, title: currentApplication.answers.advert?.title, - }) + }) + } catch (error) { + console.error('Failed to generate title preview:', error); + return ''; // Or a fallback preview + }libs/application/templates/official-journal-of-iceland/src/components/signatures/Institution.tsx (3)
144-144: Consider enhancing input validation feedback.While adding the
maxLengthconstraint is good, consider these improvements:
- Add a helper text to inform users about the 100-character limit
- Document why 100 characters was chosen as the limit
<Input maxLength={100} + helperText={f(signatures.inputs.institution.helperText)} name={`signature.${type}.institution${
Line range hint
41-119: Consider refactoring the handleInstitutionChange function for better maintainability.The function handles multiple responsibilities and could be split into smaller, more focused functions:
- Separate committee and regular signature handlers
- Extract HTML generation logic
- Add explicit error handling for state updates
Example refactor:
const handleRegularSignature = ( value: string, key: SignatureInstitutionKeys, signatureIndex: number, currentAnswers: any ) => { const updatedRegularSignature = signature?.map((signature, index) => index === signatureIndex ? { ...signature, [key]: value, html: getSingleSignatureMarkup( { ...signature, [key]: value }, application.answers.signatures?.additionalSignature?.regular ), } : signature ) return set( currentAnswers, InputFields.signature[type], updatedRegularSignature ) } const handleCommitteeSignature = ( value: string, key: SignatureInstitutionKeys, signature: any, currentAnswers: any ) => { const html = getSingleSignatureMarkup( { ...signature, [key]: value }, application.answers.signatures?.additionalSignature?.committee, signature.chairman ) return set(currentAnswers, InputFields.signature[type], { ...signature, html, [key]: value, }) }
Line range hint
1-25: Enhance TypeScript type safety.While the TypeScript usage is generally good, consider these improvements:
- Replace
anytypes with proper interfaces- Add return type annotations to functions
- Consider making the component a named export for better tree-shaking
Example improvements:
interface SignatureState { chairman?: string; institution: string; date: string; html: string; } interface ApplicationAnswers { signatures?: { additionalSignature?: { regular?: string; committee?: string; }; }; }libs/island-ui/core/src/lib/Hyphen/Hyphen.tsx (2)
Line range hint
15-29: Fix incorrect property assignment in minRight handlingThere's a bug in the options handling where
minRightis incorrectly assigned toleftmin.Apply this fix:
if (minLeft) { is.leftmin = minLeft } if (minRight) { - is.leftmin = minRight + is.rightmin = minRight }
Line range hint
15-29: Consider optimizing text processing performanceThe current implementation creates a new Hypher instance for each call and processes words individually.
Consider caching the Hypher instance and using array methods more efficiently:
+ const hypherInstances = { + is: new Hypher(is), + en: new Hypher(en) + }; export const hyphenateText: HyphenateText = ( content, { minLeft, minRight, locale = 'is' }, ) => { if (minLeft) { is.leftmin = minLeft } if (minRight) { is.rightmin = minRight } - const h = new Hypher(locale === 'is' ? is : en) + const h = hypherInstances[locale] const softHyphen = '\u00AD' - return content.split(' ').reduce((text, word) => { - const hyphenedWord = h.hyphenate(word).join(softHyphen) - text += ' ' + hyphenedWord - return text - }, '') + return content.split(' ') + .map(word => h.hyphenate(word).join(softHyphen)) + .join(' ') }libs/application/templates/official-journal-of-iceland/src/lib/utils.ts (1)
199-202: Consider memoizing hyphenated text for performanceMultiple calls to
hypenatewith the same input will recalculate the hyphenation unnecessarily.Consider using a memoization utility:
const memoizedHyphenate = memoize((text = '') => hyphenateText(text, { locale: 'is', minLeft: 4, minRight: 4 }) )
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (20)
apps/web/components/OfficialJournalOfIceland/OJOIAdvertCard.tsx(1 hunks)apps/web/screens/OfficialJournalOfIceland/OJOISearch.tsx(3 hunks)apps/web/screens/OfficialJournalOfIceland/hooks/useAdverts.ts(1 hunks)libs/api/domains/official-journal-of-iceland/src/lib/models/advert.input.ts(1 hunks)libs/application/templates/official-journal-of-iceland/src/components/additions/Additions.tsx(4 hunks)libs/application/templates/official-journal-of-iceland/src/components/input/OJOIInputController.tsx(3 hunks)libs/application/templates/official-journal-of-iceland/src/components/input/OJOISelectController.tsx(1 hunks)libs/application/templates/official-journal-of-iceland/src/components/signatures/Institution.tsx(1 hunks)libs/application/templates/official-journal-of-iceland/src/components/signatures/Member.tsx(1 hunks)libs/application/templates/official-journal-of-iceland/src/fields/Advert.tsx(1 hunks)libs/application/templates/official-journal-of-iceland/src/fields/Attachments.tsx(2 hunks)libs/application/templates/official-journal-of-iceland/src/fields/Publishing.tsx(1 hunks)libs/application/templates/official-journal-of-iceland/src/lib/dataSchema.ts(1 hunks)libs/application/templates/official-journal-of-iceland/src/lib/messages/attachments.ts(1 hunks)libs/application/templates/official-journal-of-iceland/src/lib/types.ts(1 hunks)libs/application/templates/official-journal-of-iceland/src/lib/utils.ts(4 hunks)libs/island-ui/core/src/lib/Header/Header.css.ts(1 hunks)libs/island-ui/core/src/lib/Header/Header.tsx(2 hunks)libs/island-ui/core/src/lib/Hyphen/Hyphen.tsx(1 hunks)libs/shared/form-fields/src/lib/SelectController/SelectController.tsx(4 hunks)
🧰 Additional context used
📓 Path-based instructions (20)
libs/application/templates/official-journal-of-iceland/src/lib/messages/attachments.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/official-journal-of-iceland/src/fields/Publishing.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."
apps/web/components/OfficialJournalOfIceland/OJOIAdvertCard.tsx (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
libs/application/templates/official-journal-of-iceland/src/lib/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/official-journal-of-iceland/src/fields/Advert.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/official-journal-of-iceland/src/components/signatures/Member.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/official-journal-of-iceland/src/components/signatures/Institution.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/island-ui/core/src/lib/Header/Header.css.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/official-journal-of-iceland/src/components/input/OJOISelectController.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/official-journal-of-iceland/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."
apps/web/screens/OfficialJournalOfIceland/hooks/useAdverts.ts (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
libs/application/templates/official-journal-of-iceland/src/components/input/OJOIInputController.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/island-ui/core/src/lib/Header/Header.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/island-ui/core/src/lib/Hyphen/Hyphen.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/official-journal-of-iceland/src/components/additions/Additions.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/shared/form-fields/src/lib/SelectController/SelectController.tsx (2)
Pattern libs/shared/**/*: "Confirm that the code adheres to the following:
- Cross-application utility functions, types, and constants.
- TypeScript best practices to ensure type safety and reusability.
- Documentation and examples for library consumers."
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/official-journal-of-iceland/src/lib/utils.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."
apps/web/screens/OfficialJournalOfIceland/OJOISearch.tsx (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
libs/application/templates/official-journal-of-iceland/src/fields/Attachments.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/api/domains/official-journal-of-iceland/src/lib/models/advert.input.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 (19)
libs/application/templates/official-journal-of-iceland/src/lib/messages/attachments.ts (1)
37-37: LGTM! Improved terminology consistency.
The updated message 'Bæta við viðauka' aligns better with other button labels in the file, creating a more consistent user experience.
libs/shared/form-fields/src/lib/SelectController/SelectController.tsx (3)
4-9: LGTM: Clean import restructuring
The imports have been properly organized to include SelectProps type.
34-34: LGTM: Type-safe prop addition
The filterConfig prop is properly typed using SelectProps type, maintaining type safety and making it optional.
56-56: LGTM: Proper prop forwarding
The filterConfig prop is correctly destructured and passed to the underlying Select component.
Also applies to: 101-101
libs/application/templates/official-journal-of-iceland/src/components/input/OJOISelectController.tsx (1)
81-82: LGTM: Enhanced search functionality
The addition of search capability with start-of-string matching improves user experience when filtering options.
libs/application/templates/official-journal-of-iceland/src/fields/Publishing.tsx (1)
130-132: LGTM: Consistent filtering behavior
The filterConfig implementation matches the pattern used in OJOISelectController.
libs/island-ui/core/src/lib/Header/Header.tsx (3)
132-135: LGTM! Improved layout control with Box components.
The replacement of Inline with Box components provides better layout control. The use of flexShrink: 0 ensures the logo maintains its size, while nowrap prevents unwanted content wrapping.
136-139: LGTM! Consistent layout approach for header items.
The header items layout follows the same Box-based approach, ensuring consistent spacing and preventing unwanted shrinking of the elements.
96-96: LGTM! Balanced padding for scrollable content.
The addition of responsive paddingRight ensures proper spacing when content is scrollable, matching the marginLeft values for visual balance.
apps/web/screens/OfficialJournalOfIceland/hooks/useAdverts.ts (1)
23-24: Verify date string format handling across the application
The type change from Date to string for dateFrom and dateTo aligns with the GraphQL schema changes. However, we should ensure consistent date string format handling.
Consider adding a type guard or validation function to ensure date strings are in the correct format before they reach the GraphQL layer.
✅ Verification successful
Let me gather more information about the date handling in the codebase to verify the impact of the type changes.
Date handling is properly implemented for the type change
The type change from Date to string in useAdverts.ts is safe because:
- The
formatDateutility inOJOIUtils.tsalready expects string input and handles undefined values - The component properly converts dates using
toISOString().split('T')[0]format before sending to the GraphQL layer - Date validation is present through the DatePicker component's built-in constraints (
minDate,maxDate) - The codebase consistently handles date strings in ISO format for API interactions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for date format handling and validation
rg -A 2 "formatDate|parseDate|new Date\(" apps/web/screens/OfficialJournalOfIceland/
Length of output: 4283
Script:
#!/bin/bash
# Search for the formatDate utility implementation and its usages
rg -A 3 "export.*formatDate" apps/web/components/
# Search for date-related types and interfaces
ast-grep --pattern 'interface $_ {
$$$
date: $_
$$$
}'
# Check for any date validation utilities
rg -A 2 "validateDate|isValidDate|dateValidation"
Length of output: 15717
apps/web/screens/OfficialJournalOfIceland/OJOISearch.tsx (2)
591-592: LGTM! Date handling is properly validated
The server-side implementation correctly uses the validated and formatted date strings, maintaining consistency with the client-side changes and GraphQL schema.
108-109: LGTM! Verify date format consistency
The change to use string dates directly in the GraphQL query is correct and aligns with the backend DTO changes.
Let's verify the date format consistency across the codebase:
✅ Verification successful
Date format consistency verified
The change to use string dates directly is correct and consistent with the codebase:
- The
dagsFraanddagsTilparameters are explicitly typed as string in the interface - The date handling in
getPropsensures ISO date format (YYYY-MM-DD) by usingtoISOString().split('T')[0] - The
useAdvertshook accepts and passes these string dates directly to the GraphQL query - The DatePicker components handle the date-string conversion appropriately when updating state
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check date format handling in related files
# Expected: ISO date string format (YYYY-MM-DD)
# Check date handling in related files
rg -A 3 'dateFrom|dateTo' --type typescript
Length of output: 80
Script:
#!/bin/bash
# Let's try with different approaches to find date-related patterns
# 1. Search for date-related fields in all text files
rg -A 3 'dateFrom|dateTo|dagsFra|dagsTil'
# 2. Search specifically for defaultSearchParams to understand the date format
rg -A 3 'defaultSearchParams'
# 3. Look for any date format patterns
rg -A 3 '\d{4}-\d{2}-\d{2}|Date\('
Length of output: 68615
Script:
#!/bin/bash
# Let's look specifically at the OfficialJournalOfIceland related files and date handling
rg -A 5 'defaultSearchParams|dagsFra|dagsTil' apps/web/screens/OfficialJournalOfIceland/
# Also check the useAdverts hook implementation
ast-grep --pattern 'const useAdverts = $_' apps/web/screens/OfficialJournalOfIceland/
Length of output: 15924
libs/application/templates/official-journal-of-iceland/src/lib/types.ts (1)
37-38: LGTM! Type definitions are well-structured
The new fields asDocument and asRoman are properly integrated into the InputFields object, maintaining type safety and following existing naming conventions.
libs/application/templates/official-journal-of-iceland/src/components/additions/Additions.tsx (1)
98-109: 🛠️ Refactor suggestion
Optimize state updates to reduce operations
The state update logic involves multiple operations that could be optimized.
- let updatedAnswers = set(
+ const updatedAnswers = set(
currentAnswers,
- InputFields.advert.additions,
- updatedAdditions,
+ '',
+ {
+ ...currentAnswers,
+ advert: {
+ ...currentAnswers.advert,
+ additions: updatedAdditions
+ },
+ misc: {
+ ...currentAnswers.misc,
+ asRoman: val
+ }
+ }
)
- updatedAnswers = set(updatedAnswers, InputFields.misc.asRoman, val)
setAsRoman(val)
setValue(InputFields.advert.additions, updatedAdditions)
setValue(InputFields.misc.asRoman, val)
updateApplication(updatedAnswers)Likely invalid or redundant comment.
libs/application/templates/official-journal-of-iceland/src/lib/dataSchema.ts (1)
81-82: Schema changes look good and maintain compatibility
The new optional boolean properties are well-integrated into the existing schema:
- Maintains backward compatibility through optional fields
- Follows TypeScript best practices
- Enables proper tree-shaking
- Preserves existing validation logic
libs/application/templates/official-journal-of-iceland/src/components/signatures/Member.tsx (1)
25-25: Verify if 100 characters is an appropriate limit for names
While adding a maxLength constraint is good practice, verify if 100 characters is the appropriate limit based on:
- Legal requirements for official documents
- Database constraints
- UI/UX considerations
✅ Verification successful
100 characters is an appropriate limit for signature names
Based on the codebase analysis:
- The 100-character limit is consistently used for institutional and member signatures in the official-journal-of-iceland template
- Other name-related inputs in the system use much lower limits (e.g., 50 characters for machine names)
- For official documents and forms, the longest text input limits found are:
- 600 characters for advertisement titles
- 220 characters for organ donation text
- 250 characters for institutional constraints
- 100 characters provides ample space for full names while maintaining reasonable UI constraints
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other name input constraints in the codebase
rg -g '*.{ts,tsx}' 'maxLength.*name' --type-add 'tsx:*.{ts,tsx}'
Length of output: 66
Script:
#!/bin/bash
# Let's try a broader search for maxLength constraints in input fields
rg -g '*.{ts,tsx}' 'maxLength=\{?\d+\}?' -A 2 -B 2
# Also search for any validation schemas or constants related to name fields
rg -g '*.{ts,tsx}' '(name.*validation|NAME.*LENGTH|nameLength|NAME_MAX)'
# Search for Input component usage with name-related props
ast-grep --pattern 'Input name="$_" $$$'
Length of output: 19193
libs/application/templates/official-journal-of-iceland/src/fields/Advert.tsx (1)
108-108: Verify if 600 characters is appropriate for advertisement titles
While adding a maxLength constraint is good practice, verify if 600 characters is the appropriate limit based on:
- Database constraints
- UI display limitations
- Official journal requirements
✅ Verification successful
Based on the search results, I can now generate the final response since we have gathered sufficient context about advertisement titles in the official journal system. While we don't see explicit validation rules defining the 600-character limit, we can see that the title field is used for "Heiti auglýsingar" (Advertisement Title) and is a core part of the application template.
600 characters appears to be a reasonable limit for advertisement titles
The maxLength of 600 characters is appropriate for advertisement titles in the Official Journal of Iceland because:
- It provides ample space for descriptive titles while preventing excessive length
- The title field is used as "Heiti auglýsingar (án tegundar)" - Advertisement title without type
- The UI displays titles in a structured format with separate type/category and subtitle sections
- No conflicting length constraints were found in the codebase
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for title length constraints in similar components
rg -g '*.{ts,tsx}' 'maxLength.*title' --type-add 'tsx:*.{ts,tsx}'
Length of output: 67
Script:
#!/bin/bash
# Search for maxLength constraints in form fields
rg -g '*.{ts,tsx}' 'maxLength=' -B 2 -A 2
# Search for title-related validation or schema definitions
rg -g '*.{ts,tsx,graphql}' -i 'title.*length|title.*validation|title.*schema' -B 2 -A 2
# Look for any database schema or validation files
fd -e schema.ts -e validation.ts -e model.ts -x cat {}
Length of output: 65809
Script:
#!/bin/bash
# Search for title-related fields in GraphQL schemas or type definitions
rg -g '*.graphql' -g '*.ts' 'title.*String|title:.*String|title:.*{' -B 2 -A 2
# Look for any validation rules or constraints in the official journal template
fd -g "*official-journal*.ts" -x cat {}
Length of output: 2514
Script:
#!/bin/bash
# Search for advertisement title constraints in GraphQL schema or API definitions
rg -g '*.{ts,graphql}' -i 'advert.*title|advertisement.*title' -B 2 -A 2
# Look for validation rules in the official journal template
fd -g "*official-journal*.validation.ts" -g "*official-journal*.schema.ts" -g "*official-journal*.model.ts" -x cat {}
# Search for any maxLength usage in official journal related files
rg -g '*official-journal*.{ts,tsx}' 'maxLength.*600|maxLength:.*600|maxLength=.*600' -B 2 -A 2
Length of output: 10092
libs/application/templates/official-journal-of-iceland/src/components/signatures/Institution.tsx (1)
Line range hint 26-31: LGTM! Well-structured component with proper TypeScript definitions.
The component follows React best practices with clear prop definitions and proper use of TypeScript interfaces. The modular structure promotes reusability across different NextJS applications.
Also applies to: 33-39
libs/island-ui/core/src/lib/Hyphen/Hyphen.tsx (1)
15-15: LGTM! Exporting utility function improves reusability
The export of hyphenateText aligns with the core library's goal of providing reusable utilities across different NextJS apps.
libs/api/domains/official-journal-of-iceland/src/lib/models/advert.input.ts
Show resolved
Hide resolved
libs/application/templates/official-journal-of-iceland/src/lib/utils.ts
Outdated
Show resolved
Hide resolved
libs/application/templates/official-journal-of-iceland/src/lib/utils.ts
Outdated
Show resolved
Hide resolved
Datadog ReportAll test runs ✅ 32 Total Test Services: 0 Failed, 30 Passed Test ServicesThis report shows up to 10 services
|
…island.is into fix/ojoi-advert-types
|
This PR currently has a merge conflict. Please resolve this and then re-add the |
oddsson
added
automerge
* Updating select inputs to filter from the start. * Updated the UI header component to support larger titles. * Signatures are now hyphenated and max length for each field set to 100. * Fixed state issue when users changed the type of additions * Fixed 0 rendering in an conditional check * Fixed issue where date parameters were not properly working. * Fix typo --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
What
OJOI Application
Application system
SelectControllercomponentWeb
UI Core
hyphenateTextmethod from theHyphencomponentWhy
To fix bugs in the application and web project and to improve user and dev experience
Checklist:
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
These changes enhance user experience and improve the overall functionality of the application.