-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
[WEB-2843]chore: handled the cycle date time using project timezone #6187
Conversation
WalkthroughThe changes in this pull request introduce enhancements to date handling across several components of the application. Key modifications include the addition of timezone conversion logic in the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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 (
|
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.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (7)
apiserver/plane/utils/timezone_converter.py (1)
6-6
: Consider renaming 'queryset' parameter for clarityThe parameter
queryset
may be misleading since the function operates on dictionaries or lists of dictionaries, not Django QuerySets. Renaming it todata
oritems
can improve readability.apiserver/plane/app/views/cycle/base.py (5)
58-58
: Remove unused import 'convert_utc_to_project_timezone'The function
convert_utc_to_project_timezone
is imported but not used in this file. Removing the unused import cleans up the code:from plane.utils.timezone_converter import ( - convert_utc_to_project_timezone, convert_to_utc, user_timezone_converter, )
🧰 Tools
🪛 Ruff (0.8.2)
58-58:
plane.utils.timezone_converter.convert_utc_to_project_timezone
imported but unusedRemove unused import:
plane.utils.timezone_converter.convert_utc_to_project_timezone
(F401)
77-89
: Refactor timezone conversion logic into a helper functionThe code for fetching the project and converting the current time to the project's timezone is repeated. Extracting this logic into a helper function enhances reusability and maintainability:
🧰 Tools
🪛 Ruff (0.8.2)
87-87: Line too long (93 > 88)
(E501)
186-197
: Duplicate code detected: Refactor timezone conversion logicThe timezone conversion logic is duplicated from the
get_queryset
method. Consider refactoring it into a shared helper function to avoid code redundancy.🧰 Tools
🪛 Ruff (0.8.2)
195-195: Line too long (93 > 88)
(E501)
87-87
: Line exceeds maximum recommended lengthLine 87 exceeds 88 characters. Consider wrapping the line to improve readability:
current_time_in_project_tz = timezone.now().astimezone(local_tz)🧰 Tools
🪛 Ruff (0.8.2)
87-87: Line too long (93 > 88)
(E501)
195-195
: Line exceeds maximum recommended lengthLine 195 exceeds 88 characters. Consider wrapping the line to improve readability:
current_time_in_project_tz = timezone.now().astimezone(local_tz)🧰 Tools
🪛 Ruff (0.8.2)
195-195: Line too long (93 > 88)
(E501)
apiserver/plane/app/serializers/cycle.py (1)
Line range hint
75-75
: Missing comma in 'read_only_fields'There is a missing comma between
"cycle"
and"user"
inread_only_fields
. This will cause a syntax error or unintended behavior.Apply this diff to fix the issue:
class CycleUserPropertiesSerializer(BaseSerializer): class Meta: model = CycleUserProperties fields = "__all__" - read_only_fields = ["workspace", "project", "cycle" "user"] + read_only_fields = ["workspace", "project", "cycle", "user"]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apiserver/plane/app/serializers/cycle.py
(2 hunks)apiserver/plane/app/views/cycle/base.py
(10 hunks)apiserver/plane/app/views/issue/base.py
(1 hunks)apiserver/plane/app/views/issue/sub_issue.py
(1 hunks)apiserver/plane/app/views/module/archive.py
(1 hunks)apiserver/plane/app/views/module/base.py
(1 hunks)apiserver/plane/utils/timezone_converter.py
(1 hunks)apiserver/plane/utils/user_timezone_converter.py
(0 hunks)web/core/components/project/form.tsx
(6 hunks)
💤 Files with no reviewable changes (1)
- apiserver/plane/utils/user_timezone_converter.py
✅ Files skipped from review due to trivial changes (4)
- apiserver/plane/app/views/issue/base.py
- apiserver/plane/app/views/issue/sub_issue.py
- apiserver/plane/app/views/module/archive.py
- apiserver/plane/app/views/module/base.py
🧰 Additional context used
🪛 Ruff (0.8.2)
apiserver/plane/app/views/cycle/base.py
58-58: plane.utils.timezone_converter.convert_utc_to_project_timezone
imported but unused
Remove unused import: plane.utils.timezone_converter.convert_utc_to_project_timezone
(F401)
87-87: Line too long (93 > 88)
(E501)
195-195: Line too long (93 > 88)
(E501)
🔇 Additional comments (2)
web/core/components/project/form.tsx (2)
71-84
: Timezone selection logic is correctly implemented
The timezone options and label rendering are properly handled, enhancing the user experience by allowing timezone selection.
Line range hint 389-413
: Timezone field added to the project form successfully
The timezone field is correctly integrated into the project form with appropriate validation and rendering. Users can now select a timezone when updating project details.
if is_end_date: | ||
localized_datetime -= timedelta(minutes=1) |
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.
Re-evaluate the logic for 'is_end_date' adjustment
Subtracting one minute from the start of the end date may lead to unexpected results, such as the end date being the previous day. Consider setting the time to the end of the day to accurately represent the end date:
# Instead of subtracting one minute
-localized_datetime -= timedelta(minutes=1)
+localized_datetime = localized_datetime.replace(hour=23, minute=59, second=59, microsecond=999999)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
if is_end_date: | |
localized_datetime -= timedelta(minutes=1) | |
if is_end_date: | |
localized_datetime = localized_datetime.replace(hour=23, minute=59, second=59, microsecond=999999) |
…6187) * chore: handled the cycle date time using project timezone * chore: reverted the frontend commit
Description
Allow users to set a specific timezone for a project, ensuring that all cycle operations and calculations are performed according to the configured project timezone.
Type of Change
References
Summary by CodeRabbit
New Features
Bug Fixes
Chores
user_timezone_converter
utility across multiple files, consolidating timezone conversion functionality.Documentation
timezone_converter.py
file, enhancing datetime handling capabilities.