[WEB-2889] fix: global views sorting when hyper model is enabled. #6280
Conversation
prateekshourya29
requested review from
SatishGandham,
sriramveeraghanta and
rahulramesha
WalkthroughThe pull request introduces modifications to two files: Changes
Sequence DiagramsequenceDiagram
participant WorkspaceViewForm
participant BaseIssuesStore
participant Router
WorkspaceViewForm->>BaseIssuesStore: Set display filters
Router->>BaseIssuesStore: Provide project context
BaseIssuesStore->>BaseIssuesStore: Compute orderBy based on layout
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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: 0
🧹 Nitpick comments (2)
web/core/components/workspace/views/form.tsx (2)
7-7: **Use a descriptive comment or section heading for constants imports **It's good to keep imports grouped logically. Adding a small comment or heading (e.g.,
// Constants) above constants imports likeEIssueLayoutTypescan improve clarity and maintainability.
35-37: **Consider making the layout type configurable **Hardcoding the layout as
EIssueLayoutTypes.SPREADSHEETis fine for now but may limit reusability. If future requirements mandate dynamic layouts, consider passing this value as a prop or context to keep the component flexible.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web/core/components/workspace/views/form.tsx(2 hunks)web/core/store/issue/helpers/base-issues.store.ts(1 hunks)
🔇 Additional comments (1)
web/core/store/issue/helpers/base-issues.store.ts (1)
Line range hint 300-304: **Ensure consistent sorting across different layouts **
The snippet forces orderBy to "sort_order" unless the layout is EIssueLayoutTypes.SPREADSHEET. This logic likely fixes the reported bug but might affect other layouts if combined with user-defined ordering. Recommend verifying that:
- Non-spreadsheet layouts still work with the intended custom ordering when
localDBEnabledis false. - No unintended overrides occur when toggling hyper mode on or off.
Run the following script to locate any references or test coverage for this condition:
✅ Verification successful
Let me gather more information about the sorting logic.
Let me gather more information about the special ordering logic.
Based on the analysis of the code, I can now provide a definitive response about the sorting behavior.
**Special ordering behavior is correctly implemented across layouts **
The code implements consistent sorting behavior with proper handling of sort_order across different layouts:
-
For non-spreadsheet layouts when
localDBEnabledis true, the code forcessort_orderfor special ordering fields (labels, assignees, etc.) defined inSPECIAL_ORDER_BY. This is intentional and correct behavior to maintain consistent manual ordering in local database mode. -
The spreadsheet layout is explicitly excluded from this override (
layout !== EIssueLayoutTypes.SPREADSHEET) because:- Spreadsheet view needs to maintain its own column-based sorting capabilities
- The
SPECIAL_ORDER_BYmapping is specifically designed for list/kanban/calendar views where manual ordering makes sense
-
The implementation properly handles:
- Manual ordering through
sort_orderfield - Special ordering fields through
SPECIAL_ORDER_BYmapping - Layout-specific sorting requirements
- Local database mode vs server mode sorting
- Manual ordering through
The code's behavior aligns with the intended functionality and doesn't require any changes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 1) Search for tests or usage references of 'orderBy' logic.
# 2) Provide context around localDBEnabled usage.
rg -A 5 'orderBy("'
rg -A 5 'localDBEnabled'
Length of output: 7625
Script:
#!/bin/bash
# Search for SPECIAL_ORDER_BY and related sorting logic
rg -A 5 'SPECIAL_ORDER_BY'
# Search for orderBy related patterns in issue store
ast-grep --pattern 'orderBy = $$$'
ast-grep --pattern 'orderBy: $$$'
# Search for layout related code
rg -A 5 'EIssueLayoutTypes'
Length of output: 66517
Script:
#!/bin/bash
# Search for special ordering logic in query constructor
rg -A 10 'getOrderByFragment' web/core/local-db/utils/query-constructor.ts
# Search for sort_order related code
rg -A 5 'sort_order'
Length of output: 74426
Description
In this PR, I have fixed the issue with sorting in global views when hyper mode is enabled.
Type of Change
Screenshots and Media (if applicable)
Screen.Recording.2024-12-26.at.10.01.05.PM.mov
Screen.Recording.2024-12-26.at.9.59.15.PM.mov
Summary by CodeRabbit
New Features
Bug Fixes