Skip to content

Commit

Permalink
Merge branch 'cases-rbac-poc' of github.com:elastic/kibana into add-s…
Browse files Browse the repository at this point in the history
…pace-only-tests
  • Loading branch information
jonathan-buttner committed May 10, 2021
2 parents 3211311 + 21d173d commit a1ba735
Show file tree
Hide file tree
Showing 43 changed files with 1,316 additions and 498 deletions.
66 changes: 38 additions & 28 deletions STYLEGUIDE.md → STYLEGUIDE.mdx
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
# Kibana Style Guide
---
id: kibStyleGuide
slug: /kibana-dev-docs/styleguide
title: StyleGuide
summary: JavaScript/TypeScript styleguide.
date: 2021-05-06
tags: ['kibana', 'onboarding', 'dev', 'styleguide', 'typescript', 'javascript']
---

This guide applies to all development within the Kibana project and is
recommended for the development of all Kibana plugins.

- [General](#general)
- [HTML](#html)
- [API endpoints](#api-endpoints)
- [TypeScript/JavaScript](#typeScript/javaScript)
- [SASS files](#sass-files)
- [React](#react)

Besides the content in this style guide, the following style guides may also apply
to all development within the Kibana project. Please make sure to also read them:

Expand Down Expand Up @@ -52,9 +52,7 @@ This part contains style guide rules around general (framework agnostic) HTML us
Use camel case for the values of attributes such as `id` and `data-test-subj` selectors.

```html
<button id="veryImportantButton" data-test-subj="clickMeButton">
Click me
</button>
<button id="veryImportantButton" data-test-subj="clickMeButton">Click me</button>
```

The only exception is in cases where you're dynamically creating the value, and you need to use
Expand Down Expand Up @@ -378,6 +376,20 @@ import inFoo from 'foo/child';
import inSibling from '../foo/child';
```
#### Avoid export \* in top level index.ts files
The exports in `common/index.ts`, `public/index.ts` and `server/index.ts` dictate a plugin's public API. The public API should be carefully controlled, and using `export *` makes it very easy for a developer working on internal changes to export a new public API unintentionally.
```js
// good
export { foo } from 'foo';
export { child } from './child';

// bad
export * from 'foo/child';
export * from '../foo/child';
```
### Global definitions
Don't do this. Everything should be wrapped in a module that can be depended on
Expand Down Expand Up @@ -592,20 +604,20 @@ Do not use setters, they cause more problems than they can solve.
### Avoid circular dependencies
As part of a future effort to use correct and idempotent build tools we need our code to be
able to be represented as a directed acyclic graph. We must avoid having circular dependencies
both on code and type imports to achieve that. One of the most critical parts is the plugins
code. We've developed a tool to identify plugins with circular dependencies which
has allowed us to build a list of plugins who have circular dependencies
between each other.
When building plugins we should avoid importing from plugins
who are known to have circular dependencies at the moment as well as introducing
new circular dependencies. You can run the same tool we use on our CI locally by
typing `node scripts/find_plugins_with_circular_deps --debug`. It will error out in
case new circular dependencies has been added with your changes
able to be represented as a directed acyclic graph. We must avoid having circular dependencies
both on code and type imports to achieve that. One of the most critical parts is the plugins
code. We've developed a tool to identify plugins with circular dependencies which
has allowed us to build a list of plugins who have circular dependencies
between each other.
When building plugins we should avoid importing from plugins
who are known to have circular dependencies at the moment as well as introducing
new circular dependencies. You can run the same tool we use on our CI locally by
typing `node scripts/find_plugins_with_circular_deps --debug`. It will error out in
case new circular dependencies has been added with your changes
(which will also happen in the CI) as well as print out the current list of
the known circular dependencies which, as mentioned before, should not be imported
by your code until the circular dependencies on these have been solved.
the known circular dependencies which, as mentioned before, should not be imported
by your code until the circular dependencies on these have been solved.
## SASS files
Expand All @@ -626,10 +638,8 @@ import './component.scss';
// All other imports below the SASS import

export const Component = () => {
return (
<div className="plgComponent" />
);
}
return <div className="plgComponent" />;
};
```
```scss
Expand Down
3 changes: 1 addition & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@
"proxy-from-env": "1.0.0",
"proxyquire": "1.8.0",
"puid": "1.0.7",
"puppeteer": "npm:@elastic/puppeteer@5.4.1-patch.1",
"puppeteer": "^8.0.0",
"query-string": "^6.13.2",
"raw-loader": "^3.1.0",
"rbush": "^3.0.1",
Expand Down Expand Up @@ -586,7 +586,6 @@
"@types/pretty-ms": "^5.0.0",
"@types/prop-types": "^15.7.3",
"@types/proper-lockfile": "^3.0.1",
"@types/puppeteer": "^5.4.1",
"@types/rbush": "^3.0.0",
"@types/reach__router": "^1.2.6",
"@types/react": "^16.9.36",
Expand Down
26 changes: 24 additions & 2 deletions src/plugins/data/common/search/search_source/search_source.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const mockSource2 = { excludes: ['bar-*'] };

const indexPattern = ({
title: 'foo',
fields: [{ name: 'foo-bar' }, { name: 'field1' }, { name: 'field2' }],
fields: [{ name: 'foo-bar' }, { name: 'field1' }, { name: 'field2' }, { name: '_id' }],
getComputedFields,
getSourceFiltering: () => mockSource,
} as unknown) as IndexPattern;
Expand Down Expand Up @@ -68,7 +68,7 @@ describe('SearchSource', () => {
beforeEach(() => {
const getConfigMock = jest
.fn()
.mockImplementation((param) => param === 'metaFields' && ['_type', '_source'])
.mockImplementation((param) => param === 'metaFields' && ['_type', '_source', '_id'])
.mockName('getConfig');

mockSearchMethod = jest
Expand Down Expand Up @@ -458,6 +458,28 @@ describe('SearchSource', () => {
expect(request.fields).toEqual([{ field: 'field1' }, { field: 'field2' }]);
});

test('excludes metafields from the request', async () => {
searchSource.setField('index', ({
...indexPattern,
getComputedFields: () => ({
storedFields: [],
scriptFields: [],
docvalueFields: [],
}),
} as unknown) as IndexPattern);
searchSource.setField('fields', [{ field: '*', include_unmapped: 'true' }]);

const request = searchSource.getSearchRequestBody();
expect(request.fields).toEqual([{ field: 'field1' }, { field: 'field2' }]);

searchSource.setField('fields', ['foo-bar', 'foo--bar', 'field1', 'field2']);
expect(request.fields).toEqual([{ field: 'field1' }, { field: 'field2' }]);

searchSource.removeField('fields');
searchSource.setField('fieldsFromSource', ['foo-bar', 'foo--bar', 'field1', 'field2']);
expect(request.fields).toEqual([{ field: 'field1' }, { field: 'field2' }]);
});

test('returns all scripted fields when one fields entry is *', async () => {
searchSource.setField('index', ({
...indexPattern,
Expand Down
29 changes: 17 additions & 12 deletions src/plugins/data/common/search/search_source/search_source.ts
Original file line number Diff line number Diff line change
Expand Up @@ -682,6 +682,7 @@ export class SearchSource {
searchRequest.body = searchRequest.body || {};
const { body, index, query, filters, highlightAll } = searchRequest;
searchRequest.indexType = this.getIndexType(index);
const metaFields = getConfig(UI_SETTINGS.META_FIELDS);

// get some special field types from the index pattern
const { docvalueFields, scriptFields, storedFields, runtimeFields } = index
Expand Down Expand Up @@ -712,7 +713,7 @@ export class SearchSource {
body._source = sourceFilters;
}

const filter = fieldWildcardFilter(body._source.excludes, getConfig(UI_SETTINGS.META_FIELDS));
const filter = fieldWildcardFilter(body._source.excludes, metaFields);
// also apply filters to provided fields & default docvalueFields
body.fields = body.fields.filter((fld: SearchFieldValue) => filter(this.getFieldName(fld)));
fieldsFromSource = fieldsFromSource.filter((fld: SearchFieldValue) =>
Expand Down Expand Up @@ -793,17 +794,21 @@ export class SearchSource {
const field2Name = this.getFieldName(fld2);
return field1Name === field2Name;
}
).map((fld: SearchFieldValue) => {
const fieldName = this.getFieldName(fld);
if (Object.keys(docvaluesIndex).includes(fieldName)) {
// either provide the field object from computed docvalues,
// or merge the user-provided field with the one in docvalues
return typeof fld === 'string'
? docvaluesIndex[fld]
: this.getFieldFromDocValueFieldsOrIndexPattern(docvaluesIndex, fld, index);
}
return fld;
});
)
.filter((fld: SearchFieldValue) => {
return !metaFields.includes(this.getFieldName(fld));
})
.map((fld: SearchFieldValue) => {
const fieldName = this.getFieldName(fld);
if (Object.keys(docvaluesIndex).includes(fieldName)) {
// either provide the field object from computed docvalues,
// or merge the user-provided field with the one in docvalues
return typeof fld === 'string'
? docvaluesIndex[fld]
: this.getFieldFromDocValueFieldsOrIndexPattern(docvaluesIndex, fld, index);
}
return fld;
});
}
} else {
body.fields = filteredDocvalueFields;
Expand Down
3 changes: 2 additions & 1 deletion test/functional/apps/management/_test_huge_fields.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ export default function ({ getService, getPageObjects }) {
const security = getService('security');
const PageObjects = getPageObjects(['common', 'home', 'settings']);

describe('test large number of fields', function () {
// FLAKY: https://github.com/elastic/kibana/issues/89031
describe.skip('test large number of fields', function () {
this.tags(['skipCloud']);

const EXPECTED_FIELD_COUNT = '10006';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
const PageObjects = getPageObjects(['common', 'visualize', 'visEditor', 'header', 'timePicker']);
const comboBox = getService('comboBox');

describe('dynamic options', () => {
// FLAKY: https://github.com/elastic/kibana/issues/98974
describe.skip('dynamic options', () => {
describe('without chained controls', () => {
beforeEach(async () => {
await PageObjects.common.navigateToApp('visualize');
Expand Down
Loading

0 comments on commit a1ba735

Please sign in to comment.