feat: Edit correlation rule #30
Conversation
going-confetti
commented
Jul 12, 2024
•
going-confetti
commented
| <Routes> | ||
| <Route path="/" element={<ScrollableContent />}> | ||
| <Route path="rule/:id" element={<RuleForm />} /> | ||
| <Route path="loadProfile" element={<LoadProfile />} /> | ||
| <Route path="thresholds" element={<VariablesEditor />} /> | ||
| <Route path="thinkTime" element={<ThinkTime />} /> | ||
| <Route path="testData" element={<VariablesEditor />} /> | ||
| <Route path="imports" element={<ImportSelector />} /> | ||
| <Route path="requestFilters" element={<RequestFilters />} /> | ||
| </Route> | ||
| </Routes> |
There was a problem hiding this comment.
I think router simplifies handling drawer content, especially since we may need to open some of the tabs from elsewhere (rule from the list of rules, request filters from the list of requests etc.)
| // value is always a string in Radix Select | ||
| const handleFromChange = (value: string) => { | ||
| const type = allowedTypes[value as Selector['from']].includes(selector.type) |
There was a problem hiding this comment.
This is really annoying
There was a problem hiding this comment.
now I see why the typing issues 🫠
| style={{ | ||
| borderRadius: 'var(--radius-1)', | ||
| backgroundColor: 'var(--gray-2)', | ||
| border: '1px solid transparent', | ||
| borderColor: isSelected ? 'var(--accent-5)' : 'transparent', | ||
| backgroundColor: isSelected ? 'var(--accent-3)' : 'var(--gray-2)', | ||
| }} |
There was a problem hiding this comment.
Looks like we should add emotion already
| selectRule(ruleId) | ||
| navigate(`/generator/rule/${ruleId}`) |
There was a problem hiding this comment.
See my comment in useGeneratorStore
| case 'correlation': | ||
| return { | ||
| type: 'correlation', | ||
| id: self.crypto.randomUUID(), |
There was a problem hiding this comment.
regarding ids do we want to use UUIDs or actually numbers starting from 0 🤔
There was a problem hiding this comment.
I think ids don't need to be sequential, because the user can (or rather will be able to) rearrange rules as they want.
Will we store rule ids in the generator file or is it a runtime only thing? Right now, I can't think of a good reason to store ids, so UUIDs can definitely be an overkill.
There was a problem hiding this comment.
I agree, nothing comes up for storing IDs in the saved file
| case 'parameterization': | ||
| return { | ||
| type: 'parameterization', | ||
| id: self.crypto.randomUUID(), | ||
| filter: { path: '' }, | ||
| selector: { | ||
| type: 'begin-end', | ||
| from: 'body', | ||
| begin: '', | ||
| end: '', | ||
| }, | ||
| value: { type: 'variable', variableName: '' }, | ||
| } | ||
| case 'verification': | ||
| return { | ||
| type: 'verification', | ||
| id: self.crypto.randomUUID(), | ||
| filter: { path: '' }, | ||
| selector: { | ||
| type: 'begin-end', | ||
| from: 'body', | ||
| begin: '', | ||
| end: '', | ||
| }, | ||
| value: { | ||
| type: 'recordedValue', | ||
| }, | ||
| } |
There was a problem hiding this comment.
should we be hiding these since we don't yet support them ? 🤔
There was a problem hiding this comment.
They are disabled in the UI, but yeah, we should remove these options from there.
As for the code, cases for verification and parameterization were suggested by copilot and I left them here, because it was easier than resorting to hacks to make exhaustive work (the objects are correct, maybe with the exception of an empty variableName, but we'll probably have a check for that anyway since the user can remove a test variable that is used in a rule)
| // value is always a string in Radix Select | ||
| const handleFromChange = (value: string) => { | ||
| const type = allowedTypes[value as Selector['from']].includes(selector.type) |
There was a problem hiding this comment.
now I see why the typing issues 🫠
