-
Notifications
You must be signed in to change notification settings - Fork 23
Feature: customize weekends #61
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
Conversation
WalkthroughThe changes introduce new test suites for multiple calculation functions and implement modifications in the weekend detection logic. Specifically, tests for Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Calculation as calcNonWorkingHours/calcWeekendMinutes
participant WeekendCheck as checkWeekend
participant Config as getMultipleValuesInput & date-fns getDay
Caller->>Calculation: Invoke calculation function with date
Calculation->>WeekendCheck: Call checkWeekend(date)
WeekendCheck->>Config: Retrieve weekend config (getMultipleValuesInput("WEEKENDS"))
Config-->>WeekendCheck: Return weekend days list
WeekendCheck->>Config: Call getDay(date)
Config-->>WeekendCheck: Return day number
WeekendCheck-->>Calculation: Return weekend boolean
Calculation-->>Caller: Return computed minutes
Poem
Tip 🌐 Web search-backed reviews and chat
✨ Finishing Touches
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🔭 Outside diff range comments (2)
src/converters/utils/calculations/calcWeekendMinutes.ts (1)
37-57: Consider simplifying the edge case handling logic.The logic for handling start and end dates could be simplified by extracting the minute calculations into helper functions.
+ const calculateStartMinutes = (date: Date, referenceDate: Date) => { + return differenceInMinutes(date, referenceDate); + }; + + const calculateEndMinutes = (date: Date, referenceDate: Date) => { + return differenceInMinutes( + setMinutes(setHours(referenceDate, 23), 59), + date + ) + 1; + }; + if (isStartedAtWeekend) { - minutesInWeekend -= differenceInMinutes(firstDate, weekendsOfInterval[0]); + minutesInWeekend -= calculateStartMinutes(firstDate, weekendsOfInterval[0]); } else if (isStartedOnHoliday) { - minutesInWeekend -= differenceInMinutes(firstDate, days[0]); + minutesInWeekend -= calculateStartMinutes(firstDate, days[0]); } if (isEndedAtWeekend) { - minutesInWeekend -= - differenceInMinutes( - setMinutes( - setHours(weekendsOfInterval[weekendsOfInterval.length - 1], 23), - 59 - ), - secondDate - ) + 1; + minutesInWeekend -= calculateEndMinutes(secondDate, weekendsOfInterval[weekendsOfInterval.length - 1]); } else if (isEndedOnHoliday) { - minutesInWeekend -= - differenceInMinutes( - setMinutes(setHours(days[days.length - 1], 23), 59), - secondDate - ) + 1; + minutesInWeekend -= calculateEndMinutes(secondDate, days[days.length - 1]); }src/converters/utils/calculations/calcNonWorkingHours.ts (1)
16-21: Add return type annotation for better type safety.Consider adding a return type annotation to improve type safety and documentation.
export const calcNonWorkingHours = ( firstDate: Date, secondDate: Date, { startOfWorkingTime, endOfWorkingTime }: CoreHours, holidays?: string[] -) => { +): number => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (4)
README.mdis excluded by none and included by noneaction.ymlis excluded by none and included by nonebuild/index.jsis excluded by!build/**and included by nonepackage.jsonis excluded by none and included by none
📒 Files selected for processing (8)
src/converters/utils/calculations/calcAverageValue.spec.ts(1 hunks)src/converters/utils/calculations/calcDraftTime.spec.ts(1 hunks)src/converters/utils/calculations/calcNonWorkingHours.ts(2 hunks)src/converters/utils/calculations/calcWeekendMinutes.ts(1 hunks)src/converters/utils/calculations/checkUserInclusive.ts(1 hunks)src/converters/utils/calculations/checkWeekend.spec.ts(1 hunks)src/converters/utils/calculations/checkWeekend.ts(1 hunks)src/view/utils/createConfigParamsCode.ts(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/converters/utils/calculations/checkWeekend.ts
[error] 6-6: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.
(lint/style/useNumberNamespace)
🔇 Additional comments (4)
src/converters/utils/calculations/checkUserInclusive.ts (1)
1-4: LGTM! Clean and focused implementation.The function is well-named and follows a clear single responsibility principle.
src/view/utils/createConfigParamsCode.ts (1)
28-28: LGTM! Well-placed configuration parameter.The "WEEKENDS" parameter is logically placed near other time-related configuration parameters.
src/converters/utils/calculations/calcAverageValue.spec.ts (1)
1-33: LGTM! Comprehensive test coverage.The test suite is well-structured and covers all essential scenarios:
- Edge cases
- Basic functionality
- Ceiling behavior
- Negative numbers
- Mixed numbers
src/converters/utils/calculations/calcDraftTime.spec.ts (1)
7-101: Well-structured and comprehensive test suite!The test cases are thorough and cover all major scenarios including edge cases. Good job on including tests for:
- Empty input handling
- Single event calculation
- Multiple events calculation
- Edge cases with undefined values
Pull Request
Description
Added parameter(
WEEKENDS) to customize weekends. By default it is Saturday and Sunday.Type of Change
How Has This Been Tested?
Checked default behaviour. Checked Friday, Saturday weekends.
Checklist:
Additional Information
Any additional information about the pull request.
Summary by CodeRabbit