feat(webui): Move timestamp key selector in guided Presto UI into date picker. #1631
Conversation
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
Contributor
WalkthroughThe refactoring relocates timestamp key selection from the GuidedControls component to a new TimeRangeFooter within TimeRangeInput, gated by Presto guided mode configuration. TimestampKey component removal is paired with extraction of initialization logic into a reusable Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Areas requiring extra attention:
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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}
Contributor
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 (10)
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/TimestampKey/TimestampKeySelect.tsx(0 hunks)components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/TimestampKey/index.tsx(0 hunks)components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/index.module.css(0 hunks)components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/index.tsx(2 hunks)components/webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/Presto/TimeRangeFooter/TimestampKeySelect/index.tsx(1 hunks)components/webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/Presto/TimeRangeFooter/index.module.css(1 hunks)components/webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/Presto/TimeRangeFooter/index.tsx(1 hunks)components/webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/index.tsx(4 hunks)components/webui/client/src/pages/SearchPage/SearchState/Presto/useTimestampKeyInit/index.tsx(1 hunks)components/webui/client/src/pages/SearchPage/SearchState/Presto/useTimestampKeyInit/sql.ts(1 hunks)
💤 Files with no reviewable changes (3)
- components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/TimestampKey/index.tsx
- components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/TimestampKey/TimestampKeySelect.tsx
- components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/index.module.css
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit configuration file
- Prefer
false == <expression>rather than!<expression>.
Files:
components/webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/index.tsxcomponents/webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/Presto/TimeRangeFooter/TimestampKeySelect/index.tsxcomponents/webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/Presto/TimeRangeFooter/index.tsxcomponents/webui/client/src/pages/SearchPage/SearchState/Presto/useTimestampKeyInit/index.tsxcomponents/webui/client/src/pages/SearchPage/SearchState/Presto/useTimestampKeyInit/sql.tscomponents/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/index.tsx
🧠 Learnings (3)
📓 Common learnings
Learnt from: haiqi96
Repo: y-scope/clp PR: 0
File: :0-0
Timestamp: 2025-07-29T14:04:13.769Z
Learning: User haiqi96 requested creating a GitHub issue to document a bug fix from PR #1136, which addressed MySQL compatibility issues with invalid SQL CAST operations in the WebUI component.
📚 Learning: 2025-07-18T20:00:50.288Z
Learnt from: hoophalab
Repo: y-scope/clp PR: 1108
File: components/webui/client/src/pages/SearchPage/SearchControls/Presto/SqlQueryInput/index.tsx:15-15
Timestamp: 2025-07-18T20:00:50.288Z
Learning: In the y-scope/clp React webui client codebase, for Zustand store usage: use `useStore.getState().method` for callbacks since the output is not reactive and doesn't need state as a dependency in the hook, and use `useStore((state) => state.property)` with proper selectors for reactive components that need to re-render when state changes.
Applied to files:
components/webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/index.tsx
📚 Learning: 2025-05-29T20:33:40.653Z
Learnt from: junhaoliao
Repo: y-scope/clp PR: 937
File: components/log-viewer-webui/client/src/AntdApp.tsx:16-24
Timestamp: 2025-05-29T20:33:40.653Z
Learning: In components/log-viewer-webui React codebase: Return type annotations (like `: JSX.Element`) are unnecessary and not preferred for React components in JSX/TSX files.
Applied to files:
components/webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/Presto/TimeRangeFooter/index.tsx
🧬 Code graph analysis (3)
components/webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/index.tsx (1)
components/webui/client/src/config/index.ts (1)
SETTINGS_QUERY_ENGINE(27-27)
components/webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/Presto/TimeRangeFooter/TimestampKeySelect/index.tsx (2)
components/webui/client/src/pages/SearchPage/SearchState/Presto/useTimestampKeyInit/sql.ts (1)
fetchTimestampColumns(50-50)components/webui/client/src/pages/SearchPage/SearchState/typings.ts (1)
SEARCH_UI_STATE(31-31)
components/webui/client/src/pages/SearchPage/SearchState/Presto/useTimestampKeyInit/index.tsx (1)
components/webui/client/src/pages/SearchPage/SearchState/Presto/useTimestampKeyInit/sql.ts (1)
fetchTimestampColumns(50-50)
⏰ 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)
- GitHub Check: package-image
- GitHub Check: lint-check (macos-15)
- GitHub Check: lint-check (ubuntu-24.04)
🔇 Additional comments (3)
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/index.tsx (3)
1-1: LGTM: Hook import aligns with PR objectives.The import of
useTimestampKeyInitcorrectly integrates the new hook that handles timestamp key initialization, replacing the previous inline timestamp key selector logic.
22-45: LGTM: Component structure is well-organized and aligns with PR objectives.The refactored layout correctly:
- Renders
contextHolderat the top for Ant Design's message context- Maintains the existing button row with
TimeRangeInputandSqlSearchButton- Introduces a clean grid-based layout for Select, From, Where, and OrderBy controls
- Separates status display into its own section
The structural changes properly support moving the timestamp key selector into the date picker while maintaining a logical component hierarchy.
19-21: Verification complète — intégration du hook correcte et conforme.L'inspection de la mise en œuvre du hook
useTimestampKeyInitconfirme que tous les scénarios d'initialisation sont correctement gérés :
- Changements de dataset (ligne 76-83) : L'effet réinitialise
timestampKeylors du changement de dataset- États d'erreur (ligne 52-61) : L'effet affiche un message d'erreur via
messageApi.error()en cas d'échec de récupération- Auto-assignation (ligne 41-45) : Assigne automatiquement la première colonne de timestamp si aucune n'est sélectionnée
Le destructuring de
contextHolderau sein du composantGuidedControlsest approprié et conforme à l'API Ant Designmessage.useMessage(). L'intégration du hook est propre et fonctionnelle.
components/webui/client/src/pages/SearchPage/SearchState/Presto/useTimestampKeyInit/index.tsx
Outdated
Show resolved
Hide resolved
3 tasks
hoophalab
previously approved these changes
Nov 20, 2025
Contributor
hoophalab
left a comment
hoophalab
left a comment
There was a problem hiding this comment.
lgtm
Validations: timestamp keys are shown in the selector. Changing the drop-down option changes the constructed query.
Contributor
Author
|
sorry i addressed a rabbit comment. I had to make a change to the old effect logic which worked but supposedly not idiomatic react. I found another change to the effect logic that also works, and the rabbit will be more content with. |
hoophalab
previously approved these changes
Nov 21, 2025
junhaoliao
previously approved these changes
Nov 21, 2025
Member
junhaoliao
left a comment
junhaoliao
left a comment
There was a problem hiding this comment.
deferring to @hoophalab 's review
Contributor
Author
|
@junhaoliao can you approve this. Merging main created a lint error |
junhaoliao
approved these changes
Nov 25, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Description
Moves the timestamp key selector into date picker. Previously the timestamp key selector was always rendered on guided page, moving into the date picker is not ideal since it is not always rendered, and required a change to the fetching logic. There is a new hook which is rendered on guided page which sets the timestamp key and does the fetching. The selector is now simpler and just does selection, and it not responsible for fetching or error hanlding.
Checklist
breaking change.
Validation performed
Loads timestamp keys
Summary by CodeRabbit
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.