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

Validate grok pattern name on the client to avoid duplicate names. #1150

Merged
merged 3 commits into from Mar 6, 2015

Conversation

Projects
None yet
2 participants
@bernd
Contributor

bernd commented Mar 4, 2015

Implements suggested solutions for Graylog2/graylog2-server#937.

@edmundoa Please check if that's the way to go or if I should do it different. Thanks!

@edmundoa edmundoa self-assigned this Mar 4, 2015

@bernd bernd added this to the 1.0.1 milestone Mar 4, 2015

validPatternName(name) {
// Check if patterns already contain a pattern with the given name.
return this.state.patterns.map((p) => p.name).indexOf(name) === -1;
},

This comment has been minimized.

@edmundoa

edmundoa Mar 5, 2015

Member

I'd use a some loop there, as you don't need to go through the whole array when there's a duplicated element.

This comment has been minimized.

@bernd

bernd Mar 5, 2015

Contributor

Nice trick, thanks!

pattern: this.props.pattern
pattern: this.props.pattern,
_error: false,
_error_message: ""

This comment has been minimized.

@edmundoa

edmundoa Mar 5, 2015

Member

You can remove the _ prefix from error and error message, in the end is the component status and it's private :)

@edmundoa

This comment has been minimized.

Member

edmundoa commented Mar 5, 2015

I really like what I see, great job!

In case you want to add some validation to the pattern itself, I'd pass a single validation function to EditPatternModal to keep it clean. In a more complex form, passing several validation functions as properties could become really messy. Anyway, for now that is not a problem 👍

bernd added a commit to Graylog2/graylog2-server that referenced this pull request Mar 5, 2015

Add "replace" query param to Grok bulk import REST API endpoint.
Existing Grok patterns will be removed before adding the new ones if set
to true.

Fixes #937
Refs graylog-labs/graylog2-web-interface#1150
@bernd

This comment has been minimized.

Contributor

bernd commented Mar 5, 2015

CI will fail because it depends on Graylog2/graylog2-server#1034

@bernd

This comment has been minimized.

Contributor

bernd commented Mar 5, 2015

@edmundoa This is complete now. Please check the latest additions. Thank you!

Needs Graylog2/graylog2-server#1034 to be merged.

bernd added a commit to Graylog2/graylog2-server that referenced this pull request Mar 5, 2015

Add "replace" query param to Grok bulk import REST API endpoint.
Existing Grok patterns will be removed before adding the new ones if set
to true.

Fixes #937
Refs graylog-labs/graylog2-web-interface#1150
@edmundoa

This comment has been minimized.

Member

edmundoa commented Mar 6, 2015

Looks good to me, great job! 👍

edmundoa added a commit that referenced this pull request Mar 6, 2015

Merge pull request #1150 from Graylog2/unique-grok-pattern-issue-937
Validate grok pattern name on the client to avoid duplicate names.

@edmundoa edmundoa merged commit be94729 into 1.0 Mar 6, 2015

0 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/travis-ci/push The Travis CI build failed
Details

@edmundoa edmundoa deleted the unique-grok-pattern-issue-937 branch Mar 6, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment