[My Site Migration] Addition Extension Status button to status site#1347
[My Site Migration] Addition Extension Status button to status site#1347iamitprakash merged 19 commits intoRealDevSquad:developfrom
Conversation
|
@RishiChaubey31 is attempting to deploy a commit to the RDS-Team Team on Vercel. A member of the Team first needs to authorize it. |
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Summary by CodeRabbit
WalkthroughThis update introduces a complete extension request workflow for tasks in development mode. It adds new API endpoints and Redux Toolkit Query tags for handling extension requests, including fetching and creating requests. New React components and modals allow users to view, submit, and manage extension requests for tasks, with associated SCSS modules for styling. The Card component is updated to show an "Extension Status" button and modal in dev mode for authorized users. Several new TypeScript types are introduced to support extension request data structures. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CardComponent
participant ExtensionStatusModal
participant ExtensionRequestForm
participant tasksApi (RTK Query)
participant API Server
User->>CardComponent: Clicks "Extension Status"
CardComponent->>ExtensionStatusModal: Opens modal with task info
ExtensionStatusModal->>tasksApi (RTK Query): Fetch extension requests (getSelfExtensionRequests)
tasksApi (RTK Query)->>API Server: GET /extension-requests/self?taskId=...&dev=...
API Server-->>tasksApi (RTK Query): Respond with extension requests
tasksApi (RTK Query)-->>ExtensionStatusModal: Provide requests data
User->>ExtensionStatusModal: Clicks "Request Extension"
ExtensionStatusModal->>ExtensionRequestForm: Opens form
User->>ExtensionRequestForm: Fills and submits form
ExtensionRequestForm->>tasksApi (RTK Query): createExtensionRequest mutation
tasksApi (RTK Query)->>API Server: POST /extension-requests
API Server-->>tasksApi (RTK Query): Respond with new request
tasksApi (RTK Query)-->>ExtensionRequestForm: Success callback
ExtensionRequestForm->>ExtensionStatusModal: Close form and refresh requests
Suggested reviewers
Poem
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. 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 23
🔭 Outside diff range comments (2)
src/components/tasks/card/index.tsx (2)
102-108: 🛠️ Refactor suggestionMissing dependency in
useEffectmay cause stale authorisation state
isUserAuthorizedis referenced inside the effect but not listed in the dependency array, so a change in the user’s auth state will not updateshowEditButton.-useEffect(() => { +useEffect(() => { const isAltKeyLongPressed = keyLongPressed === ALT_KEY; if (isAltKeyLongPressed && isUserAuthorized) { setShowEditButton(true); } -}, [keyLongPressed]); +}, [keyLongPressed, isUserAuthorized]);
466-467: 🧹 Nitpick (assertive)Debounced function recreated on every render
debouncedHandleTitleChangechanges identity each render causing pending timeouts to be lost. Memoise withuseMemo/useCallback.Example:
-const debouncedHandleTitleChange = debounceTitle(handleTitleChange, 3000); +const debouncedHandleTitleChange = useMemo( + () => debounceTitle(handleTitleChange, 3000), + [handleTitleChange] +);
♻️ Duplicate comments (1)
src/components/Form/ExtensionRequestForm.tsx (1)
37-41: Default ETA adds days in ms but may inherit wrong unitRelated to the previous comment – make sure the
+ 3*24*60*60*1000matches the chosen unit.
If you switch to seconds, multiply by3*24*60*60.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (9)
src/app/services/api.ts(1 hunks)src/app/services/tasksApi.ts(3 hunks)src/components/Form/ExtensionRequestForm.tsx(1 hunks)src/components/Form/Form.module.scss(1 hunks)src/components/Modal/ExtensionStatusModal.tsx(1 hunks)src/components/Modal/modal.module.scss(1 hunks)src/components/tasks/card/card.module.scss(1 hunks)src/components/tasks/card/index.tsx(4 hunks)src/interfaces/task.type.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/app/services/tasksApi.ts (1)
src/interfaces/task.type.ts (3)
ExtensionRequestsResponse(178-181)ExtensionRequestCreateResponse(193-196)ExtensionRequestCreatePayload(183-191)
src/components/tasks/card/index.tsx (4)
src/interfaces/task.type.ts (1)
CardProps(108-113)__tests__/Unit/Components/Tasks/TaskDetails.test.tsx (1)
useRouter(721-728)src/components/Modal/ExtensionStatusModal.tsx (1)
ExtensionStatusModal(35-284)src/components/tasks/card/TaskStatusEditMode.tsx (1)
beautifyStatus(89-89)
🪛 Biome (1.9.4)
src/components/tasks/card/index.tsx
[error] 52-52: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
(lint/complexity/noUselessTernary)
[error] 622-625: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
src/components/Modal/ExtensionStatusModal.tsx
[error] 151-154: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
[error] 252-255: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 123-126: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 260-263: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 270-273: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build (18.x)
🔇 Additional comments (12)
src/app/services/api.ts (1)
30-30: LGTM: Tag addition for extension requests caching.The new tag 'Extension_Requests' follows the existing naming convention and properly integrates with the Redux Toolkit Query cache invalidation system.
src/components/tasks/card/card.module.scss (1)
289-292: LGTM: Task control container.The flex layout for
.task-status-controlprovides a clean way to arrange the status update and extension buttons with proper spacing.src/components/Modal/modal.module.scss (1)
131-153: LGTM: Modal overlay and container styles.The modal overlay and container styles create a clean, visually separated interface for the extension request workflow.
src/components/tasks/card/index.tsx (1)
638-645: Minor: avoid hard work in render by precomputing beautified status
beautifyStatus(cardDetails.status)is invoked on every render. Cache it withuseMemoif the helper is expensive.src/components/Form/ExtensionRequestForm.tsx (1)
26-31:Details
❓ Verification inconclusive
Timestamp unit mismatch (ms vs s)
newEndsOnis stored in milliseconds, while the rest of the app (e.g.endsOninCard) uses seconds. Double-check what the backend expects; sending ms when s are expected will extend the task by ~1000×.If backend expects seconds:
-const [formData, setFormData] = useState<FormData>({ - reason: '', - newEndsOn: new Date().getTime(), +const [formData, setFormData] = useState<FormData>({ + reason: '', + newEndsOn: Math.floor(Date.now() / 1000),and similarly convert in
handleChange/defaultNewEndsOn.
🏁 Script executed:
#!/bin/bash set -eu echo "=== Searching for FormData type definitions ===" rg --color=never "type FormData" -n . rg --color=never "interface FormData" -n . echo -e "\n=== Showing top of ExtensionRequestForm.tsx ===" head -n 200 src/components/Form/ExtensionRequestForm.tsx echo -e "\n=== Searching for newEndsOn usages across codebase ===" rg --color=never "newEndsOn" -n srcLength of output: 154
🏁 Script executed:
#!/bin/bash set -eu echo "=== Showing first 200 lines of ExtensionRequestForm.tsx ===" sed -n '1,200p' src/components/Form/ExtensionRequestForm.tsx echo -e "\n=== Searching for FormData definition in the file ===" rg --color=never "interface FormData" -n src/components/Form/ExtensionRequestForm.tsx || true rg --color=never "type FormData" -n src/components/Form/ExtensionRequestForm.tsx || true echo -e "\n=== Searching for newEndsOn usages in the file ===" rg --color=never "newEndsOn" -n src/components/Form/ExtensionRequestForm.tsx || true echo -e "\n=== Searching for .getTime() calls in the file ===" rg --color=never "getTime" -n src/components/Form/ExtensionRequestForm.tsx || trueLength of output: 7600
🏁 Script executed:
#!/bin/bash set -eu echo "=== Locating tasksApi file ===" fd --extension ts --max-depth 3 tasksApi src/app/services echo -e "\n=== Searching for createExtensionRequestMutation in tasksApi ===" rg --color=never "createExtensionRequest" -n src/app/services/tasksApi* echo -e "\n=== Showing surrounding lines for endpoint definition ===" rg --context 5 "createExtensionRequest" -n src/app/services/tasksApi*Length of output: 1034
🏁 Script executed:
#!/bin/bash set -eu echo "=== Top of tasksApi.ts (imports and type imports) ===" sed -n '1,60p' src/app/services/tasksApi.ts echo -e "\n=== Searching for ExtensionRequestCreatePayload definition ===" rg --color=never "ExtensionRequestCreatePayload" -n .Length of output: 2519
🏁 Script executed:
#!/bin/bash set -eu echo "=== Searching for endsOn in task.type.ts ===" rg --color=never "endsOn" -n src/interfaces/task.type.ts echo -e "\n=== Showing definition around endsOn property ===" rg --context 3 "endsOn" -n src/interfaces/task.type.tsLength of output: 1492
Confirm timestamp units with backend
The form currently uses millisecond timestamps (
Date.now()/.getTime()), while yourtaskmodel and most endpoints treatendsOnas a simplenumber—which in practice is seconds. Sending ms when s are expected will inflate deadlines by ~1000×. Please verify the API spec and, if it expects seconds, unify your conversions:• Initial state (lines 26–31):
- newEndsOn: new Date().getTime(), + newEndsOn: Math.floor(Date.now() / 1000),• Default value in useEffect (lines 35–40):
- const defaultNewEndsOn = oldEndsOn - ? oldEndsOn + 3 * 24 * 60 * 60 * 1000 - : new Date().getTime(); + const secondsNow = Math.floor(Date.now() / 1000); + const defaultNewEndsOn = oldEndsOn + ? oldEndsOn + 3 * 24 * 60 * 60 + : secondsNow;• handleChange conversion (lines 47–53):
- const timestamp = new Date(value).getTime(); + const timestamp = Math.floor(new Date(value).getTime() / 1000);❗️ Double-check what unit the backend expects and adjust these conversions accordingly.
src/components/Form/Form.module.scss (1)
76-87:composes:requires CSS-loader support
composes:only works whencss-loader’smodulesoption is enabled withcomposes=true. Verify your build pipeline supports it; otherwise buttons will lose base styles.src/app/services/tasksApi.ts (3)
5-8: Good work adding TypeScript types for extension requests.Adding explicit TypeScript types for the extension request functionality provides good type safety and self-documentation for the API interactions.
107-119: Well implemented query endpoint for fetching extension requests.The implementation follows the RTK Query pattern consistently with proper typing and tag management. The skip option when the query shouldn't run is a good optimization.
151-152: Good job exporting the hooks for component consumption.Clear and consistent naming of the exported hooks makes them easy to use in components.
src/components/Modal/ExtensionStatusModal.tsx (3)
1-5: Good component and hook imports.The component imports all necessary dependencies and hooks.
42-45: Good query implementation with skip optimization.Using the
skipoption when the modal is closed is a good practice to avoid unnecessary API calls.
286-300: Well-implemented time formatting utility.The
formatTimeAgofunction is well-structured and handles various time ranges appropriately.
AnujChhikara
left a comment
AnujChhikara
left a comment
There was a problem hiding this comment.
test coverage missing
as mentioned in the PR description , test are in other PR and I will mention the test coverage here shortly |
RishiChaubey31
requested review from
Achintya-Chatterjee,
AnujChhikara and
yesyash
|
Hello @RishiChaubey31 I have resolve my comments please go through the comments of Yash and Achintya
|
Because then I had to see the working of API ( response, payload etc) and I don't have much clarity hence decided to put it as a separate issue after discussing with @Achintya-Chatterjee (commentor) |
RishiChaubey31
requested review from
Achintya-Chatterjee,
MayankBansal12 and
yesyash
RishiChaubey31
dismissed stale reviews from Achintya-Chatterjee and AnujChhikara
via
71ef856
6 participants
Date: 07-05-2025
Developer Name: Rishi Chaubey
Issue Ticket Number
Description
Documentation Updated?
Under Feature Flag
Database Changes
Breaking Changes
Development Tested?
Screenshots
Screenshot 1
ER.2.mp4
Test Coverage
Screenshot 1
Additional Notes