-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Fix inputs extractors list not updating after deletion #17289
Conversation
36f1bde
to
a2acbcb
Compare
a2acbcb
to
b80d396
Compare
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.
First of all: Code looks good, but I think this PR is a good opportunity to migrate both, ExtractorsList
and ExtractorsStore
to typescript. ExtractorsList
is also not too big and it's already a functional component.
Testing results
Prerequisite:
- Created a random HTTP Input and clicked on "Manage Extractors" on the Input List
✅ When an extractor is deleted it disappears from the list
✅ An extractor can be created and is shown in the list (tested with substring extractor)
✅ An extractor can be edited (e.g. updated name) and the changes are reflected in the list
✅ Clicking on the Details
button in the extractor toggles the detail view (shows and hides)
❌ I discovered one bug, where I cannot clearly tell how I got to this but what I did was:
- Create an extractor
- Click on edit (it works)
- Delete it
- Create another extractor
When I click on edit I get a console error and am not redirected to the edit view.
So far I am not able to get out of this state, even deleting the extractor again, creating a new one, making sure it has a different name, etc.
react-dom.development.js:19527 The above error occurred in the <EditExtractorsPage> component:
in EditExtractorsPage
in Unknown
in Unknown
in Suspense
in ErrorBoundary
in Unknown (created by AppRouter)
in RenderedRoute (created by DataRoutes)
in Outlet (created by PageContentLayout)
in div (created by Grid)
in Grid (created by Styled(Grid))
in Styled(Grid) (created by PageContentLayout)
in WithGlobalNotifications (created by PageContentLayout)
in div (created by styled.div)
in styled.div (created by PageContentLayout)
in PageContentLayout (created by AppRouter)
in RenderedRoute (created by DataRoutes)
in Outlet (created by Context.Consumer)
in div (created by styled.div)
in styled.div (created by Context.Consumer)
in RuntimeErrorBoundary (created by Context.Consumer)
in ReportedErrorBoundary (created by Context.Consumer)
in div (created by styled.div)
in styled.div (created by Context.Consumer)
in ScratchpadProvider (created by Context.Consumer)
in CustomHotkeysProvider (created by HotkeysProvider)
in BoundHotkeysProxyProviderProvider (created by HotkeysProvider)
in HotkeysProvider (created by HotkeysProvider)
in HotkeysProvider (created by Context.Consumer)
in PerspectivesProvider (created by Context.Consumer)
in QueryParamProviderInner (created by ReactRouter6Adapter)
in ReactRouter6Adapter (created by QueryParamProvider)
in QueryParamProvider (created by App)
in App (created by AppRouter)
in LicenseCheckProvider (created by GlobalContextProviders)
in ErrorBoundary (created by GlobalContextProviders)
in InvestigationDrawerProvider (created by GlobalContextProviders)
in ErrorBoundary (created by GlobalContextProviders)
in PaginationProvider (created by GlobalContextProviders)
in ErrorBoundary (created by GlobalContextProviders)
in GlobalContextProviders (created by AppRouter)
in RenderedRoute (created by DataRoutes)
in RenderErrorBoundary (created by DataRoutes)
in DataRoutes (created by RouterProvider)
in Router (created by RouterProvider)
in RouterProvider (created by AppRouter)
in RouterErrorBoundary (created by AppRouter)
in AppRouter (created by LoggedInPage)
in InputsProvider (created by LoggedInPage)
in Unknown
in Unknown (created by NodesProvider)
in NodesProvider (created by LoggedInPage)
in StreamsProvider (created by ConnectStoresWrapper[StreamsProvider] stores=streams)
in ConnectStoresWrapper[StreamsProvider] stores=streams (created by LoggedInPage)
in TelemetryProvider (created by LoggedInPage)
in tt (created by GraylogThemeProvider)
in ThemeProvider (created by @mantine/core/MantineProvider)
in @mantine/core/MantineProvider (created by GraylogThemeProvider)
in GraylogThemeProvider (created by ThemeAndUserProvider)
in CurrentUserPreferencesProvider (created by ThemeAndUserProvider)
in StaticTimezoneProvider (created by CurrentUserDateTimeProvider)
in CurrentUserDateTimeProvider (created by UserDateTimeProvider)
in UserDateTimeProvider (created by ThemeAndUserProvider)
in CurrentUserProvider (created by ThemeAndUserProvider)
in ThemeAndUserProvider (created by LoggedInPage)
in QueryClientProvider (created by DefaultQueryClientProvider)
in DefaultQueryClientProvider (created by LoggedInPage)
in LoggedInPage
in Suspense
in ErrorBoundary
in Unknown (created by AppFacade)
in AppFacade
in tt (created by GraylogThemeProvider)
in ThemeProvider (created by @mantine/core/MantineProvider)
in @mantine/core/MantineProvider (created by GraylogThemeProvider)
in GraylogThemeProvider
in QueryClientProvider (created by LoginQueryClientProvider)
in LoginQueryClientProvider
in PostHogProvider (created by TelemetryInit)
in TelemetryInit
in CustomizationProvider
React will try to recreate this component tree from scratch using the error boundary you provided, ErrorBoundary.
logCapturedError @ react-dom.development.js:19527
loadAsync.tsx:28 Invariant Violation: mergeIntoWithNoDuplicateKeys(): Tried to merge two objects with the same key: `extractor`. This conflict may be due to a mixin; in particular, this may be caused by two getInitialState() or getDefaultProps() methods returning objects with clashing keys.
at _invariant (webpack://__/./node_modules/create-react-class/factory.js?:41:15)
at mergeIntoWithNoDuplicateKeys (webpack://__/./node_modules/create-react-class/factory.js?:689:9)
at ReactClassComponent.mergedResult [as getInitialState] (webpack://__/./node_modules/create-react-class/factory.js?:723:7)
at new eval (webpack://__/./node_modules/create-react-class/factory.js?:911:54)
at constructClassInstance (webpack://__/./node_modules/react-dom/cjs/react-dom.development.js?:12880:18)
at updateClassComponent (webpack://__/./node_modules/react-dom/cjs/react-dom.development.js?:17100:5)
at beginWork (webpack://__/./node_modules/react-dom/cjs/react-dom.development.js?:18620:16)
at HTMLUnknownElement.callCallback (webpack://__/./node_modules/react-dom/cjs/react-dom.development.js?:188:14)
at Object.invokeGuardedCallbackDev (webpack://__/./node_modules/react-dom/cjs/react-dom.development.js?:237:16)
at invokeGuardedCallback (webpack://__/./node_modules/react-dom/cjs/react-dom.development.js?:292:31)
(anonymous) @ loadAsync.tsx:28
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 is one comment where I am not sure if it's intended or a typo. Besides that it is looking good! Thank you for the changes!
Testing Results
✅ When an extractor is deleted it disappears from the list
✅ An extractor can be created and is shown in the list (tested with substring extractor)
✅ An extractor can be edited (e.g. updated name) and the changes are reflected in the list
✅ Clicking on the Details button in the extractor toggles the detail view (shows and hides)
✅ Clicking on Sort Extractors
opens a modal with a sortable list
✅ Sorting extractors in the sortable list is reflected in the extractor list
} | ||
type TimerRateMetricsResponse = { |
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.
} | |
type TimerRateMetricsResponse = { | |
} | |
type TimerRateMetricsResponse = { |
init() { | ||
this.trigger({ extractors: this.extractors, extractor: this.extractor }); | ||
this.trigger({ extractors: this.extractors, extractor: this.extractory }); |
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.
extractor
? Is that a typo?
These changes update the extractors list to use the store to get the list of extractors. The component receives the latest value when the store is updated.
fix #16858
Types of changes
Checklist: