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

Fix inputs extractors list not updating after deletion #17289

Merged
merged 9 commits into from
Nov 17, 2023
Merged

Conversation

ousmaneo
Copy link
Contributor

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

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactoring (non-breaking change)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.

@ousmaneo ousmaneo changed the title Fix inputs extractors list update after deletion Fix inputs extractors list not updating after deletion Nov 14, 2023
Copy link
Contributor

@grotlue grotlue left a 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

@ousmaneo ousmaneo requested a review from grotlue November 16, 2023 11:37
Copy link
Contributor

@grotlue grotlue left a 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

Comment on lines 72 to 73
}
type TimerRateMetricsResponse = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
}
type TimerRateMetricsResponse = {
}
type TimerRateMetricsResponse = {

init() {
this.trigger({ extractors: this.extractors, extractor: this.extractor });
this.trigger({ extractors: this.extractors, extractor: this.extractory });
Copy link
Contributor

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?

@ousmaneo ousmaneo merged commit 6d17ce9 into master Nov 17, 2023
4 checks passed
@ousmaneo ousmaneo deleted the issue-16858 branch November 17, 2023 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Input extractors list does not update after deleting extractor.
2 participants