-
Notifications
You must be signed in to change notification settings - Fork 53
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
Wizard: Add kernel append input (HMS-5299) #2734
Conversation
/jira-epic HMS-5192 |
6199c02
to
418f4aa
Compare
418f4aa
to
265fa11
Compare
Codecov ReportAttention: Patch coverage is
@@ Coverage Diff @@
## main #2734 +/- ##
==========================================
+ Coverage 84.77% 84.81% +0.04%
==========================================
Files 187 187
Lines 21298 21388 +90
Branches 2099 2120 +21
==========================================
+ Hits 18055 18141 +86
- Misses 3221 3225 +4
Partials 22 22
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
I think there is a bug when creating an image with OScap -> required kernel added to the kernel step, |
setErrorText(''); | ||
} | ||
|
||
if (kernelAppend.some((arg) => arg.name === value)) { |
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.
should I see this error in case I am adding the same kernel name?
now I dont see any error when adding the same name
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.
Hmm, yeah, we don't want duplicates. Will add a check for that!
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.
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.
I tried 'test' value
Screen.Recording.2025-01-14.at.14.03.32.mov
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.
ok, I got you, I think I check that I am adding 2 times the same value 'test',
if I tried to add same value that comes from the oscap profile the validation also doesnt work for me.
but, maybe we also want to check if user try to add same value ?
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.
Aah, gotcha, the validation was triggered only on keyDown 😳 will add a fix in a bit. Nice catch, thanks!
Ooh, I left the default parsing there, didn't I? Great catch! Will revert this to draft |
265fa11
to
74217f1
Compare
74217f1
to
b6c7247
Compare
728676c
to
60653e7
Compare
The chipping input might be extractable from this one as well, moving to draft for the time being. #2747 will need to get merged first. |
60653e7
to
526afe0
Compare
7bab478
to
f1fb5ff
Compare
2fbfe23
to
c0c13fc
Compare
c0c13fc
to
a1415d7
Compare
/retest |
@@ -64,7 +66,7 @@ const ChippingInput = ({ | |||
}; | |||
|
|||
const handleKeyDown = (e: React.KeyboardEvent, value: string) => { | |||
if (e.key === ' ' || e.key === 'Enter' || e.key === ',') { | |||
if (e.key === ' ' || e.key === 'Enter') { |
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.
Comma should be a valid character for kernel argument, I've removed it from the keyDown for the consistency sake.
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.
Looks good, just a small nitpick that I added inline, but should block the PR.
src/Components/CreateImageWizard/steps/Kernel/components/KernelArguments.tsx
Outdated
Show resolved
Hide resolved
/retest |
This adds the kernel append input. New arguments can be added by pressing the "Add" button or hitting Enter after the argument. The kernel arguments linked to a selected OpenSCAP profile are rendered in a category marked as "Required by OpenSCAP" and are read only.
Removed redundant code and updated `addItem` to check for duplicate arguments in both required and non-required list.
c8ac4e9
to
11b7f49
Compare
/retest |
This adds the kernel append input, removes kernel append from the OpenSCAP step and adds basic tests to cover the functionality.
TO DO:
JIRA: HMS-5299