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

[Discover] Refactor discover.js controller topnav code #79062

Merged
Merged
Show file tree
Hide file tree
Changes from 37 commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
edfc3bc
Initial refactorings
kertal Oct 1, 2020
430d646
Fix async - await issue
kertal Oct 1, 2020
0db09ea
Refactor updateSearchSource to update_search_source.ts
kertal Oct 1, 2020
c075d5f
Fix types & jest tests
kertal Oct 5, 2020
b2803aa
Extract persistSavedSearch function
kertal Oct 6, 2020
b8119b8
Merge master / fix conflicts
kertal Oct 7, 2020
42be32e
Merge remote-tracking branch 'upstream/master' into kertal-pr-2020-10…
kertal Oct 8, 2020
34de861
Update types
kertal Oct 10, 2020
d4b246b
Refactor
kertal Oct 10, 2020
ca127f8
Improve code
kertal Oct 12, 2020
f4fbf2b
Remove console.log
kertal Oct 12, 2020
17d51dd
Merge branch 'master' into kertal-pr-2020-10-01-discover-refactor-top…
kibanamachine Nov 2, 2020
595e2ae
Merge branch 'kertal-pr-2020-10-01-discover-refactor-topnav-code' of …
kertal Nov 2, 2020
764d632
Merge remote-tracking branch 'upstream/master' into kertal-pr-2020-10…
kertal Nov 3, 2020
1496644
Improve index pattern loading code
kertal Nov 3, 2020
fe77851
Refactor index pattern code and add test
kertal Nov 4, 2020
828db70
Fix type
kertal Nov 4, 2020
ce3ada9
Add another test
kertal Nov 5, 2020
5ad8491
Merge and fix conflicts
kertal Nov 5, 2020
886b7c9
Add tests
kertal Nov 9, 2020
81fa22e
Update snapshot
kertal Nov 9, 2020
3735b50
Undo fixed scroll changes
kertal Nov 9, 2020
a1b0c8a
Add more tests and refactor
kertal Nov 12, 2020
3fb0982
Improve test code
kertal Nov 12, 2020
8b335b2
Merge and fix conflicts
kertal Nov 12, 2020
deef3fd
Merge branch 'master' into kertal-pr-2020-10-01-discover-refactor-top…
kibanamachine Nov 16, 2020
d8902dd
Merge remote-tracking branch 'upstream/master' into kertal-pr-2020-10…
kertal Nov 17, 2020
5a1f3ef
Merge branch 'kertal-pr-2020-10-01-discover-refactor-topnav-code' of …
kertal Nov 17, 2020
3e83e59
Improve index pattern code
kertal Nov 17, 2020
4e1f56b
Add removeField to SearchSource
kertal Nov 19, 2020
2c362e9
Address review comments
kertal Nov 19, 2020
9c87314
Address review comments
kertal Nov 19, 2020
6c09281
Merge remote-tracking branch 'upstream/master' into kertal-pr-2020-10…
kertal Nov 19, 2020
748bf48
Generate API
kertal Nov 19, 2020
797063f
Generate API part 2
kertal Nov 19, 2020
240d365
Merge remote-tracking branch 'upstream/master' into kertal-pr-2020-10…
kertal Nov 19, 2020
4a5b11c
Seems I've broken sorting ... time to fix
kertal Nov 19, 2020
f659bb0
Merge branch 'master' into kertal-pr-2020-10-01-discover-refactor-top…
kibanamachine Nov 23, 2020
1ef02fa
Merge branch 'master' into kertal-pr-2020-10-01-discover-refactor-top…
kibanamachine Nov 24, 2020
2acc852
Add jest test to search_source.test.ts
kertal Nov 24, 2020
e74c39d
Merge branch 'kertal-pr-2020-10-01-discover-refactor-topnav-code' of …
kertal Nov 24, 2020
bddbb23
Merge branch 'master' into kertal-pr-2020-10-01-discover-refactor-top…
kibanamachine Nov 25, 2020
910b5da
Improve SearchSource code
kertal Nov 25, 2020
be26672
Merge branch 'kertal-pr-2020-10-01-discover-refactor-topnav-code' of …
kertal Nov 25, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export declare class SearchSource
| [getSearchRequestBody()](./kibana-plugin-plugins-data-public.searchsource.getsearchrequestbody.md) | | Returns body contents of the search request, often referred as query DSL. |
| [getSerializedFields()](./kibana-plugin-plugins-data-public.searchsource.getserializedfields.md) | | serializes search source fields (which can later be passed to [ISearchStartSearchSource](./kibana-plugin-plugins-data-public.isearchstartsearchsource.md)<!-- -->) |
| [onRequestStart(handler)](./kibana-plugin-plugins-data-public.searchsource.onrequeststart.md) | | Add a handler that will be notified whenever requests start |
| [removeField(field)](./kibana-plugin-plugins-data-public.searchsource.removefield.md) | | remove field |
| [serialize()](./kibana-plugin-plugins-data-public.searchsource.serialize.md) | | Serializes the instance to a JSON string and a set of referenced objects. Use this method to get a representation of the search source which can be stored in a saved object.<!-- -->The references returned by this function can be mixed with other references in the same object, however make sure there are no name-collisions. The references will be named <code>kibanaSavedObjectMeta.searchSourceJSON.index</code> and <code>kibanaSavedObjectMeta.searchSourceJSON.filter[&lt;number&gt;].meta.index</code>.<!-- -->Using <code>createSearchSource</code>, the instance can be re-created. |
| [setField(field, value)](./kibana-plugin-plugins-data-public.searchsource.setfield.md) | | sets value to a single search source field |
| [setFields(newFields)](./kibana-plugin-plugins-data-public.searchsource.setfields.md) | | Internal, do not use. Overrides all search source fields with the new field array. |
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<!-- Do not edit this file. It is automatically generated by API Documenter. -->

