feat(new-webui): Adds search job ID and number of search results to search page. #962
Conversation
WalkthroughThis change adds search result count tracking and display to the log viewer web UI. It introduces new state properties for result counts, updates components to synchronise and display these counts, and refactors related UI state handling. New components and CSS are added for showing search status and result numbers. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SearchPage
participant SearchControls
participant SearchQueryStatus
participant SearchResultsTable
participant SearchResultsTimeline
participant SearchStateStore
User->>SearchControls: Submit search query
SearchControls->>SearchStateStore: Reset result counts to 0
SearchControls->>SearchStateStore: Update UI state to QUERY_ID_PENDING
SearchControls->>SearchStateStore: Submit query
SearchResultsTable->>SearchStateStore: Update numSearchResultsTable on results change
SearchResultsTimeline->>SearchStateStore: Update numSearchResultsTimeline on aggregation change
SearchQueryStatus->>SearchStateStore: Read job ID, UI state, result counts
SearchQueryStatus->>User: Display job ID and max result count
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ` rather than `!`.
🧬 Code Graph Analysis (1)components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/search-requests.ts (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🔇 Additional comments (3)
✨ Finishing Touches
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: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (10)
components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/SearchButton/SearchButton.tsx(1 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/search-requests.ts(3 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchQueryStatus/Results.tsx(1 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchQueryStatus/index.module.css(1 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchQueryStatus/index.tsx(1 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/index.tsx(2 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTimeline/index.tsx(2 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchState/index.tsx(4 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchState/useResultsMetadata.ts(1 hunks)components/log-viewer-webui/client/src/pages/SearchPage/index.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ` rather than `!`.
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}: - Preferfalse == <expression>rather than!<expression>.
components/log-viewer-webui/client/src/pages/SearchPage/SearchState/useResultsMetadata.tscomponents/log-viewer-webui/client/src/pages/SearchPage/index.tsxcomponents/log-viewer-webui/client/src/pages/SearchPage/SearchControls/search-requests.tscomponents/log-viewer-webui/client/src/pages/SearchPage/SearchControls/SearchButton/SearchButton.tsxcomponents/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTimeline/index.tsxcomponents/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/index.tsxcomponents/log-viewer-webui/client/src/pages/SearchPage/SearchState/index.tsxcomponents/log-viewer-webui/client/src/pages/SearchPage/SearchQueryStatus/Results.tsxcomponents/log-viewer-webui/client/src/pages/SearchPage/SearchQueryStatus/index.tsx
🧬 Code Graph Analysis (3)
components/log-viewer-webui/client/src/pages/SearchPage/SearchState/useResultsMetadata.ts (1)
components/webui/imports/ui/SearchView/SearchView.jsx (1)
resultsMetadata(77-98)
components/log-viewer-webui/client/src/pages/SearchPage/SearchQueryStatus/Results.tsx (1)
components/log-viewer-webui/client/src/pages/SearchPage/SearchState/typings.ts (1)
SEARCH_UI_STATE(26-26)
components/log-viewer-webui/client/src/pages/SearchPage/SearchQueryStatus/index.tsx (1)
components/log-viewer-webui/client/src/pages/SearchPage/SearchState/typings.ts (1)
SEARCH_UI_STATE(26-26)
🪛 GitHub Actions: clp-lint
components/log-viewer-webui/client/src/pages/SearchPage/SearchState/useResultsMetadata.ts
[error] 47-47: ESLint: Trailing spaces not allowed (@stylistic/no-trailing-spaces)
components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/search-requests.ts
[error] 8-8: ESLint: There should be no space after '{' (@stylistic/object-curly-spacing)
[error] 8-8: ESLint: 'SEARCH_STATE_DEFAULT' is defined but never used (@typescript-eslint/no-unused-vars)
[error] 8-8: ESLint: There should be no space before '}' (@stylistic/object-curly-spacing)
components/log-viewer-webui/client/src/pages/SearchPage/SearchQueryStatus/index.tsx
[error] 26-26: ESLint: Expected indentation of 14 space characters but found 16 (@stylistic/jsx-indent)
[error] 26-26: ESLint: Need to wrap this literal in a JSX expression (@stylistic/jsx-curly-brace-presence)
[error] 27-27: ESLint: {searchJobId} must be placed on a new line (@stylistic/jsx-one-expression-per-line)
[error] 27-27: ESLint: found must be placed on a new line (@stylistic/jsx-one-expression-per-line)
[error] 27-27: ESLint: {" "} must be placed on a new line (@stylistic/jsx-one-expression-per-line)
[error] 31-31: ESLint: Need to wrap this literal in a JSX expression (@stylistic/jsx-curly-brace-presence)
🔇 Additional comments (12)
components/log-viewer-webui/client/src/pages/SearchPage/index.tsx (1)
19-22: LGTM! Clean integration of SearchQueryStatus component.The new div wrapper properly organises the search controls and status display, maintaining good component structure.
components/log-viewer-webui/client/src/pages/SearchPage/SearchQueryStatus/index.module.css (1)
1-8: LGTM! Clean CSS styling for SearchQueryStatus component.The margin and padding values are appropriate for UI spacing, and the class names are semantic and clear.
components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/SearchButton/SearchButton.tsx (1)
13-16: LGTM! Good refactor that improves code clarity.The change simplifies state access by only destructuring the needed
searchUiStateproperty, making the code more focused and readable.components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/search-requests.ts (2)
62-63: LGTM! Proper reset of search result counts.Resetting the search result counts to zero before submitting a new query ensures the UI displays accurate information during the query process.
50-50: LGTM! Improved comment clarity.The updated comment better describes the user experience rather than implementation details.
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/index.tsx (1)
29-38: LGTM! Proper state synchronisation implementation.The useEffect correctly tracks the search results count and updates the global state when searchResults changes. The null handling and dependency array are appropriate.
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTimeline/index.tsx (1)
34-44: LGTM! Robust aggregation and state update logic.The useEffect correctly calculates the total count from aggregationResults using reduce and handles null cases gracefully with optional chaining and nullish coalescing. The dependency array is appropriate.
components/log-viewer-webui/client/src/pages/SearchPage/SearchQueryStatus/Results.tsx (1)
39-53: LGTM! Well-implemented state-to-style mapping.The switch statement properly maps all UI states to appropriate text types with a sensible default fallback. The logic for using different colours based on search state enhances the user experience.
components/log-viewer-webui/client/src/pages/SearchPage/SearchState/index.tsx (4)
19-20: LGTM! Proper initialization of search result counters.The new properties are correctly initialized to 0 in the default state object, following the established pattern.
37-46: LGTM! Well-documented interface additions.The new properties are properly typed as
numberwith clear, descriptive documentation comments that explain their purpose.
85-86: LGTM! Consistent function signature pattern.The update function signatures follow the same naming convention and parameter structure as existing functions in the interface.
101-106: LGTM! Implementation follows established patterns.The update functions correctly use the Zustand
setfunction and follow the same implementation pattern as other update methods in the store.
components/log-viewer-webui/client/src/pages/SearchPage/SearchState/useResultsMetadata.ts
Outdated
Show resolved
Hide resolved
components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/search-requests.ts
Outdated
Show resolved
Hide resolved
| {(searchUiState === SEARCH_UI_STATE.QUERYING || | ||
| searchUiState === SEARCH_UI_STATE.DONE) && ( | ||
| <Text type="secondary"> | ||
| Search job #{searchJobId} found{" "} | ||
| </Text> | ||
| )} | ||
| <Results/> | ||
| <Text type="secondary"> results</Text> |
There was a problem hiding this comment.
Fix ESLint formatting violations to pass CI checks.
The pipeline reports multiple ESLint violations that need to be resolved:
Apply this diff to fix all formatting issues:
- {(searchUiState === SEARCH_UI_STATE.QUERYING ||
- searchUiState === SEARCH_UI_STATE.DONE) && (
- <Text type="secondary">
- Search job #{searchJobId} found{" "}
- </Text>
- )}
- <Results/>
- <Text type="secondary"> results</Text>
+ {(searchUiState === SEARCH_UI_STATE.QUERYING ||
+ searchUiState === SEARCH_UI_STATE.DONE) && (
+ <Text type={"secondary"}>
+ {"Search job #"}
+ {searchJobId}
+ {" found "}
+ </Text>
+ )}
+ <Results/>
+ <Text type={"secondary"}>{" results"}</Text>📝 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.
| {(searchUiState === SEARCH_UI_STATE.QUERYING || | |
| searchUiState === SEARCH_UI_STATE.DONE) && ( | |
| <Text type="secondary"> | |
| Search job #{searchJobId} found{" "} | |
| </Text> | |
| )} | |
| <Results/> | |
| <Text type="secondary"> results</Text> | |
| {(searchUiState === SEARCH_UI_STATE.QUERYING || | |
| searchUiState === SEARCH_UI_STATE.DONE) && ( | |
| <Text type={"secondary"}> | |
| {"Search job #"} | |
| {searchJobId} | |
| {" found "} | |
| </Text> | |
| )} | |
| <Results/> | |
| <Text type={"secondary"}>{" results"}</Text> |
🧰 Tools
🪛 GitHub Actions: clp-lint
[error] 26-26: ESLint: Expected indentation of 14 space characters but found 16 (@stylistic/jsx-indent)
[error] 26-26: ESLint: Need to wrap this literal in a JSX expression (@stylistic/jsx-curly-brace-presence)
[error] 27-27: ESLint: {searchJobId} must be placed on a new line (@stylistic/jsx-one-expression-per-line)
[error] 27-27: ESLint: found must be placed on a new line (@stylistic/jsx-one-expression-per-line)
[error] 27-27: ESLint: {" "} must be placed on a new line (@stylistic/jsx-one-expression-per-line)
[error] 31-31: ESLint: Need to wrap this literal in a JSX expression (@stylistic/jsx-curly-brace-presence)
🤖 Prompt for AI Agents
In
components/log-viewer-webui/client/src/pages/SearchPage/SearchQueryStatus/index.tsx
around lines 24 to 31, the JSX formatting violates ESLint rules. Fix the
formatting by properly aligning and spacing the JSX elements, ensuring
consistent indentation and no unnecessary line breaks, so the code passes ESLint
checks and CI validation.
components/log-viewer-webui/client/src/pages/SearchPage/SearchQueryStatus/Results.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
components/log-viewer-webui/client/src/pages/SearchPage/SearchState/useResultsMetadata.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ` rather than `!`.
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}: - Preferfalse == <expression>rather than!<expression>.
components/log-viewer-webui/client/src/pages/SearchPage/SearchState/useResultsMetadata.ts
| @@ -48,4 +48,5 @@ const useResultsMetadata = () => { | |||
| return resultsMetadata; | |||
| }; | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Remove unnecessary blank line before export
The blank line added at line 51 serves no functional purpose and may introduce trailing whitespace. Please remove it to keep formatting consistent.
🤖 Prompt for AI Agents
In
components/log-viewer-webui/client/src/pages/SearchPage/SearchState/useResultsMetadata.ts
at line 51, remove the unnecessary blank line before the export statement to
avoid trailing whitespace and maintain consistent formatting.
|
@davemarco Can you address the CI failure? |
...ts/log-viewer-webui/client/src/pages/SearchPage/SearchControls/SearchButton/SearchButton.tsx
Outdated
Show resolved
Hide resolved
components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/search-requests.ts
Outdated
Show resolved
Hide resolved
components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/search-requests.ts
Outdated
Show resolved
Hide resolved
components/log-viewer-webui/client/src/pages/SearchPage/SearchQueryStatus/Results.tsx
Outdated
Show resolved
Hide resolved
components/log-viewer-webui/client/src/pages/SearchPage/SearchQueryStatus/Results.tsx
Outdated
Show resolved
Hide resolved
components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/search-requests.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/SearchButton/SearchButton.tsx (1)
13-13: Perfect implementation of Zustand selector pattern.The refactoring successfully addresses the previous performance concern by using a specific selector instead of destructuring the entire store state.
components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/search-requests.ts (2)
50-50: Clear and informative comment.The updated comment effectively communicates the user flow restriction.
62-63: Proper state initialization for new queries.Correctly resets the search result counts to default values when starting a new query, ensuring clean state for the search operation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
components/log-viewer-webui/client/settings.json(1 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/SearchButton/SearchButton.tsx(1 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/search-requests.ts(3 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchQueryStatus/Results.tsx(1 hunks)components/log-viewer-webui/client/vite.config.ts(1 hunks)components/log-viewer-webui/server/.env(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ` rather than `!`.
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}: - Preferfalse == <expression>rather than!<expression>.
components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/search-requests.tscomponents/log-viewer-webui/client/src/pages/SearchPage/SearchControls/SearchButton/SearchButton.tsxcomponents/log-viewer-webui/client/vite.config.tscomponents/log-viewer-webui/client/src/pages/SearchPage/SearchQueryStatus/Results.tsx
🧬 Code Graph Analysis (2)
components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/search-requests.ts (1)
components/log-viewer-webui/client/src/pages/SearchPage/SearchState/index.tsx (1)
SEARCH_STATE_DEFAULT(131-131)
components/log-viewer-webui/client/src/pages/SearchPage/SearchQueryStatus/Results.tsx (1)
components/log-viewer-webui/client/src/pages/SearchPage/SearchState/typings.ts (1)
SEARCH_UI_STATE(26-26)
🪛 dotenv-linter (3.3.0)
components/log-viewer-webui/server/.env
[warning] 8-8: [UnorderedKey] The CLP_DB_USER key should go before the HOST key
[warning] 9-9: [UnorderedKey] The CLP_DB_PASS key should go before the CLP_DB_USER key
🔇 Additional comments (3)
components/log-viewer-webui/client/src/pages/SearchPage/SearchQueryStatus/Results.tsx (1)
21-64: Excellent implementation following React and Zustand best practices.The component correctly uses specific Zustand selectors to prevent unnecessary re-renders, implements proper memoization for performance optimization, and provides comprehensive UI state handling with appropriate fallback logic.
components/log-viewer-webui/server/.env (1)
7-7: Port alignment confirmed.
You’ve updated the serverPORTto3001to match the client proxy settings. Ensure any documentation, Docker or dev scripts reflect this change.components/log-viewer-webui/client/vite.config.ts (1)
25-35: Update proxy targets to port 3001.
You’ve correctly adjusted the/query/,/api/, and/socket.io/proxy targets tolocalhost:3001. Confirm the backend HTTP and WebSocket endpoints are active on this port in local and staging environments.
| @@ -1,4 +1,4 @@ | |||
| { | |||
| "MongoDbSearchResultsMetadataCollectionName": "results-metadata", | |||
| "ClpStorageEngine": "clp" | |||
| "ClpStorageEngine": "clp-s" | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify storage engine configuration update.
Changing "ClpStorageEngine" from "clp" to "clp-s" must be supported by the backend and all references in code and documentation updated accordingly.
Run this script to ensure no stale references remain:
🏁 Script executed:
#!/bin/bash
# Search for hardcoded 'ClpStorageEngine' values
rg -n '"ClpStorageEngine"'Length of output: 315
Fix default ClpStorageEngine in webui settings and verify backend support
The search still shows a hardcoded "ClpStorageEngine": "clp" in your WebUI defaults. Please update that value and confirm the backend handles the new engine ID everywhere.
• components/webui/settings.json (line 14): change
"ClpStorageEngine": "clp" → "ClpStorageEngine": "clp-s"
• components/log-viewer-webui/client/settings.json is already set to "clp-s"
• components/clp-package-utils/clp_package_utils/scripts/start_clp.py (line 878): verify storage_engine can be "clp-s"
• Review any documentation or README files for lingering “clp” references and update accordingly
🤖 Prompt for AI Agents
In components/log-viewer-webui/client/settings.json at line 3, the
"ClpStorageEngine" is updated to "clp-s". Ensure consistency by also updating
components/webui/settings.json at line 14 to set "ClpStorageEngine" to "clp-s".
Verify that components/clp-package-utils/clp_package_utils/scripts/start_clp.py
at line 878 supports "clp-s" as a valid storage_engine value. Additionally,
review and update all documentation and README files to replace any remaining
"clp" references with "clp-s" to maintain alignment across the project.
| CLP_DB_USER=clp-user | ||
| CLP_DB_PASS= | ||
| CLP_DB_PASS=qO55i66GEUs |
There was a problem hiding this comment.
Avoid committing sensitive credentials.
Storing CLP_DB_PASS directly in a tracked .env poses a security risk. Consider moving this secret to a secure vault or adding .env to .gitignore and providing a template (.env.example).
🧰 Tools
🪛 dotenv-linter (3.3.0)
[warning] 9-9: [UnorderedKey] The CLP_DB_PASS key should go before the CLP_DB_USER key
🤖 Prompt for AI Agents
In components/log-viewer-webui/server/.env at line 9, the database password is
stored directly in the tracked .env file, which is a security risk. Remove the
CLP_DB_PASS entry from this file, add .env to .gitignore to prevent committing
sensitive credentials, and create a .env.example file with placeholder values to
serve as a template for environment variables.
| @@ -70,6 +82,8 @@ interface SearchState { | |||
| timelineConfig: TimelineConfig; | |||
|
|
|||
| updateAggregationJobId: (id: string | null) => void; | |||
There was a problem hiding this comment.
following our latest (undocumented yet - sorry!) Zustand practices, Zustand actions that do not perform more actions than state settings should be prefixed with set instead of update. This is fine to leave in the current PR, but we should address such in a future Zustand refactoring PR.
|
sorry i accidentally added empty file. can you re approve @junhaoliao |
Description
Adds search job ID and number of search results to search page.
I attempted to add a cancel state to display a different color for cancel, but the backend (results Metadata) is not ready for this. Attempt lead to many bugs, so i did not add. Potentially after feature parity to do properly. Also tried antd badge but it looked ugly
Checklist
breaking change.
Validation performed
Tested worked as expected
Summary by CodeRabbit
New Features
Style
Refactor
Chores