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 ui/src/views/system/identity/groups/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ import { useStore } from '@/stores'
import perm from '@/permission/index.ts'
import { LOGIN_METHOD } from '@/api/enums'
import { LOGIN_METHOD_LABELS } from '@/constants'
const { auth } = useStore()
const { auth, user } = useStore()

/* 选择工作空间列表 */
const selectedWorkspaceId = ref('default')
const selectedWorkspaceId = ref(user.userInfo?.workspace_list?.[0]?.id ?? 'default')
const workspaceOptions = ref<WorkspaceItem[]>([])
const loadingView = ref(false)

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.

Assuming this is Vue 3 + Pinia, the change is reasonable but has a few potential issues.

Potential issues

  1. Pinia destructuring may break reactivity

    const { auth, user } = useStore()

    Destructuring state/getters from a Pinia store can lose reactivity if the property is later replaced. If user is a state/getter and may update asynchronously, prefer:

    const store = useStore()
    const { auth } = storeToRefs(store)
    // access user via store.user

    If user is an action or a stable nested object and you only read it once, this is less critical.

  2. Initialization timing / async user data

    const selectedWorkspaceId = ref(
      user.userInfo?.workspace_list?.[0]?.id ?? 'default'
    )

    This reads userInfo only once during setup. If userInfo or workspace_list is loaded asynchronously after component creation, selectedWorkspaceId will stay 'default' and will not update when data arrives.

    Use a watch with immediate: true, and avoid overwriting a valid user selection.

  3. Fallback only handles null / undefined
    ?? 'default' does not handle an empty string or an invalid workspace ID. If empty IDs are possible, validate explicitly:

    const id = user.userInfo?.workspace_list?.[0]?.id
    const selectedWorkspaceId = ref(id || 'default')

    Be careful if 0 is a valid ID — in that case use an explicit check instead of ||.

  4. Type inference may be wrong
    If WorkspaceItem['id'] is number, then:

    ref(user.userInfo?.workspace_list?.[0]?.id ?? 'default')

    may infer number | "default", which can conflict with code expecting a string. Consider:

    const selectedWorkspaceId = ref<string | number>('default')

    or a proper WorkspaceId type.

  5. user may be undefined
    If there is any chance user is not always defined, use:

    user?.userInfo?.workspace_list?.[0]?.id

    Otherwise the current optional chaining only protects userInfo, not user.

  6. Selected workspace may not exist in workspaceOptions
    Defaulting to the first workspace from userInfo can produce an invalid selection if workspaceOptions is loaded later or filtered. Make sure selectedWorkspaceId is always present in workspaceOptions, or initialize it after options are loaded.

  7. Product behavior change
    Previously the default was 'default'; now it is the first workspace. Confirm this is intentional. If 'default' is a special backend value, switching to a real workspace ID may change API behavior.

Suggested safer pattern

const store = useStore()
const { auth } = storeToRefs(store)

const selectedWorkspaceId = ref<string | number>('default')

const initSelectedWorkspace = () => {
  const list = store.user?.userInfo?.workspace_list
  if (!list?.length) return

  const current = selectedWorkspaceId.value
  const isValid = list.some((item) => item.id === current)

  if (!current || current === 'default' || !isValid) {
    selectedWorkspaceId.value = list[0].id
  }
}

watch(
  () => store.user?.userInfo?.workspace_list,
  initSelectedWorkspace,
  { immediate: true }
)

Summary

The main risks are:

  • losing reactivity through Pinia destructuring,
  • initializing selectedWorkspaceId before async user/workspace data is available,
  • weak fallback/type handling,
  • and possibly selecting a workspace that is not yet valid in the loaded options list.

The change is safe only if user.userInfo is guaranteed to be available synchronously and if workspace_list[0] is always a valid selectable workspace.

Expand Down
4 changes: 2 additions & 2 deletions ui/src/views/system/identity/resource-authorization/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import UserAuthorizationList from './components/UserAuthorizationList.vue'
import UserGroupAuthorizationList from './components/user-group/UserGroupAuthorizationList.vue'
import { RESOURCE_AUTHORIZATION_LABELS } from './constants'

const { auth } = useStore()
const { auth, user } = useStore()
const route = useRoute()

const resourceType = ref<ResourceAuthorizationType>((route.meta.resource as ResourceAuthorizationType) ?? RESOURCE_TYPE.APPLICATION)
Expand All @@ -40,7 +40,7 @@ const resourceTypeOptions: OptionItem<ResourceAuthorizationType>[] = [

/* 选择工作空间列表 */
const loadingView = ref(false)
const selectedWorkspaceId = ref('default')
const selectedWorkspaceId = ref(user.userInfo?.workspace_list?.[0]?.id ?? 'default')
const workspaceOptions = ref<WorkspaceItem[]>([])

function handleWorkspaceSelect(workspace: WorkspaceItem) {

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.

The diff is syntactically fine, but there are a few potential issues to review:

  1. Initialization timing / stale value

    const selectedWorkspaceId = ref(user.userInfo?.workspace_list?.[0]?.id ?? 'default')

    This runs only once during component setup. If user.userInfo or workspace_list is loaded asynchronously, selectedWorkspaceId will remain 'default' even after the user data arrives.

    If that is possible, add a watcher:

    watch(
      () => user.userInfo?.workspace_list,
      (list) => {
        if (selectedWorkspaceId.value === 'default') {
          selectedWorkspaceId.value = list?.[0]?.id ?? 'default'
        }
      },
      { immediate: true }
    )

    Be careful not to override a workspace the user has already selected manually.

  2. The first workspace may not be the correct default
    workspace_list?.[0]?.id assumes the first workspace is the desired one. If the API provides a default/current workspace flag, prefer that:

    const defaultWorkspaceId =
      user.userInfo?.workspace_list?.find(w => w.isDefault)?.id ??
      user.userInfo?.workspace_list?.[0]?.id ??
      'default'

    Otherwise, document that the first workspace is intentionally used.

  3. Possible TypeScript type change
    If WorkspaceItem.id is not string, then selectedWorkspaceId may become string | number instead of the previous string. This can cause type errors downstream. Use an explicit type or normalize the value:

    const selectedWorkspaceId = ref<string>(
      String(user.userInfo?.workspace_list?.[0]?.id ?? 'default')
    )

    Only do this if IDs are expected to be strings.

  4. 'default' as a sentinel value
    If 'default' is also a real workspace ID, the fallback is ambiguous. Prefer undefined / null internally, or make sure 'default' is always handled specially.

  5. Reactivity/destructuring concern
    If useStore() is a Pinia store and user is a reactive store/state object, this is likely fine. If user is a ref or computed value, destructuring may lose reactivity. In that case, access it through the store or use storeToRefs.

  6. Validate selected workspace after options load
    After workspaceOptions is fetched, ensure selectedWorkspaceId exists in the options list. Otherwise the select component may appear blank or issue requests with a workspace ID not available to the user.

  7. Optional guard
    If user itself can be undefined, use:

    user?.userInfo?.workspace_list?.[0]?.id

Overall, the main risk is async user/workspace data loading. If user.userInfo.workspace_list is guaranteed to be available synchronously, the change is acceptable; otherwise, add a watcher or initialize after the workspace/user data is fetched.

Expand Down
Loading