Advanced watch cleanup #34955
Advanced watch cleanup #34955
Conversation
| @@ -1,30 +0,0 @@ | |||
| /* | |||
There was a problem hiding this comment.
since this is only being used in one place, i removed it from the constants directory
| } | ||
|
|
||
| if (prevWatchString !== watch.watchString && errors.json.length === 0) { | ||
| setWatchProperty('watch', JSON.parse(watch.watchString)); |
There was a problem hiding this comment.
I'm struggling with the best way to validate the watch json and update the watch accordingly. The ace editor requires a json string, and we also can't parse it and update the watch property unless it is valid json. This works, but not sure if there's a better approach.
There was a problem hiding this comment.
reworked this based on our discussion
💔 Build Failed |
There was a problem hiding this comment.
For the first Save modal, I'm worried that the title might get too long if the watch name is long. How about changing it to:
A watch with this ID already exists
Let's add a generic title to the other modal. Also, we'll need to edit the contents a bit
Save watch? --or-- Missing property value
This watch has a Slackmessage_default setting without a "to" property. If this property is already set in your elasticsearch.yml file, you're all set. Otherwise, you can include it here in the watch JSON. Learn more.
|
@gchaps good suggestions! The latest commit contains the changes.
|
💔 Build Failed |
|
@alisonelizabeth Is there a way to put text in code font (message_defaults) without highlighting it? |
💔 Build Failed |
|
@gchaps, good point. I checked the |
| @@ -38,7 +39,12 @@ const watchReducer = (state: any, action: any) => { | |||
| case 'setWatch': | |||
| return payload; | |||
| case 'setProperty': | |||
| return new (Watch.getWatchTypes())[state.type]({ ...state, ...payload }); | |||
| const { property, value } = payload; | |||
| if (!isEqual(state[property], value)) { | |||
There was a problem hiding this comment.
I would suggest flipping the logic here: if (isEqual(state[property], value)) { ...
There was a problem hiding this comment.
good point, fixed!
💔 Build Failed |
💚 Build Succeeded |
This PR reworks/fixes a few things:
lodashwhere applicable@gchaps - screenshots w/ updated text: