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

Hotfixes 2024-12-16 #2 #3238

Merged
merged 2 commits into from
Dec 16, 2024
Merged

Hotfixes 2024-12-16 #2 #3238

merged 2 commits into from
Dec 16, 2024

Conversation

rhahao
Copy link
Member

@rhahao rhahao commented Dec 16, 2024

No description provided.

Copy link
Contributor

coderabbitai bot commented Dec 16, 2024

Walkthrough

This pull request introduces a new utility function isMondayDate across multiple files in the meetings and scheduling features. The changes primarily focus on refining data filtering logic to ensure that only schedules corresponding to Monday dates are processed. The modifications span several components, including midweek exports, schedule publishing, week selection, and source formatting. The changes aim to standardize and improve the handling of weekly schedules by consistently applying Monday-based filtering.

Changes

File Change Summary
src/features/meetings/midweek_export/useMidweekExport.tsx Added isMondayDate to filter schedules based on Monday dates in handleExportSchedule
src/features/meetings/schedule_publish/useSchedulePublish.tsx Integrated isMondayDate in sourcesList computation to filter records
src/features/meetings/weekly_schedules/week_selector/useWeekSelector.tsx Modified filteredSources to include Monday date check
src/services/app/schedules.ts Refined assignment handling logic, changing suffix checks from includes() to endsWith()
src/states/sources.ts Updated sourcesFormattedState to filter sources using isMondayDate
src/views/meetings/midweek/S89/default/index.tsx Simplified prop passing for aux_class_1 in S89ToBeGiven component

Sequence Diagram

sequenceDiagram
    participant User
    participant Component
    participant isMondayDate
    participant DataSource

    User->>Component: Trigger Schedule Action
    Component->>isMondayDate: Check Date
    isMondayDate-->>Component: Is Monday?
    alt Is Monday
        Component->>DataSource: Filter/Process Data
    else Not Monday
        Component->>Component: Skip Data
    end
Loading

Possibly related PRs


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fa4e750 and 4ba36e4.

📒 Files selected for processing (6)
  • src/features/meetings/midweek_export/useMidweekExport.tsx (2 hunks)
  • src/features/meetings/schedule_publish/useSchedulePublish.tsx (2 hunks)
  • src/features/meetings/weekly_schedules/week_selector/useWeekSelector.tsx (2 hunks)
  • src/services/app/schedules.ts (1 hunks)
  • src/states/sources.ts (2 hunks)
  • src/views/meetings/midweek/S89/default/index.tsx (1 hunks)
🔇 Additional comments (10)
src/services/app/schedules.ts (2)

1899-1901: Confirm correct assignment to aux_class_1 property

Ensure that setting obj.aux_class_1 = true when assignment ends with _B is intentional and that obj.aux_class_1 is correctly handled in subsequent code.

Run the following script to verify the usage of obj.aux_class_1:

✅ Verification successful

Let me gather more context about how this object is used.


Let me gather more context about the specific code block where this assignment happens.


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

Setting aux_class_1 = true for _B assignments is correct and verified

The assignment is part of a well-structured system where:

  • The schema (SchedWeekType) properly defines aux_class_1 as a boolean property
  • The codebase consistently uses _B suffix to denote auxiliary class 1 assignments across multiple components (chairman, bible reading, student parts)
  • The assignment is used in S89 form generation where aux_class_1 boolean flag helps identify the classroom assignment
  • The pattern is consistently used in assignment history tracking where _B corresponds to classroom '2'
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Search for all usages of `obj.aux_class_1` to confirm its impact.

# Find assignments to `aux_class_1` in the codebase
rg -A 2 -B 2 "obj\.aux_class_1" --glob "*.ts"

Length of output: 278


Script:

#!/bin/bash
# Search for the broader context of where this assignment happens
rg -A 10 -B 10 "aux_class_1" --glob "*.ts"

# Also search for any type definitions or interfaces that might define this property
ast-grep --pattern 'interface $_ {
  $$$
  aux_class_1$_
  $$$
}'

