Fix component load bugs #1265
Fix component load bugs #1265
Conversation
| if (newData.current_index == null) { | ||
| newData.signing_key = validatorsFromValidations[d].signing_key | ||
| } |
There was a problem hiding this comment.
why can we remove this part? it seems like the properties that are being set have changed with this logic change now
There was a problem hiding this comment.
signing_key was never a property on validatorsFromValidations[d]. I don't know why this was added during recent refactor, but if we go further down the rendering part, master_key is given priority irrespective of the source of data.
There was a problem hiding this comment.
If signing_key property is not defined, then this line will not render the validator component.
explorer/src/containers/Validators/index.tsx
Line 207 in 8d317d9
There was a problem hiding this comment.
If signing_key property is not defined, then this line will not render the validator component.
master_key is given preference, and it will render the component. Won't render when both are not present.
In any case signing_key was not set by the code prior to #1232 and it's incorrectly set currently. So why the concern now?
achowdhry-ripple
left a comment
achowdhry-ripple
left a comment
There was a problem hiding this comment.
This is a good bug catch, LGTM overall
| expect(wrapper.find('.ledgers').length).toBe(1) | ||
| expect(wrapper.find('.ledger-list').length).toBe(1) | ||
| expect(wrapper.find('.ledger').length).toBe(1) | ||
| expect(wrapper.find('.ledger').length).toBe(2) |
There was a problem hiding this comment.
What are the two .ledger react components in the wrapper? Since we send one prevLedgerMessage, shouldn't we expect to see one of the .ledger react component?
There was a problem hiding this comment.
For the same reason 2 components were expected here -
One is the latest validated ledger and another from prevLedgerMessage on same reasoning -
| if (newData.current_index == null) { | ||
| newData.signing_key = validatorsFromValidations[d].signing_key | ||
| } |
There was a problem hiding this comment.
If signing_key property is not defined, then this line will not render the validator component.
explorer/src/containers/Validators/index.tsx
Line 207 in 8d317d9
| const { t } = useTranslation() | ||
| const { validators: validatorsFromValidations, metrics } = useStreams() | ||
| const { validators: validatorsFromVHS, unl } = useVHSValidators() | ||
| const [feeSettings, setFeeSettings] = useState<FeeSettings>() |
There was a problem hiding this comment.
Amongst the diff'ed lines in this file, what is the fix for this bug:
Loads Validator table with VHS data if data from live websocket connection is not available yet.
I'm trying to understand the goal of the changes in this file.
There was a problem hiding this comment.
Goal is to return the data from VHS even if data from clio is yet not available. It used to do that previous to #1232. You can test it by running it locally on a previous commit.
The difference in livenet tab and localhost tab in the video will make it more clear.
High Level Overview of Change
This PR addresses following items:
Context of Change
Type of Change
Codebase Modernization
Before / After
Before and after are captured in this video
Screen.Recording.2025-12-02.at.5.47.45.PM.mov
Test Plan
Added and updated tests