Skip to content

[CCR] UI updates #7

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.

Sign up for GitHub

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

Closed
sebelga wants to merge 1 commit into from
Closed

[CCR] UI updates #7

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions x-pack/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@
"d3": "3.5.6",
"d3-scale": "1.0.6",
"dataloader": "^1.4.0",
"date-fns": "^1.30.1",
Copy link
Copy Markdown

@cjcenizal cjcenizal Dec 14, 2018

Choose a reason for hiding this comment

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

Why was this added?

Copy link
Copy Markdown
Owner Author

@sebelga sebelga Dec 17, 2018

Choose a reason for hiding this comment

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

Mainly because

  • it's functional
  • it's immutable
  • it's faster
  • we can bundle what we need and not the whole lib

https://medium.com/@k2u4yt/momentjs-vs-date-fns-6bddc7bfa21e

Copy link
Copy Markdown

@cjcenizal cjcenizal Dec 17, 2018

Choose a reason for hiding this comment

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

Thanks, I read the link and have a good idea of why date-fns is better than moment. I'd be in support of exploring migrating over. :)

In this particular situation, is it worth adding the extra dependency though? Do we gain anything significant, e.g. noticeable performance improvement? If not, then I think we should just stick with moment (warts and all) to avoid adding another dependency.

"dedent": "^0.7.0",
"dragselect": "1.8.1",
"elasticsearch": "^15.2.0",
Expand Down
38 changes: 6 additions & 32 deletions x-pack/plugins/cross_cluster_replication/public/app/components/auto_follow_pattern_form.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,7 @@ import { INDEX_ILLEGAL_CHARACTERS_VISIBLE } from 'ui/indices';

import routing from '../services/routing';
import { API_STATUS } from '../constants';
import { SectionError } from './';
import { getPrefixSuffixFromFollowPattern, getPreviewIndicesFromAutoFollowPattern } from '../services/auto_follow_pattern';
import { SectionError, AutoFollowPatternIndicesPreview } from './';
import { validateAutoFollowPattern, validateLeaderIndexPattern } from '../services/auto_follow_pattern_validators';

