EnxDev commented on code in PR #45337:
URL: https://github.com/apache/superset/pull/45337#discussion_r4302559999
##########
superset-frontend/src/pages/ActionLog/index.tsx:
##########
@@ -215,24 +226,42 @@ function ActionLogList() {
row: {
original: { json },
},
- }: any) => (
- <Typography.Text
- css={css`
- .ant-typography-copy {
- visibility: hidden;
- }
- &:hover .ant-typography-copy {
- visibility: visible;
- }
- `}
- copyable={!!json}
- ellipsis={{
- tooltip: { styles: { root: { maxWidth: '900px' } }, title: json
},
- }}
- >
- {json}
- </Typography.Text>
- ),
+ }: any) => {
+ const jsonObject = safeJsonObjectParse(json);
+ if (jsonObject) {
+ return (
+ <JsonCellTrigger>
+ <JsonModal
Review Comment:
Not acceptable, agreed. `convertBigIntStrToNumber` was unquoting any
`"<digits>"` value, but json-bigint (with `storeAsString`) only turns number
literals longer than 15 characters into strings. So the function now only
unquotes digit strings of 16+ characters, which is the only case where the
parser could have made the string. With your example payload the tree shows
`"123"` for `dashboard_id` and `123` for `count`.
There's one leftover edge case: a real string of 16+ digits still loses its
quotes, because after `storeAsString` there's nothing left to tell it apart
from a big number. A full fix would mean parsing to native `BigInt` and
teaching the tree to render it. That changes what `safeJsonObjectParse` returns
for SQL Lab result cells too, so I'd rather do it separately if it turns out to
matter.
The Copy button was already fine, since it copies the raw payload and not
the tree. It now also matches what the tree shows for these values.
I added a unit test for the length threshold and an ActionLog test with
`{"dashboard_id":"123","count":123}`. Both failed before the change. Fixed in
b4de435.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
