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

[CCR] Refactor redux for Auto-follow pattern detail panel #27491

Merged
merged 10 commits into from
Dec 21, 2018

Conversation

sebelga
Copy link
Contributor

@sebelga sebelga commented Dec 19, 2018

Before starting with the follower indices section of CCR I wanted to quickly refactor the detail panel to follow the redux architecture that I had put in place.

Until we have more complex requirements for our crud apps, I'd prefer to keep the store/reducers and actions simple to use and reason about.

The reasoning behind this refactor is:

  • Decouple the UI state from an item selected (this gives us more flexibility and would allow for e.g. to close the panel in a CSS transition and still have an item selected). The fact that the detail panel is opened or closed is a UI state.
  • The detail panel should just render its content and not care if it is opened or not. That's the job of its parent component.
  • As we have 2 ways to select an item (clicking on a table item or change the pattern query param in the URL), having a single source of truth to open or close the detail panel makes things easier.
  • Fewer redux boilerplate (action types, action creators and reducers).

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@sebelga sebelga changed the base branch from master to feature/ccr December 19, 2018 15:48
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@cjcenizal cjcenizal added Feature:CCR and Remote Clusters Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more labels Dec 19, 2018
@elasticmachine
Copy link
Contributor

Pinging @elastic/es-ui

Copy link
Contributor

@cjcenizal cjcenizal left a comment

Choose a reason for hiding this comment

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

Looks good overall, just had a few suggestions and questions.

apiStatus: getApiStatus(scope)(state),
apiError: getApiError(scope)(state),
isAuthorized: isApiAuthorized(scope)(state),
isDetailPanelOpen: isDetailPanelOpen(state),
isDetailPanelOpened: isDetailPanelOpened(state),
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor nit, but I think we should use the present tense in all booleans for consistency, unless the tense has some clear significance.

Copy link
Contributor Author

@sebelga sebelga Dec 20, 2018

Choose a reason for hiding this comment

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

Ok! Was not sure if it was a mistake. So you prefer isClose to isClosed for example? I am not native but it sounds weird to me 😊

Copy link
Contributor

Choose a reason for hiding this comment

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

English is such a terrible language... "closed" is both the present and past tense, so complementary states would be isClosed and isOpen.

closeDetailPanel,
} = this.props;

if (!autoFollowPattern) {
return null;
}

const { name: autoFollowPatternName } = autoFollowPattern;
Copy link
Contributor

Choose a reason for hiding this comment

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

If the auto-follow pattern doesn't exist, then the name will be undefined. Compare this to Remote Clusters, which will still show you the name if the remote cluster can't be found.

image

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch

