Skip to content

Commit

Permalink
fix: renterd recommendations states and crashing
Browse files Browse the repository at this point in the history
  • Loading branch information
alexfreska committed May 8, 2024
1 parent e989296 commit c89d2e5
Show file tree
Hide file tree
Showing 6 changed files with 143 additions and 41 deletions.
5 changes: 5 additions & 0 deletions .changeset/fast-colts-turn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'renterd': patch
---

Fixed an issue where the recommendations bar was overlapping notifications.
5 changes: 5 additions & 0 deletions .changeset/old-candles-remember.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'renterd': patch
---

Fixed an issue where clearing some configuration fields would crash the app due to how recommendations were being calculated.
5 changes: 5 additions & 0 deletions .changeset/proud-dancers-train.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'renterd': patch
---

The configuration recommendations now instruct the user to first fill all fields. Closes https://github.com/SiaFoundation/renterd/issues/1214
81 changes: 49 additions & 32 deletions apps/renterd/components/Config/Recommendations.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
CaretDown16,
Information20,
CheckmarkFilled20,
PendingFilled20,
} from '@siafoundation/react-icons'
import { useApp } from '../../contexts/app'
import { routes } from '../../config/routes'
Expand All @@ -33,6 +34,7 @@ export function Recommendations() {
const {
hostMargin50,
hostTarget50,
hasDataToEvaluate,
needsRecommendations,
foundRecommendation,
recommendations,
Expand Down Expand Up @@ -113,6 +115,27 @@ export function Recommendations() {
</div>
)

if (!hasDataToEvaluate) {
return (
<Layout
maximized={maximized}
setMaximized={setMaximized}
maximizeControls={false}
title={
<>
<Text color="contrast">
<PendingFilled20 />
</Text>
<Text size="16" weight="medium">
The system will review your configuration once all fields are
filled
</Text>
</>
}
/>
)
}

if (!needsRecommendations) {
return (
<Layout
Expand Down Expand Up @@ -239,43 +262,37 @@ function Layout({
setMaximized: (maximized: boolean) => void
maximizeControls: boolean
title: React.ReactNode
tip: React.ReactNode
tip?: React.ReactNode
}) {
const el = (
<div
className={cx(
'flex justify-between items-center px-3 py-1.5',
maximized && children
? 'border-b border-gray-200 dark:border-graydark-300'
: '',
maximizeControls ? 'cursor-pointer' : ''
)}
onClick={() => {
if (maximizeControls) {
setMaximized(!maximized)
}
}}
>
<div className={cx('flex gap-2 items-center')}>{title}</div>
{maximizeControls && (
<Button variant="ghost" onClick={() => setMaximized(!maximized)}>
{maximized ? <Subtract24 /> : <Add24 />}
</Button>
)}
</div>
)
return (
<div className="relative">
<div className="z-20 absolute top-0 left-1/2 -translate-x-1/2 flex justify-center">
<div className="z-10 absolute top-0 left-1/2 -translate-x-1/2 flex justify-center">
<div className="w-[600px] flex flex-col max-h-[600px] bg-gray-50 dark:bg-graydark-50 border-b border-x border-gray-300 dark:border-graydark-400 rounded-b">
<ScrollArea>
<HoverCard
trigger={
<div
className={cx(
'flex justify-between items-center px-3 py-1.5',
maximized && children
? 'border-b border-gray-200 dark:border-graydark-300'
: '',
maximizeControls ? 'cursor-pointer' : ''
)}
onClick={() => {
if (maximizeControls) {
setMaximized(!maximized)
}
}}
>
<div className={cx('flex gap-2 items-center')}>{title}</div>
{maximizeControls && (
<Button
variant="ghost"
onClick={() => setMaximized(!maximized)}
>
{maximized ? <Subtract24 /> : <Add24 />}
</Button>
)}
</div>
}
>
{tip}
</HoverCard>
{tip ? <HoverCard trigger={el}>{tip}</HoverCard> : el}
{children}
</ScrollArea>
</div>
Expand Down
79 changes: 71 additions & 8 deletions apps/renterd/contexts/config/useAutopilotEvaluations.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,31 @@ export function useAutopilotEvaluations({
}) {
const values = form.watch()
const renterdState = useBusState()
const payloads = useMemo(() => {

const hasDataToEvaluate = useMemo(() => {
if (!checkIfAllResourcesLoaded(resources)) {
return
return false
}
if (!form.formState.isValid) {
return
return false
}
if (!renterdState.data) {
return false
}
return true
}, [form, resources, renterdState])

// We need to pass valid settings data into transformUp to get the payloads.
// The form can be invalid so we need to merge in valid data to make sure
// numbers are not undefined in the transformUp calculations that assume
// all data is valid and present.
const currentValuesWithZeroDefaults = useMemo(
() => mergeIfDefined(valuesZeroDefaults, values),
[values]
)

const payloads = useMemo(() => {
if (!hasDataToEvaluate) {
return
}

Expand All @@ -46,17 +63,17 @@ export function useAutopilotEvaluations({
isAutopilotEnabled,
showAdvanced,
estimatedSpendingPerMonth,
values,
values: currentValuesWithZeroDefaults,
})
return payloads
}, [
form,
values,
currentValuesWithZeroDefaults,
resources,
renterdState,
isAutopilotEnabled,
showAdvanced,
estimatedSpendingPerMonth,
hasDataToEvaluate,
])

const userContractCountTarget = payloads?.autopilot.contracts.amount || 0
Expand Down Expand Up @@ -187,15 +204,20 @@ export function useAutopilotEvaluations({
const rec = getRecommendationItem({
key: remoteToLocalFields[key],
recommendationDown,
values,
values: currentValuesWithZeroDefaults,
})
if (rec) {
recs.push(rec)
}
}
})
return recs
}, [recommendedGougingSettings, resources, payloads, values])
}, [
recommendedGougingSettings,
resources,
payloads,
currentValuesWithZeroDefaults,
])
const foundRecommendation = !!recommendations.length

return {
Expand All @@ -206,6 +228,7 @@ export function useAutopilotEvaluations({
usableHostsCurrent,
userContractCountTarget,
usableHostsAfterRecommendation,
hasDataToEvaluate,
needsRecommendations,
foundRecommendation,
recommendations,
Expand Down Expand Up @@ -283,3 +306,43 @@ const remoteToLocalFields: Record<keyof GougingSettings, keyof Fields> = {
minMaxEphemeralAccountBalance: 'minMaxEphemeralAccountBalance',
migrationSurchargeMultiplier: 'migrationSurchargeMultiplier',
}

export const valuesZeroDefaults: SettingsData = {
autopilotContractSet: '',
amountHosts: new BigNumber(0),
allowanceMonth: new BigNumber(0),
periodWeeks: new BigNumber(0),
renewWindowWeeks: new BigNumber(0),
downloadTBMonth: new BigNumber(0),
uploadTBMonth: new BigNumber(0),
storageTB: new BigNumber(0),
prune: false,
allowRedundantIPs: false,
maxDowntimeHours: new BigNumber(0),
minRecentScanFailures: new BigNumber(0),
minProtocolVersion: '',
defaultContractSet: '',
uploadPackingEnabled: true,
maxRpcPriceMillion: new BigNumber(0),
maxStoragePriceTBMonth: new BigNumber(0),
maxContractPrice: new BigNumber(0),
maxDownloadPriceTB: new BigNumber(0),
maxUploadPriceTB: new BigNumber(0),
hostBlockHeightLeeway: new BigNumber(0),
minPriceTableValidityMinutes: new BigNumber(0),
minAccountExpiryDays: new BigNumber(0),
minMaxEphemeralAccountBalance: new BigNumber(0),
migrationSurchargeMultiplier: new BigNumber(0),
minShards: new BigNumber(0),
totalShards: new BigNumber(0),
}

export function mergeIfDefined(defaults: SettingsData, values: SettingsData) {
const merged: SettingsData = { ...defaults }
Object.keys(values).forEach((key) => {
if (values[key] !== undefined) {
merged[key] = values[key]
}
})
return merged
}
9 changes: 8 additions & 1 deletion apps/renterd/contexts/config/useForm.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useMemo } from 'react'
import { useEffect, useMemo } from 'react'
import { defaultValues, getAdvancedDefaults } from './types'
import { getRedundancyMultiplier } from './utils'
import { useForm as useHookForm } from 'react-hook-form'
Expand Down Expand Up @@ -46,6 +46,13 @@ export function useForm({ resources }: { resources: Resources }) {
}
)

// Trigger input validation on showAdvanced change because many field validation
// objects switch from required to not required.
useEffect(() => {
form.trigger()
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [showAdvanced])

const estimates = useEstimates({
isAutopilotEnabled,
redundancyMultiplier,
Expand Down

0 comments on commit c89d2e5

Please sign in to comment.