[Workplace Search] Wire up write view for Blocked windows by scottybollinger · Pull Request #114696 · elastic/kibana

scottybollinger · 2021-10-12T17:32:57Z

Summary

This PR wraps up the Sync Scheduling UI. It enables the form for Blocked windows and persisting to the server.

blocked_windows.mp4

Checklist

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

We already added a reducer for this but forgot to implement this. Because we have shared state between the tabs, we need to overrule the unsaved changes prompt when simply navigating between tabs.

After wiring up the backend and converting to UTC, some changes to mocks and time formats had to be made.

This commit refactors to make use of the already-in-state schedule object. Previously, while wiring up the static views, I used a blockedWindows array directly on the state tree. This simplifies things so that equality checks can be done with one object.

It was hard to test removing an item from an array that doesn’t exist so I changed the code to expect the array to be present (! operator), since the other path is not possible. Also updated the server value from deletion to delete to match the API

(test was covered in previous commit)

One of the links was removed intentionally

The API omits the key when there are no items so we need to have the item removed as well in the UI state. Otherwise, removing the last item will cause the UI to say there are unsaved changes when there are not. I tried setting it as: schedule.blockedWindows = undefined but the selector did not see those as equal but deleting the key does.

Syncronization -> Synchronization (+h)

yakhinvadim

LGTM overall, just a couple questions:

yakhinvadim · 2021-10-12T20:53:08Z

...s/workplace_search/views/content_sources/components/synchronization/synchronization_logic.ts

+          switch (prop) {
+            case 'jobType':
+              blockedWindow.jobType = value as SyncJobType;
+              break;
+            case 'day':
+              blockedWindow.day = value as DayOfWeek | 'all';
+              break;
+            case 'start':
+              blockedWindow.start = value;
+              break;
+            case 'end':
+              blockedWindow.end = value;
+              break;
+          }


Not sure if it'll work, but maybe we can replace the switch statement with this?

blockedWindow[prop] = value

This certainly would be the easiest way to do this. However, I spent well over an hour trying to make TypeScript accept this and nothing I did could please the TypeScript gods.

yakhinvadim · 2021-10-12T21:02:18Z

x-pack/plugins/enterprise_search/public/applications/workplace_search/routes.ts

+export const SYNCHRONIZATION_DOCS_URL = `${DOCS_PREFIX}}/workplace-search-customizing-indexing-rules.html#workplace-search-customizing-indexing-rules`;
+export const DIFFERENT_SYNC_TYPES_DOCS_URL = `${DOCS_PREFIX}}/workplace-search-customizing-indexing-rules.html#_indexing_schedule`;
+export const OBJECTS_AND_ASSETS_DOCS_URL = `${DOCS_PREFIX}}/workplace-search-customizing-indexing-rules.html##workplace-search-customizing-indexing-rules`;


Is this link correct? It doesn't work for me and I'm not sure if ## is a typo.

Definitely a typo; nice catch!

Double-checking, should it be the same link as SYNCHRONIZATION_DOCS_URL?

That was the directive given here. Have asked for clarification. Nice catch!

Was unable to figure out the TypeScript but did some more digging

kibanamachine · 2021-10-13T01:32:05Z

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`enterpriseSearch`	1.4MB	1.4MB	+834.0B

History

💚 Build #160232 succeeded 2665bb8
💚 Build #160162 succeeded 383e7d1
💚 Build #160135 succeeded e0a1a2f

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

…4696) * Fix unsaved changes prompt showing up between tabs We already added a reducer for this but forgot to implement this. Because we have shared state between the tabs, we need to overrule the unsaved changes prompt when simply navigating between tabs. * Fix some timezone issues After wiring up the backend and converting to UTC, some changes to mocks and time formats had to be made. * Refactor to remove blockedWindows reducer This commit refactors to make use of the already-in-state schedule object. Previously, while wiring up the static views, I used a blockedWindows array directly on the state tree. This simplifies things so that equality checks can be done with one object. * Wire up ability to remove blocked window * Fix key and remove fallback It was hard to test removing an item from an array that doesn’t exist so I changed the code to expect the array to be present (! operator), since the other path is not possible. Also updated the server value from deletion to delete to match the API * Wire up blocked windows form to change values and update state * Pass formatted blocked_windows to server (test was covered in previous commit) * Update link text, hrefs, and replace temp copy One of the links was removed intentionally * Fix typo * Fix edge case where unsaved changes shown when removing last item The API omits the key when there are no items so we need to have the item removed as well in the UI state. Otherwise, removing the last item will cause the UI to say there are unsaved changes when there are not. I tried setting it as: schedule.blockedWindows = undefined but the selector did not see those as equal but deleting the key does. * More typo fixes Syncronization -> Synchronization (+h) * Fix link address * Refactor for simplicity Was unable to figure out the TypeScript but did some more digging

kibanamachine · 2021-10-13T01:35:01Z

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.

