-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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: updated parsing for brokerConfig and brokerSalesInfo #10926
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's always better to use the built in type structures, instead of calling toJSON
in order to get all the values.
@@ -11,17 +11,17 @@ import { createNamedHook, useApi, useCall } from '@polkadot/react-hooks'; | |||
import { stringToBN } from './utils/dataProcessing.js'; | |||
|
|||
function parseConfig (config: PalletBrokerConfigRecord): SimplifiedPalletBrokerConfigRecord { | |||
const c = config.toJSON(); | |||
const c = config?.toJSON(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should actually be like this:
function parseConfig (config: PalletBrokerConfigRecord): SimplifiedPalletBrokerConfigRecord {
return {
advanceNotice: config.advanceNotice.toNumber(),
contributionTimeout: config.contributionTimeout.toNumber(),
idealBulkProportion: stringToBN(config.idealBulkProportion?.toString()),
interludeLength: config.interludeLength.toNumber(),
leadinLength: config.leadinLength.toNumber(),
limitCoresOffered: config.limitCoresOffered.isSome ? config.limitCoresOffered.unwrap().toNumber() : 0,
regionLength: config.regionLength.toNumber(),
renewalBump: stringToBN(config.renewalBump?.toString())
};
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And if config can be undefined, then the param type needs to be updated.
record: PalletBrokerSaleInfoRecord | ||
): SimplifiedPalletBrokerSaleInfoRecord { | ||
const rec = record.toJSON(); | ||
function parsePalletBrokerSaleInfoRecord (record: PalletBrokerSaleInfoRecord): SimplifiedPalletBrokerSaleInfoRecord { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same with this as above.
regionBegin: record.regionBegin.toNumber(), | ||
regionEnd: record.regionEnd.toNumber(), | ||
saleStart: record.saleStart.toNumber(), | ||
selloutPrice: record.selloutPrice.isSome ? record.selloutPrice.unwrap() : 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small nit, but I think you can set the 0
to new BN(0)
if you want the values to be inline. That being said its a small nit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good nit 🤩
…s#10926) * updated parsing for brokerConfig and brokerSalesInfo * renamed file * updated parsing for brokerConfig and brokerSalesInfo * better type conversion for BN * BN type change * lint
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
No description provided.