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

fix: use total count to check if there's enough contracts #421

Merged
merged 1 commit into from
Nov 28, 2023

Conversation

multikatt
Copy link
Contributor

@multikatt multikatt commented Nov 26, 2023

datasetCount from useContracts() is using the filtered count which makes the onboarding panel appear.
Related to SiaFoundation/renterd#765

Copy link

changeset-bot bot commented Nov 26, 2023

🦋 Changeset detected

Latest commit: 382fa43

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
renterd Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@@ -3,7 +3,8 @@ import { useRedundancySettings } from '../../../hooks/useRedundancySettings'

export function useNotEnoughContracts() {
const redundancy = useRedundancySettings()
const { datasetCount, isLoading: isContractsLoading } = useContracts()
const { dataset, isLoading: isContractsLoading } = useContracts()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Im unsure if it's better to it this way or change datasetCount in useContracts() to use the unfiltered dataset, and another variable for pagination called datasetFiltererdCount and update that code as well.

Copy link
Member

Choose a reason for hiding this comment

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

Hey, thanks so much for the PR! I think it would be clearest to have datasetCount and datasetFilteredCount (and pageCount).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright, I've given that a try as well!

Copy link
Member

@alexfreska alexfreska left a comment

Choose a reason for hiding this comment

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

@multikatt perfect, could you add a changeset so that the bug fix is included in the release notes.

npx changeset

Use the arrow keys and hit spacebar to select renterd, skip major and minor version with enter key to select patch version bump. Enter a message like:

Fixed an issue where the filtered contract count was being used in the onboarding checks.

And then commit the generated file to this PR, it should be .changesets/some-strange-name.md

Copy link
Member

@alexfreska alexfreska left a comment

Choose a reason for hiding this comment

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

@multikatt looks great! The CI will probably fail because the second two commits are not formatted as conventional commits, maybe squash them into the first commit and force push, after that we are good to merge!

@multikatt
Copy link
Contributor Author

Hope I did that right 😃

@alexfreska
Copy link
Member

Thank you so much! 🚀

@alexfreska alexfreska merged commit f57ac16 into SiaFoundation:main Nov 28, 2023
1 check 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.

2 participants