Refact: Added a private helper _visibility_and_status_filter#13627
Refact: Added a private helper _visibility_and_status_filter#13627Sank-WoT wants to merge 19 commits intoinfiniflow:mainfrom
Conversation
dosubot
bot
added
the
size:M
Sank-WoT
changed the title
|
I added refactoring changes 1db5409 |
Sank-WoT
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #13627 +/- ##
==========================================
- Coverage 98.14% 96.72% -1.43%
==========================================
Files 10 10
Lines 702 702
Branches 112 112
==========================================
- Hits 689 679 -10
- Misses 3 5 +2
- Partials 10 18 +8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR introduces a private query helper on KnowledgebaseService to centralize the Peewee WHERE condition for “KB visible to user (team or own) AND status is VALID”, and refactors several query methods to reuse it.
Changes:
- Added
KnowledgebaseService._visibility_and_status_filter(joined_tenant_ids, user_id)to encapsulate visibility +StatusEnum.VALIDfiltering. - Refactored
get_by_tenant_idsandget_listto reuse the helper instead of repeating the same condition. - Updated
get_all_kb_by_tenant_idsto use the helper as well.
| cls.model.tenant_id == user_id | ||
| ) | ||
| ) | ||
| kbs = cls.model.select(*fields).where(cls._visibility_and_status_filter(tenant_ids, user_id)) |
There was a problem hiding this comment.
get_all_kb_by_tenant_ids previously returned all visible KBs regardless of status, but switching to _visibility_and_status_filter(...) now implicitly filters to StatusEnum.VALID. If this method is used for admin/debug flows (e.g., showing soft-deleted KBs), this changes behavior beyond a refactor—either keep the old semantics (separate visibility vs status helpers) or update the PR description/callers to reflect the intentional filtering.
| - Team KBs (`permission == TenantPermission.TEAM`) owned by any tenant in `joined_tenant_ids` | ||
| - KBs owned by the current user (`tenant_id == user_id`) | ||
| Always constrained to `StatusEnum.VALID`. |
There was a problem hiding this comment.
The helper docstring says permission == TenantPermission.TEAM, but the actual predicate compares against TenantPermission.TEAM.value. Please align the docstring with the real condition to avoid confusion for future callers.
| - Team KBs (`permission == TenantPermission.TEAM`) owned by any tenant in `joined_tenant_ids` | |
| - KBs owned by the current user (`tenant_id == user_id`) | |
| Always constrained to `StatusEnum.VALID`. | |
| - Team KBs (`permission == TenantPermission.TEAM.value`) owned by any tenant in `joined_tenant_ids` | |
| - KBs owned by the current user (`tenant_id == user_id`) | |
| Always constrained to `StatusEnum.VALID.value`. |
What problem does this PR solve?
Added a private helper _visibility_and_status_filter(joined_tenant_ids, user_id) that returns the Peewee condition: visible to user (team or own) and status is VALID.
Type of change