Skip to content

Conversation

@nikhilsaikethe
Copy link
Contributor

This pr fixes the issue in #9455 , stream should be selected before creating scheduled job

@github-actions github-actions bot added the ☢️ Bug Something isn't working label Dec 4, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Dec 4, 2025

Failed to generate code suggestions for PR

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 4, 2025

Greptile Overview

Greptile Summary

This PR addresses issue #9455 by adding validation checks in the logs search bar download functionality:

  • Added validation in addJobScheduler() to require stream selection before scheduling jobs
  • Added validation in downloadRangeData() to ensure a query exists before custom range downloads
  • Moved empty data validation to the beginning of downloadLogs() function, checking before processing rather than after conversion
  • Changed notification positions from "top" to "bottom" for consistency

Issues to address:

  • The notification changes will break an existing test in SearchBar.spec.ts that expects different message text and notification type
  • Code formatting has inconsistent indentation in the new downloadRangeData() validation block

Confidence Score: 3/5

  • This PR adds useful validation but will break an existing test and has minor code quality issues that should be addressed before merging.
  • Score of 3 reflects that while the functional changes are correct and address the issue requirements, there is a breaking change to an existing test case that will cause test failures, and minor code formatting inconsistencies. The core logic is sound.
  • web/src/plugins/logs/SearchBar.vue needs test updates in the corresponding spec file and formatting fixes.

Important Files Changed

File Analysis

Filename Score Overview
web/src/plugins/logs/SearchBar.vue 3/5 Added validations for stream selection and query existence before download operations. Contains style inconsistencies (indentation) and notification color using "positive" for informational "no data" messages. Existing tests will fail due to changed notification parameters.

Sequence Diagram

sequenceDiagram
    participant User
    participant SearchBar
    participant searchService
    participant Notification
    
    Note over User,Notification: Download Flow with New Validations
    
    User->>SearchBar: Click Download (CSV/JSON)
    SearchBar->>SearchBar: downloadLogs(data, format)
    alt data is empty or null
        SearchBar->>Notification: "No data found to download"
        Notification-->>User: Show notification
    else data exists
        SearchBar->>SearchBar: Convert to CSV/JSON
        SearchBar-->>User: Download file
    end
    
    User->>SearchBar: Click Custom Range Download
    SearchBar->>SearchBar: downloadRangeData()
    alt customDownloadQueryObj.query missing
        SearchBar->>Notification: "Please run a query first"
        Notification-->>User: Show notification
    else query exists
        SearchBar->>searchService: search(query)
        searchService-->>SearchBar: results
        alt results.hits.length > 0
            SearchBar->>SearchBar: downloadLogs(hits, format)
        else no hits
            SearchBar->>Notification: "No data found to download"
        end
    end
    
    User->>SearchBar: Schedule Job
    SearchBar->>SearchBar: addJobScheduler()
    alt no stream selected
        SearchBar->>Notification: "Please select a stream"
        Notification-->>User: Show notification
    else stream selected
        SearchBar->>SearchBar: proceed with scheduling
    end
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

@nikhilsaikethe nikhilsaikethe merged commit 2c6c83b into main Dec 4, 2025
51 of 54 checks passed
@nikhilsaikethe nikhilsaikethe deleted the fix/issue-9455 branch December 4, 2025 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

☢️ Bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants