feat: expose 'wikiAvailable', match on uniqueName #718
Conversation
WalkthroughThis pull request refines how Wikia data is processed by switching the matching criteria from the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (10)
🚧 Files skipped from review as they are similar to previous changes (10)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🪧 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)
test/lookup.mjs (1)
7-7: Consider parameterizing the lookup value.While hardcoding 'Acceltra' works for basic testing, consider making this configurable, perhaps through command-line arguments, to enable testing with different items without code changes.
-const lookup = 'Acceltra'; +const lookup = process.argv[2] || 'Acceltra';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
build/parser.mjs(1 hunks)build/wikia/transformers/transformArcanes.mjs(1 hunks)build/wikia/transformers/transformCompanion.mjs(1 hunks)build/wikia/transformers/transformMod.mjs(1 hunks)build/wikia/transformers/transformWarframe.mjs(1 hunks)build/wikia/transformers/transformWeapon.mjs(1 hunks)build/wikia/transformers/transformerArchwing.mjs(1 hunks)index.d.ts(2 hunks)package.json(1 hunks)test/lookup.mjs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build
🔇 Additional comments (20)
package.json (1)
73-73: Good addition of json-diff dependencyAdding json-diff as a development dependency is appropriate. This will help with comparing JSON objects in tests, particularly for the weapon data comparison mentioned in the AI summary.
build/wikia/transformers/transformWeapon.mjs (2)
205-205: Consistently extracting InternalName propertyGood addition of InternalName to the destructured properties, which supports the PR's goal of matching on uniqueName instead of name.
211-211: Adding uniqueName property to enable better matchingExcellent implementation. The uniqueName property is now set to the InternalName value, which will provide a more reliable identifier for matching items compared to the name property.
build/wikia/transformers/transformArcanes.mjs (2)
8-8: Consistent extraction of InternalName from source dataGood addition of InternalName to the destructured properties, maintaining consistency with the other transformer modules.
12-12: Adding uniqueName property for arcanes matches implementation in other transformersThe implementation is consistent with the other transformers and aligns with the PR's goal of matching on uniqueName instead of name.
build/wikia/transformers/transformMod.mjs (2)
8-8: Consistent extraction of InternalName across transformersGood addition of InternalName to the destructured properties, maintaining consistency with the other transformer modules.
12-12:Details
✅ Verification successful
Adding uniqueName property for mods completes the consistent pattern
The implementation is consistent with the other transformers and helps fulfill the PR objective of matching on uniqueName rather than name.
I'd like to verify that the Parser module (mentioned in the AI summary) has been properly updated to use this new uniqueName property for matching. Can you confirm this change has been made?
🏁 Script executed:
#!/bin/bash # Check if Parser has been updated to use uniqueName instead of name rg "uniqueName" -A 3 -B 3 build/parser.mjsLength of output: 8706
Parser module update verified for uniqueName usage
I reviewed the Parser module (build/parser.mjs) and confirmed that it consistently uses the
uniqueNameproperty for matching (e.g., in blueprint filtering, item processing, and comparison logic). This aligns with the changes made in the mods transformer and fulfills the PR objective.build/wikia/transformers/transformerArchwing.mjs (2)
16-16: Addition of InternalName extraction is appropriate.The addition of
InternalNameto the destructuring assignment aligns perfectly with the PR objective of matching onuniqueName. This change enables access to the internal identifier from the source data.
21-21: Appropriate implementation of uniqueName property.Adding the
uniqueNameproperty based onInternalNameprovides a consistent identifier for matching across the codebase, which is in line with the PR objective. This change enhances the identification mechanism for archwing items.build/wikia/transformers/transformWarframe.mjs (2)
37-38: Good addition of InternalName to destructuring assignment.The inclusion of
InternalNamein the destructuring maintains consistency with the other transformer modules and supports the PR objective of usinguniqueNamefor matching.
43-43: Appropriate implementation of uniqueName property.Adding the
uniqueNameproperty based onInternalNameis consistent with other transformer modules and provides a reliable identifier for warframes.test/lookup.mjs (4)
3-3: Good ESLint rule addition for development dependencies.Adding the
import/no-extraneous-dependenciesdisable rule is appropriate since the test file is importing development-only packages likejson-diff.
5-5: Appropriate addition of json-diff library.The import of
json-diffaligns with the PR description mentioning a new development dependency and provides useful functionality for comparing JSON objects in testing.
10-11: Good implementation of API comparison logic.The implementation to fetch the item from the API and find the corresponding item in the local data is well-structured and effective for testing the changes.
13-13: Effective use of json-diff for output.Using
json-diffto display differences between the API data and local data is an excellent way to visualize the impact of your changes and helps with debugging.build/wikia/transformers/transformCompanion.mjs (2)
16-17: Good addition of InternalName to destructuring assignment.The inclusion of
InternalNamein the destructuring maintains consistency with the other transformer modules and supports the PR objective.
22-22: Appropriate implementation of uniqueName property.Adding the
uniqueNameproperty based onInternalNameis consistent with other transformer modules and enhances item identification.build/parser.mjs (1)
915-917: Good improvement to wiki data matchingSwitching from matching on
nametouniqueNameis a good improvement that should make the matching more reliable, as unique names are guaranteed to be distinct. The addition of thewikiAvailableflag provides a clean way to check if wiki data exists for an item.index.d.ts (2)
158-158: LGTM: New wikiAvailable property addedAdding this optional property to the WikiaItem interface aligns properly with the implementation in the parser.
401-401: Good interface extensionMaking Component extend WikiaItem is appropriate since components can now have wiki-related properties, including the new wikiAvailable flag. The removal of redundant properties (wikiaThumbnail and wikiaUrl) that were previously defined directly on Component makes sense as they're now inherited from WikiaItem.
|
@coderabbitai see #600 (comment) for how this applies to #600 |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 1438 @TobiTenno I've reviewed the comment and understand how this PR addresses issue #600. Instead of adding a direct The PR also enhances the matching mechanism by using |
TobiTenno
force-pushed
the
wiki-matching
branch
from
1e98295 to
c014128
Compare
|
Perhaps this goes without saying, but I'm not so sure that this really solves the original issue. As with anything Wiki-related, there's going to be a concern of reliability with the data we're working with. Depending on the use case, this could hamper external services relying on this API. However, in addition, there may be a case where a page exists for an item, an item technically exists, and is not obtainable nor tradable. Also, I worry some items may not directly translate well to a Wiki URL. TL; DR: The original issue and this solution may provide the same information at the moment or any given time, but technically represent two different ideas and uses. |
|
I realize that, but there's literally no non-manual way of maintaining "is this obtainable" that isn't an inference based on data outside DE's manifest; and at best it would be cases of negative enforcement, and positive defaults |
|
🎉 This PR is included in version 1.1269.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
What did you fix?
closes #600
Reproduction steps
Evidence/screenshot/link to line
Considerations
Summary by CodeRabbit
New Features
Chores
Tests
Documentation