fixing embed mode

[ppisljar]

Fixes #16595

@Bargs would it ever be a problem if we apply the same filter twice ?

dashboard currently set the filters and query by setting a new root search source
visualize editor however does't do that and we need to add them at the visualizations search source.

thats not ideal and this is just ment as a quick fix, look at the #16641 for more information on what we plan to do in the future.

with this fix we would always get the parameters out of appState, which means that on the dashboard same filters would be set twice (once by setting the rootsearchsource and once by courier response handler reading them out of appState)

we can't remove setting of the rootsearchsource as that would break the embeddable saved search

[elasticmachine]

💔 Build Failed

• continuous-integration/kibana-ci/pull-request

[ppisljar]

jenkins, test this

[timroes]

If Matt doesn't see an issue with that, that we missed, LGTM

[elasticmachine]

💔 Build Failed

• continuous-integration/kibana-ci/pull-request

[Bargs]

This does have the unfortunate effect of sending duplicate queries to ES if the same filter is added to two different SearchSources in the same SearchSource hierarchy. See the example request below, which I generated by clicking on a vis on a dashboard with this PR checked out. I don't know if such duplication will cause any problems, but it's probably best if we avoid it. What changed between 6.1.2 and 6.2.0? How did this used to work?

"query": {
    "bool": {
      "must": [
        {
          "match_all": {}
        },
        {
          "match_all": {}
        },
        {
          "match_phrase": {
            "machine.os.raw": {
              "query": "win 8"
            }
          }
        },
        {
          "match_phrase": {
            "machine.os.raw": {
              "query": "win 8"
            }
          }
        },
        {
          "range": {
            "@timestamp": {
              "gte": 1510762300048,
              "lte": 1518538300048,
              "format": "epoch_millis"
            }
          }
        }
      ],
      "filter": [],
      "should": [],
      "must_not": []
    }

[timroes]

@Bargs unfortunately the embedding mode was just broken for quite some time, that's why this didn't work for quite some time. I think embedding was broken since 6.0(?) and so it broke at the same time, that larger refactorings were made, meaning we cannot simply convert back to the working state.

If you consider this problematic, let's first wait until we had some discussions about #16641 and see if we will come up there also with a short term fix for the issue.

[Bargs]

The original bug report says this showed up after 6.1.2. I wouldn’t want to merge this without checking with someone on the ES team to confirm it will have no ill effects. Creating duplicate queries for every filter created via a vis on a dashboard really doesn’t seem good.

…

On Wed, Feb 14, 2018 at 4:11 AM Tim Roes ***@***.***> wrote: @Bargs <https://github.com/bargs> unfortunately the embedding mode was just broken for quite some time, that's why this didn't work for quite some time. I think embedding was broken since 6.0(?) and so it broke at the same time, that larger refactorings were made, meaning we cannot simply convert back to the working state. If you consider this problematic, let's first wait until we had some discussions about #16641 <#16641> and see if we will come up there also with a short term fix for the issue. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#16642 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AF8zyJDbYyZjAzWIGT4CBJuSbdwoGso_ks5tUqMrgaJpZM4R_-tb> .

[stacey-gammon]

Don't know if this is related, but I noticed before I went on leave (so early January) that filterBarClickHandler wasn't being used at all in the visualize embeddable factory. If you are trying to track down where the filtering broke, maybe that info will help. It was being used at some point...

[stacey-gammon]

Also, I think we should try to add a functional test that would have caught this issue. With all the refactoring that will be happening soon in regards to filtering and search source, it will be good to cover the corner cases. I can help too, since it'll probably be a test in the dashboard suite, but I think it'll be good to get it in with this PR so we can make sure it fails before this fix, and passes after it.

[elasticmachine]

💔 Build Failed

• continuous-integration/kibana-ci/pull-request

[poor-bob]

This is awesome, can't wait!

I figured I'd share why some of your edge cases exist:

we hijack a lot of the functionality into easy to select drop-downs, and steal the on-click event to switch between different loaded visualizations to achieve a drilldown effect.

final drilldown layer:

[elasticmachine]

💔 Build Failed

• continuous-integration/kibana-ci/pull-request

[ppisljar]

closing in favour of #19172