Length of output: 27252


Script:

#!/bin/bash
# Get more context around the assignment
rg -A 10 -B 10 "if \(assignment\.endsWith\('_B'\)\)" --glob "*.ts"

Length of output: 2243


1895-1897: Confirm correct assignment to main_hall property

Ensure that setting obj.main_hall = true when assignment ends with _A aligns with the expected logic and that obj.main_hall is properly utilized downstream.

Run the following script to verify the usage of obj.main_hall:

✅ Verification successful

Let me gather more context about how this object is used in the codebase.


Let me search for the object initialization and its usage pattern to understand the complete flow.


Let me check how this object is initialized and what other properties are set before the final usage.


Let me check one final thing - the object initialization and the function signature to understand the complete context.


Setting main_hall = true for '_A' assignments is correct

The code is part of the schedulesS89Data function that processes meeting assignments. The logic correctly sets obj.main_hall = true for assignments ending with '_A', which represents assignments for the main hall, while assignments ending with '_B' set aux_class_1 = true. This is consistent with the assignment pattern shown in the assignments array where each assignment has either '_A' or '_B' suffix to distinguish between main hall and auxiliary class assignments.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Search for all usages of `obj.main_hall` to confirm its impact.

# Find assignments to `main_hall` in the codebase
rg -A 2 -B 2 "obj\.main_hall" --glob "*.ts"

Length of output: 274


Script:

#!/bin/bash
# Get more context around the usage of this object
rg -B 10 -A 10 "obj\.main_hall = true" --glob "*.ts"

# Find where this object is used after the assignment
ast-grep --pattern 'const obj = { $$$'

Length of output: 1083


Script:

#!/bin/bash
# Find the object initialization and its complete usage context
rg -B 20 -A 20 "obj\.main_hall = true" --glob "*.ts"

# Find where this result array is used
ast-grep --pattern 'result.push(obj)'

Length of output: 2559


Script:

#!/bin/bash
# Find the object initialization and the complete function context
rg -B 50 -A 10 "obj\.main_hall = true" --glob "*.ts" --context-separator="=========="

Length of output: 3433


Script:

#!/bin/bash
# Find the function declaration and object initialization
rg -B 100 -A 10 "obj\.main_hall = true" --glob "*.ts" --context-separator="==========" 

Length of output: 6156

src/views/meetings/midweek/S89/default/index.tsx (1)

57-57: Ensure data.aux_class_1 is reliably defined

By directly passing data.aux_class_1, make sure that this property is always defined and is of the expected boolean type to prevent potential rendering issues in S89ToBeGiven.

Run the following script to check where data.aux_class_1 is set:

✅ Verification successful

aux_class_1 is properly typed and initialized

From the codebase analysis, aux_class_1 is:

  • Properly typed as a boolean in S89ToBeGivenType interface
  • Reliably set in the schedules.ts service where it's assigned based on assignment suffix '_B'
  • Used consistently across the codebase for auxiliary class assignments
  • Has a default schema in schema.ts ensuring initialization
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Find all assignments to `data.aux_class_1` to ensure it is properly initialized.

# Search for assignments to `data.aux_class_1`
rg -A 2 -B 2 "data\.aux_class_1\s*=" --glob "*.ts*" --glob "!node_modules/*"

Length of output: 78


Script:

#!/bin/bash
# Let's try a broader search to understand the context and usage of aux_class_1

# Search for any occurrence of aux_class_1
rg "aux_class_1" --glob "*.ts*" --glob "!node_modules/*" -A 2 -B 2

# Also search for the interface or type definition that might contain aux_class_1
ast-grep --pattern 'interface $_ {
  $$$
  aux_class_1$_
  $$$
}'

Length of output: 19334

src/states/sources.ts (2)

7-7: Verify the import of isMondayDate

Ensure that isMondayDate is correctly imported from @utils/date and that no circular dependencies are introduced.