const indexPatternIllegalCharacters = INDEX_PATTERN_ILLEGAL_CHARACTERS_VISIBLE.join(' ');
Expand Down Expand Up @@ -83,7 +82,6 @@ export class AutoFollowPatternFormUI extends PureComponent {
? getEmptyAutoFollowPattern(this.props.remoteClusters)
: {
...this.props.autoFollowPattern,
...getPrefixSuffixFromFollowPattern(this.props.autoFollowPattern.followIndexPattern)
};

this.state = {
Expand Down Expand Up @@ -438,34 +436,6 @@ export class AutoFollowPatternFormUI extends PureComponent {
const isPrefixInvalid = areErrorsVisible && !!fieldsErrors.followIndexPatternPrefix;
const isSuffixInvalid = areErrorsVisible && !!fieldsErrors.followIndexPatternSuffix;

const renderFollowIndicesPreview = () => {
const { indicesPreview } = getPreviewIndicesFromAutoFollowPattern({
prefix: followIndexPatternPrefix,
suffix: followIndexPatternSuffix,
leaderIndexPatterns
});

const title = intl.formatMessage({
id: 'xpack.crossClusterReplication.autoFollowPatternForm.indicesPreviewTitle',
defaultMessage: 'Index name examples',
});

return (
<EuiCallOut
title={title}
iconType="indexMapping"
>
<FormattedMessage
id="xpack.crossClusterReplication.autoFollowPatternForm.indicesPreviewDescription"
defaultMessage="The above settings will generate index names that look like this:"
/>
<ul>
{indicesPreview.map((followerIndex, i) => <li key={i}>{followerIndex}</li>)}
</ul>
</EuiCallOut>
);
};

return (
<EuiDescribedFormGroup
title={(
Expand Down Expand Up @@ -545,7 +515,11 @@ export class AutoFollowPatternFormUI extends PureComponent {
{!!leaderIndexPatterns.length && (
<Fragment>
<EuiSpacer size="m" />
{renderFollowIndicesPreview()}
<AutoFollowPatternIndicesPreview
prefix={followIndexPatternPrefix}
suffix={followIndexPatternSuffix}
leaderIndexPatterns={leaderIndexPatterns}
/>
</Fragment>
)}
</EuiDescribedFormGroup>
Expand Down
39 changes: 39 additions & 0 deletions ...ns/cross_cluster_replication/public/app/components/auto_follow_pattern_indices_preview.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import React from 'react';

import { EuiCallOut } from '@elastic/eui';
import { injectI18n, FormattedMessage } from '@kbn/i18n/react';
import { getPreviewIndicesFromAutoFollowPattern } from '../services/auto_follow_pattern';

export const AutoFollowPatternIndicesPreview = injectI18n(({ prefix, suffix, leaderIndexPatterns, intl }) => {
const { indicesPreview } = getPreviewIndicesFromAutoFollowPattern({
prefix,
suffix,
leaderIndexPatterns
});

const title = intl.formatMessage({
id: 'xpack.crossClusterReplication.autoFollowPatternForm.indicesPreviewTitle',
defaultMessage: 'Index name examples',
});

return (
<EuiCallOut
title={title}
iconType="indexMapping"
>
<FormattedMessage
id="xpack.crossClusterReplication.autoFollowPatternForm.indicesPreviewDescription"
defaultMessage="The above settings will generate index names that look like this:"
/>
<ul>
{indicesPreview.map((followerIndex, i) => <li key={i}>{followerIndex}</li>)}
</ul>
</EuiCallOut>
);
});
1 change: 1 addition & 0 deletions x-pack/plugins/cross_cluster_replication/public/app/components/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ export { RemoteClustersProvider } from './remote_clusters_provider';
export { AutoFollowPatternForm } from './auto_follow_pattern_form';
export { AutoFollowPatternDeleteProvider } from './auto_follow_pattern_delete_provider';
export { AutoFollowPatternPageTitle } from './auto_follow_pattern_page_title';
export { AutoFollowPatternIndicesPreview } from './auto_follow_pattern_indices_preview';
19 changes: 6 additions & 13 deletions ...uto_follow_pattern_list/components/auto_follow_pattern_table/auto_follow_pattern_table.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import {
import { API_STATUS } from '../../../../../constants';
import { AutoFollowPatternDeleteProvider } from '../../../../../components';
import routing from '../../../../../services/routing';
import { getPrefixSuffixFromFollowPattern } from '../../../../../services/auto_follow_pattern';

export class AutoFollowPatternTableUI extends PureComponent {
static propTypes = {
Expand All @@ -44,12 +43,14 @@ export class AutoFollowPatternTableUI extends PureComponent {

if(queryText) {
return autoFollowPatterns.filter(autoFollowPattern => {
const { name, remoteCluster } = autoFollowPattern;
const { name, remoteCluster, followIndexPatternPrefix, followIndexPatternSuffix } = autoFollowPattern;

const inName = name.toLowerCase().includes(queryText);
const inRemoteCluster = remoteCluster.toLowerCase().includes(queryText);
const inPrefix = followIndexPatternPrefix.toLowerCase().includes(queryText);
const inSuffix = followIndexPatternSuffix.toLowerCase().includes(queryText);

return inName || inRemoteCluster;
return inName || inRemoteCluster || inPrefix || inSuffix;
});
}

Expand Down Expand Up @@ -89,29 +90,21 @@ export class AutoFollowPatternTableUI extends PureComponent {
),
render: (leaderPatterns) => leaderPatterns.join(', '),
}, {
field: 'followIndexPattern',
field: 'followIndexPatternPrefix',
Copy link
Copy Markdown

@cjcenizal cjcenizal Dec 14, 2018

Choose a reason for hiding this comment

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

Can we add sortable: true to the prefix and suffix columns?

Copy link
Copy Markdown
Owner Author

@sebelga sebelga Dec 17, 2018

Choose a reason for hiding this comment

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

Yes, don't know why I thought it was already set to true 😊

name: (
<FormattedMessage
id="xpack.cross_cluster_replication.autofollow_pattern_list.table.connected_nodes_column_title"
defaultMessage="Follower pattern prefix"
/>
),
render: (followIndexPattern) => {
const { followIndexPatternPrefix } = getPrefixSuffixFromFollowPattern(followIndexPattern);
return followIndexPatternPrefix;
}
}, {
field: 'followIndexPattern',
field: 'followIndexPatternSuffix',
name: (
<FormattedMessage
id="xpack.cross_cluster_replication.autofollow_pattern_list.table.connected_nodes_column_title"
defaultMessage="Follower pattern suffix"
/>
),
render: (followIndexPattern) => {
const { followIndexPatternSuffix } = getPrefixSuffixFromFollowPattern(followIndexPattern);
return followIndexPatternSuffix;
}
}, {
name: '',
actions: [
Expand Down
15 changes: 13 additions & 2 deletions x-pack/plugins/cross_cluster_replication/public/app/services/auto_follow_pattern.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,15 @@
* you may not use this file except in compliance with the Elastic License.
*/

import React, { Fragment } from 'react';
import { format, addDays } from 'date-fns';

export const getFollowPattern = (prefix = '', suffix = '', template = '{{leader_index}}') => (
prefix + template + suffix
<Fragment>
Copy link
Copy Markdown

@cjcenizal cjcenizal Dec 14, 2018

Choose a reason for hiding this comment

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

I have a few suggestions!

  1. Could this return an object of the shape { prefix, suffix, template } and can we delegate the rendering logic to the AutoFollowPatternPreview component? This way we can keep all rendering logic in the sections dir.
  2. Can we use <strong> instead of <span>?
  3. I'm on the fence, but do you think it might make more sense to bold the index pattern instead, since it is the part that's changing?

image

Copy link
Copy Markdown
Owner Author

@sebelga sebelga Dec 17, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Ok, good points. About where the bold is, I asked Yaron and he agrees with you.

<span style={{ fontWeight: 'bold' }}>{prefix}</span>
{template}
<span style={{ fontWeight: 'bold' }}>{suffix}</span>
</Fragment>
);

/**
Expand All @@ -20,7 +27,11 @@ export const getPreviewIndicesFromAutoFollowPattern = ({
suffix,
leaderIndexPatterns,
limit = 5,
wildcardPlaceHolders = ['0', '1', '2']
wildcardPlaceHolders = [
format(new Date(), 'YYYY-MM-DD'),
format(addDays(new Date(), 1), 'YYYY-MM-DD'),
format(addDays(new Date(), 2), 'YYYY-MM-DD'),
]
}) => {
const indicesPreview = [];
let indexPreview;
Expand Down
12 changes: 10 additions & 2 deletions x-pack/plugins/cross_cluster_replication/public/app/store/reducers/auto_follow_pattern.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import * as t from '../action_types';
import { arrayToObject } from '../../services/utils';
import { getPrefixSuffixFromFollowPattern } from '../../services/auto_follow_pattern';

const initialState = {
byId: {},
Expand All @@ -14,13 +15,20 @@ const initialState = {

const success = action => `${action}_SUCCESS`;

const parseAutoFollowPattern = (autoFollowPattern) => {
// Extract prefix and suffix from follow index pattern
const { followIndexPatternPrefix, followIndexPatternSuffix } = getPrefixSuffixFromFollowPattern(autoFollowPattern.followIndexPattern);

return { ...autoFollowPattern, followIndexPatternPrefix, followIndexPatternSuffix };
};

export const reducer = (state = initialState, action) => {
switch (action.type) {
case success(t.AUTO_FOLLOW_PATTERN_LOAD): {
return { ...state, byId: arrayToObject(action.payload.patterns, 'name') };
return { ...state, byId: arrayToObject(action.payload.patterns.map(parseAutoFollowPattern), 'name') };
}
case success(t.AUTO_FOLLOW_PATTERN_GET): {
return { ...state, byId: { ...state.byId, [action.payload.name]: action.payload } };
return { ...state, byId: { ...state.byId, [action.payload.name]: parseAutoFollowPattern(action.payload) } };
}
case t.AUTO_FOLLOW_PATTERN_SELECT: {
return { ...state, selectedId: action.payload };
Expand Down
5 changes: 5 additions & 0 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6476,6 +6476,11 @@ date-fns@^1.27.2:
resolved "https://registry.yarnpkg.com/date-fns/-/date-fns-1.29.0.tgz#12e609cdcb935127311d04d33334e2960a2a54e6"
integrity sha512-lbTXWZ6M20cWH8N9S6afb0SBm6tMk+uUg6z3MqHPKE9atmsY3kJkTm8vKe93izJ2B2+q5MV990sM2CHgtAZaOw==

date-fns@^1.30.1:
version "1.30.1"
resolved "https://registry.yarnpkg.com/date-fns/-/date-fns-1.30.1.tgz#2e71bf0b119153dbb4cc4e88d9ea5acfb50dc05c"
integrity sha512-hBSVCvSmWC+QypYObzwGOd9wqdDpOt+0wl0KbU+R+uuZBS1jN8VsD1ss3irQDknRj5NvxiTF6oj/nDRnN/UQNw==

date-now@^0.1.4:
version "0.1.4"
resolved "https://registry.yarnpkg.com/date-now/-/date-now-0.1.4.tgz#eaf439fd4d4848ad74e5cc7dbef200672b9e345b"
Expand Down