Skip to content

Commit

Permalink
[Feature] Allow order control in createAggConfig (#3160)
Browse files Browse the repository at this point in the history
* [Feature] Allow order control in `createAggConfig`

Add new optional boolean param that adds the new `AggConfig` to the beginning of the array rather than the end.
Makes it easier to work with Pie or other visualizations with `Schemas` that set `mustBeFirst`

Signed-off-by: Josh Romero <rmerqg@amazon.com>

* [Chore] Add changelog

Signed-off-by: Josh Romero <rmerqg@amazon.com>

Signed-off-by: Josh Romero <rmerqg@amazon.com>
Co-authored-by: Qingyang(Abby) Hu <abigailhu2000@gmail.com>
  • Loading branch information
joshuarrrr and abbyhu2000 authored Jan 9, 2023
1 parent 46186a4 commit b750846
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 4 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- [Multi DataSource] Improve test connection ([#3110](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3110))
- [Vis Builder] Add app filter and query persistence without using state container ([#3100](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3100))
- [Optimizer] Increase timeout waiting for the exiting of an optimizer worker ([#3193](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3193))
- [Data] Update `createAggConfig` so that newly created configs can be added to beginning of `aggConfig` array ([#3160](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3160))


### 🐛 Bug Fixes

Expand Down
52 changes: 51 additions & 1 deletion src/plugins/data/common/search/aggs/agg_configs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ describe('AggConfigs', () => {
expect(ac.aggs).toHaveLength(3);
});

it('adds new AggConfig entries to AggConfigs by default', () => {
it('adds new AggConfig entries to end of AggConfigs by default', () => {
const configStates = [
{
enabled: true,
Expand All @@ -136,6 +136,7 @@ describe('AggConfigs', () => {
schema: 'split',
});
expect(ac.aggs).toHaveLength(2);
expect(ac.aggs[1].schema).toBe('split');
});

it('does not add an agg to AggConfigs if addToAggConfigs: false', () => {
Expand All @@ -161,6 +162,55 @@ describe('AggConfigs', () => {
);
expect(ac.aggs).toHaveLength(1);
});

it('adds new AggConfig entries to beginning of AggConfigs if mustBeFirst: true', () => {
const configStates = [
{
enabled: true,
type: 'histogram',
params: {},
},
];

const ac = new AggConfigs(indexPattern, configStates, { typesRegistry });
expect(ac.aggs).toHaveLength(1);

ac.createAggConfig(
{
enabled: true,
type: 'terms',
params: {},
schema: 'split',
},
{ mustBeFirst: true }
);
expect(ac.aggs).toHaveLength(2);
expect(ac.aggs[0].schema).toBe('split');
});

it('does not add an agg to AggConfigs if addToAggConfigs: false and mustBeFirst: true', () => {
const configStates = [
{
enabled: true,
type: 'histogram',
params: {},
},
];

const ac = new AggConfigs(indexPattern, configStates, { typesRegistry });
expect(ac.aggs).toHaveLength(1);

ac.createAggConfig(
{
enabled: true,
type: 'terms',
params: {},
schema: 'split',
},
{ addToAggConfigs: false, mustBeFirst: true }
);
expect(ac.aggs).toHaveLength(1);
});
});

describe('#getRequestAggs', () => {
Expand Down
10 changes: 7 additions & 3 deletions src/plugins/data/common/search/aggs/agg_configs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,10 +130,10 @@ export class AggConfigs {

createAggConfig = <T extends AggConfig = AggConfig>(
params: CreateAggConfigParams,
{ addToAggConfigs = true } = {}
{ addToAggConfigs = true, mustBeFirst = false } = {}
) => {
const { type } = params;
let aggConfig;
let aggConfig: AggConfig;

if (params instanceof AggConfig) {
aggConfig = params;
Expand All @@ -145,8 +145,12 @@ export class AggConfigs {
});
}

const addAggConfig = () => {
return mustBeFirst ? this.aggs.unshift(aggConfig) : this.aggs.push(aggConfig);
};

if (addToAggConfigs) {
this.aggs.push(aggConfig);
addAggConfig();
}

return aggConfig as T;
Expand Down

0 comments on commit b750846

Please sign in to comment.