fix: missing ingest settings for individual files (backport of #2066)#2095
Conversation
…hare all" toggle (#2025) * feature Flag Share all * style: apply biome auto-fixes [skip ci] --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* fix: COS shared-toggle owner not propagated on bucket sync + stale on dedupe skip Two related bugs kept "Make documents available to all users" from actually sharing documents: 1. connector_sync's bucket_filter branch (the path the COS connector UI's "Ingest N Buckets" button always hits) never forwarded body.shared to either of its two sync_specific_files calls (new files / changed files), so newly-ingested files always ended up privately owned regardless of the toggle. 2. ConnectorFileProcessor.process_item's duplicate-filename and unchanged-hash skip branches returned before resolve_shared_owner_fields() ever ran, so re-syncing a bucket whose files were already indexed left their ownership exactly as it was on the prior sync. Fix (2) via _reconcile_shared_owner(): a metadata-only update_by_query scoped to owner==self OR ownerless (same boundary as delete_document_by_filename), called from both skip paths instead of silently leaving stale ownership in place. Also fixes two pre-existing test_shared_flag.py tests that had drifted from connector_sync's current signature (missing request/session/rbac args). * test: guard is_dev_ibm_cos_enabled's default against re-regressing to enabled Found during review: is_dev_ibm_cos_enabled() previously defaulted OPENRAG_DEV_IBM_COS to "true" when unset (now fixed to "false", matching is_dev_azure_blob_enabled's pattern), which made the Enterprise/SaaS-only IBM COS connector available in every deployment by default. The existing tests only monkeypatched the function away, so they wouldn't have caught this. Add a test asserting the unset-env-var default. * style: ruff autofix (auto) --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Root cause: FileBrowserDialog (the "Browse Files" individual-file picker) never read the "Make documents available to all users" toggle or ingest settings — it built its sync request with only selected_files, so shared silently defaulted to false on the backend. The whole-bucket ("folder") ingest path in SharedBucketView already did this correctly.
Changes (frontend-only, no backend changes needed — it already threads shared uniformly):
- frontend/components/file-browser-dialog.tsx: added showShared/ingestSettings props, and now includes settings and shared in the sync request body, mirroring the bucket-ingest path.
- frontend/components/connectors/shared-bucket-view.tsx: passes its existing showSharedToggle/ingestSettings state down into FileBrowserDialog.
WalkthroughAdds IBM COS development gating, exposes a shared-upload visibility setting, propagates shared ingest configuration through frontend and backend flows, and reconciles ownership metadata for existing indexed documents. ChangesIBM COS shared upload
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant SharedBucketView
participant FileBrowserDialog
participant connector_sync
participant ConnectorFileProcessor
participant OpenSearch
SharedBucketView->>FileBrowserDialog: pass ingestSettings and shared
FileBrowserDialog->>connector_sync: submit ingestion request
connector_sync->>ConnectorFileProcessor: sync new and changed files with shared
ConnectorFileProcessor->>OpenSearch: reconcile owner fields for duplicate or unchanged files
Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
|
React Doctor found no new issues. 🎉 Reviewed by React Doctor for commit |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/components/cloud-picker/ingest-settings.tsx (1)
232-246: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove commented-out code.
This
Table Structuresettings block is commented out. If this feature is no longer needed or is tracked elsewhere, consider removing the dead code to keep the file clean.♻️ Proposed refactor
- {/* <div className="flex gap-2 items-center justify-between"> - <div> - <div className="text-sm font-semibold pb-2">Table Structure</div> - <div className="text-sm text-muted-foreground"> - Capture table structure during ingest. - </div> - </div> - <Switch - id="table-structure" - checked={currentSettings.tableStructure} - onCheckedChange={(checked) => - handleSettingsChange({ tableStructure: checked }) - } - /> - </div> */}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/components/cloud-picker/ingest-settings.tsx` around lines 232 - 246, Remove the commented-out Table Structure settings block from the component, including its unused Switch and handler code, while leaving the surrounding ingest settings UI unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@frontend/components/cloud-picker/ingest-settings.tsx`:
- Around line 232-246: Remove the commented-out Table Structure settings block
from the component, including its unused Switch and handler code, while leaving
the surrounding ingest settings UI unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d715287a-9278-4519-a6f6-3dff5d4c5aac
📒 Files selected for processing (17)
.env.exampleenhancements/connectors/ibm_cos/connector.pyfrontend/app/api/queries/useGetSettingsQuery.tsfrontend/components/cloud-picker/ingest-settings.tsxfrontend/components/connectors/shared-bucket-view.tsxfrontend/components/file-browser-dialog.tsxfrontend/lib/brand.tssrc/api/connectors.pysrc/api/settings/endpoints.pysrc/api/settings/models.pysrc/config/settings.pysrc/models/processors.pytests/unit/config/test_show_shared_upload_toggle.pytests/unit/connectors/test_ibm_cos_connector.pytests/unit/test_connector_sync_bucket_filter_shared.pytests/unit/test_reconcile_shared_owner.pytests/unit/test_shared_flag.py
mpawlow
left a comment
There was a problem hiding this comment.
Code Review 1
- ✅ Approved / LGTM 🚀
- ℹ️ Verified: Cherry-pick conflict resolution: Confirmed
onIngestSuccessis fully intact and coexists correctly alongside the newshowShared/ingestSettingsprops in both the prop interface and theSharedBucketViewcall site (shared-bucket-view.tsx:346-364)
Summary
Backports #2066 from
release-saas-ga-0.6.2tomain.Depends on #2094 (backport of #2050), #2093 (#2035), #2092 (#2025) — stacked because it touches
shared-bucket-view.tsx/file-browser-dialog.tsxin the same regions. Merge order: #2092 → #2093 → #2094 → this PR.Note: cherry-pick had conflicts in
frontend/components/file-browser-dialog.tsxandfrontend/components/connectors/shared-bucket-view.tsxagainst an unrelatedonIngestSuccessfeature already onmain; resolved by keeping both additive changes (verified withtsc --noEmit, no errors in either file).Test plan
Summary by CodeRabbit
New Features
Bug Fixes
Tests