-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Claude/optimize api performance b rdzj #8543
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
Claude/optimize api performance b rdzj #8543
Conversation
…collaborators_production Feature/iss 274028/remove recent collaborators production
…h-fix ISS-274818 changes added for projectId fetching in case of custom pro…
…-for-sequence-id [Feature] [Prod] ISS-274744 : Lock on db for fixing sequence id RACE condition
…d-notification-count-production [Production] Add limit to unread notification count - ISS-274914
…ultiple-language-Support [Feature][Prob]-ISS-275671-Multiple-language-Support-for-Portuguese
…emove-hide-profile [Feature] [Prod] ISS-273505/remove hide profile
* Basic Dev to add dropdown functionallity * change in loading * wb url * to selected the value from the shown list * to check and save value * sending some extra infromation for the px * Fix WB API URL in custom_property.py Updated the WB API URL to production url * env calling * error handling * the reponse handlling * making the dropdowm option generalised as others * new view for dropdown * to send the get properly * some removal of issue_type_custom_property which was not used * the final changes * Remove unused variable assignment in custom_property.py * remove_commit * Fix WB_API_URL environment variable assignment * Final url changes * API changes * remove label from dropdown
* PT translation * Some final translation * change in the translation of stays * type safety error
* sub-issue creation idempotency * 200 status return on existing issue
…ination (#244) * Optimizing issueViewSet and pagination count * Show 100+ instead of count in project issue view details * Grouping paginator Skip_Count changes * Reverting grouped paginator changes
…ination (#250) * Optimizing issueViewSet and pagination count * Show 100+ instead of count in project issue view details * Grouping paginator Skip_Count changes * Reverting grouped paginator changes * type issues fix - issueView set pagination improvement * type issue fixes * type issue fixes * type issue fixes * type issue fixes
* whitelist Return Type / Return Detail * final fix
- Remove redundant prefetch_related for assignees, labels, and modules
(ArrayAgg annotations already fetch this data, avoiding duplicate queries)
- Optimize sub_issues_count to use base Issue.objects manager instead of
IssueManager, avoiding unnecessary filters on simple child count
- Replace duplicate ProjectMember JOINs with Exists subquery for role-based
filtering, eliminating cartesian product risk
- Add database indexes for common query patterns:
- Issue: created_at desc, parent+deleted_at, project+archived_at+is_draft,
workspace+created_at
- IssueCustomProperty: issue+key, key, key+value
|
Important Review skippedToo many files! 150 files out of 300 files are above the max files limit of 150. You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
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.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| request.META.get("HTTP_ORIGIN"), | ||
| user, | ||
| serializer | ||
| ) |
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.
Project creation crashes due to undefined variable user
High Severity
The post method calls self.create_project() with an undefined variable user on line 253. This will cause a NameError crash when attempting to create a project. The variable should be request.user. Additionally, create_project is defined as a module-level function (not a class method), so calling it via self.create_project() will raise an AttributeError.
| Available on One | ||
| <SquareArrowOutUpRight className="h-3.5 w-3.5 p-0.5" /> | ||
| </a> | ||
| <a /> |
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.
UpgradeButton renders empty non-functional anchor element
Medium Severity
The UpgradeButton component now renders just <a /> - an empty self-closing anchor tag with no href, content, or styling. The original functional button with the link to "plane.so/one" and "Available on One" text is commented out below it. This appears to be debugging or temporary code that was accidentally committed.
| else: | ||
| user = User.objects.filter(is_superuser=True).first() | ||
| self.rewite_project_id_in_url() | ||
| return (user, token) |
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.
Static token authentication returns None user without validation
High Severity
When using the static API token with an X-Assume-Role header, if no user exists with the specified username, User.objects.filter(...).first() returns None. The authentication then returns (None, token) without raising an error, potentially allowing requests to proceed with an unauthenticated/null user, which could cause downstream failures or authorization bypasses.
| # Validate the API token | ||
| user, token = self.validate_api_token(token) | ||
| assume_role_value = request.headers.get(self.assume_header_role, None) | ||
| print("assume_role",assume_role_value) |
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.
Debug print statements left in production API code
Medium Severity
Multiple debug print() statements were added across the API codebase that log sensitive information including exception details, request data, user assume-role values, and parsed HTML content. These statements pollute server logs, may expose sensitive data, and indicate incomplete cleanup of debugging code before commit.
Additional Locations (2)
|
|
||
| return user | ||
|
|
||
| return request.user # Default user if no assume role is found |
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.
Base API view creates users from untrusted headers
High Severity
The get_or_create_user_from_headers method automatically creates new user accounts when an X-Assume-Role header is provided with a username that doesn't exist. This runs before authentication checks in dispatch(), allowing any request with this header to create arbitrary user accounts in the database with fake email addresses.
| serializer = WebhookSerializer( | ||
| webhook, | ||
| data=request.data, | ||
| context={request: request}, |
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.
Webhook serializer context uses object as dictionary key
Medium Severity
The patch method passes context={request: request} to the serializer, using the request object as a dictionary key instead of the string "request". This means self.context.get("request") in the serializer will return None, breaking any validation or processing that depends on accessing the request from context.
| data['created_by'] = User.objects.get(username=data['created_by']) | ||
| else: | ||
| user_data = { | ||
| "email": data['created_by'] + '@plane-shipsy.com', |
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.
Issue serializer crashes on missing created_by field
Medium Severity
When created_by is not provided in the request (it's required=False), the code checks is_uuid(data.get('created_by')) which returns False for None. It then accesses data['created_by'] directly, causing a KeyError if the field wasn't provided, or a TypeError from None + '@plane-shipsy.com' if it was explicitly set to None.
| """ | ||
|
|
||
| // Debugging: Show the contents of the .env file (optional for development only) | ||
| sh "cat apiserver/${configStoragePath}/.env" |
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.
Jenkinsfile logs secrets to console output
High Severity
The Jenkinsfile contains debug statements that output secrets to the Jenkins console: line 78 echoes the CONFIG variable value, and line 85 runs cat on the .env file. These expose sensitive configuration and credentials in build logs which may be accessible to unauthorized users or stored indefinitely.
| current_instance=current_instance, | ||
| epoch=epoch_timestamp, | ||
| ) | ||
| return Response(serializer.data, status=status.HTTP_200_OK) |
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.
Custom property PATCH returns stale cached serializer data
Medium Severity
The patch method creates a serializer before updating the model, then accesses serializer.data at line 1249 to capture the current instance for activity logging. This caches the old values. After modifying and saving custom_property, the method returns serializer.data which contains the cached pre-update values, not the newly saved values. Clients receive stale data in the response.
Additional Locations (1)
| for item in data : | ||
| if item.get("data_type") == "number": | ||
| int_value = int(item.get("value")) | ||
| item["int_value"] = int_value |
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.
Number type custom property parsing crashes on invalid input
Medium Severity
When processing custom properties with data_type="number", the code calls int(item.get("value")) without error handling. If value is None, a non-numeric string like "abc", or missing, this raises TypeError or ValueError, causing a 500 error instead of a proper validation error response. The date type handling has try/except but number does not.
Description
Type of Change
Screenshots and Media (if applicable)
Test Scenarios
References
Note
Implements end-to-end deployment and expands API capabilities while optimizing runtime and permissions.
Jenkinsfilefetches secrets from Vault, builds/pushesweb,admin,apiserverimages, and deploys ECS services (frontend, admin, apiserver, celery, beat) with Slack notification.apiserver/Dockerfile.apiadds Flower, multi-modeENV_TYPEentrypoint (api/celery/beat), port5555, and copies env viaENV_FILE_PATH;admin/Dockerfile.adminexposes 3000 with startCMD;aio/nginx.confincreases body size and enables chunked encoding.APIKeyAuthenticationsupports staticSTATIC_API_TOKENandX-Assume-Role; throttles bypass static token.ProjectEntityGuestPermission; broadens member checks and allows superuser bypass.IssueCustomPropertysupport across serializers/views; create/update handles typed values and user auto-provision by username; filtering and aggregation on custom properties; new dropdown options endpoint.IssueAttachmentV2Endpointusing S3 presigned uploads and download URLs.strproject_id; new issue type CRUD and custom property endpoints.UpgradeButton.Written by Cursor Bugbot for commit 307828f. This will update automatically on new commits. Configure here.