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

Upgrade page fixes #181

Merged
merged 2 commits into from
Jan 8, 2025
Merged

Upgrade page fixes #181

merged 2 commits into from
Jan 8, 2025

Conversation

chalabi2
Copy link
Collaborator

@chalabi2 chalabi2 commented Jan 7, 2025

  • Added a query to fetch and refetch block height every 6 seconds
  • Added validation to upgrade form to ensure inputted block height is 1000 > current height
  • Added binary download links + commit hash to upgrade message info param

Summary by CodeRabbit

Release Notes

  • New Features

    • Added block height tracking functionality to improve upgrade modal validation.
    • Enhanced upgrade modal with more robust version selection and validation.
  • Improvements

    • Updated input label styling for better text selection.
    • Refined upgrade process with more detailed version information.
  • Technical Enhancements

    • Introduced a new hook for retrieving the current block height.
    • Improved form validation for upgrade height selection.

Copy link
Contributor

coderabbitai bot commented Jan 7, 2025

Walkthrough

This pull request introduces enhancements to the UpgradeModal component by adding a new useBlockHeight hook to dynamically retrieve the current blockchain block height. The changes improve validation for upgrade heights, update the data handling process for upgrade information, and modify the input field for block height selection. Modifications span across three files: upgradeModal.tsx, BaseInput.tsx, and useQueries.ts, focusing on robust and dynamic upgrade validation and information retrieval.

Changes

File Change Summary
components/admins/modals/upgradeModal.tsx - Added useBlockHeight hook to get current block height
- Updated parseReleaseBody regex patterns
- Enhanced UpgradeSchema with dynamic height validation
- Modified handleUpgrade to include selectedVersion
- Updated form initial values and validation
components/react/inputs/BaseInput.tsx - Added select-text class to label span element
hooks/useQueries.ts - Created new useBlockHeight hook to fetch block height
- Implemented query with 6-second refresh interval

Sequence Diagram

sequenceDiagram
    participant User
    participant UpgradeModal
    participant useBlockHeight
    participant LCDClient
    
    User->>UpgradeModal: Open Upgrade Modal
    UpgradeModal->>useBlockHeight: Fetch Current Block Height
    useBlockHeight->>LCDClient: Query Block Status
    LCDClient-->>useBlockHeight: Return Block Height
    useBlockHeight-->>UpgradeModal: Provide Block Height
    UpgradeModal->>UpgradeModal: Validate Upgrade Height
Loading

Possibly related PRs

  • chalabi/modal fixes #87: Modal functionality improvements that might share similar validation and data handling strategies.

Poem

🐰 Upgrade Modal's Rabbit Hop 🚀

Blocks dancing, heights so bright,
A rabbit's query takes its flight,
Validation leaps with graceful might,
Upgrade magic in pure delight!

Hop, hop, code so light! 🌈


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8bd59d2 and e5a7d31.

📒 Files selected for processing (1)
  • components/admins/modals/upgradeModal.tsx (7 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • components/admins/modals/upgradeModal.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Analyze (javascript-typescript)

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

codecov bot commented Jan 7, 2025

Codecov Report

Attention: Patch coverage is 11.53846% with 92 lines in your changes missing coverage. Please review.

Project coverage is 54.78%. Comparing base (12555f6) to head (e5a7d31).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
components/admins/modals/upgradeModal.tsx 10.12% 71 Missing ⚠️
hooks/useQueries.ts 4.54% 21 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #181      +/-   ##
==========================================
- Coverage   55.04%   54.78%   -0.27%     
==========================================
  Files         153      153              
  Lines       15515    15606      +91     
==========================================
+ Hits         8540     8549       +9     
- Misses       6975     7057      +82     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (3)
components/admins/modals/upgradeModal.tsx (2)

57-69: Add unit tests for upgrade height validation logic

The custom validation in UpgradeSchema checks that the proposed upgrade height is at least 1000 blocks above the current height. Unit tests will help verify that this validation works correctly under different conditions.

Would you like help in writing unit tests for this validation logic?

🧰 Tools
🪛 Biome (1.9.4)

[error] 62-62: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.

See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.

(lint/suspicious/noGlobalIsNan)


[error] 62-62: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.

See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.

(lint/suspicious/noGlobalIsNan)

🪛 GitHub Check: codecov/patch

[warning] 57-67: components/admins/modals/upgradeModal.tsx#L57-L67
Added lines #L57 - L67 were not covered by tests


336-336: Provide a default value for blockHeight in the label

When blockHeight is undefined, the label displays "HEIGHT (Current: undefined)". To enhance user experience, consider providing a default value like "..." or "Loading".

Apply this diff to handle the undefined case:

-label={`HEIGHT (Current: ${blockHeight})`}
+label={`HEIGHT (Current: ${blockHeight || '...'})`}
🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 336-336: components/admins/modals/upgradeModal.tsx#L336
Added line #L336 was not covered by tests

hooks/useQueries.ts (1)

962-982: Add unit tests for useBlockHeight hook

The new useBlockHeight hook is crucial for retrieving the current block height at regular intervals. Adding unit tests will ensure its reliability and help catch potential issues early.

Would you like assistance in creating unit tests for this hook?

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 962-982: hooks/useQueries.ts#L962-L982
Added lines #L962 - L982 were not covered by tests

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 12555f6 and 8bd59d2.

📒 Files selected for processing (3)
  • components/admins/modals/upgradeModal.tsx (7 hunks)
  • components/react/inputs/BaseInput.tsx (1 hunks)
  • hooks/useQueries.ts (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • components/react/inputs/BaseInput.tsx
🧰 Additional context used
🪛 GitHub Check: codecov/patch
hooks/useQueries.ts

[warning] 962-982: hooks/useQueries.ts#L962-L982
Added lines #L962 - L982 were not covered by tests

components/admins/modals/upgradeModal.tsx

[warning] 30-32: components/admins/modals/upgradeModal.tsx#L30-L32
Added lines #L30 - L32 were not covered by tests


[warning] 35-35: components/admins/modals/upgradeModal.tsx#L35
Added line #L35 was not covered by tests


[warning] 40-42: components/admins/modals/upgradeModal.tsx#L40-L42
Added lines #L40 - L42 were not covered by tests


[warning] 57-67: components/admins/modals/upgradeModal.tsx#L57-L67
Added lines #L57 - L67 were not covered by tests


[warning] 77-78: components/admins/modals/upgradeModal.tsx#L77-L78
Added lines #L77 - L78 were not covered by tests


[warning] 110-115: components/admins/modals/upgradeModal.tsx#L110-L115
Added lines #L110 - L115 were not covered by tests


[warning] 117-138: components/admins/modals/upgradeModal.tsx#L117-L138
Added lines #L117 - L138 were not covered by tests


[warning] 144-144: components/admins/modals/upgradeModal.tsx#L144
Added line #L144 was not covered by tests


[warning] 181-188: components/admins/modals/upgradeModal.tsx#L181-L188
Added lines #L181 - L188 were not covered by tests


[warning] 212-221: components/admins/modals/upgradeModal.tsx#L212-L221
Added lines #L212 - L221 were not covered by tests


[warning] 227-227: components/admins/modals/upgradeModal.tsx#L227
Added line #L227 was not covered by tests


[warning] 336-336: components/admins/modals/upgradeModal.tsx#L336
Added line #L336 was not covered by tests


[warning] 338-338: components/admins/modals/upgradeModal.tsx#L338
Added line #L338 was not covered by tests


[warning] 342-342: components/admins/modals/upgradeModal.tsx#L342
Added line #L342 was not covered by tests

🪛 Biome (1.9.4)
components/admins/modals/upgradeModal.tsx

[error] 62-62: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.

See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.

(lint/suspicious/noGlobalIsNan)


[error] 62-62: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.

See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.

(lint/suspicious/noGlobalIsNan)

🔇 Additional comments (1)
components/admins/modals/upgradeModal.tsx (1)

77-78: Handle potential undefined blockHeight

The blockHeight retrieved from useBlockHeight might be undefined if the query fails or is loading. Ensure that the code handles this scenario to prevent unexpected behavior.

Please verify that blockHeight is defined before using it in the component.

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 77-78: components/admins/modals/upgradeModal.tsx#L77-L78
Added lines #L77 - L78 were not covered by tests

Comment on lines +30 to +35
const nameMatch = body.match(/- \*\*Upgrade Handler Name\*\*: (.*?)(?:\r?\n|$)/);
const upgradeableMatch = body.match(/- \*\*Upgradeable\*\*: (.*?)(?:\r?\n|$)/);
const commitHashMatch = body.match(/- \*\*Commit Hash\*\*: (.*?)(?:\r?\n|$)/);

if (!nameMatch || !upgradeableMatch || !commitHashMatch) {
console.warn('Failed matches:', { nameMatch, upgradeableMatch, commitHashMatch });
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add unit tests for parseReleaseBody function

The parseReleaseBody function has been updated with new regex patterns and logging for failed matches. Adding unit tests will ensure that it accurately parses the release body in various scenarios and handles edge cases effectively.

Would you like assistance in creating unit tests for this function?

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 30-32: components/admins/modals/upgradeModal.tsx#L30-L32
Added lines #L30 - L32 were not covered by tests


[warning] 35-35: components/admins/modals/upgradeModal.tsx#L35
Added line #L35 was not covered by tests

components/admins/modals/upgradeModal.tsx Outdated Show resolved Hide resolved
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Signed-off-by: Félix C. Morency <[email protected]>
@fmorency fmorency merged commit fe9bf87 into main Jan 8, 2025
7 of 8 checks passed
@fmorency fmorency deleted the chalabi/upgrade-releases branch January 8, 2025 14:26
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.

2 participants