[Home](./index.md) &gt; [kibana-plugin-plugins-data-public](./kibana-plugin-plugins-data-public.md) &gt; [SearchSource](./kibana-plugin-plugins-data-public.searchsource.md) &gt; [removeField](./kibana-plugin-plugins-data-public.searchsource.removefield.md)

## SearchSource.removeField() method

remove field

<b>Signature:</b>

```typescript
removeField<K extends keyof SearchSourceFields>(field: K): this;
```

## Parameters

| Parameter | Type | Description |
| --- | --- | --- |
| field | <code>K</code> | |

<b>Returns:</b>

`this`

1 change: 1 addition & 0 deletions src/plugins/data/common/search/search_source/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export const searchSourceInstanceMock: MockedKeys<ISearchSource> = {
setPreferredSearchStrategyId: jest.fn(),
setFields: jest.fn().mockReturnThis(),
setField: jest.fn().mockReturnThis(),
removeField: jest.fn().mockReturnThis(),
getId: jest.fn(),
getFields: jest.fn(),
getField: jest.fn(),
Expand Down
11 changes: 10 additions & 1 deletion src/plugins/data/common/search/search_source/search_source.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,13 +142,22 @@ export class SearchSource {
*/
setField<K extends keyof SearchSourceFields>(field: K, value: SearchSourceFields[K]) {
if (value == null) {
delete this.fields[field];
return this.removeField(field);
} else {
kertal marked this conversation as resolved.
Show resolved Hide resolved
this.fields[field] = value;
}
return this;
}

/**
* remove field
* @param field: field name
*/
removeField<K extends keyof SearchSourceFields>(field: K) {
delete this.fields[field];
return this;
}
Copy link
Member Author

@kertal kertal Nov 19, 2020

Choose a reason for hiding this comment

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

Why did I add this? To prevent TypeScript confusion, because when I used 'setFields(key, null)' to remove it, I got depending what I was trying to remove TypeScript errors

Copy link
Member

Choose a reason for hiding this comment

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

Would be nice if we could add a quick unit test for the new method

Copy link
Member Author

Choose a reason for hiding this comment

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

Absolutely! I've added a jest test.


/**
* Internal, do not use. Overrides all search source fields with the new field array.
*
Expand Down
1 change: 1 addition & 0 deletions src/plugins/data/public/public.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -2157,6 +2157,7 @@ export class SearchSource {
// (undocumented)
history: SearchRequest[];
onRequestStart(handler: (searchSource: SearchSource, options?: ISearchOptions) => Promise<unknown>): void;
removeField<K extends keyof SearchSourceFields>(field: K): this;
serialize(): {
searchSourceJSON: string;
references: import("src/core/server").SavedObjectReference[];
Expand Down
30 changes: 30 additions & 0 deletions src/plugins/discover/public/__mocks__/config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { IUiSettingsClient } from '../../../../core/public';

export const configMock = ({
get: (key: string) => {
if (key === 'defaultIndex') {
return 'the-index-pattern-id';
}

return '';
},
} as unknown) as IUiSettingsClient;
74 changes: 74 additions & 0 deletions src/plugins/discover/public/__mocks__/index_pattern.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { IndexPattern, indexPatterns } from '../kibana_services';
import { IIndexPatternFieldList } from '../../../data/common/index_patterns/fields';

const fields = [
{
name: '_index',
type: 'string',
scripted: false,
filterable: true,
},
{
name: 'message',
type: 'string',
scripted: false,
filterable: false,
},
{
name: 'extension',
type: 'string',
scripted: false,
filterable: true,
},
{
name: 'bytes',
type: 'number',
scripted: false,
filterable: true,
},
{
name: 'scripted',
type: 'number',
scripted: true,
filterable: false,
},
] as IIndexPatternFieldList;

fields.getByName = (name: string) => {
return fields.find((field) => field.name === name);
};

const indexPattern = ({
id: 'the-index-pattern-id',
title: 'the-index-pattern-title',
metaFields: ['_index', '_score'],
flattenHit: undefined,
formatHit: jest.fn((hit) => hit._source),
fields,
getComputedFields: () => ({}),
getSourceFiltering: () => ({}),
getFieldByName: () => ({}),
} as unknown) as IndexPattern;

indexPattern.flattenHit = indexPatterns.flattenHitWrapper(indexPattern, indexPattern.metaFields);

export const indexPatternMock = indexPattern;
32 changes: 32 additions & 0 deletions src/plugins/discover/public/__mocks__/index_patterns.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { IndexPatternsService } from '../../../data/common';
import { indexPatternMock } from './index_pattern';

export const indexPatternsMock = ({
getCache: () => {
return [indexPatternMock];
},
get: (id: string) => {
if (id === 'the-index-pattern-id') {
return indexPatternMock;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we return an empty string here as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

for the case when the id is different? of course we can build up on this very basic mock, will be useful in other tests, however the original code here provides different error handling, so mocking this would work differently

},
} as unknown) as IndexPatternsService;
41 changes: 41 additions & 0 deletions src/plugins/discover/public/__mocks__/saved_search.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { SavedSearch } from '../saved_searches';

export const savedSearchMock = ({
id: 'the-saved-search-id',
type: 'search',
attributes: {
title: 'the-saved-search-title',
kibanaSavedObjectMeta: {
searchSourceJSON:
'{"highlightAll":true,"version":true,"query":{"query":"foo : \\"bar\\" ","language":"kuery"},"filter":[],"indexRefName":"kibanaSavedObjectMeta.searchSourceJSON.index"}',
},
},
references: [
{
name: 'kibanaSavedObjectMeta.searchSourceJSON.index',
type: 'index-pattern',
id: 'the-index-pattern-id',
},
],
migrationVersion: { search: '7.5.0' },
error: undefined,
} as unknown) as SavedSearch;
Loading