[WEB-2843] chore: updated the cycle end date logic #6194
Conversation
WalkthroughThe changes in this pull request involve updates to several serializers and views related to cycle management within the application. Key modifications include the introduction of UTC date 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (3)
apiserver/plane/app/serializers/cycle.py (1)
Line range hint
93-93: Fix syntax error in read_only_fieldsThere's a missing comma between
"cycle"and"user"in the read_only_fields list.- read_only_fields = ["workspace", "project", "cycle" "user"] + read_only_fields = ["workspace", "project", "cycle", "user"]apiserver/plane/app/views/cycle/base.py (2)
262-262: LGTM! Consider fixing line length.The change to use user timezone is appropriate. However, the line exceeds the length limit.
- data = user_timezone_converter(data, datetime_fields, request.user.user_timezone) + data = user_timezone_converter( + data, datetime_fields, request.user.user_timezone + )🧰 Tools
🪛 Ruff (0.8.2)
262-262: Line too long (89 > 88)
(E501)
460-460: LGTM! Consider fixing line length.The change to use user timezone is appropriate. However, the line exceeds the length limit.
- data = user_timezone_converter(data, datetime_fields, request.user.user_timezone) + data = user_timezone_converter( + data, datetime_fields, request.user.user_timezone + )🧰 Tools
🪛 Ruff (0.8.2)
460-460: Line too long (89 > 88)
(E501)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apiserver/plane/api/serializers/cycle.py(2 hunks)apiserver/plane/app/serializers/cycle.py(1 hunks)apiserver/plane/app/views/cycle/base.py(3 hunks)apiserver/plane/app/views/workspace/cycle.py(1 hunks)apiserver/plane/utils/timezone_converter.py(2 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
apiserver/plane/app/views/cycle/base.py
262-262: Line too long (89 > 88)
(E501)
460-460: Line too long (89 > 88)
(E501)
apiserver/plane/app/views/workspace/cycle.py
13-13: plane.utils.timezone_converter.user_timezone_converter imported but unused
Remove unused import: plane.utils.timezone_converter.user_timezone_converter
(F401)
🔇 Additional comments (3)
apiserver/plane/utils/timezone_converter.py (1)
31-31: Verify the impact of timezone conversion logic change
The change from subtracting a minute from end dates to adding a minute to start dates represents a significant shift in date handling logic. This could affect:
- Existing cycles in the database
- Integration points that depend on this behavior
- Date range calculations in queries
Consider adding a migration plan or documentation for this behavioral change, especially if it affects existing data.
Also applies to: 61-63
✅ Verification successful
Based on the search results, I can now provide a final response:
The timezone conversion change is safely implemented across the codebase
The code search reveals that convert_to_utc is consistently used in cycle-related serializers and views, with proper handling of both start and end dates. The change from is_end_date to is_start_date is consistently applied:
- In
plane/api/serializers/cycle.pyandplane/app/serializers/cycle.py, start dates are correctly passed withis_start_date=True - End dates are passed without the parameter, defaulting to
False - All date range queries in views use Django's
timezone.now()for comparisons, which is timezone-aware and not affected by this change
The behavioral change from subtracting a minute from end dates to adding a minute to start dates maintains the same effective date ranges, just implemented differently. This change doesn't impact existing data consistency as:
- The database stores dates in UTC format
- All queries properly use timezone-aware comparisons
- The conversion is only applied during serialization/deserialization
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for potential impacts of the timezone conversion change
# Find all references to convert_to_utc to ensure consistent usage
echo "Checking all references to convert_to_utc:"
rg "convert_to_utc"
# Look for any date range queries that might be affected
echo "Checking date range queries in cycle-related files:"
rg -A 3 "start_date|end_date" --glob "**/*cycle*.py"
Length of output: 12377
apiserver/plane/app/serializers/cycle.py (1)
25-25: Maintain consistency with API serializer implementation
The UTC conversion implementation here should match the error handling improvements suggested for the API serializer.
Also applies to: 28-28
apiserver/plane/app/views/cycle/base.py (1)
536-537: Verify consistent usage of convert_to_utc parameters.
The addition of is_start_date parameter needs to be verified across the codebase to ensure consistent usage.
✅ Verification successful
Let me check the implementation of convert_to_utc function to ensure the parameter is properly handled.
Let me try a different approach to find the function definition using ripgrep.
Let me check the complete implementation of the function to verify the parameter handling.
Let me get the complete implementation as the function seems to continue beyond what we see.
The convert_to_utc parameter usage is consistent across the codebase
Based on the verification:
- The
convert_to_utcfunction properly implements theis_start_dateparameter with a default value ofFalse - All start date conversions across the codebase correctly pass
is_start_date=True - All end date conversions correctly omit the parameter, using the default
Falsevalue - The implementation adds one minute to start dates when
is_start_date=True, which is consistently applied
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for all usages of convert_to_utc to ensure consistent parameter usage
# Search for convert_to_utc function calls
ast-grep --pattern 'convert_to_utc($$$)'
Length of output: 1352
Script:
#!/bin/bash
# Search for convert_to_utc function definition
ast-grep --pattern 'def convert_to_utc($$$)'
Length of output: 46
Script:
#!/bin/bash
# Search for convert_to_utc function definition using ripgrep
rg "def convert_to_utc" -A 5
Length of output: 494
Script:
#!/bin/bash
# Get the complete function implementation
rg "def convert_to_utc" -A 20 apiserver/plane/utils/timezone_converter.py
Length of output: 832
Script:
#!/bin/bash
# Get the complete file content to see full implementation
cat apiserver/plane/utils/timezone_converter.py
Length of output: 3492
| if ( | ||
| data.get("start_date", None) is not None | ||
| and data.get("end_date", None) is not None | ||
| ): | ||
| project_id = self.initial_data.get("project_id") or self.instance.project_id | ||
| data["start_date"] = convert_to_utc( | ||
| str(data.get("start_date").date()), project_id, is_start_date=True | ||
| ) | ||
| data["end_date"] = convert_to_utc( | ||
| str(data.get("end_date", None).date()), project_id | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider additional error handling for date conversion
While the validation logic is sound, consider adding error handling for the convert_to_utc calls which could raise exceptions if:
- Project doesn't exist
- Invalid timezone in project
- Invalid date format
if (
data.get("start_date", None) is not None
and data.get("end_date", None) is not None
):
project_id = self.initial_data.get("project_id") or self.instance.project_id
- data["start_date"] = convert_to_utc(
- str(data.get("start_date").date()), project_id, is_start_date=True
- )
- data["end_date"] = convert_to_utc(
- str(data.get("end_date", None).date()), project_id
- )
+ try:
+ data["start_date"] = convert_to_utc(
+ str(data.get("start_date").date()), project_id, is_start_date=True
+ )
+ data["end_date"] = convert_to_utc(
+ str(data.get("end_date", None).date()), project_id
+ )
+ except (Project.DoesNotExist, ValueError) as e:
+ raise serializers.ValidationError(f"Date conversion failed: {str(e)}")📝 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 ( | |
| data.get("start_date", None) is not None | |
| and data.get("end_date", None) is not None | |
| ): | |
| project_id = self.initial_data.get("project_id") or self.instance.project_id | |
| data["start_date"] = convert_to_utc( | |
| str(data.get("start_date").date()), project_id, is_start_date=True | |
| ) | |
| data["end_date"] = convert_to_utc( | |
| str(data.get("end_date", None).date()), project_id | |
| ) | |
| if ( | |
| data.get("start_date", None) is not None | |
| and data.get("end_date", None) is not None | |
| ): | |
| project_id = self.initial_data.get("project_id") or self.instance.project_id | |
| try: | |
| data["start_date"] = convert_to_utc( | |
| str(data.get("start_date").date()), project_id, is_start_date=True | |
| ) | |
| data["end_date"] = convert_to_utc( | |
| str(data.get("end_date", None).date()), project_id | |
| ) | |
| except (Project.DoesNotExist, ValueError) as e: | |
| raise serializers.ValidationError(f"Date conversion failed: {str(e)}") |
| @@ -10,7 +10,7 @@ | |||
| from plane.db.models import Cycle | |||
| from plane.app.permissions import WorkspaceViewerPermission | |||
| from plane.app.serializers.cycle import CycleSerializer | |||
|
|
|||
| from plane.utils.timezone_converter import user_timezone_converter | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unused import.
The user_timezone_converter import is not used in this file. Consider removing it until needed.
-from plane.utils.timezone_converter import user_timezone_converter🧰 Tools
🪛 Ruff (0.8.2)
13-13: plane.utils.timezone_converter.user_timezone_converter imported but unused
Remove unused import: plane.utils.timezone_converter.user_timezone_converter
(F401)
pushya22
changed the title
Description
updated the end date of the cycle to reflect the finish date.
Type of Change
Summary by CodeRabbit
New Features
Bug Fixes
CycleUserPropertiesSerializeraffecting field definitions.Improvements