[AMBARI-24551] [Log Search UI] get rid of redundant requests after undoing or redoing several history steps #5

tobias-istvan · 2018-10-08T11:47:03Z

What changes were proposed in this pull request?

We can prevent extra calls by using the LogsContainerService resetFiltersForms method.

How was this patch tested?

It was tested manually checking the Network tab in dev tools and by running unit tests:

PhantomJS 2.1.1 (Mac OS X 0.0.0): Executed 269 of 269 SUCCESS (9.276 secs / 9.178 secs)
✨  Done in 35.58s.

Please review Ambari Contributing Guide before opening a pull request.

asfgit · 2018-10-08T11:50:50Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ambari-LogSearch-Github-PR-Builder/7/
Test PASSed.

oleewere

lgtm

aBabiichuk · 2018-10-08T15:32:10Z

ambari-logsearch-web/src/app/services/history-manager.service.ts

@@ -292,19 +292,12 @@ export class HistoryManagerService {
   * @param {object} value
   */
  private handleUndoOrRedo(value: object): void {
-    const filtersForm = this.logsContainerService.filtersForm;
    this.hasNoPendingUndoOrRedo = false;


Looks like hasNoPendingUndoOrRedo and isUndoOrRedo aren't needed any more, as well as onFormValueChanges which handles those values

asfgit · 2018-10-09T12:49:18Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ambari-LogSearch-Github-PR-Builder/9/
Test PASSed.

aBabiichuk · 2018-10-09T13:08:16Z

ambari-logsearch-web/src/app/services/history-manager.service.ts

    if (startIndex === -1) {
      startIndex = allItems.length;
    }
-    if (endIndex === -1) {


This isUndoOrRedo/endIndex logic shouldn't be removed here. It's needed to handle the situation when after some undoing/redoing user goes on with non-historical filters changes, so there's nothing to redo. Sorry for my omission in previous comment.
P.S. Getting rid of hasNoPendingUndoOrRedo looks OK to me.

asfgit · 2018-10-30T17:02:13Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ambari-LogSearch-Github-PR-Builder/21/
Test FAILed.
Test FAILured.

asfgit · 2018-11-08T14:46:08Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ambari-LogSearch-Github-PR-Builder/28/
Test FAILed.
Test FAILured.

asfgit · 2018-11-09T16:59:32Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ambari-LogSearch-Github-PR-Builder/29/
Test FAILed.
Test FAILured.

asfgit · 2018-11-10T12:27:43Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ambari-LogSearch-Github-PR-Builder/35/
Test FAILed.
Test FAILured.

…doing or redoing several history steps

…doing or redoing several history steps - PR fixes

…doing or redoing several history steps - fixing form control implementations

…doing or redoing several history steps - change history manager with actions/reducers/effects focused solution.

…doing or redoing several history steps - working history manager

…doing or redoing several history steps - request in progress indicators

…doing or redoing several history steps - fixing dropdown with icons, optimize code readibility, changing labels

asfgit · 2018-11-12T16:31:25Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ambari-LogSearch-Github-PR-Builder/38/
Test FAILed.
Test FAILured.

asfgit · 2018-11-12T16:34:29Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ambari-LogSearch-Github-PR-Builder/39/
Test FAILed.
Test FAILured.

asfgit · 2018-11-12T16:42:57Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ambari-LogSearch-Github-PR-Builder/40/
Test FAILed.
Test FAILured.

…doing or redoing several history steps - cleaning up the branch, writing tests fixing issues revealed by unit tests.

asfgit · 2018-11-12T17:02:46Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ambari-LogSearch-Github-PR-Builder/41/
Test PASSed.

tobias-istvan · 2018-11-12T17:04:11Z

@aBabiichuk
I completely refactored the history manager. The initial reason was to simplify the business logic around the history management, also make it more URL handling compatible since the history is connected to the linkable URL.

I followed the previous approach which is use more action/effects/reducers over services in order to decouple the direct dependencies.

The main reason of the redundant request was the wrong implementation of the ControlValueAccessor interface.

I also added some UX improvements to indicate when a request is in progress in the background. There is a global notification and added some specific ones for log tables and graphs.

asfgit · 2018-11-12T17:07:07Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ambari-LogSearch-Github-PR-Builder/42/
Test PASSed.

aBabiichuk

History items in undo dropdown are listed incorrectly: newer entries should be higher than the older ones

...-logsearch-web/src/app/components/filter-history-manager/filter-history-manager.component.ts

ambari-logsearch-web/src/app/components/action-menu/action-menu.component.ts

…doing or redoing several history steps - PR change requests

asfgit · 2018-11-13T13:22:14Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ambari-LogSearch-Github-PR-Builder/44/
Test PASSed.

tobias-istvan added the web label Oct 8, 2018

tobias-istvan self-assigned this Oct 8, 2018

tobias-istvan requested a review from aBabiichuk October 8, 2018 11:47

oleewere approved these changes Oct 8, 2018

View reviewed changes

aBabiichuk reviewed Oct 8, 2018

View reviewed changes

aBabiichuk reviewed Oct 9, 2018

View reviewed changes

tobias-istvan force-pushed the AMBARI-24551-master branch from 227475d to a7c227f Compare November 8, 2018 14:40

tobias-istvan added 8 commits November 12, 2018 17:29

[AMBARI-24551] [Log Search UI] get rid of redundant requests after un…

d09e5c3

…doing or redoing several history steps

[AMBARI-24551] [Log Search UI] get rid of redundant requests after un…

dcadc20

…doing or redoing several history steps - PR fixes

In progress

2c14a70

[AMBARI-24551] [Log Search UI] get rid of redundant requests after un…

30c3157

…doing or redoing several history steps - fixing form control implementations

[AMBARI-24551] [Log Search UI] get rid of redundant requests after un…

1990a75

…doing or redoing several history steps - change history manager with actions/reducers/effects focused solution.

[AMBARI-24551] [Log Search UI] get rid of redundant requests after un…

12fc1b7

…doing or redoing several history steps - working history manager

[AMBARI-24551] [Log Search UI] get rid of redundant requests after un…

bc2b995

…doing or redoing several history steps - request in progress indicators

[AMBARI-24551] [Log Search UI] get rid of redundant requests after un…

5df64b3

…doing or redoing several history steps - fixing dropdown with icons, optimize code readibility, changing labels

tobias-istvan force-pushed the AMBARI-24551-master branch from 8312ada to 58ee472 Compare November 12, 2018 16:29

tobias-istvan force-pushed the AMBARI-24551-master branch from 58ee472 to b080f60 Compare November 12, 2018 16:39

tobias-istvan force-pushed the AMBARI-24551-master branch from b080f60 to d6559bd Compare November 12, 2018 16:58

[AMBARI-24551] [Log Search UI] get rid of redundant requests after un…

20c27ea

…doing or redoing several history steps - cleaning up the branch, writing tests fixing issues revealed by unit tests.

tobias-istvan force-pushed the AMBARI-24551-master branch from d6559bd to 20c27ea Compare November 12, 2018 17:02

aBabiichuk suggested changes Nov 13, 2018

View reviewed changes

...-logsearch-web/src/app/components/filter-history-manager/filter-history-manager.component.ts Outdated Show resolved Hide resolved

aBabiichuk suggested changes Nov 13, 2018

View reviewed changes

ambari-logsearch-web/src/app/components/action-menu/action-menu.component.ts Outdated Show resolved Hide resolved

[AMBARI-24551] [Log Search UI] get rid of redundant requests after un…

75c120d

…doing or redoing several history steps - PR change requests

aBabiichuk approved these changes Nov 13, 2018

View reviewed changes

tobias-istvan merged commit f40991f into apache:master Nov 13, 2018

tobias-istvan deleted the AMBARI-24551-master branch November 13, 2018 13:52

tobias-istvan restored the AMBARI-24551-master branch November 13, 2018 15:17

tobias-istvan mentioned this pull request Nov 13, 2018

[AMBARI-24551] [Log Search UI] get rid of redundant requests after undoing or redoing several history steps #22

Merged

tobias-istvan deleted the AMBARI-24551-master branch November 13, 2018 15:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[AMBARI-24551] [Log Search UI] get rid of redundant requests after undoing or redoing several history steps #5

[AMBARI-24551] [Log Search UI] get rid of redundant requests after undoing or redoing several history steps #5

tobias-istvan commented Oct 8, 2018

asfgit commented Oct 8, 2018

oleewere left a comment

aBabiichuk Oct 8, 2018 •

edited

asfgit commented Oct 9, 2018

aBabiichuk Oct 9, 2018 •

edited

asfgit commented Oct 30, 2018

asfgit commented Nov 8, 2018

asfgit commented Nov 9, 2018

asfgit commented Nov 10, 2018

asfgit commented Nov 12, 2018

asfgit commented Nov 12, 2018

asfgit commented Nov 12, 2018

asfgit commented Nov 12, 2018

tobias-istvan commented Nov 12, 2018

asfgit commented Nov 12, 2018

aBabiichuk left a comment

asfgit commented Nov 13, 2018

[AMBARI-24551] [Log Search UI] get rid of redundant requests after undoing or redoing several history steps #5

[AMBARI-24551] [Log Search UI] get rid of redundant requests after undoing or redoing several history steps #5

Conversation

tobias-istvan commented Oct 8, 2018

What changes were proposed in this pull request?

How was this patch tested?

asfgit commented Oct 8, 2018

oleewere left a comment

Choose a reason for hiding this comment

aBabiichuk Oct 8, 2018 • edited

Choose a reason for hiding this comment

asfgit commented Oct 9, 2018

aBabiichuk Oct 9, 2018 • edited

Choose a reason for hiding this comment

asfgit commented Oct 30, 2018

asfgit commented Nov 8, 2018

asfgit commented Nov 9, 2018

asfgit commented Nov 10, 2018

asfgit commented Nov 12, 2018

asfgit commented Nov 12, 2018

asfgit commented Nov 12, 2018

asfgit commented Nov 12, 2018

tobias-istvan commented Nov 12, 2018

asfgit commented Nov 12, 2018

aBabiichuk left a comment

Choose a reason for hiding this comment

asfgit commented Nov 13, 2018

aBabiichuk Oct 8, 2018 •

edited

aBabiichuk Oct 9, 2018 •

edited