fix(locales): source materials issues for tagalog language #3178
Conversation
WalkthroughThis pull request introduces multiple new React components and hooks to facilitate import and export functionality in the application. Key additions include three SVG icon components ( Changes
Possibly related PRs
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
CodeRabbit Configuration File (
|
✅ Deploy Preview for staging-organized ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Outside diff range and nitpick comments (15)
src/pages/meetings/schedules/useWeeklySchedules.tsx (1)
18-26: Consider adding 'scheduleType' to the dependency array of useMemoThe
useMemohook depends onscheduleType, but the dependency array is empty. To ensurevalueupdates whenscheduleTypechanges, include it in the dependency array.Apply this diff to address the issue:
const value = useMemo(() => { if (!scheduleType) return 0; if (scheduleType === 'midweek') return 0; if (scheduleType === 'weekend') return 1; if (scheduleType === 'outgoing') return 2; - }, []); + }, [scheduleType]);src/services/app/sources.ts (1)
107-113: Refactor repeated logic for 'assType' assignmentThe conditional blocks assigning
assTypebased onsource_lang,obj.weekOf, andassTypeare duplicated. Consider extracting this logic into a helper function to reduce code duplication and improve maintainability.Example refactor:
+ const adjustAssType = (assType: number): number => { + if (source_lang === 'TG' && obj.weekOf >= '2024/01/01' && assType === 102) { + return 124; + } + return assType; + }; + // Within each assignment block assType = assTypeList.find((type) => type.label === src.mwb_ayf_part1_type)?.value || 127; - if ( - source_lang === 'TG' && - obj.weekOf >= '2024/01/01' && - assType === 102 - ) { - assType = 124; - } + assType = adjustAssType(assType);Also applies to: 127-133, 148-154, 169-175
src/features/congregation/settings/import_export/index.types.ts (1)
1-4: Add JSDoc documentation for the type definition.The type definition is well-structured, but adding documentation would improve maintainability.
+/** + * Props type for the ImportExport component + * @property open - Controls the visibility of the import/export dialog + * @property onClose - Callback function to handle dialog close action + */ export type ImportExportType = { open: boolean; onClose: VoidFunction; };src/pages/congregation/settings/useCongregationSettings.tsx (1)
6-8: Consider memoizing handler functions.The handler functions could be memoized using useCallback to prevent unnecessary re-renders in child components that receive these functions as props.
- const handleOpenExchange = () => setIsDataExchangeOpen(true); + const handleOpenExchange = useCallback(() => setIsDataExchangeOpen(true), []); - const handleCloseExchange = () => setIsDataExchangeOpen(false); + const handleCloseExchange = useCallback(() => setIsDataExchangeOpen(false), []);Don't forget to import useCallback:
-import { useState } from 'react'; +import { useState, useCallback } from 'react';src/features/congregation/settings/import_export/useImportExport.tsx (1)
15-15: Define translation keys as constants.Hardcoded translation keys can lead to maintenance issues. Consider defining them as constants.
+const TRANSLATION_KEYS = { + EXPORT: 'tr_export', + IMPORT: 'tr_import', +} as const; const tabs = useMemo(() => { return [ { - label: t('tr_export'), + label: t(TRANSLATION_KEYS.EXPORT), component: Export, }, { - label: t('tr_import'), + label: t(TRANSLATION_KEYS.IMPORT), component: Import, }, ]; }, [t, onClose]);Also applies to: 19-19
src/features/congregation/settings/import_export/index.tsx (2)
15-15: Consider using theme spacing instead of hardcoded padding.Replace the hardcoded padding with theme spacing for consistency and maintainability.
-<Dialog onClose={props.onClose} open={props.open} sx={{ padding: '24px' }}> +<Dialog onClose={props.onClose} open={props.open} sx={{ padding: (theme) => theme.spacing(3) }}>
19-21: Consider using an enum or constant for tab values.Using magic numbers (0) for tab comparison makes the code less maintainable. Consider defining an enum for tab values.
+enum TabValue { + EXPORT = 0, + IMPORT = 1, +} // Then use it in the comparison -{value === 0 ? t('tr_exportDesc') : t('tr_importDesc')} +{value === TabValue.EXPORT ? t('tr_exportDesc') : t('tr_importDesc')}src/features/congregation/settings/import_export/import/useImport.tsx (1)
36-42: Consider extracting configuration constants.File size and type restrictions should be defined as named constants for better maintainability.
+const MAX_FILE_SIZE = 20 * 1024 * 1024; // 20MB +const ACCEPTED_FILE_TYPES = { 'text/json': ['.json'] }; +const MAX_FILES = 1; const { getRootProps, getInputProps } = useDropzone({ onDrop, - accept: { 'text/json': ['.json'] }, - maxFiles: 1, - maxSize: 20971520, + accept: ACCEPTED_FILE_TYPES, + maxFiles: MAX_FILES, + maxSize: MAX_FILE_SIZE, multiple: false, });src/pages/congregation/settings/index.tsx (1)
23-25: Consider lazy loading the ImportExport dialog.Since the ImportExport dialog is only shown when needed, consider lazy loading it to improve initial bundle size and page load performance.
-import ImportExport from '@features/congregation/settings/import_export'; +const ImportExport = React.lazy(() => import('@features/congregation/settings/import_export')); // In the render: {isDataExchangeOpen && ( + <React.Suspense fallback={<CircularProgress />}> <ImportExport open={isDataExchangeOpen} onClose={handleCloseExchange} /> + </React.Suspense> )}src/services/dexie/weekType.ts (1)
13-17: Consider enhancing language fallback mechanismThe current implementation provides basic language fallback to English, but could be more robust by:
- Validating the language code format
- Using a constant for the default language
- Handling potential localStorage errors
-const language = localStorage.getItem('ui_lang') || 'en'; -const languages = [language]; - -if (!languages.includes('en')) languages.push('en'); +const DEFAULT_LANGUAGE = 'en'; +let language; +try { + language = localStorage.getItem('ui_lang') || DEFAULT_LANGUAGE; + // Basic validation of language code format + if (!/^[a-z]{2}(-[A-Z]{2})?$/.test(language)) { + language = DEFAULT_LANGUAGE; + } +} catch (error) { + language = DEFAULT_LANGUAGE; +} +const languages = [language]; +if (language !== DEFAULT_LANGUAGE) { + languages.push(DEFAULT_LANGUAGE); +}src/components/icons/IconImportExport.tsx (2)
3-17: Props interface looks good, consider minor improvementsThe props interface is well-defined with appropriate types. Consider these minor enhancements:
- Add prop documentation
- Consider making className concatenation more robust
type IconProps = { + /** Custom color for the icon */ color?: string; + /** Width in pixels */ width?: number; + /** Height in pixels */ height?: number; + /** MUI System props */ sx?: SxProps<Theme>; + /** Additional CSS class names */ className?: string; };
19-49: Improve className handling and SVG optimizationThe component implementation is solid, but could benefit from:
- More robust className handling
- SVG optimization
- className={`organized-icon-import-export ${className}`} + className={`organized-icon-import-export${className ? ` ${className}` : ''}`} sx={{ width: `${width}px`, height: `${height}px`, ...sx }} >Consider running the SVG through an optimizer to reduce the path complexity while maintaining visual quality.
src/components/icons/IconImportJson.tsx (1)
12-12: Standardize color handling across icon componentsThe color handling differs between IconImportJson and IconImportExport:
- IconImportExport uses a default color prop
- IconImportJson falls back to CSS variable
Standardize the approach:
- color, + color = 'var(--accent-dark)', width = 24, // ... <path - fill={color || 'var(--accent-dark)'} + fill={color}Also applies to: 44-44
src/components/icons/IconBackupOrganized.tsx (1)
19-22: Consider memoizing className concatenation.The className template literal is recreated on every render. Consider memoizing it with useMemo for better performance in high-frequency updates.
- className={`organized-icon-backup-organized ${className}`} + className={useMemo(() => `organized-icon-backup-organized ${className || ''}`, [className])}src/features/congregation/settings/import_export/export/useExport.tsx (1)
68-68: Consider moving filter logic to the selector level.The persons filter could be moved to a Recoil selector for better performance and reusability.
- persons: persons.filter((record) => !record._deleted.value), + persons: useRecoilValue(personsActiveState), // Create a new selector
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (8)
converter/svg/sources/name=backup-organized.svgis excluded by!**/*.svg,!**/*.svgconverter/svg/sources/name=import-export.svgis excluded by!**/*.svg,!**/*.svgconverter/svg/sources/name=import-json.svgis excluded by!**/*.svg,!**/*.svgpackage-lock.jsonis excluded by!**/package-lock.json,!**/*.jsonpackage.jsonis excluded by!**/*.jsonsrc/locales/en/congregation.jsonis excluded by!**/*.jsonsrc/locales/en/errors.jsonis excluded by!**/*.jsonsrc/locales/en/general.jsonis excluded by!**/*.json
📒 Files selected for processing (21)
src/components/icons/IconBackupOrganized.tsx(1 hunks)src/components/icons/IconImportExport.tsx(1 hunks)src/components/icons/IconImportJson.tsx(1 hunks)src/components/icons/index.ts(3 hunks)src/features/congregation/settings/import_export/export/index.tsx(1 hunks)src/features/congregation/settings/import_export/export/index.types.ts(1 hunks)src/features/congregation/settings/import_export/export/useExport.tsx(1 hunks)src/features/congregation/settings/import_export/import/index.tsx(1 hunks)src/features/congregation/settings/import_export/import/index.types.ts(1 hunks)src/features/congregation/settings/import_export/import/useImport.tsx(1 hunks)src/features/congregation/settings/import_export/index.tsx(1 hunks)src/features/congregation/settings/import_export/index.types.ts(1 hunks)src/features/congregation/settings/import_export/useImportExport.tsx(1 hunks)src/pages/congregation/settings/index.tsx(1 hunks)src/pages/congregation/settings/useCongregationSettings.tsx(1 hunks)src/pages/meetings/schedules/index.tsx(1 hunks)src/pages/meetings/schedules/useWeeklySchedules.tsx(2 hunks)src/services/app/sources.ts(2 hunks)src/services/dexie/assignment.ts(1 hunks)src/services/dexie/weekType.ts(1 hunks)src/services/worker/backupUtils.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/features/congregation/settings/import_export/export/index.types.ts
🔇 Additional comments (10)
src/services/dexie/assignment.ts (1)
Line range hint 40-183: Language handling logic is correctly updated
The changes to process only the user's language and English improve efficiency and ensure relevant translations are loaded.
src/services/app/sources.ts (1)
64-67: Verify date comparison logic for consistency
Ensure that obj.weekOf and mondayDate are in the same format and time zone to prevent any mismatches due to formatting or locale differences.
Run the following script to check the date formats:
Also applies to: 69-70
✅ Verification successful
Let me gather more information about the date formatting implementation and usage.
Let me check the implementation of the date formatting functions to verify the consistency.
Based on the code analysis, I can now provide a final response:
Date formatting is consistent and safe
The code shows that:
obj.weekOfcomes from source data and is in 'yyyy/MM/dd' formatgetWeekDate()returns a Date object that is consistently formatted to 'yyyy/MM/dd' usingformatDate()- The comparison is done after ensuring both dates are in the same 'yyyy/MM/dd' format
- The
formatDatefunction uses date-fns library which handles timezone conversions properly
The date comparison logic is implemented correctly with consistent formatting and timezone handling.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that 'obj.weekOf' and 'mondayDate' have consistent formats.
# Expected format: 'yyyy/MM/dd'
echo "obj.weekOf sample value:"
grep -r "obj.weekOf =" src/ | head -n 1
echo "Format of 'mondayDate':"
grep -r "formatDate(" src/ | grep "mondayDate" | head -n 1
Length of output: 385
Script:
#!/bin/bash
# Check the implementation of formatDate and getWeekDate functions
ast-grep --pattern 'function formatDate($_) { $$$ }'
ast-grep --pattern 'function getWeekDate($_) { $$$ }'
# Check how weekOf is being used in the codebase
rg "weekOf" -A 2
# Check the type definitions related to weekOf
rg "weekOf.*:" -A 1
Length of output: 74864
Script:
#!/bin/bash
# Search for getWeekDate implementation
rg "getWeekDate" -B 2 -A 5
# Search for formatDate implementation
rg "formatDate" -B 2 -A 5
# Check how obj.weekOf is being used in comparison
rg "obj\.weekOf.*===" -A 2
rg "obj\.weekOf.*>=" -A 2
Length of output: 70914
src/features/congregation/settings/import_export/import/index.types.ts (1)
1-3: Type definition for 'ImportType' is correctly implemented
The ImportType interface appropriately defines the onClose callback, enhancing type safety for the Import component.
src/features/congregation/settings/import_export/useImportExport.tsx (1)
8-8: Add error handling for translation failures.
The translation function could fail if keys are missing. Consider adding error handling.
- const { t } = useAppTranslation();
+ const { t, ready } = useAppTranslation();
+ if (!ready) {
+ return { tabs: [], value: 0, handleTabChange: () => {} };
+ }src/pages/meetings/schedules/index.tsx (1)
10-10: LGTM! Clean refactor moving complexity to the hook.
The changes simplify the component by delegating tab management to the useWeeklySchedules hook, improving maintainability.
src/features/congregation/settings/import_export/import/useImport.tsx (1)
1-47: Verify PR objective alignment.
This file introduces import functionality, but the PR title suggests it's meant to fix Tagalog language source materials issues. Please clarify if this change is related to the PR objective.
src/services/dexie/weekType.ts (1)
19-50: Verify translation key consistency
The translation key pattern is consistent across all week types. However, let's verify that all keys exist in the translation files to prevent runtime errors.
✅ Verification successful
Let me search for the actual translation files and their content since the previous search might have missed some locations.
All translation keys are present and properly translated
All required translation keys are found in the locale files under src/locales/*/forms-templates.json and src/locales/*/meetings.json. Each language has proper translations for:
- tr_normalWeek
- tr_circuitOverseerWeek
- tr_conventionWeek
- tr_assemblyWeek
- tr_memorialWeek
- tr_specialTalkWeek
- tr_noMeeting
- tr_noMeetingWeek
The translation key pattern is consistent and all keys exist in the translation files, preventing any potential runtime errors.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify translation keys exist in translation files
# Find all translation files
fd -e json -e ts --full-path "translations|i18n" | while read -r file; do
echo "Checking $file for translation keys..."
# Search for translation keys
rg -q "tr_(normalWeek|circuitOverseerWeek|assemblyWeek|conventionWeek|memorialWeek|specialTalkWeek|noMeeting)" "$file" || echo "Missing keys in $file"
done
Length of output: 1990
Script:
#!/bin/bash
# First, let's find all JSON files that might contain translations
fd -e json | while read -r file; do
echo "=== Checking $file ==="
# Look for any of these translation keys
rg "tr_(normalWeek|circuitOverseerWeek|assemblyWeek|conventionWeek|memorialWeek|specialTalkWeek|noMeeting)" "$file" || echo "No matches found"
done
Length of output: 111267
src/components/icons/IconBackupOrganized.tsx (1)
3-9: LGTM! Well-structured props interface.
The props interface is well-defined with appropriate optional properties for customization.
src/components/icons/index.ts (1)
24-24: LGTM! Consistent export pattern maintained.
The new exports follow the established pattern and maintain alphabetical ordering.
Also applies to: 124-126, 194-194
src/services/worker/backupUtils.ts (1)
854-862: LGTM: Preserving local type configurations during restore.
The code correctly ensures that local type values for Apply Yourself to the Field Ministry (AYF) parts are preserved during the restore process, which helps maintain language-specific configurations.
Let's verify the structure and usage of these type configurations:
✅ Verification successful
The search results show that the type property in AYF parts is used extensively throughout the codebase for determining assignment types, timing calculations, and UI rendering. The changes in backupUtils.ts correctly preserve these type values during restore operations, which is crucial for maintaining language-specific configurations.
LGTM: Type preservation is correctly implemented and necessary
The code change is verified to be correct and necessary because:
- The
typeproperty is used to determine assignment types (e.g., discussions, explaining beliefs) which can vary by language - These types affect meeting timing calculations and UI rendering across multiple components
- The changes ensure that language-specific configurations aren't overwritten during restore operations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check the structure and usage of AYF type configurations
# Search for type property usage in AYF parts
ast-grep --pattern 'midweek_meeting: {
$$$
ayf_part1: {
$$$
type: $_
$$$
}
$$$
}'
# Search for any other restore operations that might need similar type preservation
rg -A 5 "midweek_meeting.*type"
Length of output: 19772
organized-app
|
||||||||||||||||||||||||||||
| Project |
organized-app
|
| Branch Review |
main
|
| Run status |
|
| Run duration | 00m 05s |
| Commit |
|
| Committer | rhahao |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
1
|
| View all changes introduced in this branch ↗︎ | |
|
🎉 This PR is included in version 3.4.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
No description provided.