Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions apps/chat/serializers/chat_authentication.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ def profile():
"login_value": ["LOCAL"],
"default_value": "LOCAL",
"rsa_key": get_key_pair_by_sql().get("key"),
"meta": {},
"meta": {"portal_name": ""},
"logo": "./favicon.ico",
},
**result,
Expand All @@ -132,7 +132,7 @@ def profile():
"login_value": auth_config.get("login_value", []),
"default_value": auth_config.get("type", "LOCAL"),
"rsa_key": get_key_pair_by_sql().get("key"),
"meta": {},
"meta": {"portal_name": portal.name},
"logo": portal.logo or "./favicon.ico",
},
**result,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review of the Diff

Potential Issues

  1. Inconsistent meta semantics between branches

    • Branch 1 returns "meta": {"portal_name": ""}, branch 2 returns "meta": {"portal_name": portal.name}.
    • If both branches serve the same endpoint (profile()), clients must handle portal_name being either empty or populated. This is acceptable only if Branch 1 truly has no portal context (e.g., unauthenticated/default profile). Confirm that assumption.
  2. portal.name may be None or missing

    • If name is nullable, the API may return "portal_name": null (or raise AttributeError if portal is None).
    • Safer: "portal_name": getattr(portal, "name", "") or "".
  3. Silent override by **result

    • "meta" is defined before **result. In Python, later keys win, so if result ever contains a "meta" key it will silently replace the one set here. Verify result never includes meta, or move meta after **result if you intend it to win.
  4. Repeated DB call per request

    • get_key_pair_by_sql().get("key") is invoked on every profile() call (and possibly twice if both branches execute). Consider caching the key pair (e.g., functools.lru_cache or a short-TTL cache) since RSA keys rarely change.
  5. Branch divergence / duplication

    • The two config blocks differ in login_value, default_value, meta, and logo, but share rsa_key. This duplication invites drift (e.g., someone will update one and forget the other). Consider building a base dict and merging branch-specific overrides.
  6. Indentation in the second hunk looks slightly off

    • The second hunk's "login_value" etc. are indented less than the first branch's block. Verify both blocks are aligned within their parent dict; Python will still parse but it suggests the branches may be at different nesting levels — confirm they're both intended as sibling return values.

Optimization Suggestions

  • Extract a helper/constant: DEFAULT_META = {"portal_name": ""} and DEFAULT_LOGO = "./favicon.ico" to keep branches consistent.
  • Use portal.name or "" (or getattr) to guarantee a string type in the response schema.
  • Cache or memoize get_key_pair_by_sql() result.
  • If auth_config is fetched per-request, consider whether login_value and default_value need consistency validation (e.g., type should be present in login_value), since an unexpected type could produce an invalid default.

Minor

  • "meta": {"portal_name": ""} vs. None: empty string is fine, but document the contract so the frontend doesn't render a blank label unintentionally.

Overall the change is small and likely fine; the main risks are (a) inconsistent/absent portal_name, (b) portal.name nullability, and (c) the **result override order.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ import AccountMenu from '../account-menu/index.vue'
import { usePortalConversationListStore, type PortalApplicationGroup } from './index'
import RenameConversation from '../../components/rename/index.vue'
import { useMessageListStore } from '../../components/message-list/index'

import auth from '@/api/chat/auth.ts'
import { useStore } from '@/stores/chat/index.ts'
const { auth: authStore }=useStore()
defineOptions({ name: 'PortalConversationList' })

const msgs = useMessageListStore()
Expand Down Expand Up @@ -78,15 +80,22 @@ const handleClearConversations = () => {
clearing.value = false
})
}
const name=computed(()=>{
const meta:any=authStore.authProfile?.meta
return meta.portal_name ||'智能体门户'
})
const logo=computed(()=>{
return authStore.authProfile?.logo
})
</script>

<template>
<div class="flex-column h-full overflow-hidden p-4">
<!-- 门户名称与收起侧栏 -->
<div class="mb-4 flex-between shrink-0 gap-2">
<div class="min-w-0 flex-align-center gap-2">
<PortalIcon />
<h4 class="truncate">智能体门户</h4>
<PortalIcon :icon="logo"/>
<h4 class="truncate">{{name}}</h4>
</div>
<!-- 收起左侧 -->
<MkTooltip content="收起侧边栏">

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

1. 🐞 Potential runtime error (bug)

const meta: any = authStore.authProfile?.meta
return meta.portal_name || '智能体门户'

authStore.authProfile?.meta only guards authProfile. If authProfile is undefined/null → meta is undefined → meta.portal_name throws TypeError: Cannot read properties of undefined.

Fix:

const portalName = computed(() => authStore.authProfile?.meta?.portal_name || '智能体门户')

Also drop : any — use a proper type, e.g. { portal_name?: string }.


2. 🐞 Pinia destructuring breaks reactivity

const { auth: authStore } = useStore()

Destructuring a store loses reactivity for reassigned references. If store.auth is ever replaced with a new object, authStore keeps pointing at the stale one, and both computeds silently stop updating.

Preferred:

const store = useStore()
// then use store.auth.authProfile

or const { auth } = storeToRefs(store) if auth is state.


3. ⚠️ Unused import + wrong module path

import auth from '@/api/chat/auth.ts'
  • auth is never used → ESLint no-unused-vars failure.
  • Importing with an explicit .ts extension is non-standard in Vite/TS source (fine for allowImportingTsExtensions, but inconsistent here). Remove the import entirely if unused.

4. ⚠️ Naming

  • name and logo are very generic and risk collisions in <script setup> scope (and name is a common conflict with window.name / component name). Prefer portalName / portalLogo.

5. 💅 Formatting / lint

const logo=computed(()=>{
    return authStore.authProfile?.logo
})
  • Missing spaces around = and => (Prettier).
  • Inconsistent indentation (4 spaces vs 2-space file style).
  • ||'智能体门户' missing a space.
  • Prefer the expression body for logo: computed(() => authStore.authProfile?.logo).

6. 🌐 i18n

'智能体门户' is hardcoded. If this project is localized, use the i18n t() key (check how the previous hardcoded string was handled — if it was also hardcoded, at least extract a constant).


7. 🔍 PortalIcon :icon="logo"

  • logo may be undefined; confirm the prop is optional and PortalIcon has a sensible default/fallback (empty src can render a broken-image).
  • Consider a fallback: computed(() => authStore.authProfile?.logo ?? defaultLogo).
  • Also confirm the prop name is really icon (not src/url) — passing an unknown prop silently produces an attribute on the root element.

8. 🧹 Minor

  • computed isn't imported in the shown diff — verify unplugin-auto-import covers it, otherwise add import { computed } from 'vue'.
  • The two computeds can be merged, but separate is fine and arguably clearer.
  • Consider moving the authStore/portal metadata read into a dedicated composable (e.g. usePortalMeta()) — it's likely reused by the header/favicon/title logic elsewhere.

Suggested final version

import { storeToRefs } from 'pinia'
import { useStore } from '@/stores/chat/index'

const store = useStore()

const portalName = computed(
  () => store.auth.authProfile?.meta?.portal_name || '智能体门户'
)
const portalLogo = computed(() => store.auth.authProfile?.logo)
<PortalIcon :icon="portalLogo" />
<h4 class="truncate">{{ portalName }}</h4>

Expand Down
Loading