Skip to content

🐛(backend) use computed_link_reach in handle_onboarding_document#2305

Open
lunika wants to merge 1 commit into
mainfrom
fix/onboarding-link-reach
Open

🐛(backend) use computed_link_reach in handle_onboarding_document#2305
lunika wants to merge 1 commit into
mainfrom
fix/onboarding-link-reach

Conversation

@lunika
Copy link
Copy Markdown
Member

@lunika lunika commented May 20, 2026

Purpose

In the model method User::_handle_onboarding_documents_access we do not allow using documents with restricted link_reach to be added as onboarding documents. To check the real link_reach of the document, we must use instead the computed_link_reach to be sure that a sub document can also be used and compute its correct link_reach.

Proposal

  • 🐛(backend) use computed_link_reach in handle_onboarding_document

Fix #2015

@lunika lunika self-assigned this May 20, 2026
@lunika lunika added the bug Something isn't working label May 20, 2026
@lunika
Copy link
Copy Markdown
Member Author

lunika commented May 20, 2026

@CodeRabbit review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 20, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 20, 2026

Review Change Stack

Walkthrough

This PR fixes a bug in onboarding document handling where sub-documents with inherited public access were not being added as favorites. The fix changes the restriction check in handle_onboarding_document from using raw link_reach to computed_link_reach, allowing documents that inherit public access from their parent to bypass the restriction exception. A new test verifies this behavior, and a changelog entry documents the fix.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: using computed_link_reach instead of link_reach in the handle_onboarding_document method, which aligns with the actual code changes.
Description check ✅ Passed The description accurately explains the purpose of the changes, references the correct issue (#2015), and clearly states what was changed and why.
Linked Issues check ✅ Passed The PR fully addresses issue #2015 by replacing link_reach checks with computed_link_reach to allow sub-documents with effective public reach to be added as onboarding documents.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the onboarding document access logic using computed_link_reach; no unrelated modifications are present.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/onboarding-link-reach

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a 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

🤖 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.

Inline comments:
In `@src/backend/core/tests/test_models_users.py`:
- Around line 208-212: The test currently checks that a LinkTrace is created
after creating a user with
override_settings(USER_ONBOARDING_DOCUMENTS=[str(document.id)]) and
factories.UserFactory(); add an assertion that a DocumentFavorite was also
created for that user-document pair. Locate the test block around
override_settings and factories.UserFactory(), then assert using the
DocumentFavorite model (e.g., models.DocumentFavorite.objects.filter(user=user,
document=document).exists()) to ensure the favorite creation is verified
alongside models.LinkTrace.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: bab05a46-5088-4786-898a-58c72b884240

📥 Commits

Reviewing files that changed from the base of the PR and between 0bfc697 and 6b1c746.

📒 Files selected for processing (3)
  • CHANGELOG.md
  • src/backend/core/models.py
  • src/backend/core/tests/test_models_users.py

Comment thread src/backend/core/tests/test_models_users.py
In the model method User::_handle_onboarding_documents_access we do not
allow using documents with restricted link_reach to be added as
onboarding documents. To check the real link_reach of the document, we
must use instead the computed_link_reach to be sure that a sub document
can also be used and compute its correct link_reach.
@lunika lunika force-pushed the fix/onboarding-link-reach branch from 6b1c746 to ea53b21 Compare May 20, 2026 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sub-doc are not added as favourite at onboarding

1 participant