Skip to content

Conversation

@tomerqodo
Copy link

@tomerqodo tomerqodo commented Dec 4, 2025

User description

Benchmark PR getsentry#102314

Type: Clean (correct implementation)

Original PR Title: feat(search-bar): Add conditionals section to filter key dropdown
Original PR Description: This PR adds in a new Conditionals section that contains AND and OR. We conditionally render this section, once the user has moved off of the first item as you can not start a filter with a conditional.

Ticket: EXP-562

Example:

Screenshot 2025-10-29 at 07 33 46 **Original PR URL**: https://github.com/getsentry/pull/102314

PR Type

Enhancement


Description

  • Add disallowLogicalOperators prop to conditionally hide Logic section

  • Rename "boolean operator" terminology to "logic operator" throughout

  • Implement Logic category in filter key dropdown menu with AND/OR/(/)

  • Add conditional rendering of Logic section based on filter position

  • Replace hardcoded spacing values with theme space tokens


Diagram Walkthrough

flowchart LR
  A["disallowLogicalOperators prop"] -->|controls visibility| B["Logic Category"]
  C["Filter position check"] -->|isFirstItem| B
  B -->|renders when enabled| D["AND/OR/Parentheses options"]
  D -->|user selects| E["Update query with logic operator"]
  F["Terminology updates"] -->|boolean → logic| G["Consistent naming"]
Loading

File Walkthrough

Relevant files
Enhancement
8 files
context.tsx
Add disallowLogicalOperators context property                       
+3/-0     
useQueryBuilderState.tsx
Rename boolean operator to logic operator                               
+7/-7     
boolean.tsx
Update boolean terminology to logic operator                         
+5/-5     
index.tsx
Replace space function with theme space tokens                     
+15/-11 
types.tsx
Add LogicFilterItem type definition                                           
+9/-2     
useFilterKeyListBox.tsx
Implement Logic category section with conditional rendering
+69/-6   
utils.tsx
Add logic filter item creation and category constants       
+19/-0   
freeText.tsx
Handle logic filter selection and pass filterItem prop     
+13/-0   
Tests
1 files
index.spec.tsx
Add tests for Logic category conditional rendering             
+55/-8   

@qodo-code-review
Copy link

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
No Audit Logs: The new logic to insert logic-filter tokens (AND/OR/parentheses) performs state updates
without any added auditing or logging of these critical user actions, making it unclear if
they are captured elsewhere.

Referred Code
  dispatch({
    type: 'UPDATE_QUERY',
    query: option.value,
    focusOverride: {itemKey: 'end'},
  });
  return;
}

if (option.type === 'logic-filter') {
  dispatch({
    type: 'UPDATE_FREE_TEXT_ON_SELECT',
    tokens: [token],
    text: option.value,
    shouldCommitQuery: true,
    focusOverride: calculateNextFocusForInsertedToken(item),
  });
  resetInputValue();
  return;
}

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Missing Guards: New logic for the Logic section depends on feature flags and state (e.g., selectedSection,
filterItem) but adds no explicit error handling or fallbacks for invalid selections or
undefined option values.

Referred Code
    ...makeRecentSearchQueryItems({
      recentSearches,
      filterKeys,
      getFieldDefinition,
    }),
  ];
}

if (
  !disallowLogicalOperators &&
  selectedSection === LOGIC_CATEGORY_VALUE &&
  hasConditionalsInCombobox
) {
  return [...askSeerItem, ...conditionalFilterItems];
}

const filteredByCategory = sectionedItems.filter(item => {
  if (itemIsSection(item)) {
    if (selectedSection === ALL_CATEGORY_VALUE) {
      return true;
    }


 ... (clipped 16 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Input Handling: New creation and insertion of logic filter items allow direct token text insertion without
visible validation or sanitization of option values, relying on assumptions that may be
enforced elsewhere.

Referred Code
  value,
}: {
  value: 'AND' | 'OR' | '(' | ')';
}): LogicFilterItem {
  return {
    key: getEscapedKey(value),
    type: 'logic-filter' as const,
    value,
    label: value,
    textValue: value,
    hideCheck: true,
    showDetailsInOverlay: true,
  };
}

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Refactor to remove duplicated logic

Refactor the useMemo hook to remove duplicated logic by first defining the base
sections and then conditionally adding the LOGIC_CATEGORY.

static/app/components/searchQueryBuilder/tokens/filterKeyListBox/useFilterKeyListBox.tsx [148-185]

 const sections = useMemo<Section[]>(() => {
   const definedSections = filterKeySections.map(section => ({
     value: section.value,
     label: section.label,
   }));
 
   if (!definedSections.length) {
     return [];
   }
 
+  const baseSections =
+    recentSearches?.length && !query
+      ? [RECENT_SEARCH_CATEGORY, ALL_CATEGORY, ...definedSections]
+      : [ALL_CATEGORY, ...definedSections];
+
   const isFirstItem = filterItem.key.toString().endsWith(':0');
-  if (recentSearches?.length && !query) {
-    const recentSearchesSections: Section[] = [
-      RECENT_SEARCH_CATEGORY,
-      ALL_CATEGORY,
-      ...definedSections,
-    ];
-
-    if (!disallowLogicalOperators && !isFirstItem && hasConditionalsInCombobox) {
-      recentSearchesSections.push(LOGIC_CATEGORY);
-    }
-    return recentSearchesSections;
+  if (!disallowLogicalOperators && !isFirstItem && hasConditionalsInCombobox) {
+    baseSections.push(LOGIC_CATEGORY);
   }
 
-  const customSections: Section[] = [ALL_CATEGORY, ...definedSections];
-  if (!disallowLogicalOperators && !isFirstItem && hasConditionalsInCombobox) {
-    customSections.push(LOGIC_CATEGORY);
-  }
-
-  return customSections;
+  return baseSections;
 }, [
   disallowLogicalOperators,
   filterKeySections,
   hasConditionalsInCombobox,
   filterItem.key,
   query,
   recentSearches?.length,
 ]);
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why: The suggestion correctly identifies duplicated logic and proposes a valid refactoring that improves code readability and maintainability without changing functionality.

Low
  • More

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants