UN-2773 [FEAT] Add sharing functionality for API deployments#1520
Conversation
- Add shared_users and shared_to_org fields to APIDeployment model - Update APIDeploymentViewSet with sharing permissions and endpoints - Add list_of_shared_users endpoint for retrieving sharing details - Implement sharing notifications via email when users are added - Update frontend with Share button and SharePermission modal integration - Include owner column in API deployments table - Support individual user sharing with organization member filtering 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Summary by CodeRabbit
WalkthroughAdds sharing to APIDeployments: model fields and migration, manager method for_user, serializers and endpoint to list shared users, view permission adjustments and PATCH handling to update sharing with optional notifications, plus frontend UI, actions, and service methods to view/update sharing. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant FE as Frontend (ApiDeployment.jsx)
participant SVC as api-deployments-service.js
participant BE as APIDeploymentViewSet
participant DB as Database
U->>FE: Click "Share" on a deployment
FE->>SVC: getAllUsers()
SVC->>BE: GET /users/
BE->>DB: Query users
DB-->>BE: Users
BE-->>SVC: 200 Users
SVC-->>FE: Users
FE->>SVC: getSharedUsers(deploymentId)
SVC->>BE: GET /api/deployment/{id}/users/
BE->>DB: Fetch deployment + shared_users
DB-->>BE: Deployment + shared list
BE-->>SVC: 200 Shared users
SVC-->>FE: Shared users
FE-->>U: Open Share modal with data
sequenceDiagram
autonumber
actor U as User
participant FE as Frontend
participant SVC as api-deployments-service.js
participant BE as APIDeploymentViewSet
participant DB as Database
participant NP as NotificationPlugin (optional)
U->>FE: Apply sharing changes
FE->>SVC: updateSharing(id, sharedUsers, shareWithEveryone)
SVC->>BE: PATCH /api/deployment/{id} {shared_users, shared_to_org}
BE->>DB: Load deployment (permission check via IsOwner/IsOwnerOrSharedUser)
BE->>DB: Update `shared_users` M2M and `shared_to_org`
DB-->>BE: Saved
alt Notification plugin available & new users added
BE->>NP: send_sharing_notification(resource, participants)
NP-->>BE: ack or error (logged, non-fatal)
end
BE-->>SVC: 200 Updated deployment
SVC-->>FE: Success
FE-->>U: Success toast & refresh
sequenceDiagram
autonumber
actor User
participant FE as Frontend
participant BE as APIDeploymentViewSet
participant DB as Database
User->>FE: View deployments
FE->>BE: GET /api/deployment/
BE->>DB: APIDeployment.objects.for_user(request.user)
DB-->>BE: Owned ∪ Shared deployments
BE-->>FE: 200 List
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
Handle cases where notification plugin may not be available by wrapping imports in try-catch block and checking availability before use. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
johnyrahul
requested review from
chandrasekharan-zipstack,
jaseemjaskp,
kirtimanmishrazipstack and
vishnuszipstack
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (9)
backend/api_v2/urls.py (1)
31-35: Endpoint wiring is fine; consider @action for cohesionDefining
list_of_shared_usersas a DRF@action(detail=True, methods=["get"])on the ViewSet keeps routing/permissions consistent and testable.frontend/src/components/deployments/api-deployment/ApiDeployment.jsx (2)
149-162: Make owner check type‑stableIDs may differ in type/format (e.g., UUID vs quoted string). Normalize before comparison to avoid false negatives.
Apply this diff:
-const currentUser = sessionDetails?.userId; -const isOwner = record?.created_by === currentUser; +const currentUser = sessionDetails?.userId; +const isOwner = String(record?.created_by || "") === String(currentUser || "");
369-384: Gate “Share” action to owners client‑sideTo reduce noise and prevent inevitable 403s, disable the menu item when the current user isn’t the owner. Server‑side checks remain the source of truth.
Apply this diff:
{ key: "3", label: ( <Space direction="horizontal" className="action-items" onClick={handleShare} > @@ <div> <Typography.Text>Share</Typography.Text> </div> </Space> ), + disabled: String(selectedRow?.created_by || "") !== String(sessionDetails?.userId || ""), },backend/api_v2/api_deployment_views.py (3)
170-170: Avoid N+1 for created_by_email in list.Select the relation once at the queryset level.
Apply this diff:
- queryset = APIDeployment.objects.for_user(self.request.user) + queryset = ( + APIDeployment.objects.for_user(self.request.user) + .select_related("created_by") + )
268-274: Silence false-positive unused args warning (Ruff ARG002).DRF requires the standard signature even if not used.
Apply this diff:
- def list_of_shared_users(self, request: Request, pk: str | None = None) -> Response: + def list_of_shared_users( # noqa: ARG002 + self, request: Request, pk: str | None = None + ) -> Response:
275-309: Notification path: compare by IDs, reuse singleton, log with stack.
- Set-diff model instances can be fragile; compare by IDs.
- Reuse the module-level service instead of re-instantiating.
- Use logger.exception for tracebacks.
Apply this diff:
def partial_update(self, request: Request, *args: Any, **kwargs: Any) -> Response: """Override partial_update to handle sharing notifications.""" - # Get current instance and shared users + # Get current instance and shared users instance = self.get_object() - current_shared_users = set(instance.shared_users.all()) + current_shared_user_ids = set( + instance.shared_users.values_list("id", flat=True) + ) # Perform the update response = super().partial_update(request, *args, **kwargs) # If successful and shared_users changed, send notifications if ( response.status_code == 200 and "shared_users" in request.data and NOTIFICATION_PLUGIN_AVAILABLE ): try: instance.refresh_from_db() - new_shared_users = set(instance.shared_users.all()) - newly_shared_users = new_shared_users - current_shared_users + new_shared_user_ids = set( + instance.shared_users.values_list("id", flat=True) + ) + newly_added_ids = new_shared_user_ids - current_shared_user_ids - if newly_shared_users: - notification_service = SharingNotificationService() - notification_service.send_sharing_notification( + if newly_added_ids and sharing_notification_service: + newly_shared_users = list( + instance.shared_users.filter(id__in=newly_added_ids) + ) + sharing_notification_service.send_sharing_notification( resource_type=ResourceType.API_DEPLOYMENT.value, resource_name=instance.display_name, resource_id=str(instance.id), shared_by=request.user, - shared_to=list(newly_shared_users), + shared_to=newly_shared_users, resource_instance=instance, ) except Exception as e: - logger.error(f"Failed to send sharing notification: {e}") + logger.exception("Failed to send sharing notification")backend/api_v2/serializers.py (3)
382-382: Prefer source= for simple related fields; drop method.Cuts boilerplate and pairs well with select_related above.
Apply this diff:
- created_by_email = SerializerMethodField() + created_by_email = CharField(source="created_by.email", read_only=True) @@ - def get_created_by_email(self, obj): - """Get the email of the creator.""" - return obj.created_by.email if obj.created_by else None + # get_created_by_email no longer neededAlso applies to: 399-402
446-449: Avoid loading full user objects; fetch only needed fields.Use values to reduce overhead (even though it’s a single object endpoint).
Apply this diff:
def get_shared_users(self, obj): """Return list of shared users with id and email.""" - return [{"id": user.id, "email": user.email} for user in obj.shared_users.all()] + return list(obj.shared_users.values("id", "email"))
444-444: Ruff RUF012 on Meta.fields is a common false positive in Django.Either ignore or silence inline.
Apply this diff:
- fields = ["id", "display_name", "shared_users", "shared_to_org", "created_by"] + fields = ["id", "display_name", "shared_users", "shared_to_org", "created_by"] # noqa: RUF012
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (8)
backend/api_v2/api_deployment_views.py(4 hunks)backend/api_v2/api_key_views.py(2 hunks)backend/api_v2/migrations/0002_apideployment_shared_to_org_and_more.py(1 hunks)backend/api_v2/models.py(2 hunks)backend/api_v2/serializers.py(4 hunks)backend/api_v2/urls.py(2 hunks)frontend/src/components/deployments/api-deployment/ApiDeployment.jsx(11 hunks)frontend/src/components/deployments/api-deployment/api-deployments-service.js(1 hunks)
🧰 Additional context used
🪛 Ruff (0.12.2)
backend/api_v2/migrations/0002_apideployment_shared_to_org_and_more.py
8-11: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
13-31: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
backend/api_v2/serializers.py
444-444: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
backend/api_v2/api_key_views.py
18-18: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
backend/api_v2/api_deployment_views.py
269-269: Unused method argument: request
(ARG002)
269-269: Unused method argument: pk
(ARG002)
305-305: Do not catch blind exception: Exception
(BLE001)
306-306: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (11)
backend/api_v2/migrations/0002_apideployment_shared_to_org_and_more.py (2)
1-31: Migration looks correctAdds fields with expected types and dependency ordering. No issues.
1-31: Ruff RUF012 on migrations can be ignoredRUF012 flags mutable class attrs, but Django migration
dependencies/operationsas lists are idiomatic and safe here.frontend/src/components/deployments/api-deployment/ApiDeployment.jsx (3)
269-303: Good UX flow; minor nitOpening the modal early with a loading spinner is fine. Consider handling the case where
membersis absent by defaulting to[](already done). No changes required.
563-572: SharePermission wiring LGTMProps look consistent with the service payload and the current choice to hide org‑wide sharing.
305-323: Don't drop the 2nd parameter — callers pass three argsTwo callers invoke onApply with three arguments:
- frontend/src/components/widgets/share-permission/SharePermission.jsx:155 — onApply(selectedUsers, adapter, shareWithEveryone)
- frontend/src/components/custom-tools/export-tool/ExportTool.jsx:168 — onApply(selectedUsers, toolDetails, sharingOption === SHARE_ALL)
Either:
- keep the second parameter in ApiDeployment.jsx (rename to _api or _unusedApi to indicate it's intentionally unused), or
- remove the second parameter and update both callers above to stop passing the second arg (so they call onApply(selectedUsers, shareWithEveryone)).
backend/api_v2/api_key_views.py (1)
1-1: Import path is correct — no change requiredIsOwnerOrSharedUser is defined in backend/permissions/permission.py and is imported by backend/api_v2/api_key_views.py (line 1).
backend/api_v2/models.py (1)
88-95: Enforce same-organization invariant for shared_usersAdd server-side validation so users from other organizations cannot be added to shared_users — enforce in serializer.validate_shared_users or model.clean. Location: backend/api_v2/models.py (shared_users field, lines ~88–95). A search for
validate_shared_users|cleanin backend/api_v2 returned no matches; add/verify this check and accompanying tests.Suggested serializer validator (adjust to your org model):
def validate_shared_users(self, users): org = self.instance.organization if self.instance else self.context["organization"] invalid = [u for u in users if getattr(u, "organization_id", None) != getattr(org, "id", None)] if invalid: raise serializers.ValidationError("All shared users must belong to the same organization.") return usersfrontend/src/components/deployments/api-deployment/api-deployments-service.js (1)
99-124: Localize request options and verify endpoint/payload contracts
- Use a function‑local
const optionsto avoid mutating the module‑scopedoptionsduring concurrent requests (frontend/src/components/deployments/api-deployment/api-deployments-service.js).- Verified: backend defines the deployment users endpoint and sharing field (backend/api_v2/urls.py:
deployment/<uuid:pk>/users/; backend/api_v2/models.py:APIDeployment.shared_usersis a ManyToManyField; backend/api_v2/api_deployment_views.py checks for"shared_users"inrequest.dataduringpartial_update). Confirm the APIDeployment serializer acceptsshared_usersas an array of user IDs and that${path}/users/is the intended org‑scoped users endpoint before merging.Apply this diff:
getSharedUsers: (id) => { - options = { + const options = { method: "GET", url: `${path}/api/deployment/${id}/users/`, }; return axiosPrivate(options); }, updateSharing: (id, sharedUsers, shareWithEveryone) => { - options = { + const options = { method: "PATCH", url: `${path}/api/deployment/${id}/`, headers: requestHeaders, data: { shared_users: sharedUsers, shared_to_org: shareWithEveryone, }, }; return axiosPrivate(options); }, getAllUsers: () => { - options = { + const options = { method: "GET", url: `${path}/users/`, }; return axiosPrivate(options); },backend/api_v2/urls.py (1)
61-65: Object‑level permissions are enforced — no change requiredlist_of_shared_users already calls self.get_object() (backend/api_v2/api_deployment_views.py:269–273); DRF's GenericAPIView.get_object calls self.check_object_permissions(self.request, obj), so object‑level checks will run. (django-rest-framework.org)
backend/api_v2/api_deployment_views.py (2)
8-8: LGTM: permissions import.Import of IsOwner and IsOwnerOrSharedUser aligns with usage below.
34-43: Good plugin gating + safe fallback.Optional notification plugin is correctly feature-gated and won’t break OSS builds.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Rahul Johny <116638720+johnyrahul@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
backend/api_v2/api_deployment_views.py (1)
165-166: Nice: PUT now owner-only (addresses prior review).
🧹 Nitpick comments (3)
backend/api_v2/api_deployment_views.py (3)
169-177: Optional: prefetch relations to avoid N+1 in list views.If serializers touch
created_byand/orshared_users, prefetch for efficiency.Apply this diff if applicable:
- queryset = APIDeployment.objects.for_user(self.request.user) + queryset = ( + APIDeployment.objects.for_user(self.request.user) + .select_related("created_by") + .prefetch_related("shared_users") + )
268-274: Silence Ruff ARG002 on unused args.Rename parameters to underscores; logic unchanged.
Apply this diff:
- def list_of_shared_users(self, request: Request, pk: str | None = None) -> Response: + def list_of_shared_users(self, _request: Request, _pk: str | None = None) -> Response:
275-309: Harden notification flow: 2xx check, reuse module service, drop brittle data gate, log stacktrace.
- Check for any 2xx, not only 200.
- Use the module-level
sharing_notification_serviceinstead of re-instantiating.- Don’t rely on
"shared_users" in request.data; compute diff regardless.- Use
logger.exception(also satisfies Ruff TRY400) and avoid blind catch message loss.Apply this diff:
- if ( - response.status_code == 200 - and "shared_users" in request.data - and NOTIFICATION_PLUGIN_AVAILABLE - ): + if ( + 200 <= response.status_code < 300 + and sharing_notification_service + ): try: instance.refresh_from_db() new_shared_users = set(instance.shared_users.all()) newly_shared_users = new_shared_users - current_shared_users - if newly_shared_users: - notification_service = SharingNotificationService() - notification_service.send_sharing_notification( + if newly_shared_users: + sharing_notification_service.send_sharing_notification( resource_type=ResourceType.API_DEPLOYMENT.value, resource_name=instance.display_name, resource_id=str(instance.id), shared_by=request.user, shared_to=list(newly_shared_users), resource_instance=instance, ) - except Exception as e: - logger.error(f"Failed to send sharing notification: {e}") + except Exception: + logger.exception("Failed to send sharing notification")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
backend/api_v2/api_deployment_views.py(4 hunks)
🧰 Additional context used
🪛 Ruff (0.12.2)
backend/api_v2/api_deployment_views.py
269-269: Unused method argument: request
(ARG002)
269-269: Unused method argument: pk
(ARG002)
305-305: Do not catch blind exception: Exception
(BLE001)
306-306: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (2)
backend/api_v2/api_deployment_views.py (2)
8-8: LGTM: permission imports are correct.
31-31: LGTM: serializer import added for sharing list.
|
@johnyrahul pls add screenshots. |
Co-authored-by: Chandrasekharan M <117059509+chandrasekharan-zipstack@users.noreply.github.com> Signed-off-by: Rahul Johny <116638720+johnyrahul@users.noreply.github.com>
…Zipstack/unstract into UN-2773-add-sharing-for-api-deployments
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (2)
backend/api_v2/api_deployment_views.py(4 hunks)frontend/src/components/deployments/api-deployment/ApiDeployment.jsx(11 hunks)
🧰 Additional context used
🪛 Ruff (0.13.1)
backend/api_v2/api_deployment_views.py
269-269: Unused method argument: request
(ARG002)
269-269: Unused method argument: pk
(ARG002)
306-306: Redundant exception object included in logging.exception call
(TRY401)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (2)
backend/api_v2/api_deployment_views.py (2)
164-174: Lock downlist_of_shared_usersto ownersBecause
get_permissionsoverrides per-actionpermission_classes, shared users can currently calllist_of_shared_usersand see the entire sharing roster. Include this action in the owner-only set.- def get_permissions(self) -> list[Any]: - if self.action in ["destroy", "partial_update", "update"]: + def get_permissions(self) -> list[Any]: + owner_only_actions = { + "destroy", + "partial_update", + "update", + "list_of_shared_users", + } + if self.action in owner_only_actions: return [IsOwner()] return [IsOwnerOrSharedUser()]
34-43: Don’t let notification plugin init crash imports
SharingNotificationService()is constructed at import time outside theImportErrorguard, so any runtime misconfiguration raises and prevents Django from loading the module. Wrap the constructor in its owntry/exceptand fall back to disabling notifications when initialization fails.-try: - from plugins.notification.constants import ResourceType - from plugins.notification.sharing_notification import SharingNotificationService - - NOTIFICATION_PLUGIN_AVAILABLE = True - sharing_notification_service = SharingNotificationService() -except ImportError: +try: + from plugins.notification.constants import ResourceType + from plugins.notification.sharing_notification import SharingNotificationService +except ImportError: NOTIFICATION_PLUGIN_AVAILABLE = False sharing_notification_service = None +else: + try: + sharing_notification_service = SharingNotificationService() + NOTIFICATION_PLUGIN_AVAILABLE = True + except Exception: + logging.exception( + "Failed to initialize SharingNotificationService; notifications disabled" + ) + NOTIFICATION_PLUGIN_AVAILABLE = False + sharing_notification_service = None
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/api_v2/api_deployment_views.py (1)
277-311: PUT path missing notifications; parity with PATCH needed.Updating shared_users via PUT won’t send notifications. Mirror the partial_update logic in update().
Would you like me to open a follow-up PR? If you prefer inline, add:
def update(self, request: Request, *args: Any, **kwargs: Any) -> Response: instance = self.get_object() current_shared_users = set(instance.shared_users.all()) response = super().update(request, *args, **kwargs) if ( response.status_code == 200 and "shared_users" in request.data and NOTIFICATION_PLUGIN_AVAILABLE ): try: instance.refresh_from_db() new_shared_users = set(instance.shared_users.all()) newly_shared_users = new_shared_users - current_shared_users if newly_shared_users and sharing_notification_service: sharing_notification_service.send_sharing_notification( resource_type=ResourceType.API_DEPLOYMENT.value, resource_name=instance.display_name, resource_id=str(instance.id), shared_by=request.user, shared_to=list(newly_shared_users), resource_instance=instance, ) except Exception: logger.exception("Failed to send sharing notification") return response
🧹 Nitpick comments (8)
backend/api_v2/api_deployment_views.py (6)
34-43: Avoid double-initializing the notification service.You create SharingNotificationService() at import time and again per request in partial_update. Reuse the module-level instance to reduce overhead and keep behavior consistent with fail-fast init.
Apply this diff in the partial_update block (see lines 297-305 below) to use the module-level singleton instead of re-instantiating.
172-172: Prevent N+1 on created_by_email by selecting the relation.APIDeploymentListSerializer uses created_by_email; select_related('created_by') avoids extra queries during list.
- queryset = APIDeployment.objects.for_user(self.request.user) + queryset = ( + APIDeployment.objects.for_user(self.request.user) + .select_related("created_by") + )
270-276: Make intent clear: remove redundant permission override and silence ARG002.Since get_permissions returns IsOwnerOrSharedUser for this action (by design), the @action(permission_classes=[IsOwner]) is misleading and ignored. Also silence Ruff ARG002 for unused args.
- @action(detail=True, methods=["get"], permission_classes=[IsOwner]) - def list_of_shared_users(self, request: Request, pk: str | None = None) -> Response: + @action(detail=True, methods=["get"]) + def list_of_shared_users(self, _request: Request, _pk: str | None = None) -> Response: """List users who have access to this API deployment.""" instance = self.get_object() serializer = SharedUserListSerializer(instance) return Response(serializer.data)Confirm that shared users should see this list. If owner-only is desired instead, add "list_of_shared_users" to the owner-only set in get_permissions.
298-305: Reuse the module-level service, don’t instantiate per request.Prevents repeated construction and aligns with your fail-fast init choice.
- notification_service = SharingNotificationService() - notification_service.send_sharing_notification( + if sharing_notification_service: + sharing_notification_service.send_sharing_notification( resource_type=ResourceType.API_DEPLOYMENT.value, resource_name=instance.display_name, resource_id=str(instance.id), shared_by=request.user, shared_to=list(newly_shared_users), resource_instance=instance, - ) + )
308-308: Use logger.exception without interpolating the exception.logging.exception already includes the traceback; embedding {e} is redundant (Ruff TRY401).
- logger.exception(f"Failed to send sharing notification: {e}") + logger.exception("Failed to send sharing notification")
232-235: Respect sharing in by_prompt_studio_tool.Currently filtered by created_by only; shared users won’t see deployments they can access. Filter via for_user to match list semantics.
# Outside changed lines; suggested replacement for the queryset in by_prompt_studio_tool: deployments = ( APIDeployment.objects.for_user(request.user) .filter(workflow_id__in=workflow_ids) )Confirm if this endpoint should reflect sharing. If owner-only is intended, ignore.
backend/api_v2/serializers.py (2)
395-410: Good addition; ensure queryset optimizes creator lookups.created_by_email is useful. Pair with select_related('created_by') in the view (suggested) to avoid N+1 during list.
449-468: Minor: make Meta.fields immutable to quiet lint and reduce accidental mutation.Using a tuple instead of list is a small improvement and may satisfy RUF012.
- fields = ["id", "display_name", "shared_users", "shared_to_org", "created_by"] + fields = ("id", "display_name", "shared_users", "shared_to_org", "created_by")Also confirm that exposing user emails to all shared users is acceptable per org privacy policy.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (2)
backend/api_v2/api_deployment_views.py(4 hunks)backend/api_v2/serializers.py(4 hunks)
🧰 Additional context used
🪛 Ruff (0.13.1)
backend/api_v2/api_deployment_views.py
271-271: Unused method argument: request
(ARG002)
271-271: Unused method argument: pk
(ARG002)
308-308: Redundant exception object included in logging.exception call
(TRY401)
backend/api_v2/serializers.py
457-457: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (4)
backend/api_v2/api_deployment_views.py (2)
8-8: LGTM: Permission import and serializer wiring are correct.Using IsOwnerOrSharedUser aligns with the new sharing model, and importing SharedUserListSerializer is consistent with the new endpoint.
Also applies to: 31-31
166-169: LGTM: PUT now owner-only.Including "update" alongside "destroy" and "partial_update" is correct and closes the prior gap.
backend/api_v2/serializers.py (2)
19-20: LGTM: SerializerMethodField import added.Import aligns with new computed fields.
412-415: LGTM.Method is concise and safe if creator is missing.
johnyrahul
changed the title
|
|
* [FEAT] Add sharing functionality for API deployments - Add shared_users and shared_to_org fields to APIDeployment model - Update APIDeploymentViewSet with sharing permissions and endpoints - Add list_of_shared_users endpoint for retrieving sharing details - Implement sharing notifications via email when users are added - Update frontend with Share button and SharePermission modal integration - Include owner column in API deployments table - Support individual user sharing with organization member filtering 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * [FIX] Import notification plugin conditionally in API deployment views Handle cases where notification plugin may not be available by wrapping imports in try-catch block and checking availability before use. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * Update backend/api_v2/api_deployment_views.py Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Rahul Johny <116638720+johnyrahul@users.noreply.github.com> * Update backend/api_v2/api_deployment_views.py Co-authored-by: Chandrasekharan M <117059509+chandrasekharan-zipstack@users.noreply.github.com> Signed-off-by: Rahul Johny <116638720+johnyrahul@users.noreply.github.com> * Addressing review comments --------- Signed-off-by: Rahul Johny <116638720+johnyrahul@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Chandrasekharan M <117059509+chandrasekharan-zipstack@users.noreply.github.com>
4 participants
Summary
Changes
Backend
shared_usersandshared_to_orgfields to APIDeployment modelIsOwnerOrSharedUserpermissionlist_of_shared_usersendpoint for retrieving sharing detailsFrontend
Test Plan
Notes
shared_to_org) field is included but not actively used (similar to workflow sharing)Screenshots
🤖 Generated with Claude Code