kdump: Port KdumpTestDialog to Dialogs API - #23421
Conversation
| const [task, setTask] = useState(null); | ||
|
|
||
| function crash() { | ||
| setTask(onCrashKernel() |
There was a problem hiding this comment.
This doesn't show any error in the dialog when onCrashKernel() throws an exception, right?
I think the old code didn't do that either, and would actually crash JavaScript in that case, because it doesn't provide a set_error function in the footer props. Hmm,
There was a problem hiding this comment.
I think the old code didn't do that either, and would actually crash JavaScript in that case, because it doesn't provide a
set_errorfunction in the footer props. Hmm,
No, show_modal_dialog sets set_error by itself. So we are loosing error reporting here.
mvollmer
left a comment
mvollmer
left a comment
There was a problem hiding this comment.
The dialog needs to continue to show errors from running the crashkernel.sh script.
Move away from the `show_modal_dialog` to the new Dialog API. Related: COCKPIT-1886
8a4ad32 to
855a949
Compare
| const KdumpTestDialog = ({ verifyMessage, onCrashKernel }) => { | ||
| const Dialogs = useDialogs(); | ||
| const [task, setTask] = useState(null); | ||
| const [error, setError] = useState(null); |
There was a problem hiding this comment.
These 4 added lines are not executed by any test. Details
| function crash() { | ||
| setError(null); | ||
| setTask(onCrashKernel() | ||
| .then(Dialogs.close) | ||
| .catch(error => { | ||
| setTask(null); | ||
| setError(error); |
There was a problem hiding this comment.
These 7 added lines are not executed by any test. Details
| return ( | ||
| <Modal position="top" variant="small" id="kdump-test-dialog" isOpen | ||
| onClose={Dialogs.close}> | ||
| <ModalHeader title={_("Test kdump settings")} titleIconVariant="warning" /> | ||
| <ModalBody> | ||
| {error && <ModalError dialogError={error.message || error} />} | ||
| <Content> | ||
| <Content component={ContentVariants.p}> | ||
| {_("Test kdump settings by crashing the kernel. This may take a while and the system might not automatically reboot. Do not purposefully crash the system while any important task is running.")} | ||
| </Content> | ||
| {verifyMessage && <Content component={ContentVariants.p}> | ||
| {verifyMessage} | ||
| </Content>} | ||
| </Content> | ||
| </ModalBody> | ||
| <ModalFooter> | ||
| <Button variant="danger" | ||
| isLoading={!!task} | ||
| isDisabled={!!task} | ||
| onClick={crash}> | ||
| {_("Crash system")} | ||
| </Button> | ||
| <Button variant="link" | ||
| isDisabled={!!task} | ||
| onClick={Dialogs.close}> | ||
| {_("Cancel")} | ||
| </Button> | ||
| </ModalFooter> | ||
| </Modal>); |
There was a problem hiding this comment.
These 29 added lines are not executed by any test. Details
| } | ||
|
|
||
| handleTestSettingsClick() { | ||
| const Dialogs = this.context; |
There was a problem hiding this comment.
This added line is not executed by any test. Details
| Dialogs.show(<KdumpTestDialog verifyMessage={verifyMessage} | ||
| onCrashKernel={this.props.onCrashKernel} />); |
There was a problem hiding this comment.
These 2 added lines are not executed by any test. Details
|
@tomasmatus maybe you can take over the review? |
3 participants
Move away from the
show_modal_dialogto the new Dialog API.Related: COCKPIT-1886