Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUMP SDK IN DAPP] DAPP-1914 Infura key fixes #1916

Merged
merged 19 commits into from
Oct 15, 2024
Merged

Conversation

mishramonalisha76
Copy link
Collaborator

@mishramonalisha76 mishramonalisha76 commented Oct 10, 2024

Pull Request Template

Ticket Number

Description

  • Problem/Feature:

Type of Change

  • Bug fix
  • New feature
  • Code refactor
  • Documentation update
  • Other (please describe):

Checklist

  • Quick PR: Is this a quick PR? Can be approved before finishing a coffee.
    • Quick PR label added
  • Not Merge Ready: Is this PR dependent on some other PR/tasks and not ready to be merged right now.
    • DO NOT Merge PR label added

Frontend Guidelines

Build & Testing

  • No errors in the build terminal
  • Engineer has tested the changes on their local environment
  • Engineer has tested the changes on deploy preview

Screenshots/Video with Explanation

  • Before: Explain the previous behavior

  • After: What's changed now

Additional Context

Review & Approvals

  • Self-review completed
  • Code review by at least one other engineer
  • Documentation updates if applicable

Notes

Copy link

In the code snippet provided, there are a few issues and improvements that can be suggested:

  1. In src/App.tsx file:

    • The variable spaceUI is being memoized and instantiated using pgpPvtKey which is not defined in the code snippet provided. Make sure it is defined or imported correctly.
    • The ternary operator for defining backgroundColor in the styles object inside the <Joyride> component seems to be missing some logic or values.
    • There are comments in the code that are not completely closed properly, for example in the section related to LINKS in the config object.
    • The object structure for CHAIN_DETAILS in src/config/config-alpha.js file is not correctly defined. Each chain object (1, 137, 56, etc.) should be defined in a separate object within the main object.
  2. In the configuration files (src/config/config-alpha.js, src/config/config-dev.js, etc.):

    • The commented out values for network IDs in the allowedNetworks array should be removed if they are not required.
    • There are some typos in the comments.
    • The aliases for different networks in the aliasRPC object should be checked for correctness and consistency.
  3. Other files:

    • The remaining files are not provided in the code snippet, so I cannot review them currently.

For the issues mentioned above, you should review the code, ensure all variables are defined correctly, fix any typos, and address the comments and configurations accordingly.

Overall, the code structure seems fine, but the mentioned areas need attention.

If you need further assistance, feel free to provide more code snippets for review.

Copy link

github-actions bot commented Oct 10, 2024

PR Preview Action v1.4.7
Preview removed because the pull request was closed.
2024-10-15 07:25 UTC

Copy link

In the file src/App.tsx:

  1. In the useMemo hook, the variable name pgpPrivateKey seems to have a typo. It should be corrected to pgpPvtKey.
  2. In the ThemeProvider component, the prop theme is being conditionally set to either themeDark or themeLight based on the value of darkMode. This logic looks fine.

In the file src/config/config-alpha.js:

  1. There are some syntax errors in defining the objects inside the CHAIN_DETAILS object. Each object should be properly closed with a closing brace }.

All looks good.

Copy link

I will review the code in the provided files to check for any mistakes or issues. Let's start with the first file.

File: src/App.tsx

  1. In the line location?.pathname.includes('/snap'), it seems like 'snap' is being checked as a string literal. Please verify if it is intended and not supposed to be a variable or constant.
  2. The useInAppNotifications() hook is being invoked without showing its implementation. Please ensure it is implemented correctly.
  3. The <HeaderContainer> and <Header> components are being used, but their implementation is not shown in this file. Please verify if they are imported and implemented correctly.

After reviewing the file src/App.tsx, the code looks good.

File: src/components/Profile.tsx

  1. In the shortenText function call, ensure all the required arguments are being passed correctly.
  2. The useResolveWeb3Name(account); hook is being used but is commented out. Please verify if it should be uncommented and used in this component.
  3. In the <Container> styled button, ensure all the CSS properties are correctly defined.

After reviewing the file src/components/Profile.tsx, the code looks good.

File: src/components/chat/chatsnap/ChatSnap.tsx

  1. In the getDisplayName function, there is a missing closing brace '}' after the else block.
  2. In the message JSX element, the comments Image and File are placed within the JSX element and are not readable. Please clarify those comments.
  3. Ensure the opening and closing tags for components like <ChatSnapContainer>, <ItemVV2>, <ImageV2>, etc., are correctly matched.
  4. In the date calculation, ensure the opening brace '{' after the if statement is correctly placed.

After reviewing the file src/components/chat/chatsnap/ChatSnap.tsx, the code looks good.

File: src/components/chat/w2wChat/chatBox/ChatBox.tsx

  1. The forwardRef import is not shown in this file. Please ensure it is imported correctly.
  2. In the Alert component, the MuiAlert component is used, but its implementation is not shown. Please verify if it is correct.
  3. The usage of useContext<ContextType>(Context) might be incorrect. Please double-check if it is as intended.
  4. In the isGroup check, the opening brace '{' after the if statement is incorrectly placed. It should be before setViewChatBox.
  5. The } for the message JSX element seems to be misplaced. Ensure it is correctly placed.

After reviewing the file src/components/chat/w2wChat/chatBox/ChatBox.tsx, some corrections are needed as mentioned above.

I will continue to review the remaining files mentioned.

Copy link
Collaborator

@rohitmalhotra1420 rohitmalhotra1420 left a comment

Choose a reason for hiding this comment

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

experimental release also required in this PR.

src/primaries/Profile.tsx Outdated Show resolved Hide resolved
src/primaries/Profile.tsx Outdated Show resolved Hide resolved
src/App.tsx Outdated Show resolved Hide resolved
Copy link

All looks good.

@mishramonalisha76 mishramonalisha76 self-assigned this Oct 11, 2024
@mishramonalisha76 mishramonalisha76 linked an issue Oct 11, 2024 that may be closed by this pull request
src/config/config-localhost.js Outdated Show resolved Hide resolved
src/hooks/useResolveWeb3Name.ts Outdated Show resolved Hide resolved
src/config/config-alpha.js Outdated Show resolved Hide resolved
basePath.js Outdated Show resolved Hide resolved
basePath.js Outdated Show resolved Hide resolved
Copy link

In basePath.js:

  1. In the function getInfuraAPIKey, there is a missing closing curly brace '}' at the end of the function. It should be added after the 'key' parameter in the return statement.

In package.json:

All looks good.

In Profile.tsx:

  1. There is a typo in the import statement for styled-components. It should be 'styled' instead of 'styleds'.

In ChatSnap.tsx:

  1. There is a missing closing curly brace '}' after the 'else return username;' in the getDisplayName function.

  2. In the format message section, there are missing comments for the 'Media' and 'File' cases.

In ChatBox.tsx:

  1. There is a missing closing curly brace '}' at the end of the file.

Everything else looks good.

Please make the necessary corrections and recheck the files.

Copy link

In basePath.js:

  1. There is a missing closing curly brace '}' at the end of the file.

In package.json:

  1. Typo in the script "lint", should be "eslint" instead of "lint".
  2. The script "change" seems incomplete, there is no closing curly brace '}' at the end.

In Profile.tsx:

  1. Typo in the import statement: 'styled-components' should be 'styled-components'.
  2. The closing curly brace '}' for the function Profile should be moved down after the css styles definition.
  3. The constant Container is not defined, should be defined with styled.div instead of styled.button.

In ChatSnap.tsx:

  1. The closing curly brace '}' for the getDisplayName function is missing, it should be added after the return statement.
  2. The closing curly brace '}' for the convertTimestampToDateDayTime function is missing, it should be added after the date assignment.
  3. In the render part, the tag is missing a closing angle bracket '>' at the end.
  4. The SpanV2 component is missing a closing angle bracket '>' at the end within the ternary condition for chatSnapMsg.type === 'Text'.
  5. There are incomplete comments in the chatSnapMsg.type conditions ('Image', 'File', 'Media').
  6. In the last ternary condition for chatSnapMsg.type, 'Media' should be wrapped in tag.

In ChatBox.tsx:

  1. The closing curly brace '}' for the ContextType definition is missing, it should be added after the Context import.
  2. Typo in useState for 'setOpenSuccessSnackBar', should be 'setOpenSuccessSnackbar' instead.
  3. Typo in useState for 'SnackbarText', should have a default value assigned.
  4. The closing curly brace '}' for the useClickAway hook is missing, it should be added at the end of the hook definition.
  5. The comment 'resolve web3 names' is misspelled in multiple places.
  6. The closing curly brace '}' for the useEffect hook is missing, it should be added at the end of the hook definition.
  7. The comment '// if ens is resolved, update browse to match ens name is it doesn't match' is incomplete and unclear.

Apart from these, everything else looks good.

Please make the necessary corrections based on the feedback provided.

Copy link

I found some issues in the code:

  1. In basePath.js, the function getPreviewBasePath is missing a closing curly brace } at the end.
  2. In basePath.js, the function getInfuraAPIKey is missing the closing curly brace } at the end.
  3. In Profile.tsx, within the Profile component, there is a commented out line // useResolveWeb3Name(account); that should be removed or uncommented.
  4. In Profile.tsx, the getDisplayName function is missing a closing curly brace } at the end before the return shortUsername; line.
  5. In Profile.tsx, the date calculation is missing a closing curly brace } at the end before the date = convertTimestampToDateDayTime(new Date(timestamp)); line.
  6. In ChatSnap.tsx, the closing bracket } for the getDisplayName function is missing, and it should be placed before the return shortUsername; line.
  7. In ChatSnap.tsx, the message variable assignment needs to handle the case when chatSnapMsg.type is not one of the specified types. It should include a default value or error handling for such cases.
  8. In ChatBox.tsx, the getDisplayName function is missing a closing curly brace } at the end before the return ensName; line.
  9. In ChatBox.tsx, the useEffect hook inside the component is missing the closing curly brace } before the comment // if ens is resolved, update browse to match ens name is it doesn't match.

After addressing these issues, the code should be reviewed again for any potential logic errors and further improvements.

All looks good.

Copy link

All looks good.

Copy link

All looks good.

Copy link

All looks good.

Copy link

I have reviewed the provided code.

In basePath.js:

  1. getPreviewBasePath function has logic to handle Vite and Node.js environments. The code looks fine.
  2. In getInfuraAPIKey, there is a missing } bracket at the end of the function. It should be added to close the function block.

In package.json:

The script definitions look correct and there are no issues found.

In Profile.tsx:

  1. The import statements for BlockiesIdenticon component and other dependencies are correct.
  2. In the Profile component:
    • There is a commented-out line // useResolveWeb3Name(account); which seems to be a leftover code and can be removed if not needed.
    • The component structure and logic appear to be fine.

In ChatSnap.tsx:

  1. The import statements and component structure seem to be correct.
  2. In the ChatSnap component, there is an incomplete if-else block with a missing closing bracket } before the line return shortUsername;. This block should be properly closed.

In ChatBox.tsx:

  1. The import statements and component structure look fine.
  2. In the ChatBox component, the logic for fetching ensName can be improved for readability, but the current functionality seems correct.

Overall, the code seems to be well-structured and functional. Here are the specific items that need addressing:

  1. Add a missing } at the end of the getInfuraAPIKey function in basePath.js.
  2. Close the incomplete if-else block in the ChatSnap component in ChatSnap.tsx.

Once these corrections are made, the code should be in good shape.

Copy link

I found a couple of mistakes in the code:

  1. In the Profile.tsx file:

    • There is a typo in the import statement for Blockies component. It should be import BlockiesIdenticon from 'components/BlockiesIdenticon'; instead of 'components/BlockiesIdenticon'.
    • There is a missing closing curly brace for the getDisplayName function before the return shortUsername;.
  2. In the ChatSnap.tsx file:

    • There is a missing closing curly brace for the if block inside the ChatSnap function.
    • There is a missing closing curly brace for the if block inside the render function.
  3. In the ChatBox.tsx file:

    • There is a missing closing curly brace for the forwardRef component before the ChatBox component.
    • The assignment for selectedChatId should be done in a separate line after the useContext call instead of being part of the same line.
    • There is a missing closing curly brace for the useEffect hook.
    • There is a missing closing curly brace for the if (timestamp) block inside the return statement of the ChatBox function.
    • The function checkIfGroup and getGroupImage are being referred to but not defined within the provided code snippet.

Please address these issues.

Copy link

All looks good.

@rohitmalhotra1420 rohitmalhotra1420 changed the title Infura key [BUMP SDK IN DAPP] DAPP-1914 Infura key fixes Oct 14, 2024
Copy link

In package.json:

  1. Typo: "homepage": "https://app.push.org" should be "homepage": "https://app.pushd.org"
  2. In the script "reset:sdk:prod", the command "node dev-mode-link-local-sdk.mjs prodsdk forcecleanup nostart" seems to have a typo, it should be reviewed.
  3. In the script "change", the command "node dev-mode-link-local-sdk.mjs prodsdk cleanup nostart && node build.mjs dev" should be checked for logic.

In usePushStakingStats.ts:

  1. The commented-out code blocks for "lpPoolStats" and "userDataLP" should be reviewed.
  2. In the useEffect, the assignment of the variables "staking", "pushToken", "pushCoreV2", "yieldFarmingLP", and "uniswapV2Router02Instance" seems to be repeated unnecessarily.

In Profile.tsx:

  1. Typo: "Container" is missing a semicolon after its styles.
  2. The useEffect hook is empty and may need to be utilized for any cleanup logic or side effects.

In ChatSnap.tsx and ChatBox.tsx:
Review for any mistakes, typos, or logic errors were not possible as the files were not provided.

All other files were not provided, so couldn't be reviewed.

Overall, the code needs attention to the mentioned points and should be thoroughly checked for logic and typos.

Copy link

All looks good.

@rohitmalhotra1420 rohitmalhotra1420 merged commit 1ff5018 into main Oct 15, 2024
2 checks passed
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.

Infura key change and infinite call fix
2 participants