Repository navigation
fix: chat bug #7841
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
fix: chat bug #7841
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,27 +15,11 @@ interface HomeResourceCard { | |
| details: [string, number | undefined][] | ||
| } | ||
|
|
||
| const ZERO_APPLICATION: HomeApplicationAggregation = { total: 0, publish_count: 0, un_publish_count: 0 } | ||
| const ZERO_KNOWLEDGE: HomeKnowledgeAggregation = { total: 0, document_count: 0, failure_count: 0 } | ||
| const ZERO_TOOL: HomeToolAggregation = { | ||
| total: 0, | ||
| custom_count: 0, | ||
| workflow_count: 0, | ||
| skill_count: 0, | ||
| mcp_count: 0, | ||
| data_source_count: 0, | ||
| } | ||
| const ZERO_MODEL: HomeModelAggregation = { total: 0, llm_count: 0, embedding_count: 0 } | ||
|
|
||
| /* 资源概况与列表导航 */ | ||
| const props = defineProps<{ workspaceId: string; api: typeof HomepageApi }>() | ||
| const router = useRouter() | ||
| const route = useRoute() | ||
| const isSystemScope = computed(() => route.meta.scope === 'system') | ||
| const canReadApplication = computed(() => !isSystemScope.value || perm.application.system.read()) | ||
| const canReadKnowledge = computed(() => !isSystemScope.value || perm.knowledge.system.read()) | ||
| const canReadTool = computed(() => !isSystemScope.value || perm.tool.system.read()) | ||
| const canReadModel = computed(() => !isSystemScope.value || perm.model.system.read()) | ||
| const loading = ref(false) | ||
| const application = ref<HomeApplicationAggregation>() | ||
| const knowledge = ref<HomeKnowledgeAggregation>() | ||
|
|
@@ -97,54 +81,38 @@ function loadResources() { | |
| if (loading.value) return | ||
| loading.value = true | ||
| return Promise.allSettled([ | ||
| canReadApplication.value | ||
| ? props.api | ||
| .getApplicationAggregation(props.workspaceId) | ||
| .then((value) => { | ||
| application.value = value | ||
| }) | ||
| .catch(() => { | ||
| application.value = undefined | ||
| }) | ||
| : Promise.resolve().then(() => { | ||
| application.value = { ...ZERO_APPLICATION } | ||
| }), | ||
| canReadKnowledge.value | ||
| ? props.api | ||
| .getKnowledgeAggregation(props.workspaceId) | ||
| .then((value) => { | ||
| knowledge.value = value | ||
| }) | ||
| .catch(() => { | ||
| knowledge.value = undefined | ||
| }) | ||
| : Promise.resolve().then(() => { | ||
| knowledge.value = { ...ZERO_KNOWLEDGE } | ||
| }), | ||
| canReadTool.value | ||
| ? props.api | ||
| .getToolAggregation(props.workspaceId) | ||
| .then((value) => { | ||
| tool.value = value | ||
| }) | ||
| .catch(() => { | ||
| tool.value = undefined | ||
| }) | ||
| : Promise.resolve().then(() => { | ||
| tool.value = { ...ZERO_TOOL } | ||
| }), | ||
| canReadModel.value | ||
| ? props.api | ||
| .getModelAggregation(props.workspaceId) | ||
| .then((value) => { | ||
| model.value = value | ||
| }) | ||
| .catch(() => { | ||
| model.value = undefined | ||
| }) | ||
| : Promise.resolve().then(() => { | ||
| model.value = { ...ZERO_MODEL } | ||
| }), | ||
| props.api | ||
| .getApplicationAggregation(props.workspaceId) | ||
| .then((value) => { | ||
| application.value = value | ||
| }) | ||
| .catch(() => { | ||
| application.value = undefined | ||
| }), | ||
| props.api | ||
| .getKnowledgeAggregation(props.workspaceId) | ||
| .then((value) => { | ||
| knowledge.value = value | ||
| }) | ||
| .catch(() => { | ||
| knowledge.value = undefined | ||
| }), | ||
| props.api | ||
| .getToolAggregation(props.workspaceId) | ||
| .then((value) => { | ||
| tool.value = value | ||
| }) | ||
| .catch(() => { | ||
| tool.value = undefined | ||
| }), | ||
| props.api | ||
| .getModelAggregation(props.workspaceId) | ||
| .then((value) => { | ||
| model.value = value | ||
| }) | ||
| .catch(() => { | ||
| model.value = undefined | ||
| }), | ||
| ]).finally(() => { | ||
| loading.value = false | ||
| }) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Review of the diffMain issue: removed permission gating (behavior regression)The diff removes the four Consequences to consider:
Other observations
RecommendationUnless the backend reliably returns zeroed aggregations for unauthorized resources, restore the |
||
|
|
||
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.
Code Review
Did the diff apply cleanly?
The provided diff is a delta, not standalone code, so I reviewed the changes in context. The changes are logically consistent, but there are a few issues worth flagging.
Issues & Observations
1.
portalNamemeta handling is over-defensive but inconsistentmeta.portal_nameon possiblyundefined— that was a latent runtime crash). Good fix.authProfile.metaproperly (e.g.{ portal_name?: string; [k: string]: unknown }) so you can drop the runtime guards. Thein+typeofchain is verbose here.(meta as Record<string, unknown> | undefined)?.portal_name.2.
groupConversationschanged semantics — verify with consumersPreviously it always sliced to 5. Now it returns all conversations when total ≤ 20. It's used in:
historyApplicationGroupsfilter (.length > 0) — fine.3.
totalConversationCount > 20threshold is a magic numberExtract constants for clarity:
And use in both the computed and template (
v-if). Right now20is duplicated in script and template.4.
groupConversationCountmay double-count the current applicationIf
applicationGroupsincludes a group whosecountalready reflects server-side totals, the sum intotalConversationCountcould mismatch actual rendered counts. Confirmgroup.countsemantics (server total vs. loaded). If the server count is larger than loaded, users may see "查看更多" even though only some are loaded — acceptable, but document intent.5. Redundant import path change
@/stores/chat/index.ts→@/stores/chatis cleaner. ✅ But ensure no other file relies on the explicit.tspath consistency style; align project-wide.6.
RECENT_CONVERSATION_LIMITcomment now stale elsewhere?The inline comment near line ~230 was updated ("总数超过 20 条…"). Good. But check that any docs/tests referencing "每个智能体最多展示最近 5 条会话" are updated to reflect the new conditional behavior.
7. Minor: template binding order
class="pl-8!"beforev-if— putv-iffirst for readability (convention).8.
portalLogotypeauthStore.authProfile?.logo— iflogocan beundefined,<PortalIcon :icon="portalLogo" />must tolerateundefined. VerifyPortalIconprop allows optional icon (else add default).Suggested refined snippet
Template:
Summary
The refactor is an improvement — it fixes a real null-access bug (
meta.portal_name) and moves the name/logo logic into computed properties. Main follow-ups: define named constants, typemeta, confirm rendering of >5 items per group, and verifygroup.countsemantics to avoid incorrect "查看更多" visibility.