Conversation
…close automation page
WalkthroughAdded memoized helpers to compute active state in two automation components (auto-archive and auto-close); replaced inline checks with these helpers so undefined project details are treated as inactive. No exported APIs or public signatures changed. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant UI as Automation Component
participant M as useMemo helper
U->>UI: view component / toggle
UI->>M: compute status from currentProjectDetails
M-->>UI: boolean (false if details undefined or 0)
UI->>UI: render ToggleSwitch (value = status)
alt status true
UI->>UI: render details panel
else status false
UI->>UI: hide details panel
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Pull Request Linked with Plane Work Items
Comment Automatically Generated by Plane |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (2)
apps/web/core/components/automation/auto-archive-automation.tsx (1)
80-95: Wrap onChange in try/catch and track failuresCurrently, failures in handleChange skip tracking altogether. Wrap with try/catch to record both outcomes and avoid silent failures.
- onChange={async () => { - if (currentProjectDetails?.archive_in === 0) { - await handleChange({ archive_in: 1 }); - } else { - await handleChange({ archive_in: 0 }); - } - captureElementAndEvent({ - element: { - elementName: PROJECT_SETTINGS_TRACKER_ELEMENTS.AUTOMATIONS_ARCHIVE_TOGGLE_BUTTON, - }, - event: { - eventName: PROJECT_SETTINGS_TRACKER_EVENTS.auto_archive_workitems, - state: "SUCCESS", - }, - }); - }} + onChange={async () => { + try { + if (currentProjectDetails?.archive_in === 0) { + await handleChange({ archive_in: 1 }); + } else { + await handleChange({ archive_in: 0 }); + } + captureElementAndEvent({ + element: { + elementName: PROJECT_SETTINGS_TRACKER_ELEMENTS.AUTOMATIONS_ARCHIVE_TOGGLE_BUTTON, + }, + event: { + eventName: PROJECT_SETTINGS_TRACKER_EVENTS.auto_archive_workitems, + state: "SUCCESS", + }, + }); + } catch (error) { + captureElementAndEvent({ + element: { + elementName: PROJECT_SETTINGS_TRACKER_ELEMENTS.AUTOMATIONS_ARCHIVE_TOGGLE_BUTTON, + }, + event: { + eventName: PROJECT_SETTINGS_TRACKER_EVENTS.auto_archive_workitems, + state: "ERROR", + }, + }); + } + }}apps/web/core/components/automation/auto-close-automation.tsx (1)
107-122: Wrap onChange in try/catch and track failuresCapture both success and error states to improve observability and avoid silent failures.
- onChange={async () => { - if (currentProjectDetails?.close_in === 0) { - await handleChange({ close_in: 1, default_state: defaultState }); - } else { - await handleChange({ close_in: 0, default_state: null }); - } - captureElementAndEvent({ - element: { - elementName: PROJECT_SETTINGS_TRACKER_ELEMENTS.AUTOMATIONS_CLOSE_TOGGLE_BUTTON, - }, - event: { - eventName: PROJECT_SETTINGS_TRACKER_EVENTS.auto_close_workitems, - state: "SUCCESS", - }, - }); - }} + onChange={async () => { + try { + if (currentProjectDetails?.close_in === 0) { + await handleChange({ close_in: 1, default_state: defaultState }); + } else { + await handleChange({ close_in: 0, default_state: null }); + } + captureElementAndEvent({ + element: { + elementName: PROJECT_SETTINGS_TRACKER_ELEMENTS.AUTOMATIONS_CLOSE_TOGGLE_BUTTON, + }, + event: { + eventName: PROJECT_SETTINGS_TRACKER_EVENTS.auto_close_workitems, + state: "SUCCESS", + }, + }); + } catch (error) { + captureElementAndEvent({ + element: { + elementName: PROJECT_SETTINGS_TRACKER_ELEMENTS.AUTOMATIONS_CLOSE_TOGGLE_BUTTON, + }, + event: { + eventName: PROJECT_SETTINGS_TRACKER_EVENTS.auto_close_workitems, + state: "ERROR", + }, + }); + } + }}
🧹 Nitpick comments (4)
apps/web/core/components/automation/auto-archive-automation.tsx (1)
97-97: Disable toggle while project details are not loadedPrevents firing handleChange against incomplete data and aligns with the “inactive while undefined” intent.
- disabled={!isAdmin} + disabled={!isAdmin || !currentProjectDetails}apps/web/core/components/automation/auto-close-automation.tsx (3)
124-124: Disable toggle while project details are not loadedPrevents updates against incomplete data and aligns with the inactive-until-defined UX.
- disabled={!isAdmin} + disabled={!isAdmin || !currentProjectDetails}
166-166: Typo in className: remove stray “ppy” utility“ppy” is not a valid Tailwind class and is likely a leftover. It could cause confusion.
- <div className="ppy sm:py-10 flex w-full items-center justify-between gap-2 px-5 py-4"> + <div className="sm:py-10 flex w-full items-center justify-between gap-2 px-5 py-4">
108-112: Verify default_state behavior when enabling auto-closeWhen enabling, default_state is set to defaultState, which may be null if projectStates haven’t loaded. Confirm backend allows null here or defer toggling until defaultState is known. If not allowed, guard or compute a safe fallback.
Possible guards:
- Disable the toggle when enabling if defaultState is null.
- Or, compute a fallback cancelled state once projectStates load before allowing enable.
Also applies to: 171-173
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/core/components/automation/auto-archive-automation.tsx(3 hunks)apps/web/core/components/automation/auto-close-automation.tsx(3 hunks)
🔇 Additional comments (2)
apps/web/core/components/automation/auto-archive-automation.tsx (1)
79-79: LGTM: Centralized active-state check removes initial flickerUsing a single helper for both the ToggleSwitch value and conditional rendering is clearer and fixes the undefined→true flicker.
Also applies to: 102-102
apps/web/core/components/automation/auto-close-automation.tsx (1)
106-106: LGTM: Centralized active-state check removes initial flickerConsistent with auto-archive; improves readability and correctness on initial load.
Also applies to: 129-129
apps/web/core/components/automation/auto-archive-automation.tsx
Outdated
Show resolved
Hide resolved
| ); | ||
|
|
||
| const getAutoArchiveStatus = () => { | ||
| if (currentProjectDetails === undefined || currentProjectDetails.archive_in === undefined) return false; |
There was a problem hiding this comment.
There's no need of this check.
There was a problem hiding this comment.
It’s required because this was the cause of the flicker. We’re directly comparing currentProjectDetails?.archive_in !== 0 and rendering the details, which is true even if the archive_in is undefined.
There was a problem hiding this comment.
What I mean is, there's no need to check for currentProjectDetails === undefined
There was a problem hiding this comment.
It will remain the same. Without that, we’ll check currentProjectDetails?.archive_in === undefined with optional chaining, which essentially combines both these conditions.
apps/web/core/components/automation/auto-archive-automation.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
apps/web/core/components/automation/auto-close-automation.tsx (1)
123-125: Disable the toggle while project details are loadingAvoid invoking
handleChangebefore the project is loaded (could lead to unintended updates whencurrentProjectDetailsis undefined).Apply this diff:
- disabled={!isAdmin} + disabled={!isAdmin || !currentProjectDetails}
♻️ Duplicate comments (3)
apps/web/core/components/automation/auto-close-automation.tsx (3)
106-106: Value prop wiring is correct; ensure the memo above updates promptly
value={autoCloseStatus}is correct. With the fix to the memo dependencies/null-safety above, the toggle will reflect store changes immediately.Please verify that toggling updates the UI instantly (no extra render or stale state).
129-129: Conditional details rendering depends on the same autoCloseStatus fixThis usage is fine and should remove the initial flicker. It will still rely on the corrected
autoCloseStatusto avoid stale visibility.
78-82: Fix stale memoization and add null-safety for autoCloseStatus
- Dependency on the whole
currentProjectDetailsobject can keep the memo stale if MobX mutates the same object reference (common pattern).- Null isn’t handled for
close_in; onlyundefinedis checked.Prefer deriving from the primitive and guard both null/undefined to avoid runtime edge-cases and stale UI.
Apply this diff:
- const autoCloseStatus = useMemo(() => { - if (currentProjectDetails === undefined || currentProjectDetails.close_in === undefined) return false; - return currentProjectDetails.close_in !== 0; - }, [currentProjectDetails]); + const autoCloseStatus = useMemo(() => { + const v = currentProjectDetails?.close_in; + return v != null && v !== 0; + }, [currentProjectDetails?.close_in]);
🧹 Nitpick comments (2)
apps/web/core/components/automation/auto-close-automation.tsx (2)
3-3: useMemo import is fine; optionally simplify by removing memoizationIf you adopt the revised dependency fix above, keep
useMemo. Alternatively, you can compute the boolean directly withoutuseMemo(MobX observer will re-render whenclose_inchanges), then drop the import.Optional change if you remove memoization:
-import React, { useMemo, useState } from "react"; +import React, { useState } from "react";And replace the memo block with:
const v = currentProjectDetails?.close_in; const autoCloseStatus = v != null && v !== 0;
166-166: Typo in className: stray "ppy" tokenThe class list includes an invalid "ppy" token which won’t map to any Tailwind class.
Apply this diff:
- <div className="ppy sm:py-10 flex w-full items-center justify-between gap-2 px-5 py-4"> + <div className="sm:py-10 flex w-full items-center justify-between gap-2 px-5 py-4">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/core/components/automation/auto-archive-automation.tsx(4 hunks)apps/web/core/components/automation/auto-close-automation.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/core/components/automation/auto-archive-automation.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/web/core/components/automation/auto-close-automation.tsx (1)
apps/web/core/store/project/project.store.ts (1)
currentProjectDetails(217-220)
3 participants
Description
Updated the
ToggleSwitchcomponents in bothauto-archive-automation.tsxandauto-close-automation.tsxto resolve the flickering issue that occurred during the initial page load.Type of Change
Summary by CodeRabbit
Here are the updated release notes:
Bug Fixes
Refactor