fix(search): author is ??? in result
#741
Conversation
Signed-off-by: Kevin Cui <bh@bugs.cc>
WalkthroughThe pull request introduces modifications to the Changes
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/core/service/PackageSearchService.ts (1)
144-152: LGTM! Publisher field mapping looks correctThe implementation properly maps the
_npmUserfield to the newpublisherfield format required by npm CLI v10.6.0+, while maintaining backward compatibility.Consider adding a test case to verify this mapping behavior, especially for packages with and without the publisher field. Would you like me to help create the test cases?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/core/service/PackageSearchService.ts(1 hunks)app/repository/SearchRepository.ts(1 hunks)
🔇 Additional comments (2)
app/repository/SearchRepository.ts (1)
20-23: LGTM! Type changes align with npm CLI requirements
The addition of the optional publisher type with username and email fields correctly matches the structure expected by npm CLI v10.6.0+, while maintaining backward compatibility.
app/core/service/PackageSearchService.ts (1)
144-152: Verify the impact on search functionality
Let's verify the usage of publisher/author fields in the codebase.
✅ Verification successful
Let me analyze the search results and generate the final response.
Publisher field change is safe and properly implemented
The code change to use publisher field instead of _npmUser field is safe and properly implemented because:
- The change only adds publisher field when it's missing and _npmUser exists, maintaining backward compatibility
- The search functionality in PackageSearchService.ts only uses these fields for display purposes
- The publisher field is consistently used across the codebase, mainly in test files and core services
- The _npmUser field is only used in test data and sync operations, not in core functionality
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any other occurrences of publisher or author field usage that might need updates
# Search for publisher field usage
echo "Checking publisher field usage:"
rg -p "publisher['\"]?\s*[:=]" --type ts
# Search for _npmUser field usage
echo "\nChecking _npmUser field usage:"
rg -p "_npmUser['\"]?\s*[:=]" --type ts
# Search for author-related field usage
echo "\nChecking author field usage:"
rg -p "author['\"]?\s*[:=]" --type ts
# Search for maintainers field usage that might need updates
echo "\nChecking maintainers field usage:"
rg -p "maintainers['\"]?\s*[:=]" --type ts
Length of output: 103099
|
@BlackHole1 Nice work! |
[skip ci] ## [3.71.2](v3.71.1...v3.71.2) (2024-12-18) ### Bug Fixes * **search:** author is `???` in result ([#741](#741)) ([acffb14](acffb14)), closes [/github.com/npm/cli/pull/7407/files#diff-4bc15933c685fc9a9ce8be0c13a2f067f5e2b3334bacd6664bdfa7ddc46aedb6L58](https://github.com/cnpm//github.com/npm/cli/pull/7407/files/issues/diff-4bc15933c685fc9a9ce8be0c13a2f067f5e2b3334bacd6664bdfa7ddc46aedb6L58) [/github.com/npm/cli/pull/7407/files#diff-4bc15933c685fc9a9ce8be0c13a2f067f5e2b3334bacd6664bdfa7ddc46aedb6R162](https://github.com/cnpm//github.com/npm/cli/pull/7407/files/issues/diff-4bc15933c685fc9a9ce8be0c13a2f067f5e2b3334bacd6664bdfa7ddc46aedb6R162)
|
已生效
|
改动原因
在今年 4 月份，npm cli 对 search 做了一些改动，在 4 月份以前(npm cli <
v10.6.0)，采用的是maintainers字段，而在 npm/cli#7407 后，改为使用:publisher字段。导致当 npm 版本大于等于
v10.6.0后，search 结果中的author将变成???，如图：预期的结果应该为:
技术细节说明
当前改动没有为
es增加新的索引，原因是处于以下考虑:mapping一旦创建，就无法修改（虽然使用了dynamic: true但无法细粒度的进行控制）_npmUser已经有相关信息了，没有必要为此浪费额外的磁盘空间npm cli 老版本: https://github.com/npm/cli/pull/7407/files#diff-4bc15933c685fc9a9ce8be0c13a2f067f5e2b3334bacd6664bdfa7ddc46aedb6L58
npm cli 新版本: https://github.com/npm/cli/pull/7407/files#diff-4bc15933c685fc9a9ce8be0c13a2f067f5e2b3334bacd6664bdfa7ddc46aedb6R162
其他
相关 PR: #513
PTAL @Beace @fengmk2 @elrrrrrrr