-
Notifications
You must be signed in to change notification settings - Fork 0
refactor: move WebWrapper files from packages/platform to apps/web/modules #57
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
base: sentry_combined_20260121_augment_sentry_coderabbit_1_base_refactor_move_webwrapper_files_from_packages_platform_to_apps_web_modules_pr708
Are you sure you want to change the base?
refactor: move WebWrapper files from packages/platform to apps/web/modules #57
Conversation
| import { useSession } from "next-auth/react"; | ||
| import { useSearchParams, usePathname, useRouter } from "next/navigation"; | ||
| import { useMemo, useCallback, useEffect, useRef } from "react"; | ||
| import { useSearchParams, usePathname } from "next/navigation"; |
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.
Bug: The router variable is used in multiple callbacks but is not defined because the useRouter hook is missing. This will cause a runtime ReferenceError.
Severity: HIGH
Suggested Fix
Import the useRouter hook from next/navigation at the top of the file. Then, inside the BookerWebWrapper component, initialize the router constant by calling the hook: const router = useRouter();.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: apps/web/modules/bookings/components/BookerWebWrapper.tsx#L4
Potential issue: The component `BookerWebWrapper` uses a `router` variable within four
separate callback functions: `onOverlaySwitchStateChange`, `onConnectNowInstantMeeting`,
`onOverlayClickNoCalendar`, and `onClickOverlayContinue`. However, the `router` variable
is never defined because the `useRouter` hook from `next/navigation` is not imported or
called. When a user performs an action that triggers any of these callbacks, such as
toggling the overlay calendar or attempting to navigate, the application will attempt to
call a method on the undefined `router` object. This will result in a `ReferenceError`
at runtime, causing a crash and preventing the user from completing their action.
Did we get this right? 👍 / 👎 to inform future reviews.
|
|
||
| if (isRRWeightsEnabled) { | ||
| if (weightA === weightB) { | ||
| return priorityA - priorityB; |
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.
Bug: The host sorting logic was reversed from descending to ascending priority when host weights are equal, leading to incorrect host selection order in round-robin events.
Severity: MEDIUM
Suggested Fix
In hostGroupUtils.ts, locate the conditional block for when host weights are equal. Change the return statement from return priorityA - priorityB; back to return priorityB - priorityA; to restore the correct descending sort order by priority.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: packages/lib/bookings/hostGroupUtils.ts#L53
Potential issue: In the refactored host sorting logic, the order for hosts with equal
weights has been inadvertently reversed. The original implementation sorted hosts by
descending priority (`priorityB - priorityA`). The new logic in `hostGroupUtils.ts`,
specifically when `isRRWeightsEnabled` is true and `weightA === weightB`, now sorts by
ascending priority (`priorityA - priorityB`). This means that for a round-robin event
type, if multiple hosts have the same weight, the one with the lowest priority will be
selected first, which is the opposite of the intended behavior.
Did we get this right? 👍 / 👎 to inform future reviews.
Benchmark PR from qodo-benchmark#708