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
6 changes: 5 additions & 1 deletion ui/src/api/types/chat-auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,15 @@ export interface ApplicationAuthProfileMeta {
application_name: string
}

export interface PortalAuthProfileMeta {
portal_name: string
}

/** 单应用对话的认证前置配置。 */
export type ApplicationAuthProfile = AuthProfile<ApplicationAuthProfileMeta>

/** 门户的认证前置配置。 */
export type PortalAuthProfile = AuthProfile
export type PortalAuthProfile = AuthProfile<PortalAuthProfileMeta>

export interface ChatLoginRequest {
username: string

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

  1. Type-safety break introduced (potential bug in callers).
    Changing PortalAuthProfile from AuthProfile (default generic) to AuthProfile<PortalAuthProfileMeta> narrows the type. Any code that previously constructed a PortalAuthProfile with the old meta shape (or relied on the default generic) will now fail to compile or pass an incompatible object. Verify all usages:

    • Usages of AuthProfile without generics — what is the default? If it's AuthProfile<unknown> or AuthProfile<any>, existing assignments may silently continue working but lose safety.
    • Any serialized/deserialized portal configs in tests, fixtures, or persisted data that lack portal_name.
  2. Consistency with ApplicationAuthProfile. The pattern matches — good. But confirm:

    • Is there an analogous runtime validator / schema (e.g., zod, JSON schema) that must also gain portal_name? Type-only changes won't enforce anything at runtime.
    • Backend API contract: does the portal auth profile endpoint actually return portal_name? If not, the declared type lies.
  3. Required vs optional field. portal_name: string is required. If older putaways/records may omit it (backwards compatibility), consider portal_name?: string or a migration path. Same question for application_name — if it's already required, fine, but be explicit about why.

  4. Naming/generic default. Check AuthProfile's definition: if its generic has a default like = unknown, then AuthProfile writes remain valid and the change is safe-ish; if the default is something else (e.g., Record<string, any>), document the intent. Consider making the generic constraint explicit (AuthProfile<T extends ...>).

  5. Optimization (minor).

    • Consider defining a shared base for name-bearing metas:
      interface NamedPortalMeta { portal_name: string }
      Not necessary, but avoids drift if more fields converge later.
    • No performance concerns — compile-time only.
  6. Diff hygiene. The hunk header says @@ -20,11 +20,15 @@ but the visible context is small; ensure surrounding declarations (e.g., other *AuthProfileMeta interfaces) are consistent in JSDoc style (English comment added /** 单应用… */ exists; the new PortalAuthProfileMeta lacks a doc comment). Add one for consistency.

  7. No tests visible. Add/adjust a type/unit test asserting a valid PortalAuthProfile requires portal_name and that downstream consumers accept it.

Verdict: Structurally correct and consistent with the application counterpart, but semantically breaking — grep all PortalAuthProfile usages, check runtime schemas and backend contracts, and decide whether portal_name should be optional for backward compatibility before merging.

Expand Down
6 changes: 4 additions & 2 deletions ui/src/views/chat/login/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { v7 as uuidv7 } from 'uuid'
import ChatAuthApi from '@/api/chat/auth'
import { CHAT_API_BASE_PATH } from '@/api/constants'
import { LOGIN_METHOD } from '@/api/enums'
import type { ApplicationAuthProfileMeta, LoginConfig, LoginMethod } from '@/api/types'
import type { ApplicationAuthProfileMeta, LoginConfig, LoginMethod, PortalAuthProfileMeta } from '@/api/types'
import { useStore } from '@/stores/chat'
import LoginModePanel from '@/views/login/components/LoginModePanel.vue'
import type { LoginService } from '@/views/login/types'
Expand All @@ -23,7 +23,9 @@ const { auth } = useStore()
const isClientLoggingIn = ref(false)
console.log(auth)
const loginTitle = computed(() => {
if (auth.isPortal) return '门户登录'
if (auth.isPortal) {
return (auth.authProfile?.meta as PortalAuthProfileMeta | undefined)?.portal_name || '门户登录'
}
return (auth.authProfile?.meta as ApplicationAuthProfileMeta | undefined)?.application_name || '登录'
})

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.

Here's the review of the provided diff:

Issues / Potential Problems

  1. Unsafe type cast (as): The code casts auth.authProfile?.meta to PortalAuthProfileMeta | undefined and ApplicationAuthProfileMeta | undefined. Since meta is likely typed as a union or generic object, this cast can hide real type errors. Prefer a type guard or discriminated union narrowing (e.g., based on a type field) instead of as.

  2. Leftover debug statement: console.log(auth) is still present. This should be removed before merging (or gated behind a dev flag).

  3. || vs ?? for fallback: Using || '门户登录' will also fall back when portal_name is an empty string ''. If an empty portal_name is invalid data anyway, this is fine; otherwise use ?? to only fall back on null/undefined. Decide intentionally which behavior you want.

  4. Reduced testability/typing: The inline cast inside computed makes the computed less self-documenting. Consider extracting a helper like getPortalName(meta) with proper typing and unit tests.

Optimization Suggestions

  • Cache meta lookup: Minor, but the computed re-runs whenever auth identity changes; since auth.isPortal and auth.authProfile.meta are related, consider using separate computeds (isPortal, meta) for clarity and better reactivity granularity — not required but cleaner.
  • Import ordering / formatting: The import line is long; if ESLint max-len applies, moving PortalAuthProfileMeta to a separate import type block may help readability.
  • Consistency: Ensure Chinese default strings ('门户登录', '登录') align with i18n strategy if the project uses localization — hardcoding titles reduces maintainability.

Minor correctness note
The logic itself is sound: portal → portal_name fallback, otherwise application_name fallback. No functional bug introduced by the change, assuming meta really holds those shapes per mode.

Expand Down
Loading