-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Uptime] Add basic ui monitor management table, basic monitor editing, and basic location selection #120038
[Uptime] Add basic ui monitor management table, basic monitor editing, and basic location selection #120038
Conversation
export * from './state'; | ||
|
||
export const MonitorManagementListResultType = t.type({ | ||
monitors: t.array(t.unknown), |
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.
Need runtime types for this.
}: Props) => any = ({ monitorList: { list, error, loading }, pageSize, setPageSize }) => { | ||
const items = list.monitors ?? []; | ||
|
||
const columns = [ |
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.
All of the types for the render functions should be replaced with types from the runtime monitor type
…tor-management-table
pathname: MONITOR_ADD_ROUTE, | ||
})} | ||
> | ||
Add monitor |
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.
i18n
monitorType, | ||
tlsConfig: defaultTLSConfig, | ||
} = useMemo(() => { | ||
/* TODO: fetch current monitor to be edited from saved objects based on url param */ |
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.
is this TODO still applicable?
<EuiComboBox | ||
placeholder={PLACEHOLDER_LABEL} | ||
options={locations} | ||
selectedOptions={selectedLocations} | ||
inputRef={setLocationsInputRef} | ||
onChange={onLocationChange} | ||
onSearchChange={onSearchChange} | ||
onBlur={onBlur} | ||
data-test-subj="syntheticsServiceLocationsComboBox" | ||
/> |
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.
this should have a required or invalid state, since there should always be one location. Though i suspect combobox isn't the most ideal component, but let's move forward and iterate.
I think we will end up with something like checkbox for each location, and initially all of them selected and then user can uncheck some.
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.
With a combobox, we certainly can have some initial state where all or the most common locations are selected, so I don't think it has to be checkboxes. Iterating sounds good.
const monitorList = useSelector(monitorManagementListSelector); | ||
|
||
useEffect(() => { | ||
if (refresh) { |
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 think refresh will always be there?
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.
After this effect runs, it sets refresh to false
. I pass setRefresh
to children, so when things like pagination or deleting monitors happenes, it calls setRefresh
and sets it back to true
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 think i confused it with lastRefresh
builder | ||
.addCase(getMonitors, (state: WritableDraft<MonitorManagementList>) => ({ | ||
...state, | ||
loading: { | ||
...state.loading, | ||
monitorList: true, | ||
}, | ||
})) | ||
.addCase( |
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.
Love it ❤️
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.
If I'm being honest, it hasn't saved me very much time writing boilerplate yet, but maybe that's because everything else still isn't using redux toolkit.
@@ -14,5 +14,5 @@ export const getServiceLocationsRoute: UMRestApiRouteFactory = () => ({ | |||
path: API_URLS.SERVICE_LOCATIONS, | |||
validate: {}, | |||
handler: async ({ server }): Promise<any> => | |||
getServiceLocations({ manifestUrl: server.config.service.manifestUrl }), | |||
getServiceLocations({ manifestUrl: server.config.unsafe.service.manifestUrl }), |
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.
told you this type isn't working. Something weird is going on, i will investigate.
Pinging @elastic/uptime (Team:uptime) |
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.
LGTM, looks great, really excited about it !!
WFG, pending some tests failures !!
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: |
💔 Backport failedThe backport operation could not be completed due to the following error: The backport PRs will be merged automatically after passing CI. To backport manually run: |
…, and basic location selection (elastic#120038) * add basic ui monitor management table * Creating io-ts types for synthetics monitors. * add runtime types * adjust types in components * add monitor editing and deleting * add management link * add monitor btn * fix conflicts * fix conflicts * hide * add tags component * add locations * add pagination * adjust types * add unit tests * update tests * adjust types and i18n * fix types * eslint * update i18n * update i18n * update monitor types Co-authored-by: Abdul Zahid <[email protected]> Co-authored-by: shahzad31 <[email protected]>
Summary
Fixes elastic/uptime#413
Fixes elastic/uptime#412
Fixes elastic/uptime#416