Run the following script to verify the import:

✅ Verification successful

Import of isMondayDate is correctly implemented

The function isMondayDate is properly exported from the utils/date module and is being imported correctly in src/states/sources.ts. The implementation is simple and has no circular dependencies, as verified by checking imports across the codebase.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check that `isMondayDate` is properly exported and imported.

# Find the export of `isMondayDate`
rg -A 2 -B 2 "export function isMondayDate" --glob "*.ts"

# Check for imports of `isMondayDate` in other files
rg -A 0 -B 0 "import.*isMondayDate.*from.*@utils/date" --glob "*.ts"

Length of output: 252


Script:

#!/bin/bash
# Let's check the actual implementation of isMondayDate in the utils directory
fd "date.ts" --type f --exec cat {}

# Also check for any potential circular dependencies by looking for imports from states/sources
rg -A 2 "from.*states/sources" --glob "*.ts"

Length of output: 13796


20-20: Confirm isMondayDate filters dates as expected

Verify that the isMondayDate function correctly identifies Mondays to prevent unintended exclusion of valid weeks.

Run the following script to test isMondayDate function with sample dates:

✅ Verification successful

isMondayDate implementation is correct and consistently used

The function correctly identifies Mondays by checking if the day of the week equals 1 (Monday in JavaScript's Date.getDay()). The implementation is consistent across the codebase:

export const isMondayDate = (date: string) => {
  const inputDate = new Date(date);
  const dayOfWeek = inputDate.getDay();
  return dayOfWeek === 1;
};

The function is used consistently in multiple places for filtering Monday dates:

  • src/states/sources.ts (the file under review)
  • src/features/meetings/weekly_schedules/week_selector
  • src/features/meetings/schedule_publish
  • src/features/meetings/midweek_export
  • src/migration/action_steps
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Validate `isMondayDate` function with sample dates.

# Create a temporary TypeScript file to test `isMondayDate`
echo -e "import { isMondayDate } from '@utils/date';\nconsole.log(isMondayDate('2024/12/16')); // Expected: true\nconsole.log(isMondayDate('2024/12/17')); // Expected: false" > tempTest.ts

# Since we cannot run TypeScript code here, please run this test in your development environment.

Length of output: 194


Script:

#!/bin/bash
# Let's first locate the isMondayDate function implementation
ast-grep --pattern 'function isMondayDate($_) { $$$ }'

# Also search for any tests related to this function
rg "isMondayDate.*test" -l

# Let's also see the actual implementation with context
rg "isMondayDate" -A 5

Length of output: 8840

src/features/meetings/weekly_schedules/week_selector/useWeekSelector.tsx (1)

28-30: Ensure date filtering includes all valid weeks

When filtering sources, confirm that the isMondayDate function and date comparison with minDate correctly include all necessary weeks, and no valid weeks are omitted due to date format discrepancies.

Run the following script to list all sources after filtering:

✅ Verification successful

Let me gather more information about the isMondayDate function and minDate usage to verify the date filtering logic.


Based on the gathered information, I can now provide a final response:

Date filtering logic is correctly implemented

The date filtering implementation is correct and includes all valid weeks because:

  1. minDate is consistently set to 2 months in the past using addMonths(new Date(), -2)
  2. isMondayDate is used in conjunction with getWeekDate() utility which properly handles week calculations
  3. The date comparison record.weekOf >= minDate ensures all weeks from 2 months ago up to the current date are included
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: List all `sources` after applying the filter to verify correctness.

# Output filtered `sources` with `weekOf` dates
rg -A 2 -B 2 "sources\.filter" --glob "*.ts*" | grep -E "weekOf"

# Verify that all expected weeks starting from `minDate` are included

Length of output: 821


Script:

#!/bin/bash
# Check implementation of isMondayDate function
ast-grep --pattern 'function isMondayDate($_) {
  $$$
}'

# Check minDate usage and initialization
rg "minDate" --glob "*.ts*" -A 2 -B 2

# Check date-related utility functions
rg "getWeekDate|getMonday" --glob "*.ts*" -A 2 -B 2

Length of output: 25763

src/features/meetings/midweek_export/useMidweekExport.tsx (2)

39-39: LGTM!

Clean integration of the isMondayDate utility function.


184-187: Fix indentation for better readability.

The indentation of the date comparison logic after the Monday check appears inconsistent. Consider adjusting the indentation to maintain a clear logical flow.

 const weeksList = schedules.filter((schedule) => {
   const isMonday = isMondayDate(schedule.weekOf);

   if (!isMonday) return false;
-
   const [yearStart, monthStart] = startMonth.split('/');
   const [yearEnd, monthEnd] = endMonth.split('/');

Additionally, let's verify that this change doesn't break existing schedule exports.

src/features/meetings/schedule_publish/useSchedulePublish.tsx (2)

4-4: LGTM!

Clean integration of isMondayDate alongside other date utilities.


65-68: Verify impact on existing schedule data.

The addition of the Monday date filter could potentially impact the display of existing schedules. We should verify:

  1. That existing schedules are not inadvertently hidden
  2. That the UI properly handles potentially fewer visible schedules
✅ Verification successful

No impact on existing schedule data

The implementation is safe and won't affect existing schedules because:

  1. The isMondayDate function is already used throughout the codebase for validation
  2. The implementation in src/utils/date.ts correctly validates Monday dates (day === 1)
  3. Existing data is already filtered for Monday dates in multiple places:
    • Sources state management (src/states/sources.ts)
    • Data migration (src/migration)
    • Schedule exports and imports

The added Monday date filter aligns with the existing data validation patterns in the codebase and won't hide any valid schedules.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any non-Monday dates in existing schedule data that might be affected

# Look for schedule data files
echo "Searching for schedule data files..."
fd -e json -e ts -e tsx | grep -i "schedule\|source"

# Check for date patterns in these files
echo "Checking date patterns in found files..."
rg "weekOf.*[0-9]{4}/[0-9]{2}/[0-9]{2}" -l | while read -r file; do
  echo "=== $file ==="
  rg "weekOf.*[0-9]{4}/[0-9]{2}/[0-9]{2}" "$file"
done

Length of output: 7787


Script:

#!/bin/bash
# Let's check the implementation of isMondayDate function and verify if there's any data validation
echo "=== Checking isMondayDate implementation ==="
ast-grep --pattern 'function isMondayDate'

echo -e "\n=== Checking for date validation in sources handling ==="
rg "weekOf.*validation|validateDate|isValid.*date" -A 3

echo -e "\n=== Looking for test data or sample dates ==="
rg "weekOf.*=" -A 1

Length of output: 30362


Script:

#!/bin/bash
# Let's check the implementation of isMondayDate function
echo "=== Checking isMondayDate implementation ==="
rg "isMondayDate.*=" -B 2 -A 5

echo -e "\n=== Checking for any existing date validation or Monday checks ==="
rg "getDay\(\).*===.*1|isMonday|monday.*check" -A 2

Length of output: 10405


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 generate docstrings to generate docstrings for this PR. (Beta)
  • @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

vercel bot commented Dec 16, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
organized-app ✅ Ready (Inspect) Visit Preview Dec 16, 2024 2:38pm

@rhahao rhahao merged commit 8054bf6 into sws2apps:main Dec 16, 2024
11 checks passed
@rhahao
Copy link
Member Author

rhahao commented Dec 16, 2024

🎉 This PR is included in version 3.4.2 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Copy link

cypress bot commented Dec 16, 2024

organized-app    Run #1821

Run Properties:  status check passed Passed #1821  •  git commit 8054bf61f5: fix(meetings): exclude not monday week
Project organized-app
Branch Review main
Run status status check passed Passed #1821
Run duration 00m 09s
Commit git commit 8054bf61f5: fix(meetings): exclude not monday week
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 ↗︎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant