Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(app): add hotfixes #3173

Merged
merged 16 commits into from
Dec 8, 2024
Merged

fix(app): add hotfixes #3173

merged 16 commits into from
Dec 8, 2024

Conversation

rhahao
Copy link
Member

@rhahao rhahao commented Dec 8, 2024

No description provided.

Copy link
Contributor

coderabbitai bot commented Dec 8, 2024

Caution

Review failed

The pull request is closed.

Walkthrough

This pull request includes a variety of changes across multiple files, primarily focusing on simplifying logic, enhancing data handling, and improving localization features. Key modifications involve the removal of unnecessary configurations, updates to hooks for better state management, and enhancements to report handling in the database. Additionally, improvements to language handling and translation mechanisms are introduced, alongside adjustments to user interface components for better clarity and functionality.

Changes

File Change Summary
.firebaserc Deleted file containing Firebase project configuration settings.
src/RootWrap.tsx Simplified font retrieval logic by directly assigning font from local storage with a fallback to 'Inter'.
src/constants/index.ts Updated LANGUAGE_LIST for Tagalog by adding an identifier and corrected the locale for Hungarian.
src/definition/ministry.ts Removed report_id property from IncomingReport type.
src/features/app_notification/container/useContainer.tsx Removed imports for congAccessCodeState and congMasterKeyState, renamed isPending to isFetching, and updated logic to fetch settings from appDb.
src/features/app_start/shared/hooks/useFeedback.tsx Enhanced error handling in hideMessage function by adding a check for the existence of onboardingError element.
src/features/app_start/vip/congregation_create/congregation_access_code/useCongregationAccessCode.tsx Added new property 'cong_settings.cong_new': true in database update operation.
src/features/app_start/vip/congregation_create/congregation_details/useCongregationDetails.tsx Added new property 'cong_settings.cong_new': true in database update operation.
src/features/app_start/vip/congregation_create/congregation_master_key/useCongregationMasterKey.tsx Added new property 'cong_settings.cong_new': true in database update operation.
src/features/app_start/vip/startup/index.tsx Restructured rendering logic to group conditions under a loading check, improving control flow.
src/features/app_start/vip/startup/useStartup.tsx Updated condition for checking congAccessCode length to include a check for congName.
src/features/language_switcher/useLanguage.tsx Removed useSearchParams and Recoil state imports, simplifying language change handling by directly updating localStorage.
src/features/meetings/weekly_schedules/midweek_meeting/useMidweekMeeting.tsx Introduced hour24FormatState for time format handling and updated partTimings logic accordingly.
src/features/meetings/weekly_schedules/weekend_meeting/useWeekendMeeting.tsx Added hour24FormatState for time format handling and updated partTimings logic accordingly.
src/features/ministry/report/publisher_report/monthly_report/submit_report/useSubmitReport.tsx Enhanced report submission logic with type safety and improved handling of report existence.
src/features/ministry/report/publisher_report/monthly_report/withdraw_report/useWithdrawReport.tsx Updated report creation logic to check for existing reports and retrieve congReport if necessary.
src/features/persons/spiritual_status/baptized_publisher/useBaptizedPublisher.tsx Modified logic for handling baptism dates and history records.
src/features/reminders/useReminders.tsx Refactored reminder-checking functions and consolidated logic into useEffect.
src/features/reports/hooks/useCongregationCard.tsx Replaced currentReportMonth utility with currentServiceYear for year calculations.
src/features/whats_new/useWhatsNew.tsx Enhanced localization handling by using LANGUAGE_LIST for release notes.
src/hooks/useAppTranslation.tsx Updated to dynamically select language from local storage.
src/hooks/useUserAutoLogin.tsx Removed backupAuto variable and simplified message posting logic.
src/pages/ministry/ministry_report/index.tsx Renamed status variable to reportStatus in the MinistryReport component.
src/pages/ministry/ministry_report/useMinistryReport.tsx Added monthlyReports and reportStatus for improved report handling.
src/services/app/index.ts Added dbRemoveDuplicateReports function and updated local storage management logic.
src/services/app/schedules.ts Integrated hour24FormatState for time formatting in scheduling functions.
src/services/dexie/app.ts Removed localStorage.clear(); from dbAppDelete function.
src/services/dexie/cong_field_service_reports.ts Enhanced report handling logic in dbHandleIncomingReports and added dbRemoveDuplicateReports function.
src/services/i18n/index.ts Simplified application language retrieval and updated languages structure.
src/services/i18n/public_talks.ts Modified publicTalksBuildList function to use identifiers for fetching translations.
src/services/i18n/songs.ts Modified songsBuildList function to use identifiers for fetching translations.
src/services/i18n/translation.ts Updated getTranslation function to handle language identifiers dynamically.
src/services/worker/backupUtils.ts Enhanced backup and restoration logic with refined criteria for report inclusion and user role checks.
src/states/app.ts Removed getAppLang function and streamlined language state management.
src/views/reports/S21/shared/TableHeader.tsx Modified translation rendering logic to replace hyphens with hyphen followed by a newline.
src/utils/dev.ts Added first_report property to person_data for handling baptism statuses.

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 4ba5a97 and bf56c1e.

📒 Files selected for processing (1)
  • src/utils/dev.ts (2 hunks)

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

netlify bot commented Dec 8, 2024

Deploy Preview for staging-organized ready!

Name Link
🔨 Latest commit bf56c1e
🔍 Latest deploy log https://app.netlify.com/sites/staging-organized/deploys/67558fe6070399000893dba6
😎 Deploy Preview https://deploy-preview-3173--staging-organized.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

This was linked to issues Dec 8, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 11

🧹 Outside diff range and nitpick comments (13)
src/features/meetings/weekly_schedules/midweek_meeting/useMidweekMeeting.tsx (1)

173-189: LGTM! Consider extracting time format logic to a utility function.

The time format handling logic is correct and properly integrated with the existing code. However, since this logic is also used in the weekend meeting hook, consider extracting it to a shared utility function to promote code reuse and maintainability.

// Create a new utility function in @utils/date.ts
+export const formatTimeByPreference = (time: string, use24: boolean): string => {
+  if (!use24) {
+    const date = generateDateFromTime(time);
+    return formatDate(date, 'h:mm');
+  }
+  return time;
+};

// Then use it in both hooks
-if (!use24) {
-  const date = generateDateFromTime(pgmStart);
-  meetingStart = formatDate(date, 'h:mm');
-}
+meetingStart = formatTimeByPreference(pgmStart, use24);
src/services/app/schedules.ts (1)

Line range hint 1-2500: File is too large and should be split into smaller modules.

This file is over 2000 lines long and handles multiple concerns. Consider splitting it into smaller, focused modules:

  • schedulesMidweek.ts: Midweek meeting related functions
  • schedulesWeekend.ts: Weekend meeting related functions
  • schedulesHistory.ts: Assignment history related functions
  • schedulesAutofill.ts: Auto-fill related functions
  • schedulesCommon.ts: Shared utilities and types

This would improve maintainability, readability, and testability.

src/services/dexie/cong_field_service_reports.ts (1)

175-177: Consider using Date objects for accurate date comparison

In the sorting of month.reports to find the least recently updated report, using localeCompare on date strings may not handle all cases reliably. Consider parsing the updatedAt strings into Date objects and comparing them numerically for more accurate results.

Apply this diff to improve date comparison:

- .sort((a, b) =>
-   a.report_data.updatedAt.localeCompare(b.report_data.updatedAt)
- )
+ .sort((a, b) =>
+   new Date(a.report_data.updatedAt) - new Date(b.report_data.updatedAt)
+ )
src/services/i18n/public_talks.ts (1)

5-5: Add type safety for the language parameter

Consider adding type safety to ensure the language parameter matches the expected format.

-export const publicTalksBuildList = (language: string) => {
+type SupportedLanguage = typeof LANGUAGE_LIST[number]['locale'];
+export const publicTalksBuildList = (language: SupportedLanguage) => {
src/features/app_start/shared/hooks/useFeedback.tsx (2)

Line range hint 21-25: Add null check to showMessage function

The showMessage function directly accesses onboardingError without checking if it exists, which could cause runtime errors.

Apply this fix:

  const showMessage = () => {
+   if (!onboardingError) return;
    onboardingError.style.opacity = '0';
    onboardingError.style.display = 'block';
    onboardingError.style.animation = 'fade-in 0.15s forwards';
  };

Line range hint 11-25: Consider refactoring DOM manipulation to use React refs

Direct DOM manipulation using document.querySelector is an anti-pattern in React. Consider using refs instead.

Here's a suggested refactor:

const useFeedback = () => {
  const title = useRecoilValue(onboardingTitleState);
  const message = useRecoilValue(onboardingMessageState);
  const variant = useRecoilValue(onboardingVariantState);
  const errorRef = useRef<HTMLElement>(null);

  const hideMessage = () => {
    if (errorRef.current) {
      errorRef.current.style.animation = 'fade-out 0.15s forwards';
    }
  };

  const showMessage = () => {
    if (!errorRef.current) return;
    errorRef.current.style.opacity = '0';
    errorRef.current.style.display = 'block';
    errorRef.current.style.animation = 'fade-in 0.15s forwards';
  };

  return { title, message, variant, hideMessage, showMessage, errorRef };
};
src/features/app_start/vip/startup/index.tsx (2)

31-48: Simplify conditional rendering logic

The nested conditional structure makes the code harder to maintain and reason about. Consider using a more declarative approach.

Here's a suggested refactor:

const renderContent = () => {
  if (isUserSignIn) return <Signin />;
  if (isUserMfaVerify) return <VerifyMFA />;
  if (isUserAccountCreated) return <UserAccountCreated />;
  if (isCongCreate) return <CongregationCreate />;
  if (isEmailAuth) return <EmailAuth />;
  if (isEmailLinkAuth) return <EmailLinkAuthentication />;
  if (!isCongCreate && isEncryptionCodeOpen) return <CongregationEncryption />;
  return null;
};

return (
  <>
    <TermsUse />
    {!isCongCreate && !isEncryptionCodeOpen && isLoading && (
      <WaitingLoader type="lottie" variant="standard" />
    )}
    {!isLoading && renderContent()}
  </>
);

31-48: Consider memoizing conditional components

The multiple boolean checks could lead to unnecessary re-renders. Consider using React.memo for the child components and useMemo for the conditions.

Example:

const ConditionalEncryption = memo(() => (
  !isCongCreate && isEncryptionCodeOpen ? <CongregationEncryption /> : null
));

const AuthContent = memo(() => (
  <>
    {isUserMfaVerify && <VerifyMFA />}
    {isUserAccountCreated && <UserAccountCreated />}
    {isCongCreate && <CongregationCreate />}
    {isEmailAuth && <EmailAuth />}
    {isEmailLinkAuth && <EmailLinkAuthentication />}
    <ConditionalEncryption />
  </>
));
src/services/i18n/index.ts (1)

Line range hint 22-72: Consider optimizing locale file loading.

The current implementation loads locale files sequentially. Consider loading them in parallel for better performance.

-for await (const record of languages) {
+await Promise.all(languages.map(async (record) => {
   const language = record.locale;
   
   const [
     activities,
     congregation,
     // ... other imports
   ] = await Promise.all([
     import(`@locales/${language}/activities.json`),
     import(`@locales/${language}/congregation.json`),
     // ... other imports
   ]).then(modules => modules.map(m => m.default));

   resources[record.identifier] = {
     ui: {
       ...activities,
       ...congregation,
       // ... other spreads
     },
     // ... other namespaces
   };
+}));
src/features/reminders/useReminders.tsx (1)

47-67: Consider extracting date comparison logic

The date comparison logic is duplicated between checkPubReport and checkBranchReport. Consider extracting this into a shared utility function.

+const isWithinReportWindow = (nextMonth: number, startDay: number, endDay: number) => {
+  const todayMonth = new Date().getMonth();
+  const todayDate = new Date().getDate();
+  return nextMonth === todayMonth && todayDate >= startDay && todayDate <= endDay;
+};

 const checkBranchReport = useMemo(() => {
   if (!isSecretary) return false;
   // ... other checks ...
-  const nextMonth = addMonths(`${currentReport}/01`, 1).getMonth();
-  const todayMonth = new Date().getMonth();
-  const todayDate = new Date().getDate();
-  if (nextMonth === todayMonth && todayDate >= 10 && todayDate <= 28) {
+  if (isWithinReportWindow(nextMonth, 10, 28)) {
     return true;
   }
   return false;
 }, [branchReports, currentReport, isSecretary]);
src/features/app_start/vip/congregation_create/congregation_access_code/useCongregationAccessCode.tsx (1)

Line range hint 35-80: Consider enhancing error handling for access code setup

While reviewing the access code setup, consider adding more specific error handling for different failure scenarios (network issues, validation failures, etc.).

 try {
   const encryptionKey = generateKey();
+  if (!encryptionKey) {
+    throw new Error('ENCRYPTION_KEY_GENERATION_FAILED');
+  }
   const encryptedKey = encryptData(encryptionKey, tmpAccessCode);
+  if (!encryptedKey) {
+    throw new Error('ENCRYPTION_FAILED');
+  }

   const { status, data } = await apiSetCongregationAccessCode(encryptedKey);
src/features/ministry/report/publisher_report/monthly_report/submit_report/useSubmitReport.tsx (1)

150-162: LGTM: Improved type safety and report handling

The changes enhance type safety and immutability through proper typing and structuredClone usage. Consider extracting the UUID generation into a utility function for reusability.

+// In a utility file
+export const generateReportId = () => crypto.randomUUID();

-      report.report_id = crypto.randomUUID();
+      report.report_id = generateReportId();
src/features/app_notification/container/useContainer.tsx (1)

309-311: LGTM: Secure access code handling for reports

The implementation follows the established pattern for secure data access. Consider enhancing error handling to specifically catch database access errors.

 try {
   const incoming = data?.result?.incoming_reports;
   if (!incoming) return;
-  const settings = await appDb.app_settings.get(1);
+  try {
+    const settings = await appDb.app_settings.get(1);
+  } catch (dbError) {
+    console.error('Failed to access app settings:', dbError);
+    throw new Error('error_app_settings_access');
+  }
   const congAccessCode = settings.cong_settings.cong_access_code;
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 115d41e and 5ef6cd5.

📒 Files selected for processing (35)
  • .firebaserc (0 hunks)
  • src/RootWrap.tsx (1 hunks)
  • src/constants/index.ts (2 hunks)
  • src/definition/ministry.ts (0 hunks)
  • src/features/app_notification/container/useContainer.tsx (10 hunks)
  • src/features/app_start/shared/hooks/useFeedback.tsx (1 hunks)
  • src/features/app_start/vip/congregation_create/congregation_access_code/useCongregationAccessCode.tsx (1 hunks)
  • src/features/app_start/vip/congregation_create/congregation_details/useCongregationDetails.tsx (1 hunks)
  • src/features/app_start/vip/congregation_create/congregation_master_key/useCongregationMasterKey.tsx (1 hunks)
  • src/features/app_start/vip/startup/index.tsx (1 hunks)
  • src/features/app_start/vip/startup/useStartup.tsx (1 hunks)
  • src/features/language_switcher/useLanguage.tsx (2 hunks)
  • src/features/meetings/weekly_schedules/midweek_meeting/useMidweekMeeting.tsx (3 hunks)
  • src/features/meetings/weekly_schedules/weekend_meeting/useWeekendMeeting.tsx (4 hunks)
  • src/features/ministry/report/publisher_report/monthly_report/submit_report/useSubmitReport.tsx (2 hunks)
  • src/features/ministry/report/publisher_report/monthly_report/withdraw_report/useWithdrawReport.tsx (3 hunks)
  • src/features/persons/spiritual_status/baptized_publisher/useBaptizedPublisher.tsx (1 hunks)
  • src/features/reminders/useReminders.tsx (2 hunks)
  • src/features/reports/hooks/useCongregationCard.tsx (2 hunks)
  • src/features/whats_new/useWhatsNew.tsx (2 hunks)
  • src/hooks/useAppTranslation.tsx (1 hunks)
  • src/hooks/useUserAutoLogin.tsx (3 hunks)
  • src/pages/ministry/ministry_report/index.tsx (2 hunks)
  • src/pages/ministry/ministry_report/useMinistryReport.tsx (4 hunks)
  • src/services/app/index.ts (3 hunks)
  • src/services/app/schedules.ts (3 hunks)
  • src/services/dexie/app.ts (0 hunks)
  • src/services/dexie/cong_field_service_reports.ts (3 hunks)
  • src/services/i18n/index.ts (3 hunks)
  • src/services/i18n/public_talks.ts (1 hunks)
  • src/services/i18n/songs.ts (1 hunks)
  • src/services/i18n/translation.ts (2 hunks)
  • src/services/worker/backupUtils.ts (1 hunks)
  • src/states/app.ts (1 hunks)
  • src/views/reports/S21/shared/TableHeader.tsx (1 hunks)
💤 Files with no reviewable changes (3)
  • .firebaserc
  • src/services/dexie/app.ts
  • src/definition/ministry.ts
🔇 Additional comments (28)
src/features/meetings/weekly_schedules/weekend_meeting/useWeekendMeeting.tsx (1)

Line range hint 189-211: Duplicate time format logic should be consolidated.

This implementation mirrors the time format handling in the midweek meeting hook. As suggested in the previous comment, this logic should be extracted to a shared utility function to avoid duplication.

src/services/app/schedules.ts (1)

2057-2064: Time format logic appears in multiple places.

This is the third occurrence of similar time format conversion logic. As suggested earlier, this should be consolidated into a shared utility function.

src/services/dexie/cong_field_service_reports.ts (1)

Line range hint 1-187: Overall, the code changes enhance report management effectively

The updates to dbHandleIncomingReports and the addition of dbRemoveDuplicateReports improve the handling of incoming reports and duplicate detection. The implementation is clear and aligns with the objectives.

src/services/worker/backupUtils.ts (1)

1241-1243: Ensure consistent report identification throughout the codebase

By changing the report identification criteria from report_id to person_uid and report_month, verify that this modification is consistently applied across all related functions to prevent mismatches or missed reports during backup and restore operations.

src/views/reports/S21/shared/TableHeader.tsx (1)

14-14: LGTM! Verify PDF rendering.

The addition of line breaks after hyphens in the service year text should improve readability in the PDF output.

Please verify the PDF rendering with different service year translations to ensure proper text wrapping:

✅ Verification successful

Let me gather more context about the translation keys and their usage.


Let me check the PDF styles and layout context to ensure this change is appropriate.


Based on the analysis of the TableHeader.tsx and its styles, I can now generate the final response:

LGTM - Line break addition is appropriate for PDF layout

The change to add line breaks after hyphens in the service year text is well-implemented:

  • It's used in a PDF-specific component (@react-pdf/renderer)
  • The column width is fixed at 80px (styles.column1)
  • The font size is small (9px for columnHeader)
  • The technique using Unicode line feed is standard for PDF text formatting
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Find all occurrences of tr_serviceYear translations to verify potential hyphenation cases
rg "tr_serviceYear" --type ts

Length of output: 696


Script:

#!/bin/bash
# Find translation files that might contain tr_serviceYear
fd -e json -e ts "translation|i18n" --exec rg -l "tr_serviceYear"

# Check the TableHeader.tsx file content for more context
cat src/views/reports/S21/shared/TableHeader.tsx

Length of output: 2005


Script:

#!/bin/bash
# Check the styles file referenced in TableHeader.tsx
cat src/views/reports/S21/shared/index.styles.ts

# Look for other PDF files that might use similar hyphenation technique
rg -l "\u000A" --type ts

Length of output: 2757

src/pages/ministry/ministry_report/useMinistryReport.tsx (2)

21-33: LGTM! Robust status computation.

The memoized reportStatus computation correctly:

  • Finds the matching report by date
  • Handles the case when no report exists
  • Properly prioritizes 'pending' status

50-56: Verify the modal state transitions.

The modal handling logic has been updated to use reportStatus with expanded conditions for the withdraw modal.

Let's verify all possible status transitions:

✅ Verification successful

Based on the search results, I can now generate the final response:

Modal state transitions are correctly implemented and aligned with the report status flow.

The codebase shows a consistent status flow:

  • 'pending' → 'submitted' → 'received' → 'confirmed'
  • The submit modal opens only for 'pending' status
  • The withdraw modal opens for both 'submitted' and 'received' statuses
  • The withdraw action correctly resets status back to 'pending'
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Find all status-related transitions and handlers
rg "status.*=.*['\"](?:pending|submitted|received|confirmed)" --type ts

# Find related modal handlers
rg "(?:handleOpenModal|setSubmitOpen|setWithdrawOpen)" --type ts

Length of output: 7307

src/pages/ministry/ministry_report/index.tsx (1)

24-24: LGTM! Clear and consistent variable renaming.

The renaming from status to reportStatus improves code clarity by being more specific about what the status represents. The button's conditional rendering logic remains functionally equivalent.

Also applies to: 36-40

src/services/i18n/index.ts (1)

94-96: LGTM! Clear i18n configuration.

The i18n configuration is properly set up with the new identifier-based approach and appropriate fallback language.

src/features/reports/hooks/useCongregationCard.tsx (1)

5-5: Verify impact on existing reports after service year calculation change

The switch to using currentServiceYear better aligns with the organizational calendar, but we should verify this doesn't affect existing reports.

Also applies to: 21-24

✅ Verification successful

Let me gather more information about the date utilities and their usage to better understand the impact.


Based on the comprehensive analysis of the codebase, I can now provide the final response:

The service year calculation change is safe and properly implemented

The switch from currentReportMonth to currentServiceYear in useCongregationCard.tsx is part of a well-structured system where:

  • currentServiceYear correctly handles the September-August service year calculation
  • The change aligns with existing service year calculations used throughout the application
  • Existing reports are protected as they use createArrayFromMonths which maintains chronological order regardless of the year calculation method
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for other uses of currentReportMonth to ensure consistent updates
rg "currentReportMonth"

# Check for any date-related calculations in report processing
ast-grep --pattern 'const $_ = $$$Month'

Length of output: 3691


Script:

#!/bin/bash
# Check the implementation of currentServiceYear and currentReportMonth
cat src/utils/date.ts

# Look for service year related calculations
rg "serviceYear" -A 3

# Check for month-based calculations in reports
ast-grep --pattern 'createArrayFromMonths($$$)'

Length of output: 49386

src/features/app_start/vip/congregation_create/congregation_master_key/useCongregationMasterKey.tsx (1)

61-61: Verify consistent usage of cong_new flag

The addition of 'cong_settings.cong_new': true needs to be consistently applied across all congregation creation flows.

Consider centralizing congregation initialization settings in a single configuration object to ensure consistency.

✅ Verification successful

Based on the search results, I can now generate the final response:

Usage of cong_settings.cong_new is consistent across the codebase

The flag is correctly set to true during congregation creation flows (in congregation_master_key, congregation_details, and congregation_access_code) and properly set to false in all other contexts (initial setup, demo notice, authentication flows). This maintains a clear distinction between new and existing congregations.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for other occurrences of cong_new setting
rg "'cong_settings\.cong_new':" -A 1

# Search for congregation creation related functions
ast-grep --pattern 'function $_($$) {
  $$$
  dbAppSettingsUpdate($$$)
  $$$
}'

Length of output: 2252

src/RootWrap.tsx (1)

24-24: Verify the impact of removing URL-based font configuration

The simplified font retrieval logic removes URL parameter handling. Please confirm if this was an intentionally removed feature or if it's still needed by users/testing.

src/features/reminders/useReminders.tsx (2)

15-16: Improved publisher check logic with proper memoization

The refactoring to useMemo with proper dependency tracking is a good improvement. The early returns make the code more maintainable.

Also applies to: 19-19, 27-45


88-109: Improved state management with single update

The refactoring to collect all reminders before a single setReminders call is a good improvement for performance.

src/features/app_start/vip/congregation_create/congregation_access_code/useCongregationAccessCode.tsx (1)

63-63: Verify the impact of the new congregation flag

The addition of cong_settings.cong_new flag needs verification across the codebase to ensure it's properly handled in all congregation-related flows.

✅ Verification successful

Based on the search results, I can now provide a definitive response about the cong_settings.cong_new flag:

The cong_new flag is properly handled across the codebase

The flag is used consistently throughout the application to manage the congregation setup flow:

  • It's set to true during initial congregation creation in the setup flows
  • It's properly cleared (false) after setup completion in multiple places:
    • Initial setup completion
    • OAuth authentication
    • Email link authentication
    • MFA verification
    • Demo notice handling

The flag is also:

  • Properly defined in schema and type definitions
  • Used in root layout for conditional rendering
  • Backed up and restored correctly during data migrations
  • Handled securely with proper encryption settings
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for other uses of cong_new setting
rg "cong_new" --type ts --type tsx

# Search for congregation settings handling
rg "cong_settings\." --type ts --type tsx

Length of output: 134


Script:

#!/bin/bash
# Search for other uses of cong_new setting without file type restrictions
rg "cong_new"

# Search for congregation settings handling without file type restrictions
rg "cong_settings\."

# Find all TypeScript/React files that might contain congregation settings
fd "\.(ts|tsx)$" | xargs rg "cong_settings"

Length of output: 71514

src/features/whats_new/useWhatsNew.tsx (2)

25-30: LGTM: Robust language identifier handling

The implementation correctly handles language mapping with a proper fallback to appLang if no identifier is found in LANGUAGE_LIST.


33-33: Verify language resources availability

Ensure that all required language resources are available in i18n options for each identifier.

src/features/app_start/vip/congregation_create/congregation_details/useCongregationDetails.tsx (1)

139-139: Verify the impact of new congregation flag

The addition of cong_settings.cong_new flag could affect other parts of the application that might rely on this setting.

✅ Verification successful

Based on the search results, I can now generate the final response as I have comprehensive information about how the cong_settings.cong_new flag is used throughout the codebase:

The cong_new flag is used consistently and safely

The flag is used appropriately across the codebase:

  • Set to true only during congregation creation process in related files
  • Properly reset to false after initial setup, authentication, and demo flows
  • Used for controlling initialization flows and UI states in dashboard and root layout
  • Backed up correctly in backup utilities
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for usages of cong_new setting
rg -A 3 "cong_settings\.cong_new|settings\['cong_settings'\]\.cong_new"

# Search for related congregation settings checks
ast-grep --pattern 'settings.cong_settings'

Length of output: 22765

src/constants/index.ts (2)

22-22: LGTM: Added standard language identifier for Tagalog

The addition of identifier: 'tgl' follows ISO 639-3 language code standards for Tagalog.


68-68: Fixed: Hungarian locale code

Corrected the Hungarian locale from 'hy-HU' to 'hu-HU'. The previous code was incorrect as 'hy' is the language code for Armenian.

src/features/app_start/vip/startup/useStartup.tsx (1)

172-175: LGTM: Enhanced validation for encryption code dialog

The additional check congName.length > 0 ensures the encryption code dialog only appears when there's an actual congregation name, preventing premature dialog display.

src/features/persons/spiritual_status/baptized_publisher/useBaptizedPublisher.tsx (2)

195-197: LGTM: Enhanced validation for baptism history creation

The additional check for firstReport.length === 0 prevents duplicate history entries by ensuring a new history record is only created when there's no existing first report.


187-187: Improved code readability

Added line break to separate the baptism date update from the history creation logic.

src/hooks/useUserAutoLogin.tsx (2)

193-205: LGTM: Worker message flow is well-structured

The sequence of worker messages follows a logical order, setting up the necessary context before starting the worker.


282-294: LGTM: Consistent worker message implementation

The worker message implementation maintains consistency with the VIP flow, ensuring uniform behavior across account types.

src/features/app_notification/container/useContainer.tsx (2)

271-273: LGTM: Consistent pattern for sensitive data handling

The implementation maintains consistency with the established pattern of secure database access for sensitive information.


159-161: LGTM: Secure handling of sensitive data

The change appropriately moves sensitive data retrieval from global state to direct database access.

✅ Verification successful

Based on the search results, I can now generate the final response since I have enough context to verify the security handling of the master key. The search shows:

  1. The master key is properly handled through database access and encryption services
  2. It's used consistently for encrypting/decrypting sensitive data
  3. The implementation follows security best practices with proper access controls
  4. The UI/UX properly guides users on secure master key management

LGTM: Secure implementation of congregation master key handling

The change from Recoil state to direct database access is appropriate and maintains security through:

  • Proper encryption/decryption utilities in services/encryption
  • Controlled access through database layer
  • Clear user guidance on master key management in UI translations
  • Consistent usage pattern across sensitive operations
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify that sensitive data is not exposed in other parts of the codebase
rg -g '!*.test.*' -g '!*.spec.*' "congMasterKey|masterKey" -A 3 -B 3

Length of output: 68130

src/states/app.ts (1)

54-54: Verify language switching functionality across the application

The simplified language setting logic might affect other parts of the application that rely on URL-based language switching.

✅ Verification successful

Let me analyze the results and generate additional verification scripts to check for URL-based language switching.

The first search didn't show appLangState usage, but we need to verify how the language state is managed and if there are any URL-based language switches in the routing or configuration files. This will help us determine if the change to use localStorage is safe.


Based on the search results, I can now provide a final response:

The change to use localStorage for language setting is safe

The codebase consistently uses localStorage for language management:

  • The language state (appLangState) is already initialized from localStorage with 'en' as fallback
  • Language switching is handled through localStorage in useLanguage.tsx and other components
  • No URL-based language switching was found in the codebase
  • The i18n configuration and all language-dependent components rely on the same localStorage-based approach
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for URL-based language parameter usage
echo "Checking for URL language parameter usage..."
rg -l "lang.*URLSearchParams|URLSearchParams.*lang"

# Check for appLangState usage
echo "Checking for appLangState usage..."
rg "appLangState" --type ts --type tsx

# Check for language-related hooks
echo "Checking for language-related hooks..."
fd -e ts -e tsx | xargs rg "useAppTranslation|useTranslation"

Length of output: 65952


Script:

#!/bin/bash
# Check for language-related state management
echo "Checking for language state management..."
rg -A 5 "appLangState|ui_lang" --type ts

# Check for language-related URL handling
echo "Checking for language URL handling..."
rg -A 5 "language.*url|url.*language" --type ts --type tsx

# Check for i18n configuration
echo "Checking for i18n configuration..."
fd -e ts -e tsx | xargs rg "i18n.*init|i18next.*init"

Length of output: 9979

src/hooks/useAppTranslation.tsx Outdated Show resolved Hide resolved
src/services/i18n/songs.ts Show resolved Hide resolved
src/services/i18n/public_talks.ts Show resolved Hide resolved
src/services/i18n/translation.ts Show resolved Hide resolved
src/services/app/index.ts Show resolved Hide resolved
src/services/app/index.ts Show resolved Hide resolved
src/states/app.ts Show resolved Hide resolved
coderabbitai[bot]
coderabbitai bot previously approved these changes Dec 8, 2024
coderabbitai[bot]
coderabbitai bot previously approved these changes Dec 8, 2024
Copy link

sonarqubecloud bot commented Dec 8, 2024

@rhahao rhahao merged commit 6596fd2 into sws2apps:main Dec 8, 2024
14 checks passed
Copy link

cypress bot commented Dec 8, 2024

organized-app    Run #1765

Run Properties:  status check passed Passed #1765  •  git commit 6596fd294f: chore(demo): add first report value for persons
Project organized-app
Branch Review main
Run status status check passed Passed #1765
Run duration 00m 05s
Commit git commit 6596fd294f: chore(demo): add first report value for persons
Committer rhahao
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 1
View all changes introduced in this branch ↗︎

@rhahao
Copy link
Member Author

rhahao commented Dec 8, 2024

🎉 This PR is included in version 3.3.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@rhahao rhahao added the released label Dec 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
1 participant