Fix/dmt feedback#51
Open
Divyasharma-samtaai wants to merge 18 commits into
Open
Conversation
…nd leaderboard sections
…ents - Migrate Newsfeed state management from Context to React Query - Add query-keys, mutation-options, and query-options for Newsfeed - Update PostCard, CommentSection, PostModal, and SocialNewsFeed to use React Query hooks - Refactor CelebrationsCard to handle empty states - Remove file extensions in Policies and Learning & Development pages
…cators, and welcome banner
… semantic classes and improve class merging
… obsolete news components
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/apps/app/lib/socket.ts (1)
142-147:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCancel pending reconnect timer on intentional disconnect.
If a reconnect is already scheduled,
disconnect()currently leaves it active, so the client may reconnect after an intentional close.Suggested fix
disconnect() { this.isIntentionalDisconnect = true; + if (this.reconnectTimer) { + clearTimeout(this.reconnectTimer); + this.reconnectTimer = null; + } + this.reconnectAttempts = 0; if (this.socket) { this.socket.close(); this.socket = null; } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/apps/app/lib/socket.ts` around lines 142 - 147, In the disconnect() method, add code to cancel any pending reconnect timer before closing the socket. This ensures that when disconnect() is intentionally called, any scheduled reconnection is properly cancelled and won't fire after the intentional close. Identify the reconnect timer property (likely something like reconnectTimeout or reconnectTimer) and clear it using the appropriate cleanup method (such as clearTimeout) before or after closing the socket.frontend/apps/app/app/(dashboard)/newsfeed/page.tsx (1)
37-37:⚠️ Potential issue | 🟠 Major | ⚡ Quick winManager gating is inconsistent with mutation authorization checks.
Line 37 allows manager access via
role === "manager", but the mutation layer authorizes onlyuser.is_manager(seefrontend/apps/app/app/(dashboard)/newsfeed/mutation-options.tsLine 22, Line 67, Line 82). This can expose create/edit/delete UI that deterministically fails.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/apps/app/app/`(dashboard)/newsfeed/page.tsx at line 37, The isManager variable on line 37 of newsfeed/page.tsx checks both user.is_manager and a role-based condition, but the mutation authorization in mutation-options.ts only checks user.is_manager. Remove the user?.role?.toLowerCase() === "manager" check from the isManager assignment to align the frontend gating with the actual backend authorization, preventing the display of manager UI actions that would fail at the mutation layer.
🧹 Nitpick comments (1)
frontend/apps/app/app/(dashboard)/policies/page.tsx (1)
43-50: 💤 Low valueDuplicate
getFileViewerUrllogic exists insearch/page.tsx.Consider extracting this helper into a shared utility (e.g., in
@dmt/apior a locallib/folder) to avoid divergence and simplify maintenance.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/apps/app/app/`(dashboard)/policies/page.tsx around lines 43 - 50, The getFileViewerUrl helper function contains duplicated logic that exists in both policies/page.tsx and search/page.tsx. Extract this function into a shared utility module (such as a new file in `@dmt/api` or a local lib/ folder), then remove the duplicate implementations from both page files and import the shared utility instead. This will prevent the logic from diverging and simplify future maintenance of the Office Viewer URL formatting logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docker-compose.yml`:
- Around line 116-120: The company-portal service in docker-compose.yml is
configured to call production endpoints (api.elevate.samta.ai) instead of the
local backend service, which risks accidentally sending development traffic and
test writes to production. Replace all production URLs in the environment
variables NEXT_PUBLIC_API_URL, NEXT_PUBLIC_WS_HOST,
NEXT_PUBLIC_WS_TELEMETRY_URL_TEMPLATE, and NEXT_PUBLIC_MEDIA_BASE_URL with their
corresponding local backend endpoints (using the backend service hostname and
port 8000 or localhost:8000 as appropriate). Update NEXT_PUBLIC_DEV_DOMAINS to
reflect local development domains instead of production domains to ensure
local-only traffic routing.
In `@frontend/apps/admin/package.json`:
- Line 6: The dev script in package.json is missing the port specification that
maintains consistency with documented team workflows and configuration
expectations. Restore the --port 3001 flag to the dev script command so that
next dev explicitly runs on port 3001 instead of defaulting to port 3000,
ensuring the admin development server runs on the documented port and avoids
port conflicts with other local services.
In `@frontend/apps/app/app/`(dashboard)/newsfeed/mutation-options.ts:
- Around line 58-61: The onError handler in the mutation options only rolls back
the cache when context.previousPosts is truthy, which means when the cache is
initially empty (undefined), a failed mutation leaves the optimistic post
inserted earlier as a phantom entry. Modify the condition to ensure the query
data is always reset regardless of whether previousPosts is truthy or undefined.
Change the check from conditionally rolling back only when context.previousPosts
exists to always setting the query data with context.previousPosts (which may be
undefined), so that failed mutations properly clean up inserted posts even when
starting from an empty cache.
- Around line 25-32: The mutations in the socket.emit calls (create_post,
update_post, and delete_post functions) resolve immediately without waiting for
server confirmation or handling error responses. Since this codebase uses native
WebSocket with a custom WSClient rather than Socket.IO, ack callbacks are not
available. Instead, add event listeners for server error responses (such as
error events or negative acknowledgement events) to catch validation errors or
permission denials from the server, following the same pattern used in the
get_posts function for listening to server responses. This ensures mutations
only resolve on successful server confirmation and reject when the server
indicates failure.
In `@frontend/apps/app/app/`(dashboard)/newsfeed/query-options.ts:
- Around line 10-39: Add a timeout mechanism to the queryFn Promise that
prevents indefinite hanging if the server never sends a 'posts' or 'error'
event. Implement a setTimeout that rejects the Promise after a reasonable
duration (e.g., 30 seconds) and ensure the timeout callback properly cleans up
the socket listeners by calling socket.off() for both the 'posts' and 'error'
event handlers (handlePosts and handleError). Additionally, clear the timeout in
both the handlePosts and handleError callbacks to prevent the timeout from
executing after a successful or failed response is already received.
In `@frontend/apps/app/app/`(dashboard)/onboarding/page.tsx:
- Around line 46-52: The getFileViewerUrl function in both files checks file
extensions against the full URL, allowing signed URLs or URLs with query
parameters to bypass the Office-viewer formatting. Fix this by extracting only
the path portion of the URL (before query strings and hash fragments) before
applying the extension regex check. In
frontend/apps/app/app/(dashboard)/onboarding/page.tsx at lines 46-52, modify the
function to split the URL at query strings and fragments using
url.split(/[?#]/)[0] before passing it to the extension regex in the
lowerUrl.match() call. Apply the same path-only extraction in the corresponding
getFileViewerUrl function in
frontend/apps/app/app/(dashboard)/learning-and-development/page.tsx at lines
28-34 to maintain consistent Office-viewer behavior across both files.
In `@frontend/apps/app/app/`(dashboard)/org-chart/page.tsx:
- Around line 141-145: The departments API call in getDepartments() is coupled
to the critical path with getHierarchy() and getRolesDropdown() in the
Promise.all() block, causing the entire org-chart to fail if departments fails.
Remove orgChart.getDepartments() from the Promise.all() call to keep only the
critical hierarchy and roles data in the main await, then fetch departments
separately as a non-blocking operation that can fail independently without
affecting the core org-chart rendering.
In `@frontend/apps/app/app/`(dashboard)/search/page.tsx:
- Around line 20-21: The limit parameter parsed from URL search params is not
bounded, allowing users to provide arbitrarily large values that can trigger
heavy requests and degrade performance. After parsing the limitParam with
parseInt on line 21, add logic to clamp the parsed limit value to a reasonable
maximum (e.g., using Math.min with a sensible upper bound like 100 or 1000)
before assigning it to the limit variable. This ensures the limit value is
safely bounded before it's used in any subsequent queries.
- Around line 29-33: When the query becomes empty (checked via !query.trim()),
the loading state is not being reset before the early return, which leaves the
page stuck in a spinner state if a request was in-flight. Add a
setLoading(false) call before the return statement in the empty-query check
block to ensure the loading state is properly reset. Apply this same fix to the
other location where this pattern occurs.
In `@frontend/apps/app/app/api/download/route.ts`:
- Around line 5-7: The development mode check in the route.ts file
unconditionally returns true, bypassing all SSRF protections and allowing the
proxy to fetch arbitrary URLs including cloud metadata endpoints and internal
network services. Instead of returning true unconditionally when NODE_ENV is
'development', keep the SSRF protection checks active (such as private IP
validation and metadata endpoint checks) even in development mode.
Alternatively, implement explicit allowlisting of only the backend host or
trusted domains rather than allowing all URLs in development.
In `@frontend/apps/app/components/ImageCropperModal.tsx`:
- Around line 15-16: The croppedAreaPixels state is initialized as null and can
be passed to the crop helper before it has been set by the crop callback,
causing the save action to fail silently. Add null checks or guards at the usage
sites (around lines 22-27 and 93-94) to prevent the save action from proceeding
when croppedAreaPixels is null. You should either disable the save button/action
when croppedAreaPixels is null, or add conditional logic that returns early or
shows an error if the value is still null when save is attempted. Change the
type annotation from any to a more specific type if possible to better represent
the actual shape of the cropped area data.
- Around line 69-77: The zoom input slider element uses aria-labelledby="Zoom"
but no element with id="Zoom" exists in the component, causing the slider to be
announced without a proper accessible name. Either add a label element with
id="Zoom" and associate it with the zoom slider input, or replace the
aria-labelledby attribute with aria-label="Zoom" (or a more descriptive label)
to provide the required accessible name for the slider control.
In `@frontend/apps/app/components/Navbar.tsx`:
- Around line 159-161: The Office file extension detection in
`frontend/apps/app/components/Navbar.tsx` (lines 159-161) and
`frontend/apps/app/app/(dashboard)/search/page.tsx` (lines 67-71) is fragile
because it tests the full URL string, which fails when query parameters or hash
fragments are present after the file extension. Extract the pathname portion of
the fileUrl by stripping query strings and fragments (remove everything from the
first `?` or `#` character onward) before applying the extension regex match for
both the isOfficeFile check in Navbar.tsx and the corresponding extension
detection logic in search/page.tsx. This ensures that signed URLs with query
strings and other parameterized document URLs are correctly identified as Office
files and routed to the viewer.
- Around line 32-52: The useEffect hook in Navbar.tsx contains a debounced
search that can display stale results if async responses complete out of order.
Additionally, the error handler doesn't clear previous results, leaving stale
data visible. Add a request version/ID mechanism (using an AbortController or
simple counter incremented with each search) to track the latest request. Update
the fetchResults function to only call setDropdownResults and setShowDropdown if
the response corresponds to the current request version. Modify the catch block
to explicitly clear both setDropdownResults and setShowDropdown on error,
ensuring stale UI state is never persisted when requests fail or are superseded
by newer searches.
In `@frontend/apps/app/components/NotificationBell.tsx`:
- Line 44: The className for the span element uses Tailwind CSS v3 syntax for
CSS variables with `var(--color-access)` and `var(--color-access-foreground)`,
but the project uses Tailwind CSS v4.2.1 which requires parenthesis syntax
instead. Update the bg-[var(--color-access)] utility to bg-(--color-access) and
the text-[var(--color-access-foreground)] utility to
text-(--color-access-foreground) in the span className to conform to Tailwind v4
syntax for CSS variable references.
In `@frontend/apps/app/components/org-chart/EmployeeModal.tsx`:
- Around line 82-83: The code is lowercasing department values using
.toLowerCase() when initializing the department field in the EmployeeModal
component, which can silently rewrite canonical department names and cause
selection mismatches during edit operations. Remove all .toLowerCase() calls
applied to department names throughout the form initialization and data handling
logic to preserve the canonical department names as they are supplied and
displayed in the application.
In `@frontend/apps/app/constants/dmtTerms.tsx`:
- Line 92: There is missing spacing after closing bold tags in the dmtTerms.tsx
file which causes text to render as merged words in the UI. In
frontend/apps/app/constants/dmtTerms.tsx at line 92, after the closing </strong>
tag in the text about defect calculation, add a space before "Count" so it reads
"...</strong> Count..." instead of "...</strong>Count...". Apply the same fix at
lines 112, 178, and 277 where similar patterns exist with bold labels followed
immediately by text without spacing, ensuring all bold label sections have
proper spacing for readable UI copy.
In `@frontend/apps/app/context/WebSocketContext.tsx`:
- Around line 34-40: The cleanup function removes the 'close' event listener
before disconnecting the client, which prevents the handleClose callback from
firing and leaves isConnected in a stale true state. To fix this, explicitly
reset isConnected to false during cleanup in the return function alongside the
client.off calls and client.disconnect() call. This ensures the state is
properly cleared when the component unmounts, preventing downstream
socket-driven queries from being incorrectly gated by the stale true value.
In `@frontend/apps/app/hooks/useNewsfeedQuery.ts`:
- Around line 48-55: The onPostCreated handler uses title, content, and author
comparison to deduplicate posts, which incorrectly removes valid distinct posts
when the same author publishes repeated content. Replace the deduplication logic
in the filter condition within queryClient.setQueryData to use post_id
comparison instead of the title/content/author combination, matching the
approach used by other handlers like onPosts, onPostUpdated, and onPostDeleted.
- Line 13: Stabilize the `queryKey` reference by memoizing the call to
`newsfeedKeys.posts()` or storing it in a constant outside the component body to
ensure it maintains the same reference across renders, preventing the effect
dependency from changing and causing unnecessary re-execution. Additionally, fix
the `onPostCreated` deduplication logic by changing the filter condition to
compare posts by `post_id` instead of filtering by title, content, and author,
which can incorrectly remove distinct posts with identical text or authorship.
---
Outside diff comments:
In `@frontend/apps/app/app/`(dashboard)/newsfeed/page.tsx:
- Line 37: The isManager variable on line 37 of newsfeed/page.tsx checks both
user.is_manager and a role-based condition, but the mutation authorization in
mutation-options.ts only checks user.is_manager. Remove the
user?.role?.toLowerCase() === "manager" check from the isManager assignment to
align the frontend gating with the actual backend authorization, preventing the
display of manager UI actions that would fail at the mutation layer.
In `@frontend/apps/app/lib/socket.ts`:
- Around line 142-147: In the disconnect() method, add code to cancel any
pending reconnect timer before closing the socket. This ensures that when
disconnect() is intentionally called, any scheduled reconnection is properly
cancelled and won't fire after the intentional close. Identify the reconnect
timer property (likely something like reconnectTimeout or reconnectTimer) and
clear it using the appropriate cleanup method (such as clearTimeout) before or
after closing the socket.
---
Nitpick comments:
In `@frontend/apps/app/app/`(dashboard)/policies/page.tsx:
- Around line 43-50: The getFileViewerUrl helper function contains duplicated
logic that exists in both policies/page.tsx and search/page.tsx. Extract this
function into a shared utility module (such as a new file in `@dmt/api` or a local
lib/ folder), then remove the duplicate implementations from both page files and
import the shared utility instead. This will prevent the logic from diverging
and simplify future maintenance of the Office Viewer URL formatting logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c43caf22-3184-45a2-909c-5c9b446902ab
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (68)
ai-docs/review/onboarding-newsfeed-review.mdai-docs/review/review_f7eb7cb.mddocker-compose.ymlfrontend/apps/admin/package.jsonfrontend/apps/app/app/(dashboard)/engagement-calendar/page.tsxfrontend/apps/app/app/(dashboard)/holiday-calendar/page.tsxfrontend/apps/app/app/(dashboard)/latest-news/page.tsxfrontend/apps/app/app/(dashboard)/learning-and-development/mutation-options.tsfrontend/apps/app/app/(dashboard)/learning-and-development/page.tsxfrontend/apps/app/app/(dashboard)/learning-and-development/query-keys.tsfrontend/apps/app/app/(dashboard)/learning-and-development/query-options.tsfrontend/apps/app/app/(dashboard)/metrics/[id]/page.tsxfrontend/apps/app/app/(dashboard)/metrics/page.tsxfrontend/apps/app/app/(dashboard)/newsfeed/mutation-options.tsfrontend/apps/app/app/(dashboard)/newsfeed/page.tsxfrontend/apps/app/app/(dashboard)/newsfeed/query-keys.tsfrontend/apps/app/app/(dashboard)/newsfeed/query-options.tsfrontend/apps/app/app/(dashboard)/notifications/send/page.tsxfrontend/apps/app/app/(dashboard)/onboarding/mutation-options.tsfrontend/apps/app/app/(dashboard)/onboarding/page.tsxfrontend/apps/app/app/(dashboard)/onboarding/query-keys.tsfrontend/apps/app/app/(dashboard)/onboarding/query-options.tsfrontend/apps/app/app/(dashboard)/org-chart/page.tsxfrontend/apps/app/app/(dashboard)/page.tsxfrontend/apps/app/app/(dashboard)/policies/page.tsxfrontend/apps/app/app/(dashboard)/profile/page.tsxfrontend/apps/app/app/(dashboard)/search/page.tsxfrontend/apps/app/app/(dashboard)/sprint-comparison/page.tsxfrontend/apps/app/app/api/download/route.tsfrontend/apps/app/app/globals.cssfrontend/apps/app/components/AssigneeDistributionCard.tsxfrontend/apps/app/components/CelebrationsCard.tsxfrontend/apps/app/components/DashboardLayout.tsxfrontend/apps/app/components/ImageCropperModal.tsxfrontend/apps/app/components/Navbar.tsxfrontend/apps/app/components/NotificationBell.tsxfrontend/apps/app/components/ProjectSelector.tsxfrontend/apps/app/components/QuickAccessTiles.tsxfrontend/apps/app/components/Sidebar.tsxfrontend/apps/app/components/SocialNewsFeed.tsxfrontend/apps/app/components/SprintSelector.tsxfrontend/apps/app/components/StarPerformer.tsxfrontend/apps/app/components/WelcomeBanner.tsxfrontend/apps/app/components/__tests__/SocialNewsFeed.test.tsxfrontend/apps/app/components/charts/VelocityChart.tsxfrontend/apps/app/components/knowledge/KnowledgeSidebar.tsxfrontend/apps/app/components/latest-news/FeaturedNews.tsxfrontend/apps/app/components/latest-news/LatestNews.tsxfrontend/apps/app/components/latest-news/NewsQueueList.tsxfrontend/apps/app/components/newsfeedUI/CommentItem.tsxfrontend/apps/app/components/newsfeedUI/CommentSection.tsxfrontend/apps/app/components/newsfeedUI/PostCard.tsxfrontend/apps/app/components/newsfeedUI/PostModal.tsxfrontend/apps/app/components/newsfeedUI/StaticNavbar.tsxfrontend/apps/app/components/org-chart/EmployeeModal.tsxfrontend/apps/app/constants/dmtTerms.tsxfrontend/apps/app/context/AuthContext.tsxfrontend/apps/app/context/WebSocketContext.tsxfrontend/apps/app/hooks/useComments.tsfrontend/apps/app/hooks/useNewsfeedData.tsfrontend/apps/app/hooks/useNewsfeedMutations.tsfrontend/apps/app/hooks/useNewsfeedQuery.tsfrontend/apps/app/lib/cropImage.tsfrontend/apps/app/lib/socket.tsfrontend/apps/app/types/newsfeed.tsfrontend/package.jsonfrontend/packages/api/index.tsfrontend/packages/api/knowledge-base-apis/records-api.ts
💤 Files with no reviewable changes (5)
- frontend/apps/app/components/latest-news/FeaturedNews.tsx
- frontend/apps/app/components/latest-news/LatestNews.tsx
- frontend/apps/app/components/latest-news/NewsQueueList.tsx
- frontend/apps/app/hooks/useNewsfeedData.ts
- frontend/apps/app/app/(dashboard)/latest-news/page.tsx
Comment on lines
+116
to
+120
| - NEXT_PUBLIC_API_URL=https://api.elevate.samta.ai/api/ | ||
| - NEXT_PUBLIC_WS_HOST=wss://api.elevate.samta.ai | ||
| - NEXT_PUBLIC_WS_TELEMETRY_URL_TEMPLATE=wss://api.elevate.samta.ai/ws/telemetry/{tenant_id}/ | ||
| - NEXT_PUBLIC_MEDIA_BASE_URL=https://api.elevate.samta.ai | ||
| - NEXT_PUBLIC_DEV_DOMAINS=localhost,127.0.0.1,api.elevate.samta.ai,admin.elevate.samta.ai,elevate.samta.ai |
There was a problem hiding this comment.
Pointing local company-portal to production endpoints is risky
In this local compose stack, company-portal is configured to call production (api.elevate.samta.ai) instead of the local backend (backend:8000/localhost:8000). That can send local dev traffic and test writes to production unintentionally.
Suggested safer dev-default diff
- - NEXT_PUBLIC_API_URL=https://api.elevate.samta.ai/api/
- - NEXT_PUBLIC_WS_HOST=wss://api.elevate.samta.ai
- - NEXT_PUBLIC_WS_TELEMETRY_URL_TEMPLATE=wss://api.elevate.samta.ai/ws/telemetry/{tenant_id}/
- - NEXT_PUBLIC_MEDIA_BASE_URL=https://api.elevate.samta.ai
- - NEXT_PUBLIC_DEV_DOMAINS=localhost,127.0.0.1,api.elevate.samta.ai,admin.elevate.samta.ai,elevate.samta.ai
+ - NEXT_PUBLIC_API_URL=http://localhost:8000/api/
+ - NEXT_PUBLIC_WS_HOST=localhost
+ - NEXT_PUBLIC_WS_TELEMETRY_URL_TEMPLATE=ws://localhost:8000/ws/telemetry/{tenant_id}/
+ - NEXT_PUBLIC_MEDIA_BASE_URL=http://localhost:8000
+ - NEXT_PUBLIC_DEV_DOMAINS=localhost,127.0.0.1As per coding guidelines, cross-file configuration contracts should validate the actual values to prevent high-impact environment misrouting.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - NEXT_PUBLIC_API_URL=https://api.elevate.samta.ai/api/ | |
| - NEXT_PUBLIC_WS_HOST=wss://api.elevate.samta.ai | |
| - NEXT_PUBLIC_WS_TELEMETRY_URL_TEMPLATE=wss://api.elevate.samta.ai/ws/telemetry/{tenant_id}/ | |
| - NEXT_PUBLIC_MEDIA_BASE_URL=https://api.elevate.samta.ai | |
| - NEXT_PUBLIC_DEV_DOMAINS=localhost,127.0.0.1,api.elevate.samta.ai,admin.elevate.samta.ai,elevate.samta.ai | |
| - NEXT_PUBLIC_API_URL=http://localhost:8000/api/ | |
| - NEXT_PUBLIC_WS_HOST=localhost | |
| - NEXT_PUBLIC_WS_TELEMETRY_URL_TEMPLATE=ws://localhost:8000/ws/telemetry/{tenant_id}/ | |
| - NEXT_PUBLIC_MEDIA_BASE_URL=http://localhost:8000 | |
| - NEXT_PUBLIC_DEV_DOMAINS=localhost,127.0.0.1 |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docker-compose.yml` around lines 116 - 120, The company-portal service in
docker-compose.yml is configured to call production endpoints
(api.elevate.samta.ai) instead of the local backend service, which risks
accidentally sending development traffic and test writes to production. Replace
all production URLs in the environment variables NEXT_PUBLIC_API_URL,
NEXT_PUBLIC_WS_HOST, NEXT_PUBLIC_WS_TELEMETRY_URL_TEMPLATE, and
NEXT_PUBLIC_MEDIA_BASE_URL with their corresponding local backend endpoints
(using the backend service hostname and port 8000 or localhost:8000 as
appropriate). Update NEXT_PUBLIC_DEV_DOMAINS to reflect local development
domains instead of production domains to ensure local-only traffic routing.
| "private": true, | ||
| "scripts": { | ||
| "dev": "next dev --port 3001", | ||
| "dev": "next dev", |
There was a problem hiding this comment.
dev script port change breaks documented standalone admin workflow
next dev now defaults to port 3000, while existing team docs indicate admin runs on 3001. Either restore --port 3001 here or update docs/workflow guidance to avoid local confusion and port clashes outside Docker.
As per coding guidelines, configuration and downstream usage expectations should remain consistent across related files.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/apps/admin/package.json` at line 6, The dev script in package.json
is missing the port specification that maintains consistency with documented
team workflows and configuration expectations. Restore the --port 3001 flag to
the dev script command so that next dev explicitly runs on port 3001 instead of
defaulting to port 3000, ensuring the admin development server runs on the
documented port and avoids port conflicts with other local services.
Comment on lines
+25
to
+32
| socket.emit('create_post', { | ||
| title, | ||
| content, | ||
| category, | ||
| author: user.id, | ||
| image_id: imageId || null | ||
| }); | ||
| return { title, content, category, imageId }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify current emit usage and whether server-side handlers expose ack callbacks.
rg -n -C2 "socket\.emit\('create_post'|socket\.emit\('update_post'|socket\.emit\('delete_post'" frontend/apps/app
rg -n -C3 "create_post|update_post|delete_post" frontendRepository: ekesel/dmt-sldp
Length of output: 10695
🏁 Script executed:
#!/bin/bash
# Search for server-side WebSocket handler implementation
find . -type f \( -name "*.py" -o -name "*.js" -o -name "*.ts" \) -path "*/backend/*" -o -path "*/server/*" | head -20
# Look for Django Channels or Socket.IO server code
fd -t f "consumers\|handlers\|websocket"
# Search for handler patterns
rg -l "create_post|update_post|delete_post" --type py --type js --type ts | grep -E "(backend|server)"Repository: ekesel/dmt-sldp
Length of output: 981
🏁 Script executed:
#!/bin/bash
# Check if there are any ack callbacks or error handlers in the mutation-options.ts file
cat -n frontend/apps/app/app/\(dashboard\)/newsfeed/mutation-options.ts | head -100Repository: ekesel/dmt-sldp
Length of output: 4168
🏁 Script executed:
#!/bin/bash
# Search for any error event listeners or ack patterns in the client code
rg -n "socket\.(on|once)\(" frontend/apps/app/app/\(dashboard\)/newsfeed/ -A 2
rg -n "socket\.(emit|off)" frontend/apps/app/app/\(dashboard\)/newsfeed/ -B 1 -A 1Repository: ekesel/dmt-sldp
Length of output: 2327
🏁 Script executed:
#!/bin/bash
# Read the server-side WebSocket consumer implementation
cat -n backend/newsapp/consumers.pyRepository: ekesel/dmt-sldp
Length of output: 17804
🏁 Script executed:
#!/bin/bash
# Check for global socket error handlers and socket initialization
rg -n "socket\.(on|once)\(" frontend/apps/app --type ts --type tsx -B 2 -A 2 | head -60
# Also search for where socket is created/connected
rg -n "socket\s*=" frontend/apps/app/app/\(dashboard\)/newsfeed/ -B 2 -A 2Repository: ekesel/dmt-sldp
Length of output: 87
🏁 Script executed:
#!/bin/bash
# Check if there's any global error handling
rg -n "socket\.on\(['\"](error|disconnect)" frontend/apps/app -B 2 -A 3Repository: ekesel/dmt-sldp
Length of output: 592
🏁 Script executed:
#!/bin/bash
# Search more broadly for socket initialization and how it handles other events
fd socket frontend/apps/app/app/\(dashboard\)/newsfeed/ -t f
# Check the context provider or hook that creates the socket
rg -n "useSocket\|createSocket\|io(" frontend/apps/app/app/\(dashboard\)/newsfeed/ -B 3 -A 3Repository: ekesel/dmt-sldp
Length of output: 166
🏁 Script executed:
#!/bin/bash
# Also check what type of socket library is being used
rg -n "from.*socket|import.*socket|import.*io" frontend/apps/app -t ts | head -20Repository: ekesel/dmt-sldp
Length of output: 2542
🏁 Script executed:
#!/bin/bash
# Check WebSocketContext and socket client
cat -n frontend/apps/app/context/WebSocketContext.tsx
# Also check the socket client implementation
cat -n frontend/apps/app/lib/socket.tsRepository: ekesel/dmt-sldp
Length of output: 7703
🏁 Script executed:
#!/bin/bash
# Check if there are global error listeners set up anywhere
rg -n "socket\.on" frontend/apps/app -B 2 -A 3 | grep -E "(error|disconnect|connect)" -B 2 -A 2Repository: ekesel/dmt-sldp
Length of output: 1695
Mutations resolve before server acknowledgement—use event listeners instead of ack callbacks.
The core issue is valid: at lines 25–32, 70–76, and 85–86, mutations emit events and immediately resolve without waiting for server confirmation. If the server rejects an operation (validation error, permission denied), the mutation still reports success because no error listener catches the server's response.
However, the suggested ack-based solution won't work here. This codebase uses native WebSocket + a custom WSClient, not Socket.IO. Ack callbacks (socket.emit(event, payload, callback)) are a Socket.IO feature and unavailable with raw WebSocket or Django Channels.
Instead, add event listeners for error responses (like get_posts does on line 36 of query-options.ts):
socket.emit('create_post', { ... });
+ const errorPromise = new Promise((_, reject) => {
+ const handler = (err: any) => {
+ socket.off('error', handler);
+ reject(err);
+ };
+ socket.on('error', handler);
+ setTimeout(() => { socket.off('error', handler); reject(new Error('timeout')); }, 5000);
+ });
- return { title, content, category, imageId };
+ await errorPromise.catch(() => {}); // Wait for error or timeout
+ return { title, content, category, imageId };Or refactor mutations to wait for broadcast confirmation (post_created, post_updated, post_deleted events) instead of resolving immediately.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/apps/app/app/`(dashboard)/newsfeed/mutation-options.ts around lines
25 - 32, The mutations in the socket.emit calls (create_post, update_post, and
delete_post functions) resolve immediately without waiting for server
confirmation or handling error responses. Since this codebase uses native
WebSocket with a custom WSClient rather than Socket.IO, ack callbacks are not
available. Instead, add event listeners for server error responses (such as
error events or negative acknowledgement events) to catch validation errors or
permission denials from the server, following the same pattern used in the
get_posts function for listening to server responses. This ensures mutations
only resolve on successful server confirmation and reject when the server
indicates failure.
Comment on lines
+58
to
+61
| onError: (err: any, newPost: any, context: any) => { | ||
| if (context?.previousPosts) { | ||
| queryClient.setQueryData(newsfeedKeys.posts(), context.previousPosts); | ||
| } |
There was a problem hiding this comment.
Rollback skips empty-cache snapshots and can leave phantom optimistic posts.
Line 59 only rolls back when context.previousPosts is truthy. If the cache was initially empty (undefined snapshot), a failed mutation can leave the temp post inserted on Line 54.
Targeted fix
- onError: (err: any, newPost: any, context: any) => {
- if (context?.previousPosts) {
- queryClient.setQueryData(newsfeedKeys.posts(), context.previousPosts);
- }
- }
+ onError: (_err: any, _newPost: any, context: any) => {
+ if (context && 'previousPosts' in context) {
+ queryClient.setQueryData(newsfeedKeys.posts(), context.previousPosts ?? []);
+ }
+ }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/apps/app/app/`(dashboard)/newsfeed/mutation-options.ts around lines
58 - 61, The onError handler in the mutation options only rolls back the cache
when context.previousPosts is truthy, which means when the cache is initially
empty (undefined), a failed mutation leaves the optimistic post inserted earlier
as a phantom entry. Modify the condition to ensure the query data is always
reset regardless of whether previousPosts is truthy or undefined. Change the
check from conditionally rolling back only when context.previousPosts exists to
always setting the query data with context.previousPosts (which may be
undefined), so that failed mutations properly clean up inserted posts even when
starting from an empty cache.
Comment on lines
+10
to
+39
| queryFn: () => { | ||
| return new Promise<Post[]>((resolve, reject) => { | ||
| if (!socket) { | ||
| return reject(new Error('WebSocket is not connected')); | ||
| } | ||
|
|
||
| const handlePosts = (payload: any) => { | ||
| socket.off('posts', handlePosts); | ||
| socket.off('error', handleError); | ||
|
|
||
| if (payload.error) { | ||
| reject(new Error(payload.error)); | ||
| } else { | ||
| handlers.setPage(payload.page || 1); | ||
| handlers.setHasNextPage(!!payload.has_next); | ||
| resolve(payload.data || []); | ||
| } | ||
| }; | ||
|
|
||
| const handleError = (err: any) => { | ||
| socket.off('posts', handlePosts); | ||
| socket.off('error', handleError); | ||
| reject(err); | ||
| }; | ||
|
|
||
| socket.on('posts', handlePosts); | ||
| socket.on('error', handleError); | ||
|
|
||
| socket.emit('get_posts', { page: 1 }); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify pending-promise risk points in the migrated newsfeed query path.
rg -n -C3 "new Promise<Post\\[\\]>|socket\\.on\\('posts'|socket\\.on\\('error'|socket\\.off\\('posts'|socket\\.off\\('error'|setTimeout\\(" frontend/apps/app/app/\(dashboard\)/newsfeed/query-options.tsRepository: ekesel/dmt-sldp
Length of output: 1001
🏁 Script executed:
#!/bin/bash
# Check if abortSignal or cancellation is used in query options or related files
rg -n "abortSignal|AbortSignal|AbortController" frontend/apps/app/app/\(dashboard\)/newsfeed/ -A 3Repository: ekesel/dmt-sldp
Length of output: 41
🏁 Script executed:
#!/bin/bash
# Look for other socket.on/emit patterns in the codebase to see if timeouts are used elsewhere
rg -n "socket\.(on|off|emit)" frontend/apps/app/ -A 2 -B 2 | head -100Repository: ekesel/dmt-sldp
Length of output: 6537
🏁 Script executed:
#!/bin/bash
# Check the context of this queryFn - how is it being used/called
rg -n "newsfeedKeys\.posts\|queryFn" frontend/apps/app/ -B 5 -A 5Repository: ekesel/dmt-sldp
Length of output: 41
Add timeout/cancellation to prevent hung queries and orphaned listeners.
The queryFn will hang indefinitely if the server never sends a posts or error event, leaving listeners attached to the socket and blocking retry behavior. Unlike the hook in useNewsfeedQuery.ts which cleans up listeners via useEffect, this Promise-based approach has no cleanup if the response never arrives.
Add a timeout with cleanup to ensure listeners are removed and the query rejects appropriately:
Suggested fix
queryFn: () => {
return new Promise<Post[]>((resolve, reject) => {
if (!socket) {
return reject(new Error('WebSocket is not connected'));
}
+ const timeoutId = setTimeout(() => {
+ socket.off('posts', handlePosts);
+ socket.off('error', handleError);
+ reject(new Error('Timed out waiting for posts response'));
+ }, 10000);
const handlePosts = (payload: any) => {
+ clearTimeout(timeoutId);
socket.off('posts', handlePosts);
socket.off('error', handleError);
...
};
const handleError = (err: any) => {
+ clearTimeout(timeoutId);
socket.off('posts', handlePosts);
socket.off('error', handleError);
reject(err);
};🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/apps/app/app/`(dashboard)/newsfeed/query-options.ts around lines 10
- 39, Add a timeout mechanism to the queryFn Promise that prevents indefinite
hanging if the server never sends a 'posts' or 'error' event. Implement a
setTimeout that rejects the Promise after a reasonable duration (e.g., 30
seconds) and ensure the timeout callback properly cleans up the socket listeners
by calling socket.off() for both the 'posts' and 'error' event handlers
(handlePosts and handleError). Additionally, clear the timeout in both the
handlePosts and handleError callbacks to prevent the timeout from executing
after a successful or failed response is already received.
Comment on lines
82
to
83
| department: employeeData ? employeeData.department : (departmentsList.length > 0 ? String(departmentsList[0].name).toLowerCase() : ''), | ||
| parentId: employeeData ? (employeeData.parentId || '') : (defaultParentId || ''), |
There was a problem hiding this comment.
Stop lowercasing department values in state/options/payload.
This flow normalizes departments to lowercase at multiple points, but departments are supplied/displayed as canonical names. That can silently rewrite existing department names and cause selection mismatch during edit when case differs.
Suggested fix
- department: employeeData ? employeeData.department : (departmentsList.length > 0 ? String(departmentsList[0].name).toLowerCase() : ''),
+ department: employeeData ? employeeData.department : (departmentsList.length > 0 ? String(departmentsList[0].name) : ''),
- const newDepartment = s.department === prev.department ? (departmentsListRef.current.length > 0 ? String(departmentsListRef.current[0].name).toLowerCase() : '') : s.department;
+ const newDepartment = s.department === prev.department ? (departmentsListRef.current.length > 0 ? String(departmentsListRef.current[0].name) : '') : s.department;
- department: finalDepartment.toLowerCase(),
+ department: finalDepartment,
- department: departmentsList.length > 0 ? String(departmentsList[0].name).toLowerCase() : ''
+ department: departmentsList.length > 0 ? String(departmentsList[0].name) : ''
- <option key={dept.id} value={String(dept.name).toLowerCase()}>
+ <option key={dept.id} value={String(dept.name)}>Also applies to: 122-123, 290-290, 441-442, 463-464
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/apps/app/components/org-chart/EmployeeModal.tsx` around lines 82 -
83, The code is lowercasing department values using .toLowerCase() when
initializing the department field in the EmployeeModal component, which can
silently rewrite canonical department names and cause selection mismatches
during edit operations. Remove all .toLowerCase() calls applied to department
names throughout the form initialization and data handling logic to preserve the
canonical department names as they are supplied and displayed in the
application.
| <div className="space-y-2"> | ||
| <p><strong>Meaning:</strong> Checks quality by measuring how many bugs are found compared to the amount of work completed. It answers: <em>Are we maintaining quality while working fast?</em></p> | ||
| <p><strong>Formula:</strong> (Total Bugs / Total Story Points Delivered) × 100</p> | ||
| <p><strong>How it is calculated: </strong>Count the total number of defects (bugs) reported for the completed work and divide it by the total story points delivered. Multiply the result by 100 to express the value as a percentage.</p> |
There was a problem hiding this comment.
Fix missing spacing after bold labels in rendered help text.
On Lines 92, 112, 178, and 277, text starts immediately after </strong> (...:</strong>Count/...) which renders as merged words. Add a space after the closing tag for readable UI copy.
Also applies to: 112-112, 178-178, 277-277
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/apps/app/constants/dmtTerms.tsx` at line 92, There is missing
spacing after closing bold tags in the dmtTerms.tsx file which causes text to
render as merged words in the UI. In frontend/apps/app/constants/dmtTerms.tsx at
line 92, after the closing </strong> tag in the text about defect calculation,
add a space before "Count" so it reads "...</strong> Count..." instead of
"...</strong>Count...". Apply the same fix at lines 112, 178, and 277 where
similar patterns exist with bold labels followed immediately by text without
spacing, ensuring all bold label sections have proper spacing for readable UI
copy.
Comment on lines
34
to
40
| return () => { | ||
| client.off('message', handleAllMessages); | ||
| client.off('open', handleOpen); | ||
| client.off('close', handleClose); | ||
| client.disconnect(); | ||
| }; | ||
| } |
There was a problem hiding this comment.
Reset isConnected during cleanup to avoid stale true state.
isConnected can stay true after teardown because Line 36–37 removes the close listener before Line 38 disconnects. That stale state can incorrectly gate downstream socket-driven queries.
Suggested fix
return () => {
+ setIsConnected(false);
client.off('message', handleAllMessages);
client.off('open', handleOpen);
client.off('close', handleClose);
client.disconnect();
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| return () => { | |
| client.off('message', handleAllMessages); | |
| client.off('open', handleOpen); | |
| client.off('close', handleClose); | |
| client.disconnect(); | |
| }; | |
| } | |
| return () => { | |
| setIsConnected(false); | |
| client.off('message', handleAllMessages); | |
| client.off('open', handleOpen); | |
| client.off('close', handleClose); | |
| client.disconnect(); | |
| }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/apps/app/context/WebSocketContext.tsx` around lines 34 - 40, The
cleanup function removes the 'close' event listener before disconnecting the
client, which prevents the handleClose callback from firing and leaves
isConnected in a stale true state. To fix this, explicitly reset isConnected to
false during cleanup in the return function alongside the client.off calls and
client.disconnect() call. This ensures the state is properly cleared when the
component unmounts, preventing downstream socket-driven queries from being
incorrectly gated by the stale true value.
| export function useNewsfeedQuery() { | ||
| const { client: socket, isConnected } = useWebSocket(); | ||
| const queryClient = useQueryClient(); | ||
| const queryKey = newsfeedKeys.posts(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify unstable key construction and effect dependency coupling.
rg -n -C2 "const queryKey = newsfeedKeys\\.posts\\(\\)|useEffect\\(|socket\\.emit\\('get_posts'" frontend/apps/app/hooks/useNewsfeedQuery.ts frontend/apps/app/app/\(dashboard\)/newsfeed/query-options.tsRepository: ekesel/dmt-sldp
Length of output: 2013
Stabilize queryKey reference to prevent effect re-execution on every render.
newsfeedKeys.posts() returns a new array each invocation. Since queryKey is a direct call in the component body and included in the effect dependency array (line 89), it changes every render, causing the effect to re-run and repeatedly emit get_posts while connected (line 80).
Suggested fix
import { useEffect, useState, useCallback } from 'react';
+import { useMemo } from 'react';
...
-const queryKey = newsfeedKeys.posts();
+const queryKey = useMemo(() => newsfeedKeys.posts(), []);Additionally, the onPostCreated deduplication logic (lines 49–52) filters posts by title, content, and author instead of post_id, which can incorrectly remove distinct posts with identical text or authorship.
Also applies to: 89–89
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/apps/app/hooks/useNewsfeedQuery.ts` at line 13, Stabilize the
`queryKey` reference by memoizing the call to `newsfeedKeys.posts()` or storing
it in a constant outside the component body to ensure it maintains the same
reference across renders, preventing the effect dependency from changing and
causing unnecessary re-execution. Additionally, fix the `onPostCreated`
deduplication logic by changing the filter condition to compare posts by
`post_id` instead of filtering by title, content, and author, which can
incorrectly remove distinct posts with identical text or authorship.
Comment on lines
+48
to
+55
| queryClient.setQueryData<Post[]>(queryKey, (old) => { | ||
| const filtered = (old || []).filter(p => | ||
| !(p.title === newPost.title && | ||
| p.content === newPost.content && | ||
| p.author?.id === newPost.author?.id) | ||
| ); | ||
| return [newPost, ...filtered]; | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify current dedupe criterion in post-created path.
rg -n -C3 "onPostCreated|title === newPost\\.title|content === newPost\\.content|author\\?\\.id === newPost\\.author\\?\\.id|post_id !== newPost\\.post_id" frontend/apps/app/hooks/useNewsfeedQuery.tsRepository: ekesel/dmt-sldp
Length of output: 1185
🏁 Script executed:
# Find Post type definition
rg -n "type Post|interface Post" frontend/apps/app/hooks/useNewsfeedQuery.ts
# Also search in common type locations
fd -e ts -e tsx -e d.ts | grep -iE "(type|model|interface)" | head -20Repository: ekesel/dmt-sldp
Length of output: 95
🏁 Script executed:
# Check if post_id is used as a primary identifier elsewhere
rg -n "post_id" frontend/apps/app/hooks/useNewsfeedQuery.ts
# Search for Post type definition more broadly
rg -n "Post\[" --type ts --type tsxRepository: ekesel/dmt-sldp
Length of output: 499
🏁 Script executed:
# Check Post type definition
cat frontend/apps/app/types/newsfeed.tsRepository: ekesel/dmt-sldp
Length of output: 501
🏁 Script executed:
# Get full context around the handlers in useNewsfeedQuery.ts
cat -n frontend/apps/app/hooks/useNewsfeedQuery.ts | head -90Repository: ekesel/dmt-sldp
Length of output: 4052
Use post_id for deduplication in onPostCreated, not title/content/author.
Current logic removes valid distinct posts when the same author publishes repeated content with identical title and body. All other handlers (onPosts, onPostUpdated, onPostDeleted) use post_id for identification; onPostCreated should follow the same pattern.
Suggested fix
queryClient.setQueryData<Post[]>(queryKey, (old) => {
- const filtered = (old || []).filter(p =>
- !(p.title === newPost.title &&
- p.content === newPost.content &&
- p.author?.id === newPost.author?.id)
- );
+ const filtered = (old || []).filter(p => p.post_id !== newPost.post_id);
return [newPost, ...filtered];
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| queryClient.setQueryData<Post[]>(queryKey, (old) => { | |
| const filtered = (old || []).filter(p => | |
| !(p.title === newPost.title && | |
| p.content === newPost.content && | |
| p.author?.id === newPost.author?.id) | |
| ); | |
| return [newPost, ...filtered]; | |
| }); | |
| queryClient.setQueryData<Post[]>(queryKey, (old) => { | |
| const filtered = (old || []).filter(p => p.post_id !== newPost.post_id); | |
| return [newPost, ...filtered]; | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/apps/app/hooks/useNewsfeedQuery.ts` around lines 48 - 55, The
onPostCreated handler uses title, content, and author comparison to deduplicate
posts, which incorrectly removes valid distinct posts when the same author
publishes repeated content. Replace the deduplication logic in the filter
condition within queryClient.setQueryData to use post_id comparison instead of
the title/content/author combination, matching the approach used by other
handlers like onPosts, onPostUpdated, and onPostDeleted.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implemented multiple enhancements across the portal based on feedbacks, including updates to the Sprint History Table styling, Messenger theme and branding, Policies document preview functionality, Newsfeed performance optimizations, Profile Picture resize/crop support, DMT content readability improvements, Org Chart direct reporting lines, and Team Workload color coding indicators , Active Tasks and total task ,Global Search .
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Style