Skip to content

Improve Account admin page titles and list URLs#3315

Draft
nighca wants to merge 1 commit into
goplus:devfrom
nighca:issue-3313
Draft

Improve Account admin page titles and list URLs#3315
nighca wants to merge 1 commit into
goplus:devfrom
nighca:issue-3313

Conversation

@nighca

@nighca nighca commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator

Closes #3313

Changes

  • Add Account admin page titles for the admin shell, list pages, and detail pages.
  • Sync Account admin list pagination, sorting, and filters to route query params.
  • Reset list pagination when sorting or filters change while keeping default URLs clean.

Checks

  • npm run type-check
  • npm run lint
  • git diff --check

Notes

  • Attempted browser verification in the in-app browser, but browser automation timed out while navigating the local admin page. Static checks passed.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request integrates page titles and synchronizes pagination, sorting, and filter states with route query parameters across various admin pages. The review feedback highlights a critical race condition in audit-logs.vue where resetting multiple route query parameters sequentially in clearFilters() causes conflicting asynchronous router updates. It is recommended to import useRouter and batch-update the query parameters in a single router push to resolve this issue.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +2 to +6
import { computed } from 'vue'

import { useQuery } from '@/utils/query'
import { useRouteQueryParamInt, useRouteQueryParamStr, useRouteQueryParamStrEnum } from '@/utils/route'
import { usePageTitle } from '@/utils/utils'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

Import useRouter from vue-router to allow batch updating the route query parameters in clearFilters and avoid race conditions.

import { computed } from 'vue'
import { useRouter } from 'vue-router'

import { useQuery } from '@/utils/query'
import { useRouteQueryParamInt, useRouteQueryParamStr, useRouteQueryParamStrEnum } from '@/utils/route'
import { usePageTitle } from '@/utils/utils'

Comment on lines +16 to +21
const pageSize = 20
const page = useRouteQueryParamInt('p', 1)
const resetPage = (query: Partial<Record<string, string | null>>) => ({ ...query, p: null })
const sortOrder = useRouteQueryParamStrEnum('order', SortOrder, SortOrder.Desc, resetPage)
const createdAfter = useRouteQueryParamStr('from', '', resetPage)
const createdBefore = useRouteQueryParamStr('to', '', resetPage)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

Synchronously setting both createdAfter.value = '' and createdBefore.value = '' inside clearFilters() triggers two consecutive, asynchronous router pushes. Because Vue Router updates the route query asynchronously, the second push will read the stale route query (which still contains the old from value) and overwrite the first push. As a result, only one of the filters will actually be cleared.

To fix this, define router here and update clearFilters() to perform a single router push to clear both query parameters at once:

function clearFilters() {
  router.push({
    query: {
      ...router.currentRoute.value.query,
      from: undefined,
      to: undefined,
      p: undefined
    }
  })
}
const router = useRouter()

const pageSize = 20
const page = useRouteQueryParamInt('p', 1)
const resetPage = (query: Partial<Record<string, string | null>>) => ({ ...query, p: null })
const sortOrder = useRouteQueryParamStrEnum('order', SortOrder, SortOrder.Desc, resetPage)
const createdAfter = useRouteQueryParamStr('from', '', resetPage)
const createdBefore = useRouteQueryParamStr('to', '', resetPage)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve Account admin page titles and list URLs

1 participant