-
Notifications
You must be signed in to change notification settings - Fork 572
UN-2873 [FEAT] Add organization-level sharing support for adapters #1588
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
UN-2873 [FEAT] Add organization-level sharing support for adapters #1588
Conversation
- Add shared_to_org field to SharedUserListSerializer to expose org-wide sharing status - Implement email notifications when adapters are shared with users - Use type-specific resource types (LLM, EMBEDDING, VECTOR_DB, X2TEXT) instead of generic adapter type - Add frontend support for org-wide sharing checkbox in adapter settings - Fix SharePermission component to disable org-wide checkbox in view-only mode - Add comprehensive documentation in prompting/tasks/ This extends the existing adapter sharing to support organization-wide access, bringing adapters to full parity with workflows and text extractors. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Summary by CodeRabbit
WalkthroughAdds org-level sharing support across backend and frontend. Backend viewsets capture pre-update shared users, perform update, compute newly shared users, and send notifications. Serializer exposes shared_to_org. Frontend passes shared_to_org in PATCH via updated onShare, enables org-share UI with a disabled state tied to permission. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant FE as ToolSettings (FE)
participant API as AdapterViewSet/AdapterInstanceViewSet (API)
participant DB as DB
participant Notif as SharingNotificationService
User->>FE: Initiate share (userIds, shareWithEveryone)
FE->>API: PATCH adapter {shared_users, shared_to_org}
API->>DB: Read current_shared_users (pre-snapshot)
API->>DB: Apply partial_update
DB-->>API: Updated adapter
API->>DB: Refresh adapter
API->>API: Compute newly_shared = after - before
alt newly_shared non-empty
API->>Notif: send(resource_type, name, id, shared_by, shared_to, resource_instance)
Note right of Notif: Exceptions are caught and logged by API
end
API-->>FE: 200 OK / error
FE-->>User: Success alert or error message
sequenceDiagram
actor User
participant SP as SharePermission (FE)
participant TS as ToolSettings.onShare (FE)
User->>SP: Open share dialog
SP->>User: Show "Share with everyone in current org" [disabled if !permissionEdit]
User->>SP: Select users, toggle org-share (if enabled)
SP->>TS: onShare(userIds, adapter, shareWithEveryone)
TS->>TS: PATCH payload includes shared_to_org
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Cache: Disabled due to Reviews > Disable Cache setting Knowledge base: Disabled due to 📒 Files selected for processing (1)
🧰 Additional context used🪛 Ruff (0.14.0)backend/adapter_processor_v2/views.py377-377: Redundant exception object included in (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)
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.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/src/components/widgets/share-permission/SharePermission.jsx (2)
35-55: Bug: shareWithEveryone resets unexpectedly; view-only state not reflected.shareWithEveryone is set inside an effect that depends on selectedUsers and permissionEdit, so selecting users can reset the checkbox to adapter.shared_to_org, and in view-only the state may never reflect adapter.shared_to_org. Move the state sync to a separate effect keyed only on adapter.
Apply:
useEffect(() => { - if (permissionEdit && adapter && adapter?.shared_users) { + if (permissionEdit && adapter && adapter?.shared_users) { // If permissionEdit is true, and adapter is available, // set the selectedUsers to the IDs of shared users const users = allUsers.filter((user) => { if (adapter?.created_by?.id !== undefined) { return isSharableToOrg ? !selectedUsers.includes(user?.id?.toString()) : user?.id !== adapter?.created_by?.id?.toString() && !selectedUsers.includes(user?.id?.toString()); } else { return isSharableToOrg ? !selectedUsers.includes(user?.id?.toString()) : user?.id !== adapter?.created_by?.toString() && !selectedUsers.includes(user?.id?.toString()); } }); setFilteredUsers(users); - setShareWithEveryone(adapter?.shared_to_org || false); } }, [permissionEdit, adapter, allUsers, selectedUsers]); + +// Keep org-share state in sync with adapter, even in view-only mode +useEffect(() => { + setShareWithEveryone(Boolean(adapter?.shared_to_org)); +}, [adapter]);
57-69: selectedUsers type mismatch can cause duplicates and failed deletions.selectedUsers contains strings while Select adds numbers; includes checks and deletions may fail. Normalize IDs to strings throughout.
Apply:
useEffect(() => { if (adapter?.shared_users) { setSelectedUsers( adapter.shared_users.map((user) => { if (user?.id !== undefined) { - return user.id.toString(); + return String(user.id); } else { - return user?.toString(); + return String(user); } }) ); } }, [adapter, allUsers]); @@ -<Popconfirm +<Popconfirm key={`${item.id}-delete`} @@ - onConfirm={(event) => handleDeleteUser(item?.id)} + onConfirm={() => handleDeleteUser(String(item?.id))} > @@ - <Select + <Select @@ - onChange={(selectedValue) => { - const isValueSelected = selectedUsers.includes(selectedValue); + onChange={(selectedValue) => { + const normalized = String(selectedValue); + const isValueSelected = selectedUsers.includes(normalized); if (!isValueSelected) { - // Update the state only if the selected value is not already present - setSelectedUsers([...selectedUsers, selectedValue]); + setSelectedUsers([...selectedUsers, normalized]); } }} - options={filteredUsers.map((user) => ({ - label: user.email, - value: user.id, - }))} + options={filteredUsers.map((user) => ({ + label: user.email, + value: String(user.id), + }))} > - {filteredUsers.map((user) => { + {filteredUsers.map((user) => { return ( - <Select.Option key={user?.id} value={user?.id}> + <Select.Option key={String(user?.id)} value={String(user?.id)}> {user?.email} </Select.Option> ); })} </Select>Also applies to: 182-193, 189-193, 103-121
🧹 Nitpick comments (2)
frontend/src/components/widgets/share-permission/SharePermission.jsx (1)
39-51: Exclude owner from selectable users even when org-share is enabled.Currently, isSharableToOrg=true allows the creator to be selected. This is confusing and unnecessary. Always exclude the creator from filteredUsers.
Apply:
- return isSharableToOrg - ? !selectedUsers.includes(user?.id?.toString()) - : user?.id !== adapter?.created_by?.id?.toString() && - !selectedUsers.includes(user?.id?.toString()); + return ( + String(user?.id) !== String(adapter?.created_by?.id) && + !selectedUsers.includes(String(user?.id)) + );And similarly for the else-branch using adapter?.created_by.
frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx (1)
180-184: Normalize payload types and boolean.Avoid backend type coercion by sending string IDs consistently or integers. Also coerce boolean explicitly.
Apply:
data: { - shared_users: userIds, - shared_to_org: shareWithEveryone || false, + // Ensure IDs are strings (or map to ints if API expects integers) + shared_users: (userIds || []).map((id) => String(id)), + shared_to_org: Boolean(shareWithEveryone), },If API strictly expects integers, use parseInt with Number.isFinite guard.
📜 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 (4)
backend/adapter_processor_v2/serializers.py(1 hunks)backend/adapter_processor_v2/views.py(2 hunks)frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx(2 hunks)frontend/src/components/widgets/share-permission/SharePermission.jsx(1 hunks)
🧰 Additional context used
🪛 Ruff (0.14.0)
backend/adapter_processor_v2/views.py
376-376: Do not catch blind exception: Exception
(BLE001)
377-377: 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 (3)
frontend/src/components/widgets/share-permission/SharePermission.jsx (1)
169-169: Good: org-share checkbox respects view-only.Disabling the org-level checkbox when permissionEdit is false is correct.
backend/adapter_processor_v2/serializers.py (1)
162-163: LGTM: expose shared_to_org in shared users list.Serializer field addition aligns FE needs.
frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx (1)
265-266: LGTM: enable org-level sharing in UI.Passing isSharableToOrg={true} is consistent with backend support.
…ter sharing - Change logger.error to logger.exception to automatically include traceback - Improves debugging when email notifications fail during adapter sharing 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
|
|
Test ResultsSummary
Runner Tests - Full Report
SDK1 Tests - Full Report
|
|
…1588) * UN-2873 [FEAT] Add organization-level sharing support for adapters - Add shared_to_org field to SharedUserListSerializer to expose org-wide sharing status - Implement email notifications when adapters are shared with users - Use type-specific resource types (LLM, EMBEDDING, VECTOR_DB, X2TEXT) instead of generic adapter type - Add frontend support for org-wide sharing checkbox in adapter settings - Fix SharePermission component to disable org-wide checkbox in view-only mode - Add comprehensive documentation in prompting/tasks/ This extends the existing adapter sharing to support organization-wide access, bringing adapters to full parity with workflows and text extractors. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * UN-2873 [FIX] Use logger.exception for better error traceback in adapter sharing - Change logger.error to logger.exception to automatically include traceback - Improves debugging when email notifications fail during adapter sharing 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> --------- Co-authored-by: Claude <[email protected]> Co-authored-by: Athul <[email protected]>
…1588) * UN-2873 [FEAT] Add organization-level sharing support for adapters - Add shared_to_org field to SharedUserListSerializer to expose org-wide sharing status - Implement email notifications when adapters are shared with users - Use type-specific resource types (LLM, EMBEDDING, VECTOR_DB, X2TEXT) instead of generic adapter type - Add frontend support for org-wide sharing checkbox in adapter settings - Fix SharePermission component to disable org-wide checkbox in view-only mode - Add comprehensive documentation in prompting/tasks/ This extends the existing adapter sharing to support organization-wide access, bringing adapters to full parity with workflows and text extractors. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * UN-2873 [FIX] Use logger.exception for better error traceback in adapter sharing - Change logger.error to logger.exception to automatically include traceback - Improves debugging when email notifications fail during adapter sharing 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> --------- Co-authored-by: Claude <[email protected]> Co-authored-by: Athul <[email protected]>



Summary
Extends the existing adapter sharing functionality to support organization-level sharing, allowing adapters to be shared with all users in an organization. This brings adapters to full parity with workflows and text extractors.
Changes
Backend
Serializer Enhancement (
adapter_processor_v2/serializers.py)shared_to_orgfield toSharedUserListSerializerto expose organization-wide sharing statusEmail Notifications (
adapter_processor_v2/views.py)partial_update()to send email notifications when adapters are sharedFrontend
ToolSettings Component (
tool-settings/ToolSettings.jsx)onShare()function to accept and sendshared_to_orgflagisSharableToOrg={true}on SharePermission componentSharePermission Component (
share-permission/SharePermission.jsx)permissionEditpropScreenshots
Key Features
Technical Details
Database Model (Already Existed)
shared_to_orgBoolean fieldshared_usersManyToMany fieldfor_user()method includesQ(shared_to_org=True)filterAccess Control
IsOwnerOrSharedUserOrSharedToOrgpermission classEmail Notification Flow
Testing
✅ Django system checks pass
✅ Backend imports verified
✅ Organization-level sharing confirmed in model
✅ Permission classes validated
✅ Frontend linter passes
✅ Adapter type to resource type mapping tested
URL Patterns
Different adapter types link to specific settings pages:
/<org_id>/settings/llms/<org_id>/settings/embedding/<org_id>/settings/vectorDbs/<org_id>/settings/textExtractorNotes
🤖 Generated with Claude Code