Skip to content

Fix case-sensitivity issues around backend unique identifiers#2964

Merged
aofei merged 2 commits intogoplus:devfrom
nighca:issue-2917
Mar 27, 2026
Merged

Fix case-sensitivity issues around backend unique identifiers#2964
aofei merged 2 commits intogoplus:devfrom
nighca:issue-2917

Conversation

@nighca
Copy link
Copy Markdown
Collaborator

@nighca nighca commented Mar 18, 2026

close #2917.

@gemini-code-assist

This comment was marked as outdated.

gemini-code-assist[bot]

This comment was marked as outdated.

@xgopilot
Copy link
Copy Markdown
Contributor

xgopilot bot commented Mar 19, 2026

Good foundational fix — the distinction between unresolved (JWT-derived) and canonical (API-confirmed) identifiers is clearly articulated in AGENTS.md and consistently applied across the codebase. The Input-suffix naming convention on route params is a nice signal at call sites.

A few things worth addressing before merge:

  • The startAutoSave watcher will create an unnecessary Saving on every mode transition (e.g., when signedInUser resolves), even with no dirty files.
  • The preferPublished callback is evaluated lazily inside the cloud load; if signedInUser hasn't resolved yet, a project owner gets the published snapshot on first load with no subsequent correction.
  • onMounted in CopilotUI.vue has no unmount guard around await untilNotNull(signedInUser).

Inline comments added on the specific locations.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

})
/** Records that the current user has used Copilot when the specified condition is met. */
async function recordCopilotUsage(shouldRecord: () => boolean) {
const signedInState = await untilLoaded(signedInStateQuery)
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

这边原来的逻辑有点乱,还有优化的空间,先不在这次处理了

<!-- eslint-disable vue/no-v-html -->
<template>
<div v-if="!signedInUser" class="sign-in">
<div v-if="!loading && signedInUser == null" class="sign-in">
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

加个 loading 的判断,这样在已登录的情况下,避免在初始化页面时短暂地渲染“登录”按钮

* - callers only need the signed-in user information
* - callers don't need to distinguish between "not signed in" and "signed in but user information not available"
*/
export function useSignedInUser() {
Copy link
Copy Markdown
Collaborator Author

@nighca nighca Mar 19, 2026

Choose a reason for hiding this comment

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

定位跟原 useSignedInUser 类似,不过考虑绝大部分调用方都不关心 queryRet.data 之外的信息,因此在原来的接口基础上把 queryRet 这一层去掉了,直接把 Ref<SignedInUser> 返回

@nighca nighca changed the title Fix frontend for case-sensitivity bugs around backend unique identifiers Fix case-sensitivity issues around backend unique identifiers Mar 19, 2026
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

router.ts 里改了路径参数,这里应该也要跟着改吧?不然修改成功后不会切换到新用户名 url

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

嗯是的,这里用的 param 名不对,route.params.* 不是类型安全的,搞不好其他地方也有类似的问题,我再整体过下

另外,这里原有的拿 oldUsername 跟路由参数做比较(然后 return)主要是针对的什么情况?一般来说如果能编辑 profile,应该肯定是在自己的 profile 页面才对

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

另外,这里原有的拿 oldUsername 跟路由参数做比较(然后 return)主要是针对的什么情况?一般来说如果能编辑 profile,应该肯定是在自己的 profile 页面才对

哦这好像是之前做成修改完用户名后不重定向刷新页面,而是就地替换的时候加的逻辑,现在不需要了,可以干掉

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

OK

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

done 5160291

if (oldUsername == null) throw new Error('Signed-in username is not available')

const updated = await apis.updateSignedInUser({ username: newUsername })
userState.username = updated.username
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

router.tsgetOwnProjectEditorRoutegetUnresolvedSignedInUsername 来获取 username,如果这里不更新的话,那后续 getOwnProjectEditorRoute 拿到的路由应该都是 old username 拼接的?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

嗯是的这里会有问题,我打算在这里 refresh 下 access token

直接 set userState.username 的话,userState 中的 username 跟其他信息会不一致,也会跟现在 username 的定位(User name parsed from access token)冲突

Copy link
Copy Markdown
Collaborator Author

@nighca nighca Mar 26, 2026

Choose a reason for hiding this comment

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

我打算在这里 refresh 下 access token

又遇到一个 casdoor 的 bug.. #2990

从代码的排查情况看,即使是改名后正常地 access token 过期触发的 refresh,也会遇到 crash;这么看的话如果我们不去改 casddoor,这边改完 username 得主动登出才能避免这个问题

Copy link
Copy Markdown
Collaborator Author

@nighca nighca Mar 27, 2026

Choose a reason for hiding this comment

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

5160291 这边我先处理成修改 username 后登出并自动跳转到登录页;Casdoor 那边我尝试提了个 PR 不过还不 ready,总感觉不太放心

@aofei aofei merged commit 646853e into goplus:dev Mar 27, 2026
4 checks passed
nighca added a commit to nighca/goplus-builder that referenced this pull request Mar 30, 2026
…#2964)

* Fix case-sensitivity issues around backend unique identifiers

* Fix route param names & username update issue
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.

Review frontend for case-sensitivity bugs around backend unique identifiers

3 participants