fix: Fix bug in which emails would be fetched during initial indexing #4959
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
Fixed asymmetric permission handling in Teams connector where email addresses were inconsistently populated between regular indexing and slim-document retrieval paths.
- Consolidated permission logic into new
fetch_external_accessutility inbackend/onyx/connectors/teams/utils.pyto ensure consistent behavior across retrieval paths - Added
public()classmethod toExternalAccessinbackend/onyx/access/models.pyfor standardized creation of public document instances - Removed redundant
_is_channel_publichelper and moved_PUBLIC_MEMBERSHIP_TYPEconstant to module level for better organization - Ensured
ExternalAccessinstances maintain identical states between slim and full document retrieval
3 files reviewed, no comments
Edit PR Review Bot Settings | Greptile
raunakab
changed the title
Orbital-Web
reviewed
Jun 27, 2025
Orbital-Web
approved these changes
Jun 27, 2025
AnkitTukatek
pushed a commit
to TukaTek/onyx
that referenced
this pull request
Sep 23, 2025
…onyx-dot-app#4959) * Add new convenience method * Fix bug in which emails would be fetched for initial indexing * Improve tests for MS Teams connector * Fix test_gdrive_perm_sync_with_real_data patching * Protect against incorrect truthiness --------- Co-authored-by: Weves <chrisweaver101@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
During initial sync, the
ExternalAccess.external_user_emailsfield of aDocumentwill always be populated with a set of actual user-emails, regardless of whether that channel is public or not.During permission-sync, the
ExternalAccess.external_user_emailswill only be populated with the actual emails if that channel is private. Otherwise (if it's public), it'll set that field to the empty set.This is asymmetric behaviour; we want both initial-indexing + permission-sync to produce identical
ExternalAccessinstances (given that the underlying document's permissions have not changed).This PR fixes that issue.
Addresses: https://linear.app/danswer/issue/DAN-2150/fix-asymmetric-bug-in-externalaccess-generation-for-perm-sync-vs.
How Has This Been Tested?
Existing tests are sufficient.