dispatch(closeAutoFollowPatternDetailPanel());
const ids = arrify(id);
const autoFollowPatternId = getSelectedAutoFollowPatternId(getState());
if (ids.some(_id => autoFollowPatternId === _id)) {
Copy link
Contributor

@cjcenizal cjcenizal Dec 19, 2018

Choose a reason for hiding this comment

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

The original code anticipated that the deletion request could fail for the selected auto-follow pattern. It looks like the new code doesn't anticipate that case and will close the detail panel regardless of the request failing or succeeding. Is this correct? If so, then I think the first case makes more sense from the user's perspective. What do you think?

Copy link
Contributor Author

@sebelga sebelga Dec 20, 2018

Choose a reason for hiding this comment

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

This is inside the onSuccess handler so will only be executed if the deletion succeeded. But it made me realize that I should change the if statement to if (ids.includes(autoFollowPatternId)) 😊

@@ -152,7 +152,7 @@ export const AutoFollowPatternTable = injectI18n(
}),
icon: 'pencil',
onClick: ({ name }) => {
editAutoFollowPattern(name);
selectAutoFollowPattern(name);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to select the auto-follow pattern here? I just tried removing this line and the editing functionality seemed to work fine.

On a separate note, I think we may want to implement this the way we did in Remote Clusters, so that this is a link with an href instead of a button with an onClick. This way people could open this in a new tab or copy the URL if they wanted to for some reason: https://github.com/elastic/kibana/blob/master/x-pack/plugins/remote_clusters/public/sections/remote_cluster_list/remote_cluster_table/remote_cluster_table.js#L183

Copy link
Contributor Author

@sebelga sebelga Dec 20, 2018

Choose a reason for hiding this comment

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

It works because, when there is no selection the edit component looks at the route param and fetches from the server (which is what happens on a page refresh). I'll change it to an href, good point

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@sebelga
Copy link
Contributor Author

sebelga commented Dec 20, 2018

Thanks for the review @cjcenizal @jen-huang ! I made all the changes and manage to simplify even more 😊 by removing the middleware to update the history and putting the state that opens or closes the detail panel locally on the <AutoFollowPatternList /> component.

The only use case not supported is if the user starts editing the pattern query param in the url. I don't think this would ever be needed as the user has the table with the list of auto-follow pattern right on the screen.

@elasticmachine
Copy link
Contributor

💔 Build Failed

@sebelga
Copy link
Contributor Author

sebelga commented Dec 20, 2018

retest

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

Copy link
Contributor

@cjcenizal cjcenizal left a comment

Choose a reason for hiding this comment

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

I had a few minor suggestions, but I'm also a bit uncomfortable with how complex AutoFollowPatternList has become. 😅

I also found a bug which we should fix. If you click the "Edit" icon in a row and then hit the back button, the detail panel opens up. I think this is because we're recycling the pattern selection for both the detail panel and edit form?

ccr_routing_bug

Because I won't be here for some time, I'll leave it to you and the rest of the team to decide how to move forward with regard to my other comments. 😄

openDetailPanel(patternName);
} else if (isDetailPanelOpen) {
closeDetailPanel();
componentDidUpdate(_, prevState) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of _ can we use prevProps, which is more conventional? It seems unusual and potentially confusing to use _.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No prob.

const mapDispatchToProps = (dispatch) => ({
// The actual closing of the panel is done in the <AutoFollowPatternList />
// component as it listens to the URL query change
closeDetailPanel: () => dispatch(selectAutoFollowPattern(null)),
Copy link
Contributor

Choose a reason for hiding this comment

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

The logic might be a little easier to follow if the owner (AutoFollowPatternList) were to provide this callback instead of the container. Then the comment becomes a little less necessary because all of the related logic is already in front of you.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great point

dispatch(closeAutoFollowPatternDetailPanel());
const ids = arrify(id);
const autoFollowPatternId = getSelectedAutoFollowPatternId(getState());
if (ids.includes(autoFollowPatternId)) {
Copy link
Contributor

@cjcenizal cjcenizal Dec 20, 2018

Choose a reason for hiding this comment

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

In the logic above this point, we're surfacing both error toasts (for when an auto-follow pattern fails to be deleted) and success toasts (for when an auto-follow pattern is deleted). So isn't this onSuccess callback invoked when there is a partial failure during a bulk delete? If this is the case, the original logic results in a better UX in the event that the auto-follow pattern in the detail panel has failed to be deleted, because we're checking response.itemsDeleted instead of ids.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry totally missed that, yes of course it should be response.itemsDeleted. Thanks for pointing it out

</EuiFlexItem>
</EuiFlexGroup>
</EuiFlexItem>
{autoFollowPattern && (
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice improvement here to render the "Close" button when the pattern is missing. Could you apply it to Remote Clusters, too?

image

}

static getDerivedStateFromProps({ autoFollowPatternId }, { lastAutoFollowPatternId }) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a personal DX anecdote... as I read this code, I found myself having to trace the logic through three lifecycle methods: componentDidMount, getDerivedStateFromProps, and componentDidUpdate. I then had to also factor in what would happen when the user clicks a row (a new pattern is selected, triggering getDerivedStateFromProps). This took me a few minutes to wrap my head around, and then I was still left wondering why we're storing lastAutoFollowPatternId. Through experimentation by removing the logic it supports, I found that its purpose is to prevent an infinite render loop.

To be honest, I feel like this code has become more complex with this change. What is the benefit this complexity brings us?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ComponentDidMount is pretty clear I think, it mounts and checks for the presence of a pattern in the query params. I understand what u say about the 2 other lifecycle methods, I will see how this could be improved.
There is never a benefit when complexity comes in 😊 I thought that using a middleware to change a URL on a single page was also a bit complex, mainly because you had to remember that it existed and that it was there that the search params were being changed. So I tried to move the logic as close as possible to the page (section) component.

@cjcenizal cjcenizal dismissed their stale review December 20, 2018 18:40

I don't want to block the PR on this while I'm out.

@sebelga
Copy link
Contributor Author

sebelga commented Dec 21, 2018

@cjcenizal @jen-huang Mea-culpa about my initial idea to have a single id for both detail and edit views. You were right that it adds a mental overhead and complexity in the code.
I added a commit to have 2 ids in the store: one for the detail panel and one for the edit view. I do think though that having the panel opened or closed is a local state of the list component so I left it like that.

About the complexity of having a lifecycle (componentDidUpdate) to manage the query param and deep link, IMO it might be a bit complex but it is easier to follow than having a separate middleware totally outside the component.

In order for you to forgive my stubbornness, I followed your advice @cjcenizal and replaced the api-middelware by a redux-thunk action. It does indeed remove another layer of complexity. 😊

@jen-huang Can you do test it locally and make sure I didn't leave anything behind? Thanks!!

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@sebelga sebelga merged commit e958eee into elastic:feature/ccr Dec 21, 2018
@sebelga sebelga deleted the feature/ccr_refactor_redux branch December 21, 2018 13:23
@sebelga
Copy link
Contributor Author

sebelga commented Dec 21, 2018

I will merge this PR and fix any regressions on a separate PR if there are any.

jen-huang added a commit to jen-huang/kibana that referenced this pull request Feb 5, 2019
* [CCR] Refactor redux for Auto-follow pattern detail panel (elastic#27491)

* [CCR] Refactor redux for Auto-follow pattern detail panel

* [CCR] Small refactor

* [CCR] Change to present tense

* [CCR] Display auto-follow pattern name even if it does not exist

* [CCR] Use href to edit auto-follow pattern + remove middelware to update "pattern" query params

* [CCR] Fix navigation back bug + set 2 ids for detail and edit an auto-follow pattern

* [CCR] Replace api middleware with redux-thunk action

* [CCR] Show detail footer close button even when cluster is not valid

* [CCR] Add endpoints for fetching and creating follower indices (elastic#27646)

* Add GET /follower_indices endpoint with deserialization logic and tests.

* Add POST /follower_indices endpoint with serialization logic and tests.

* [CCR]  Add unit tests for RemoteClusterForm, RemoteClusterList, and RemoteClusterTable (elastic#27647)

* Use componentDidUpdate instead of getDerivedStateFromProps.

* Add unit tests for RemoteClusterForm, RemoteClusterList, and RemoteClusterTable.

* Add jest mock for eui `makeId()` utility and get deterministic aria IDs for snapshots

* Update snapshot for Remote Cluster list test

* [CCR] Follower indices table and detail panel (elastic#27804)

* Store for follower indices

* Initial work for follower indices table and detail panel

* Fix load auto-follow stats load as middleware was removed

* [CCR] Create follower index UI form (elastic#27864)

* Initial setup Follower Index form

* Working form without client validation

* Add client side validation for follower index

* Add client validation to check if index already exist

* Improve error message when leader index does not exist

* Remove update method for follower index

* Clear api error on field change

* Fix i18n error

* Update snapshots

* [CCR] Add pause, resume, and unfollow actions for follower indices (elastic#28305)

* Add pause and resume follower index routes

* Add unfollow route

* Add api methods for new routes

* Adjust routes to have bulk capabilities, add corresponding actions

* Refresh list after pausing/resuming, remove items after unfollowing

* First pass at UI for pause and unfollow (and resume, but that is not visible due to ES stats response)

* Handle additional conditions needed for unfollowing leader index, add placeholder code to deduce paused status

* PR feedback

* [CCR] Advanced settings UI for follower indices (elastic#28267)

* Add client side validation of advanced settings form
* Move form entry row to separate component
* Add server side serialization for advanced settings
* Ignore advanced settings input when that section is hidden.
  - Cache and restore input when the section is shown again.

* [CCR] Show remote cluster validation in CCR forms, and add edit follower index (elastic#28778)

* [CCR] Advanced settings component

* Remove preview active on toggle settings

* Add client side validation of advanced settings form

* Move form entry row to separate component

* Add title to panel

* Add i18n translation of advanced settings

* Update Follower index form with toggle for advanced settings

* Add server side serialisation for advanced settings

* Make code review changes

* Fix test: mock constant dependency

* Add section to edit follower index

* Show confirm modal before updating follower index

* Add edit icon in table + update server endpoint to pause / resume

* [CCR] Show remote cluster validation in follower index form & auto-follow pattern form

* PR feedback, cleanup form sizes, add redirect to edit remote cluster

* Fix routing, remove unused code, adjust auto follow pattern edit loading error page

* Adjust error messages and make remote cluster not found edit page the same

* Fix functionality as result of merge

* Fix validation, reorder actions, fix tests, and address feedback

* PR feedback and fix validation pt 2

* Adjust remote cluster validation

* Fix i18n

* Fix api error not showing on add follower form

* [CCR]  Integrate new follower index info ES API (elastic#29153)

* Integrate new follower index info ES API

* Collate data from follower stats and info apis when retrieving all followers and single follower
* Add follower settings info to detail panel
* Add paused/active UI state
* Surface follower default settings to UI

* Adjust tests

* Address PR feedback

* Update snapshots

* [CCR] Surface license errors in-app and refine permissions error UI. (elastic#29228)

* Fix camelcasing bug in XPackInfo.
* Silently swallow API error when checking for index name availability.
* Fix typo in followerIndexForm fatal error.
* Add permissions check before allowing user to access the app.

* Refine wording of CCR permission denied page, to specify cluster privileges. (elastic#29533)

* [CCR] Improve form error handling and general UX (elastic#29419)

* Remove unnecessary eslint disable-line

* [CCR] Implement Advanced Settings design feedback (elastic#29543)

* Use EuiSwitch to toggle advanced settings in Create Follower Index form.
* Move 'optional' from each Advanced Setting field to the section heading.
* Change Advanced Settings switch label and description to emphasize that you can customize them or use the defaults.
* Prepopulate Advanced Settings fields with default values.
* When editing a follower index, check if advanced settings have been edited and open them if so.
* Add 'Reset to default' button below advanced settings fields if their values are different than the default.
* Remove 'Default' copy from Advanced Settings descriptions.
* Simplify toggleAdvancedSettings function, add comments, and fix React console error.

* [CCR] Follower index list fixes from design feedback (elastic#29484)

* Delete remote cluster settings before updating

* Fix detail panel z-index

* Remove default descriptor from follower index detail panel setting values

* Follower index confirm action copy adjustments

* Change z-index styling to use EUI vars

* [CCR] Improve remote clusters test coverage (elastic#29487)

* Add Jest test for RemoteClusterForm validation state.
* Extract validation functions out of RemoteClusterForm and add unit tests.
  - Return null instead of undefined from validators.
* Add unit tests for different types of remote clusters in RemoteClusterTable.
* Add unit test for RemoteClusterList empty prompt.
* Add tests verifying behavior for row link, row delete button, and detail panel delete button.
  - Use getRouterLinkProps to assign onClick and href to edit buttons in row and detail panel.

* [CCR] Adjust spacing around descriptions in list views, link to transport port docs, etc (elastic#29789)

* Adjust spacing around description around descriptions in list views so that it's even on top and bottom.
* Add link to transport port docs from Remote Cluster form.
* Move 'View in Index Management' link from the detail panel body into the footer.

* Re-order follower index form sections: remote cluster, leader index, follower index. (elastic#29885)

* Fix deep-linking to follower index after creating/updating it. (elastic#29865)

* [CCR] Copy edits (elastic#29676)

* Use 'Resume/pause data replication' in context menu and row actions.
* Update copy of 'Update' confirm modal for a paused follower index.
* Update copy of 'Update' confirm modal for an active follower index.
* Update copy of 'Pause data replication' confirm modal.
* Update copy of 'Resume data replication' confirm modal.
* Update copy for permissions check.
* Update copy of table empty state.
* Update copy around tables.
* Update form copy.
* Update copy for RemoteClustersFormField callouts.
* Convert 'data replication' -> 'replication'.
* Update copy for Unfollow confirm modal.
* Update copy for form API error and Auto-follow Patterns table.
* Update form save button labels to be 'Create' and 'Update'.
* Move API errors to bottom of form, into same position as sync validation errors. Remove spacer from SectionError implementation.

* [CCR] Open index after unfollowing leader (elastic#29887)

* Open index after unfollowing leader, fix some variable names

* Fix typo

* Add comment

* [CCR] IE and Screen reader accesibility (elastic#29731)

* Fix api endpoit for auto-follow stats

* Prevent letter wrapping in IE for the Remote cluster "connection" table column

* Move inline style to CSS class to fix IE flex bug

* [CCR] Add callout to paused follower index detail panel (elastic#30013)

* Add callout to paused follower index detail panel

* Update copy

* Skip call to ccr stats api if follower index is paused (elastic#30027)

* [CCR] Add integration tests for follower indices (elastic#30064)

* [CCR] Add integration tests for follower indices

* Import advanced settings value from app constants
jen-huang added a commit that referenced this pull request Feb 5, 2019
* [CCR] Refactor redux for Auto-follow pattern detail panel (#27491)

* [CCR] Refactor redux for Auto-follow pattern detail panel

* [CCR] Small refactor

* [CCR] Change to present tense

* [CCR] Display auto-follow pattern name even if it does not exist

* [CCR] Use href to edit auto-follow pattern + remove middelware to update "pattern" query params

* [CCR] Fix navigation back bug + set 2 ids for detail and edit an auto-follow pattern

* [CCR] Replace api middleware with redux-thunk action

* [CCR] Show detail footer close button even when cluster is not valid

* [CCR] Add endpoints for fetching and creating follower indices (#27646)

* Add GET /follower_indices endpoint with deserialization logic and tests.

* Add POST /follower_indices endpoint with serialization logic and tests.

* [CCR]  Add unit tests for RemoteClusterForm, RemoteClusterList, and RemoteClusterTable (#27647)

* Use componentDidUpdate instead of getDerivedStateFromProps.

* Add unit tests for RemoteClusterForm, RemoteClusterList, and RemoteClusterTable.

* Add jest mock for eui `makeId()` utility and get deterministic aria IDs for snapshots

* Update snapshot for Remote Cluster list test

* [CCR] Follower indices table and detail panel (#27804)

* Store for follower indices

* Initial work for follower indices table and detail panel

* Fix load auto-follow stats load as middleware was removed

* [CCR] Create follower index UI form (#27864)

* Initial setup Follower Index form

* Working form without client validation

* Add client side validation for follower index

* Add client validation to check if index already exist

* Improve error message when leader index does not exist

* Remove update method for follower index

* Clear api error on field change

* Fix i18n error

* Update snapshots

* [CCR] Add pause, resume, and unfollow actions for follower indices (#28305)

* Add pause and resume follower index routes

* Add unfollow route

* Add api methods for new routes

* Adjust routes to have bulk capabilities, add corresponding actions

* Refresh list after pausing/resuming, remove items after unfollowing

* First pass at UI for pause and unfollow (and resume, but that is not visible due to ES stats response)

* Handle additional conditions needed for unfollowing leader index, add placeholder code to deduce paused status

* PR feedback

* [CCR] Advanced settings UI for follower indices (#28267)

* Add client side validation of advanced settings form
* Move form entry row to separate component
* Add server side serialization for advanced settings
* Ignore advanced settings input when that section is hidden.
  - Cache and restore input when the section is shown again.

* [CCR] Show remote cluster validation in CCR forms, and add edit follower index (#28778)

* [CCR] Advanced settings component

* Remove preview active on toggle settings

* Add client side validation of advanced settings form

* Move form entry row to separate component

* Add title to panel

* Add i18n translation of advanced settings

* Update Follower index form with toggle for advanced settings

* Add server side serialisation for advanced settings

* Make code review changes

* Fix test: mock constant dependency

* Add section to edit follower index

* Show confirm modal before updating follower index

* Add edit icon in table + update server endpoint to pause / resume

* [CCR] Show remote cluster validation in follower index form & auto-follow pattern form

* PR feedback, cleanup form sizes, add redirect to edit remote cluster

* Fix routing, remove unused code, adjust auto follow pattern edit loading error page

* Adjust error messages and make remote cluster not found edit page the same

* Fix functionality as result of merge

* Fix validation, reorder actions, fix tests, and address feedback

* PR feedback and fix validation pt 2

* Adjust remote cluster validation

* Fix i18n

* Fix api error not showing on add follower form

* [CCR]  Integrate new follower index info ES API (#29153)

* Integrate new follower index info ES API

* Collate data from follower stats and info apis when retrieving all followers and single follower
* Add follower settings info to detail panel
* Add paused/active UI state
* Surface follower default settings to UI

* Adjust tests

* Address PR feedback

* Update snapshots

* [CCR] Surface license errors in-app and refine permissions error UI. (#29228)

* Fix camelcasing bug in XPackInfo.
* Silently swallow API error when checking for index name availability.
* Fix typo in followerIndexForm fatal error.
* Add permissions check before allowing user to access the app.

* Refine wording of CCR permission denied page, to specify cluster privileges. (#29533)

* [CCR] Improve form error handling and general UX (#29419)

* Remove unnecessary eslint disable-line

* [CCR] Implement Advanced Settings design feedback (#29543)

* Use EuiSwitch to toggle advanced settings in Create Follower Index form.
* Move 'optional' from each Advanced Setting field to the section heading.
* Change Advanced Settings switch label and description to emphasize that you can customize them or use the defaults.
* Prepopulate Advanced Settings fields with default values.
* When editing a follower index, check if advanced settings have been edited and open them if so.
* Add 'Reset to default' button below advanced settings fields if their values are different than the default.
* Remove 'Default' copy from Advanced Settings descriptions.
* Simplify toggleAdvancedSettings function, add comments, and fix React console error.

* [CCR] Follower index list fixes from design feedback (#29484)

* Delete remote cluster settings before updating

* Fix detail panel z-index

* Remove default descriptor from follower index detail panel setting values

* Follower index confirm action copy adjustments

* Change z-index styling to use EUI vars

* [CCR] Improve remote clusters test coverage (#29487)

* Add Jest test for RemoteClusterForm validation state.
* Extract validation functions out of RemoteClusterForm and add unit tests.
  - Return null instead of undefined from validators.
* Add unit tests for different types of remote clusters in RemoteClusterTable.
* Add unit test for RemoteClusterList empty prompt.
* Add tests verifying behavior for row link, row delete button, and detail panel delete button.
  - Use getRouterLinkProps to assign onClick and href to edit buttons in row and detail panel.

* [CCR] Adjust spacing around descriptions in list views, link to transport port docs, etc (#29789)

* Adjust spacing around description around descriptions in list views so that it's even on top and bottom.
* Add link to transport port docs from Remote Cluster form.
* Move 'View in Index Management' link from the detail panel body into the footer.

* Re-order follower index form sections: remote cluster, leader index, follower index. (#29885)

* Fix deep-linking to follower index after creating/updating it. (#29865)

* [CCR] Copy edits (#29676)

* Use 'Resume/pause data replication' in context menu and row actions.
* Update copy of 'Update' confirm modal for a paused follower index.
* Update copy of 'Update' confirm modal for an active follower index.
* Update copy of 'Pause data replication' confirm modal.
* Update copy of 'Resume data replication' confirm modal.
* Update copy for permissions check.
* Update copy of table empty state.
* Update copy around tables.
* Update form copy.
* Update copy for RemoteClustersFormField callouts.
* Convert 'data replication' -> 'replication'.
* Update copy for Unfollow confirm modal.
* Update copy for form API error and Auto-follow Patterns table.
* Update form save button labels to be 'Create' and 'Update'.
* Move API errors to bottom of form, into same position as sync validation errors. Remove spacer from SectionError implementation.

* [CCR] Open index after unfollowing leader (#29887)

* Open index after unfollowing leader, fix some variable names

* Fix typo

* Add comment

* [CCR] IE and Screen reader accesibility (#29731)

* Fix api endpoit for auto-follow stats

* Prevent letter wrapping in IE for the Remote cluster "connection" table column

* Move inline style to CSS class to fix IE flex bug

* [CCR] Add callout to paused follower index detail panel (#30013)

* Add callout to paused follower index detail panel

* Update copy

* Skip call to ccr stats api if follower index is paused (#30027)

* [CCR] Add integration tests for follower indices (#30064)

* [CCR] Add integration tests for follower indices

* Import advanced settings value from app constants
jen-huang added a commit that referenced this pull request Feb 6, 2019
* [CCR] Follower index CRUD (#27936)

* [CCR] Refactor redux for Auto-follow pattern detail panel (#27491)

* [CCR] Refactor redux for Auto-follow pattern detail panel

* [CCR] Small refactor

* [CCR] Change to present tense

* [CCR] Display auto-follow pattern name even if it does not exist

* [CCR] Use href to edit auto-follow pattern + remove middelware to update "pattern" query params

* [CCR] Fix navigation back bug + set 2 ids for detail and edit an auto-follow pattern

* [CCR] Replace api middleware with redux-thunk action

* [CCR] Show detail footer close button even when cluster is not valid

* [CCR] Add endpoints for fetching and creating follower indices (#27646)

* Add GET /follower_indices endpoint with deserialization logic and tests.

* Add POST /follower_indices endpoint with serialization logic and tests.

* [CCR]  Add unit tests for RemoteClusterForm, RemoteClusterList, and RemoteClusterTable (#27647)

* Use componentDidUpdate instead of getDerivedStateFromProps.

* Add unit tests for RemoteClusterForm, RemoteClusterList, and RemoteClusterTable.

* Add jest mock for eui `makeId()` utility and get deterministic aria IDs for snapshots

* Update snapshot for Remote Cluster list test

* [CCR] Follower indices table and detail panel (#27804)

* Store for follower indices

* Initial work for follower indices table and detail panel

* Fix load auto-follow stats load as middleware was removed

* [CCR] Create follower index UI form (#27864)

* Initial setup Follower Index form

* Working form without client validation

* Add client side validation for follower index

* Add client validation to check if index already exist

* Improve error message when leader index does not exist

* Remove update method for follower index

* Clear api error on field change

* Fix i18n error

* Update snapshots

* [CCR] Add pause, resume, and unfollow actions for follower indices (#28305)

* Add pause and resume follower index routes

* Add unfollow route

* Add api methods for new routes

* Adjust routes to have bulk capabilities, add corresponding actions

* Refresh list after pausing/resuming, remove items after unfollowing

* First pass at UI for pause and unfollow (and resume, but that is not visible due to ES stats response)

* Handle additional conditions needed for unfollowing leader index, add placeholder code to deduce paused status

* PR feedback

* [CCR] Advanced settings UI for follower indices (#28267)

* Add client side validation of advanced settings form
* Move form entry row to separate component
* Add server side serialization for advanced settings
* Ignore advanced settings input when that section is hidden.
  - Cache and restore input when the section is shown again.

* [CCR] Show remote cluster validation in CCR forms, and add edit follower index (#28778)

* [CCR] Advanced settings component

* Remove preview active on toggle settings

* Add client side validation of advanced settings form

* Move form entry row to separate component

* Add title to panel

* Add i18n translation of advanced settings

* Update Follower index form with toggle for advanced settings

* Add server side serialisation for advanced settings

* Make code review changes

* Fix test: mock constant dependency

* Add section to edit follower index

* Show confirm modal before updating follower index

* Add edit icon in table + update server endpoint to pause / resume

* [CCR] Show remote cluster validation in follower index form & auto-follow pattern form

* PR feedback, cleanup form sizes, add redirect to edit remote cluster

* Fix routing, remove unused code, adjust auto follow pattern edit loading error page

* Adjust error messages and make remote cluster not found edit page the same

* Fix functionality as result of merge

* Fix validation, reorder actions, fix tests, and address feedback

* PR feedback and fix validation pt 2

* Adjust remote cluster validation

* Fix i18n

* Fix api error not showing on add follower form

* [CCR]  Integrate new follower index info ES API (#29153)

* Integrate new follower index info ES API

* Collate data from follower stats and info apis when retrieving all followers and single follower
* Add follower settings info to detail panel
* Add paused/active UI state
* Surface follower default settings to UI

* Adjust tests

* Address PR feedback

* Update snapshots

* [CCR] Surface license errors in-app and refine permissions error UI. (#29228)

* Fix camelcasing bug in XPackInfo.
* Silently swallow API error when checking for index name availability.
* Fix typo in followerIndexForm fatal error.
* Add permissions check before allowing user to access the app.

* Refine wording of CCR permission denied page, to specify cluster privileges. (#29533)

* [CCR] Improve form error handling and general UX (#29419)

* Remove unnecessary eslint disable-line

* [CCR] Implement Advanced Settings design feedback (#29543)

* Use EuiSwitch to toggle advanced settings in Create Follower Index form.
* Move 'optional' from each Advanced Setting field to the section heading.
* Change Advanced Settings switch label and description to emphasize that you can customize them or use the defaults.
* Prepopulate Advanced Settings fields with default values.
* When editing a follower index, check if advanced settings have been edited and open them if so.
* Add 'Reset to default' button below advanced settings fields if their values are different than the default.
* Remove 'Default' copy from Advanced Settings descriptions.
* Simplify toggleAdvancedSettings function, add comments, and fix React console error.

* [CCR] Follower index list fixes from design feedback (#29484)

* Delete remote cluster settings before updating

* Fix detail panel z-index

* Remove default descriptor from follower index detail panel setting values

* Follower index confirm action copy adjustments

* Change z-index styling to use EUI vars

* [CCR] Improve remote clusters test coverage (#29487)

* Add Jest test for RemoteClusterForm validation state.
* Extract validation functions out of RemoteClusterForm and add unit tests.
  - Return null instead of undefined from validators.
* Add unit tests for different types of remote clusters in RemoteClusterTable.
* Add unit test for RemoteClusterList empty prompt.
* Add tests verifying behavior for row link, row delete button, and detail panel delete button.
  - Use getRouterLinkProps to assign onClick and href to edit buttons in row and detail panel.

* [CCR] Adjust spacing around descriptions in list views, link to transport port docs, etc (#29789)

* Adjust spacing around description around descriptions in list views so that it's even on top and bottom.
* Add link to transport port docs from Remote Cluster form.
* Move 'View in Index Management' link from the detail panel body into the footer.

* Re-order follower index form sections: remote cluster, leader index, follower index. (#29885)

* Fix deep-linking to follower index after creating/updating it. (#29865)

* [CCR] Copy edits (#29676)

* Use 'Resume/pause data replication' in context menu and row actions.
* Update copy of 'Update' confirm modal for a paused follower index.
* Update copy of 'Update' confirm modal for an active follower index.
* Update copy of 'Pause data replication' confirm modal.
* Update copy of 'Resume data replication' confirm modal.
* Update copy for permissions check.
* Update copy of table empty state.
* Update copy around tables.
* Update form copy.
* Update copy for RemoteClustersFormField callouts.
* Convert 'data replication' -> 'replication'.
* Update copy for Unfollow confirm modal.
* Update copy for form API error and Auto-follow Patterns table.
* Update form save button labels to be 'Create' and 'Update'.
* Move API errors to bottom of form, into same position as sync validation errors. Remove spacer from SectionError implementation.

* [CCR] Open index after unfollowing leader (#29887)

* Open index after unfollowing leader, fix some variable names

* Fix typo

* Add comment

* [CCR] IE and Screen reader accesibility (#29731)

* Fix api endpoit for auto-follow stats

* Prevent letter wrapping in IE for the Remote cluster "connection" table column

* Move inline style to CSS class to fix IE flex bug

* [CCR] Add callout to paused follower index detail panel (#30013)

* Add callout to paused follower index detail panel

* Update copy

* Skip call to ccr stats api if follower index is paused (#30027)

* [CCR] Add integration tests for follower indices (#30064)

* [CCR] Add integration tests for follower indices

* Import advanced settings value from app constants

* Disable flaky follower indices API integration tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:CCR and Remote Clusters Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants