Skip to content

Conversation

@ppisljar
Copy link
Contributor

@ppisljar ppisljar commented Feb 16, 2021

Summary

Summarize your PR. If it involves visual changes include a screenshot or gif.

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@ppisljar ppisljar requested a review from tsullivan February 16, 2021 17:02
@ppisljar ppisljar requested a review from a team as a code owner February 16, 2021 17:02
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-services (Team:AppServices)

@ppisljar ppisljar added Feature:Search Querying infrastructure in Kibana release_note:skip Skip the PR/issue when compiling release notes v7.12 v8.0.0 labels Feb 16, 2021
@timroes timroes added v7.12.0 and removed v7.12 labels Feb 17, 2021
Copy link
Contributor

@streamich streamich left a comment

Choose a reason for hiding this comment

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

Code changes LGTM, also ran Kibana on Mac/Chrome all seems to be working.

Comment on lines 178 to +181
/**
* returns all search source fields
*/
getFields(recurse = false): SearchSourceFields {
let thisFilter = this.fields.filter; // type is single value, array, or function
if (thisFilter) {
if (typeof thisFilter === 'function') {
thisFilter = thisFilter() || []; // type is single value or array
}

if (Array.isArray(thisFilter)) {
thisFilter = [...thisFilter];
} else {
thisFilter = [thisFilter];
}
} else {
thisFilter = [];
}

if (recurse) {
const parent = this.getParent();
if (parent) {
const parentFields = parent.getFields(recurse);

let parentFilter = parentFields.filter; // type is single value, array, or function
if (parentFilter) {
if (typeof parentFilter === 'function') {
parentFilter = parentFilter() || []; // type is single value or array
}

if (Array.isArray(parentFilter)) {
thisFilter.push(...parentFilter);
} else {
thisFilter.push(parentFilter);
}
}

// add combined filters to the fields
const thisFields = {
...this.fields,
filter: thisFilter,
};

return { ...parentFields, ...thisFields };
}
}
getFields(): SearchSourceFields {
Copy link
Contributor

Choose a reason for hiding this comment

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

If I understand it correctly, this new getFields will not return the parent field, as it is now removed in constructor, I guess that is intentional?

  /**
   * Returns all search source fields, except `parent` field.
   */
  getFields(): SearchSourceFields {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, this is how the method looked in 7.11

Co-authored-by: Vadim Dalecky <streamich@gmail.com>
@ppisljar ppisljar force-pushed the searchSource/serialize2 branch from 50709f4 to c6481cf Compare February 17, 2021 14:55
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
data 912.1KB 911.6KB -454.0B

History

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

Copy link
Member

@tsullivan tsullivan left a comment

Choose a reason for hiding this comment

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

LGTM

@ppisljar ppisljar merged commit 7503fd2 into elastic:master Feb 18, 2021
ppisljar added a commit to ppisljar/kibana that referenced this pull request Feb 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature:Search Querying infrastructure in Kibana release_note:skip Skip the PR/issue when compiling release notes v7.12.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants