feat: adaptive username #536
Conversation
elrrrrrrr
commented
Jun 28, 2023
elrrrrrrr
commented
- 🧶 Modify the user query api to prioritize returning users from selfRegistry
- 🧶 Ensure that the query api uniformly returns displayName
- 🧶 修改用户查询接口，优先返回 selfRegistry 内的用户
- 🧶 查询接口统一返回 displayName
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| export function getPrefixedName(prefix: string, username: string): string { | ||
| return prefix ? `${prefix}${username}` : username; | ||
| } | ||
|
|
||
| export async function calculateIntegrity(contentOrFile: Uint8Array | string) { | ||
| let integrityObj; | ||
| if (typeof contentOrFile === 'string') { |
There was a problem hiding this comment.
在给定的代码补丁中，有一个新的函数getPrefixedName被添加到代码中。这个函数将一个前缀字符串和一个用户名字符串作为参数，并返回一个带有前缀的用户名字符串。
代码的修改看起来是正确的，没有明显的错误或缺陷风险。然而，以下是一些建议改进：
- 在
getPrefixedName函数中，可以添加输入参数的类型检查器，确保传递正确的参数类型。例如，为prefix和username添加类型注解string。 - 在
calculateIntegrity函数中，你可以添加返回类型注解（可能是Promise<void>）来指示异步函数的返回值类型。 - 如有可能，对函数和变量使用更具描述性的命名，以提高代码的可读性。
如果代码的上下文中还有其他内容，或者有其他方面需要注意的限制，请提供更多信息，以便我能够作出更准确的建议。
elrrrrrrr
force-pushed
the
adaptive-username
branch
from
18a355b
to
ffb6baa
Compare
| export function getPrefixedName(prefix: string, username: string): string { | ||
| return prefix ? `${prefix}${username}` : username; | ||
| } | ||
|
|
||
| export async function calculateIntegrity(contentOrFile: Uint8Array | string) { | ||
| let integrityObj; | ||
| if (typeof contentOrFile === 'string') { |
There was a problem hiding this comment.
代码看起来没有明显的错误风险，但以下是一些建议：
-
在代码中添加适当的注释，以解释每个函数和条件的用途和意图。这将提高可读性并帮助其他开发人员理解代码。
-
在导出的函数上添加适当的类型注解。这将使代码更具可读性，并允许编辑器捕获潜在的类型错误。
-
对于
calculateIntegrity函数，请确保在可能的情况下处理错误和异常情况。例如，如果传入的参数既不是Uint8Array类型也不是字符串类型，可能需要抛出一个错误或返回一个默认值。 -
针对每个函数编写一些单元测试，以确保其正确性和预期行为。这有助于捕获潜在的错误和边界情况。
总体而言，代码看起来相当简单且功能齐全。建议继续进行更准确的测试以确保正确的行为。
| async findInRegistry(registry:Registry, name: string): Promise<UserEntity | null> { | ||
| return await this.findUserByName(getPrefixedName(registry.userPrefix, name)); | ||
| } | ||
|
|
||
| async findUserByName(name: string): Promise<UserEntity | null> { | ||
| return await this.userRepository.findUserByName(name); | ||
| } |
There was a problem hiding this comment.
这段代码对UserService进行了补丁。下面是一些代码审查的建议和改进建议：
-
在导入语句处，可以把所有的import语句按照一定规则进行分组，以提高可读性。
-
对于
findUserByNameOrDisplayName方法：- 增加了在包含前缀的情况下直接调用
findUserByName的逻辑。 - 通过
registryManagerService来获取默认的注册表（defaultRegistry）和当前注册表（selfRegistry）。 - 使用
getPrefixedName函数生成带前缀的用户名，并在默认注册表和当前注册表中进行用户查找。
- 增加了在包含前缀的情况下直接调用
-
添加了
findInRegistry方法，该方法在给定的注册表中查找用户。 -
建议在代码中添加注释，解释每个方法的功能和参数。
-
确保在使用async/await时处理异常情况，可以使用try-catch块来捕获和处理可能发生的错误。
-
检查是否有必要对参数进行验证和边界条件检查。
这些建议都是基于代码片段本身，更全面的代码审查还需要结合整个项目上下文来进行。如果有其他具体问题或需求，请提供更多细节。
| assert.equal(res.body.email, email); | ||
| }); | ||
|
|
||
|
|
||
| it('should 404 when user not exists', async () => { | ||
| const { authorization, name } = await TestUtil.createUser(); | ||
| let res = await app.httpRequest() |
There was a problem hiding this comment.
这段代码的主要目标是测试用户控制器（UserController）中的一个方法。以下是对代码进行的简要代码审查：
-
在第23行和第34行之间添加了两个新的测试用例。
- 第一个测试用例验证了当请求带有特定前缀信息时是否正确清除该前缀，并返回相应的用户信息。
- 第二个测试用例验证了当请求自身用户时是否返回正确的用户信息。
-
需要确保在每个测试用例中都提供有效的授权头部（
authorization）。 -
没有立即发现明显的错误或潜在的风险。
针对改进的建议：
- 在测试用例中提供更详细的注释，以解释每个测试的目的和预期结果。
- 添加边界测试用例，例如尝试使用无效的授权头部或无效的请求路径来测试系统的鲁棒性。
- 确保在删除前缀信息的操作上进行适当的错误处理，例如在前缀信息为空或不存在时返回合适的错误响应。
请注意，这只是对给定代码片段的初步审查，可能还有其他方面需要检查。最佳实践是进行全面而彻底的代码审查，包括阅读和理解相关的功能实现和错误处理逻辑。
Codecov Report
@@ Coverage Diff @@
## master #536 +/- ##
==========================================
- Coverage 97.03% 96.97% -0.07%
==========================================
Files 174 174
Lines 16560 16591 +31
Branches 2170 2174 +4
==========================================
+ Hits 16069 16089 +20
- Misses 491 502 +11
|
[skip ci] ## [3.35.0](v3.34.10...v3.35.0) (2023-06-28) ### Features * adaptive username ([#536](#536)) ([dd69606](dd69606)), closes [/github.com/npm/cli/blob/latest/lib/commands/owner.js#L151](https://github.com/cnpm//github.com/npm/cli/blob/latest/lib/commands/owner.js/issues/L151)
