-
Notifications
You must be signed in to change notification settings - Fork 18.2k
extract parse_time_range for console app stats related queries #27626
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
extract parse_time_range for console app stats related queries #27626
Conversation
Signed-off-by: NeatGuyCoding <15627489+NeatGuyCoding@users.noreply.github.com>
Summary of ChangesHello @NeatGuyCoding, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant refactoring of date and time parsing logic within the console application's backend. The core change involves extracting a new Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request does a great job of refactoring the time range parsing logic into a reusable parse_time_range utility function. This significantly reduces code duplication across multiple controller files. The addition of comprehensive unit tests for the new utility, including edge cases for timezones and DST, is also excellent. I've left a few comments for improvement. The main one is a bug fix and refactoring suggestion for the new parse_time_range function to make it more robust and maintainable. I've also pointed out some inconsistencies in how end times for date ranges are handled, which could lead to unexpected behavior for users.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors datetime parsing logic by introducing a centralized parse_time_range utility function to replace duplicated timezone conversion code across multiple controller files.
- Adds a new
parse_time_rangefunction indatetime_utils.pythat handles timezone conversion, DST transitions, and input validation - Replaces repetitive datetime parsing code in workflow statistics, app statistics, and conversation controllers
- Adds comprehensive unit tests covering various scenarios including DST handling, timezone conversions, and edge cases
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| api/libs/datetime_utils.py | Implements new parse_time_range function with DST handling and timezone conversion |
| api/tests/unit_tests/libs/test_datetime_utils.py | Adds 26 comprehensive test cases for the new utility function |
| api/controllers/console/app/workflow_statistic.py | Refactors 4 endpoints to use the new centralized utility function |
| api/controllers/console/app/statistic.py | Refactors 7 endpoints to use the new centralized utility function |
| api/controllers/console/app/conversation.py | Refactors 2 endpoints to use the new centralized utility function |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: NeatGuyCoding <15627489+NeatGuyCoding@users.noreply.github.com>
…030' into feature-extract-parse_time_range-1030
Important
Fixes #<issue number>.Summary
Extract parse_time_range for console app stats related queries
Screenshots
Checklist
dev/reformat(backend) andcd web && npx lint-staged(frontend) to appease the lint gods