Skip to content
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

Conversation

dominiqueclarke
Copy link
Contributor

@dominiqueclarke dominiqueclarke commented Nov 30, 2021

export * from './state';

export const MonitorManagementListResultType = t.type({
monitors: t.array(t.unknown),
Copy link
Contributor Author

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 = [
Copy link
Contributor Author

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

pathname: MONITOR_ADD_ROUTE,
})}
>
Add monitor
Copy link
Contributor

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 */
Copy link
Contributor

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?

Comment on lines +45 to +54
<EuiComboBox
placeholder={PLACEHOLDER_LABEL}
options={locations}
selectedOptions={selectedLocations}
inputRef={setLocationsInputRef}
onChange={onLocationChange}
onSearchChange={onSearchChange}
onBlur={onBlur}
data-test-subj="syntheticsServiceLocationsComboBox"
/>
Copy link
Contributor

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.

Copy link
Contributor Author

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) {
Copy link
Contributor

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?

Copy link
Contributor Author

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

Copy link
Contributor

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

Comment on lines +46 to +54
builder
.addCase(getMonitors, (state: WritableDraft<MonitorManagementList>) => ({
...state,
loading: {
...state.loading,
monitorList: true,
},
}))
.addCase(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love it ❤️

Copy link
Contributor Author

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 }),
Copy link
Contributor

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.

@dominiqueclarke dominiqueclarke changed the title add basic ui monitor management table [Uptime] Add basic ui monitor management table, basic monitor editing, and basic location selection Dec 8, 2021
@dominiqueclarke dominiqueclarke marked this pull request as ready for review December 8, 2021 03:23
@dominiqueclarke dominiqueclarke requested a review from a team as a code owner December 8, 2021 03:23
@dominiqueclarke dominiqueclarke added Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v8.1.0 labels Dec 8, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/uptime (Team:uptime)

@dominiqueclarke dominiqueclarke added release_note:skip Skip the PR/issue when compiling release notes auto-backport Deprecated - use backport:version if exact versions are needed enhancement New value added to drive a business result labels Dec 8, 2021
Copy link
Contributor

@shahzad31 shahzad31 left a 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 !!

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
uptime 648 671 +23

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
uptime 601.7KB 685.5KB +83.9KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
uptime 23.2KB 23.3KB +55.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@dominiqueclarke dominiqueclarke merged commit e667c7f into elastic:main Dec 8, 2021
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Dec 8, 2021
@dominiqueclarke dominiqueclarke deleted the feature/ui-monitor-management-table branch December 8, 2021 17:06
@kibanamachine
Copy link
Contributor

💔 Backport failed

The backport operation could not be completed due to the following error:
There are no branches to backport to. Aborting.

The backport PRs will be merged automatically after passing CI.

To backport manually run:
node scripts/backport --pr 120038

TinLe pushed a commit to TinLe/kibana that referenced this pull request Dec 22, 2021
…, 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 <awahab07@yahoo.com>
Co-authored-by: shahzad31 <shahzad.muhammad@elastic.co>
gbamparop pushed a commit to gbamparop/kibana that referenced this pull request Jan 12, 2022
…, 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 <awahab07@yahoo.com>
Co-authored-by: shahzad31 <shahzad.muhammad@elastic.co>
@tylersmalley tylersmalley added ci:cloud-deploy Create or update a Cloud deployment and removed ci:deploy-cloud labels Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed backport:skip This commit does not require backporting ci:cloud-deploy Create or update a Cloud deployment enhancement New value added to drive a business result release_note:skip Skip the PR/issue when compiling release notes Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v8.1.0
Projects
None yet
7 participants