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

Do not initialize `inputs` variable to prevent merging duplicate key in. #5954

Merged
merged 3 commits into from May 21, 2019

Conversation

@dennisoelkers
Copy link
Member

commented May 21, 2019

Description

Motivation and Context

This change is fixing an issue that is related to the introduction of
its supplied state even during first initialization, leading to an
attempt to merge a duplicate key when mounting the ExtractorsPage
component. Removing the inputs key in the initial state of that
component is fixing it.

Fixes #5948.

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • 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.
  • All new and existing tests passed.
Do not initialize `inputs` variable to prevent merging duplicate key.
This change is fixing an issue that is related to the introduction of
its supplied state even during first initialization, leading to an
attempt to merge a duplicate key when mounting the `ExtractorsPage`
component. Removing the `inputs` key in the initial state of that
component is fixing it.

Fixes #5948.

@dennisoelkers dennisoelkers added this to the 3.1.0 milestone May 21, 2019

@dennisoelkers dennisoelkers requested a review from kmerz May 21, 2019

@kmerz

This comment has been minimized.

Copy link
Member

commented May 21, 2019

We have two inputs:

    1. w/ extractors
    1. w/o extractors

Clicking on Manage extractors of the first input will display no extractors as expected. Clicking
then on the second input Manage extractors shows also no extractors. Going back and clicking again on Manage extractors of the second input shows the extractors of the second input.

Peek 2019-05-21 15-36

Replace connecting component to `InputsStore` with single action call.
Last commits were fixing the issue only partly. It turns out, that the
`InputsStore` relies on an internal state (calling `get` with an input
id) to return the correct input in the `input` key of its state.
Therefore the extractors page was returning the extractors of the last
opened input instead of the current one.

This change is replacing the `connect` call with a single call to the
`get` action and sets the current input in the callback of the promise.
@kmerz
kmerz approved these changes May 21, 2019
Copy link
Member

left a comment

Works as expected! 👍 LGTM 💯

@kmerz kmerz merged commit a7687aa into master May 21, 2019

4 checks passed

ci-web-linter Jenkins build graylog-pr-linter-check 3590 has succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
graylog-project/pr Jenkins build graylog-project-pr-snapshot 3936 has succeeded
Details
license/cla Contributor License Agreement is signed.
Details

@kmerz kmerz deleted the preventing-duplicate-key-merge-in-extractors-page branch May 21, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.