…114743) * Fix unsaved changes prompt showing up between tabs We already added a reducer for this but forgot to implement this. Because we have shared state between the tabs, we need to overrule the unsaved changes prompt when simply navigating between tabs. * Fix some timezone issues After wiring up the backend and converting to UTC, some changes to mocks and time formats had to be made. * Refactor to remove blockedWindows reducer This commit refactors to make use of the already-in-state schedule object. Previously, while wiring up the static views, I used a blockedWindows array directly on the state tree. This simplifies things so that equality checks can be done with one object. * Wire up ability to remove blocked window * Fix key and remove fallback It was hard to test removing an item from an array that doesn’t exist so I changed the code to expect the array to be present (! operator), since the other path is not possible. Also updated the server value from deletion to delete to match the API * Wire up blocked windows form to change values and update state * Pass formatted blocked_windows to server (test was covered in previous commit) * Update link text, hrefs, and replace temp copy One of the links was removed intentionally * Fix typo * Fix edge case where unsaved changes shown when removing last item The API omits the key when there are no items so we need to have the item removed as well in the UI state. Otherwise, removing the last item will cause the UI to say there are unsaved changes when there are not. I tried setting it as: schedule.blockedWindows = undefined but the selector did not see those as equal but deleting the key does. * More typo fixes Syncronization -> Synchronization (+h) * Fix link address * Refactor for simplicity Was unable to figure out the TypeScript but did some more digging Co-authored-by: Scotty Bollinger <scotty.bollinger@elastic.co>

scottybollinger added 8 commits October 12, 2021 12:29

Fix unsaved changes prompt showing up between tabs

e02ab40

We already added a reducer for this but forgot to implement this. Because we have shared state between the tabs, we need to overrule the unsaved changes prompt when simply navigating between tabs.

Fix some timezone issues

55ba75f

After wiring up the backend and converting to UTC, some changes to mocks and time formats had to be made.

Wire up ability to remove blocked window

5dba565

Fix key and remove fallback

1a0c5df

It was hard to test removing an item from an array that doesn’t exist so I changed the code to expect the array to be present (! operator), since the other path is not possible. Also updated the server value from deletion to delete to match the API

Wire up blocked windows form to change values and update state

6e49fb8

Pass formatted blocked_windows to server

9a84909

(test was covered in previous commit)

Update link text, hrefs, and replace temp copy

e0a1a2f

One of the links was removed intentionally

scottybollinger added release_note:skip Skip the PR/issue when compiling release notes auto-backport Deprecated - use backport:version if exact versions are needed v7.16.0 labels Oct 12, 2021

scottybollinger added 2 commits October 12, 2021 12:43

Fix typo

c33c42c

scottybollinger marked this pull request as ready for review October 12, 2021 18:01

scottybollinger requested a review from a team October 12, 2021 18:01

More typo fixes

383e7d1

Syncronization -> Synchronization (+h)

yakhinvadim reviewed Oct 12, 2021

View reviewed changes

Fix link address

2665bb8

scottybollinger enabled auto-merge (squash) October 12, 2021 21:20

scottybollinger disabled auto-merge October 12, 2021 23:10

Refactor for simplicity

e4ae6e6

Was unable to figure out the TypeScript but did some more digging

scottybollinger enabled auto-merge (squash) October 12, 2021 23:35

yakhinvadim approved these changes Oct 12, 2021

View reviewed changes

scottybollinger merged commit 3025942 into elastic:master Oct 13, 2021

kibanamachine mentioned this pull request Oct 13, 2021

[7.x] [Workplace Search] Wire up write view for Blocked windows (#114696) #114743

Merged

scottybollinger deleted the scottybollinger/sync-sched-5 branch March 15, 2022 15:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Workplace Search] Wire up write view for Blocked windows#114696

[Workplace Search] Wire up write view for Blocked windows#114696
scottybollinger merged 13 commits intoelastic:masterfrom
scottybollinger:scottybollinger/sync-sched-5

scottybollinger commented Oct 12, 2021 •

edited

Loading

Uh oh!

yakhinvadim left a comment

Uh oh!

yakhinvadim Oct 12, 2021

Uh oh!

scottybollinger Oct 12, 2021

Uh oh!

yakhinvadim Oct 12, 2021

Uh oh!

yakhinvadim Oct 12, 2021

Uh oh!

scottybollinger Oct 12, 2021

Uh oh!

yakhinvadim Oct 12, 2021

Uh oh!

scottybollinger Oct 12, 2021

Uh oh!

kibanamachine commented Oct 13, 2021

Uh oh!

kibanamachine commented Oct 13, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

scottybollinger commented Oct 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Uh oh!

yakhinvadim left a comment

Choose a reason for hiding this comment

Uh oh!

yakhinvadim Oct 12, 2021

Choose a reason for hiding this comment

Uh oh!

scottybollinger Oct 12, 2021

Choose a reason for hiding this comment

Uh oh!

yakhinvadim Oct 12, 2021

Choose a reason for hiding this comment

Uh oh!

yakhinvadim Oct 12, 2021

Choose a reason for hiding this comment

Uh oh!

scottybollinger Oct 12, 2021

Choose a reason for hiding this comment

Uh oh!

yakhinvadim Oct 12, 2021

Choose a reason for hiding this comment

Uh oh!

scottybollinger Oct 12, 2021

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Oct 13, 2021

💚 Build Succeeded

Metrics [docs]

Async chunks

History

Uh oh!

kibanamachine commented Oct 13, 2021

💚 Backport successful

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

scottybollinger commented Oct 12, 2021 •

edited

Loading