Update metadata Automatically #1368
Conversation
WalkthroughIn this update, a new method was added to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Extension
participant API
User->>Extension: Trigger metadata update
Extension->>API: Fetch new metadata
API-->>Extension: Return new metadata
Extension->>Extension: metadataUpdate(metadata)
Extension-->>User: Notify metadata update completed
Assessment against linked issues
Poem
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 as PR comments)
Additionally, you can add CodeRabbit Configration 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: 4
Outside diff range and nitpick comments (6)
packages/extension-polkagate/src/messaging.ts (1)
Line range hint
49-49: Avoid using theFunctiontype. Specify the function's signature explicitly for better type safety.- subscriber?: (data: any) => void; + subscriber?: (data: ResponseTypes[TMessageType]) => void;packages/extension-base/src/background/types.ts (1)
Line range hint
109-109: Remove thevoidtype from generic type arguments as it's only valid as a return type.- export interface RequestJsonRestore { - file: KeyringPair$Json; - password: string; - } - export interface RequestBatchRestore { - file: KeyringPairs$Json; - password: string; - } - export interface ResponseJsonRestore { - error: string | null; - }Also applies to: 110-110, 135-135
packages/extension-base/src/background/handlers/Extension.ts (1)
Line range hint
124-134: Refactor function to use an arrow function for consistency.According to the static analysis tool Biome, this function can be refactored to an arrow function for better readability and consistency with modern JavaScript practices.
- private lockExtension(): boolean { - const currentDomain = chrome.runtime.getURL('/'); - chrome.tabs.query({}, function (tabs) { - for (let i = 0; i < tabs.length; i++) { - const tabParsedUrl = new URL(tabs[i].url); - const tabDomain = `${tabParsedUrl?.protocol}//${tabParsedUrl?.hostname}/`; - if (tabDomain === currentDomain) { - chrome.tabs.reload(tabs[i].id).catch(console.error); - } - } - }); - return true; - } + private lockExtension = (): boolean => { + const currentDomain = chrome.runtime.getURL('/'); + chrome.tabs.query({}, (tabs) => { + tabs.forEach((tab) => { + const tabParsedUrl = new URL(tab.url); + const tabDomain = `${tabParsedUrl?.protocol}//${tabParsedUrl?.hostname}/`; + if (tabDomain === currentDomain) { + chrome.tabs.reload(tab.id).catch(console.error); + } + }); + }); + return true; + };packages/extension-ui/src/Popup/index.tsx (3)
Line range hint
188-188: Optimize deletion of properties.Instead of using the
deleteoperator, which can impact performance, consider setting the property toundefined.- delete assetsOnChains.balances[_address]; + assetsOnChains.balances[_address] = undefined;
Line range hint
221-227: Refactor function to use an arrow function for consistency.According to the static analysis tool Biome, this function can be refactored to an arrow function for better readability and consistency with modern JavaScript practices.
- function wrapWithErrorBoundary (component: React.ReactElement, trigger?: string): React.ReactElement { - return <ErrorBoundary trigger={trigger}>{component}</ErrorBoundary>; - } + const wrapWithErrorBoundary = (component: React.ReactElement, trigger?: string): React.ReactElement => { + return <ErrorBoundary trigger={trigger}>{component}</ErrorBoundary>; + };
Line range hint
261-265: Use optional chaining for better safety and readability.Refactor the code to use optional chaining to avoid potential runtime errors and improve code readability.
- if (priceIds && currency?.code && !isFetchingPricesRef.current) { + if (priceIds && currency?.code && !isFetchingPricesRef.current?) {
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- packages/extension-base/src/background/handlers/Extension.ts (2 hunks)
- packages/extension-base/src/background/types.ts (1 hunks)
- packages/extension-polkagate/src/hooks/index.ts (1 hunks)
- packages/extension-polkagate/src/hooks/useUpdateMetadata.ts (1 hunks)
- packages/extension-polkagate/src/messaging.ts (1 hunks)
- packages/extension-polkagate/src/util/types.ts (1 hunks)
- packages/extension-ui/src/Popup/index.tsx (2 hunks)
Files skipped from review due to trivial changes (1)
- packages/extension-polkagate/src/hooks/index.ts
Additional context used
Biome
packages/extension-polkagate/src/hooks/useUpdateMetadata.ts
[error] 88-88: Do not access Object.prototype method 'hasOwnProperty' from target object. (lint/suspicious/noPrototypeBuiltins)
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.packages/extension-polkagate/src/messaging.ts
[error] 167-179: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
[error] 49-49: Don't use 'Function' as a type. (lint/complexity/noBannedTypes)
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
packages/extension-base/src/background/types.ts
[error] 109-109: void is only valid as a return type or a type argument in generic type (lint/suspicious/noConfusingVoidType)
Remove void
[error] 110-110: void is only valid as a return type or a type argument in generic type (lint/suspicious/noConfusingVoidType)
Remove void
[error] 135-135: void is only valid as a return type or a type argument in generic type (lint/suspicious/noConfusingVoidType)
Remove void
packages/extension-base/src/background/handlers/Extension.ts
[error] 124-134: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.packages/extension-ui/src/Popup/index.tsx
[error] 188-188: Avoid the delete operator which can impact performance. (lint/performance/noDelete)
Unsafe fix: Use an undefined assignment instead.
[error] 221-227: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 261-261: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
[error] 263-263: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
[error] 265-265: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
Additional comments not posted (2)
packages/extension-polkagate/src/messaging.ts (1)
Line range hint
167-179: Theelseclause is unnecessary afterreturnstatements in previous branches.
[REFACTOR_SUGGESTion]- } else if (data.error) { - handler.reject(new Error(data.error)); - } else { - handler.resolve(data.response); - } + } else if (data.error) { + handler.reject(new Error(data.error)); + } else { + handler.resolve(data.response); + }packages/extension-ui/src/Popup/index.tsx (1)
139-140: EnsureuseUpdateMetadatais correctly implemented with the necessary dependencies.The usage of
useUpdateMetadatawithaccountCtx.accountsandapisas arguments seems appropriate. Ensure that the hook handles changes in these dependencies correctly.
| export interface ApiProps { | ||
| api?: ApiPromise; | ||
| endpoint?: string; | ||
| isRequested: boolean; | ||
| } | ||
|
|
||
| export interface APIs { | ||
| [genesisHash: string]: ApiProps; | ||
| [genesisHash: string]: ApiProps[]; | ||
| } | ||
|
|
||
| export interface APIsContext { | ||
| apis: { [key: string]: { api?: ApiPromise; endpoint?: string; isRequested: boolean; }[] }; | ||
| apis: APIs; | ||
| setIt: (apis: { | ||
| [key: string]: | ||
| { | ||
| api?: ApiPromise; | ||
| endpoint?: string; | ||
| isRequested: boolean; | ||
| }[] | ||
| [key: string]: ApiProps[] |
There was a problem hiding this comment.
Ensure consistency in the naming and structure of interfaces to maintain readability and ease of use.
- export interface ApiProps {
- api?: ApiPromise;
- endpoint?: string;
- isRequested: boolean;
- }
- export interface APIs {
- [genesisHash: string]: ApiProps[];
- }
- export interface APIsContext {
- apis: APIs;
- setIt: (apis: { [key: string]: ApiProps[] }) => void;
- }Committable suggestion was skipped due to low confidence.
AMIRKHANEF
force-pushed
the
UpdateMetadata
branch
from
b1667b7 to
3bfb4bf
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range and nitpick comments (4)
packages/extension-base/src/background/types.ts (3)
Line range hint
117-117: Consider replacingvoidwithundefinedin the type definition to avoid confusion, as suggested by static analysis.- export type RequestMetadataSubscribe = null; + export type RequestMetadataSubscribe = undefined;
Line range hint
118-118: Replacevoidwithundefinedin the type definition to align better with TypeScript best practices.- export type RequestSigningSubscribe = null; + export type RequestSigningSubscribe = undefined;
Line range hint
143-143: Adjust the type definition by replacingvoidwithundefinedto enhance clarity and consistency in the codebase.- export type RequestSigningSubscribe = null; + export type RequestSigningSubscribe = undefined;packages/extension-base/src/background/handlers/Extension.ts (1)
Line range hint
124-134: Consider refactoring the function expression into an arrow function for consistency and to reduce complexity.- function transformAccounts (accounts: SubjectInfo): AccountJson[] { + const transformAccounts = (accounts: SubjectInfo): AccountJson[] => {
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- packages/extension-base/src/background/handlers/Extension.ts (2 hunks)
- packages/extension-base/src/background/types.ts (1 hunks)
Additional context used
Biome
packages/extension-base/src/background/types.ts
[error] 117-117: void is confusing outside a return type or a type parameter. (lint/suspicious/noConfusingVoidType)
Unsafe fix: Use undefined instead.
[error] 118-118: void is confusing outside a return type or a type parameter. (lint/suspicious/noConfusingVoidType)
Unsafe fix: Use undefined instead.
[error] 143-143: void is confusing outside a return type or a type parameter. (lint/suspicious/noConfusingVoidType)
Unsafe fix: Use undefined instead.
packages/extension-base/src/background/handlers/Extension.ts
[error] 124-134: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
Additional comments not posted (1)
packages/extension-base/src/background/types.ts (1)
99-99: The addition of'pri(metadata.update)': [MetadataDef, boolean]aligns with the PR's objective to update metadata in the background. This update is crucial for enabling the new metadata update functionality.
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (3)
packages/extension-ui/src/Popup/index.tsx (3)
Line range hint
189-189: Avoid using thedeleteoperator as it can impact performance.Consider setting the property to
undefinedinstead of deleting it to potentially improve performance and prevent side effects on object shape optimizations.- delete assetsOnChains.balances[_address]; + assetsOnChains.balances[_address] = undefined;
Line range hint
222-228: This function expression can be turned into an arrow function.Using arrow functions can make the code more concise and improve readability.
- const fetchLoginInfo = async function() { + const fetchLoginInfo = async () => {
Line range hint
262-262: Consider using optional chaining for better safety and readability.Optional chaining (
?.) allows you to safely access deeply nested properties without having to check if each reference in the chain is nullish.- if (loginInfo.status !== 'forgot') { + if (loginInfo?.status !== 'forgot') { - } else if (loginInfo.status === 'forgot') { + } else if (loginInfo?.status === 'forgot') { - const addresses = accounts?.map((account) => account.address); + const addresses = accounts?.map((account) => account?.address);Also applies to: 264-264, 266-266
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- packages/extension-polkagate/src/hooks/useUpdateMetadata.ts (1 hunks)
- packages/extension-polkagate/src/util/types.ts (1 hunks)
- packages/extension-ui/src/Popup/index.tsx (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- packages/extension-polkagate/src/util/types.ts
Additional context used
Biome
packages/extension-polkagate/src/hooks/useUpdateMetadata.ts
[error] 88-88: Do not access Object.prototype method 'hasOwnProperty' from target object. (lint/suspicious/noPrototypeBuiltins)
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.packages/extension-ui/src/Popup/index.tsx
[error] 189-189: Avoid the delete operator which can impact performance. (lint/performance/noDelete)
Unsafe fix: Use an undefined assignment instead.
[error] 222-228: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 262-262: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
[error] 264-264: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
[error] 266-266: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
| setUpdating(true); | ||
|
|
||
| for (const chainGenesisHash in apiList) { | ||
| if (apiList.hasOwnProperty(chainGenesisHash)) { |
There was a problem hiding this comment.
Replace hasOwnProperty with Object.hasOwn for better safety and modern standards.
- if (apiList.hasOwnProperty(chainGenesisHash)) {
+ if (Object.hasOwn(apiList, chainGenesisHash)) {Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (apiList.hasOwnProperty(chainGenesisHash)) { | |
| if (Object.hasOwn(apiList, chainGenesisHash)) { |
Tools
Biome
[error] 88-88: Do not access Object.prototype method 'hasOwnProperty' from target object. (lint/suspicious/noPrototypeBuiltins)
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- packages/extension-base/src/background/handlers/Extension.ts (4 hunks)
- packages/extension-polkagate/src/util/types.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- packages/extension-polkagate/src/util/types.ts
Additional context used
Biome
packages/extension-base/src/background/handlers/Extension.ts
[error] 124-134: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
Additional comments not posted (1)
packages/extension-base/src/background/handlers/Extension.ts (1)
108-108: Consider using an arrow function foraccountsUpdateMetato enhance readability and reduce function complexity.- private accountsUpdateMeta ({ address, meta }: RequestUpdateMeta): boolean { + private accountsUpdateMeta = ({ address, meta }: RequestUpdateMeta): boolean => {Likely invalid or redundant comment.
| private metadataUpdate(metadata: MetadataDef): boolean { | ||
| assert(metadata, 'Unable to update metadata'); | ||
|
|
||
| this.#state.saveMetadata(metadata); | ||
|
|
||
| return true; | ||
| } |
There was a problem hiding this comment.
The method metadataUpdate effectively handles the metadata update process. However, consider using an arrow function here as well to align with modern JavaScript best practices and simplify the function's structure.
- private metadataUpdate(metadata: MetadataDef): boolean {
+ private metadataUpdate = (metadata: MetadataDef): boolean => {Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private metadataUpdate(metadata: MetadataDef): boolean { | |
| assert(metadata, 'Unable to update metadata'); | |
| this.#state.saveMetadata(metadata); | |
| return true; | |
| } | |
| private metadataUpdate = (metadata: MetadataDef): boolean => { | |
| assert(metadata, 'Unable to update metadata'); | |
| this.#state.saveMetadata(metadata); | |
| return true; | |
| } |
| case 'pri(metadata.update)': // added for polkagate | ||
| return this.metadataUpdate(request as MetadataDef); | ||
|
|
There was a problem hiding this comment.
Tip
Codebase Verification
The search did not reveal any test files that directly or indirectly cover the 'pri(metadata.update)' case in the Extension handler. This indicates that the new switch case is not currently covered by unit tests.
- Please add unit tests to cover the 'pri(metadata.update)' case in the
Extensionhandler to ensure its functionality is thoroughly verified.
Analysis chain
The addition of the case 'pri(metadata.update)' in the switch statement is correctly implemented. However, ensure that this case is covered by unit tests to verify its functionality thoroughly.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify if the new switch case for 'pri(metadata.update)' is covered by unit tests.
# Test: Search for the test file and check for the presence of the new case.
rg --type ts $'describe\\("Extension handle", \\(\\) => \\{[^}]*it\\("handles pri\\(metadata.update\\)"'
Length of output: 98
Script:
#!/bin/bash
# Description: Broaden the search to find any test cases that cover 'pri(metadata.update)' in the entire codebase.
# Test: Search for any occurrence of 'pri(metadata.update)' in test files.
rg --type ts 'pri\(metadata.update\)'
Length of output: 381
Script:
#!/bin/bash
# Description: Search for any test files that might indirectly test the 'pri(metadata.update)' case.
# Test: Search for any test files that import or reference the Extension handler.
rg --type ts 'Extension' --files-with-matches | xargs rg 'pri\(metadata.update\)'
Length of output: 302
|
related #1378 |
|
close in favour of #1517 |
Works Done
useUpdateMetadata.ts.pri(metadata.update).ApiContexttype.Close: #1356
Summary by CodeRabbit
New Features
Refactor