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.

Sign up for GitHub

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

Jump to bottom

[Chore] Update timeline default expression #2720

Merged
merged 15 commits into from
Mar 27, 2023
Merged

[Chore] Update timeline default expression #2720

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
327212c
[Chore] Update timeline default expression
joshuarrrr Nov 1, 2022
47303cf
[Chore] Update other timeline `es()` examples and test usage
joshuarrrr Nov 18, 2022
8413249
[Chore] Update BWC data generation to use es(*)
joshuarrrr Nov 18, 2022
8b6c195
[Chore] Update changelog
joshuarrrr Nov 18, 2022
fee215a
Merge branch 'main' into chore/update-default-timeline-exp
joshuarrrr Dec 28, 2022
7e8eba6
Merge branch 'main' into chore/update-default-timeline-exp
joshuarrrr Dec 29, 2022
0667e2b
Merge branch 'main' into chore/update-default-timeline-exp
joshuarrrr Mar 8, 2023
0e7c231
Merge branch 'main' into chore/update-default-timeline-exp
joshuarrrr Mar 9, 2023
3485523
Merge branch 'main' into chore/update-default-timeline-exp
joshuarrrr Mar 14, 2023
189a239
Test: update generate_data to account for selectAll not working
joshuarrrr Mar 14, 2023
76d6b3f
Test: clear expression before each timeline test
joshuarrrr Mar 15, 2023
db8983d
Test: fully clear expression text before each
joshuarrrr Mar 20, 2023
bdaff53
try `clear()` in before each
joshuarrrr Mar 24, 2023
0cc1d3f
Add better programmatic text clearing for monaco editor
joshuarrrr Mar 24, 2023
6401ff4
Update the expression clearing in the generate_data bwc helpers
joshuarrrr Mar 24, 2023
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
### 🛠 Maintenance

- Adding @zhongnansu as maintainer. ([#2590](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2590))
- [Timeline] Update default expressions from `.es(*)` to `.opensearch(*)`. ([2720](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2720))
- Removes `minimatch` manual resolution ([#3019](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3019))
- Remove `github-checks-reporter`, an unused dependency ([#3126](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3126))
- Upgrade `vega-lite` dependency to ^5.6.0 ([#3076](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3076))
Expand Down
45 changes: 24 additions & 21 deletions cypress/integration/with-security/check_timeline.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,87 +44,90 @@ describe('check timeline visualization', () => {
.find('[data-test-subj="newItemButton"]')
.click();
cy.get('[data-test-subj="visType-timelion"]').click();
// Because monaco editor doesn't use a contenteditable, input, or textarea, .clear() or .type('{selectall}') won't work. To clear, we just backspace for each character instead.
cy.get('[class="view-line"]')
.invoke('text')
.then((expressionText) => {
cy.get('[class="view-line"]').type('{backspace}'.repeat(expressionText.length));
});
});

it('.es(*, kibana1=true) should report search error', () => {
cy.get('[class="view-line"]').type('{selectall}{backspace}, kibana1=true)');
cy.get('[class="view-line"]').type('.es(*, kibana1=true)');
cy.get('[data-test-subj="visualizeEditorRenderButton"]').click();
cy.waitForLoader();
cy.get('[data-test-subj="globalToastList"]')
.find('[data-test-subj="errorToastMessage"]')
.contains('Timeline request error: undefined Error: Unknown argument to es: kibana1');
});

it('.es(*, kibana=true) should not report search error', () => {
cy.get('[class="view-line"]').type('{selectall}{backspace}, kibana=true)');
cy.get('[class="view-line"]').type('.es(*, kibana=true)');
cy.get('[data-test-subj="visualizeEditorRenderButton"]').click();
cy.waitForLoader();
cy.get('[data-test-subj="globalToastList"]')
.find('[data-test-subj="errorToastMessage"]')
.should('not.exist');
});

it('.es(*, opensearchDashboards=true) should not report search error', () => {
cy.get('[class="view-line"]').type('{selectall}{backspace}, opensearchDashboards=true)');
cy.get('[class="view-line"]').type('.es(*, opensearchDashboards=true)');
cy.get('[data-test-subj="visualizeEditorRenderButton"]').click();
cy.waitForLoader();
cy.get('[data-test-subj="globalToastList"]')
.find('[data-test-subj="errorToastMessage"]')
.should('not.exist');
});

it('.elasticsearch(*, kibana1=true) should report search error', () => {
cy.get('[class="view-line"]').type(
'{selectall}{backspace}{backspace}{backspace}{backspace}{backspace}elasticsearch(*, kibana1=true)'
);
cy.get('[class="view-line"]').type('.elasticsearch(*, kibana1=true)');
cy.get('[data-test-subj="visualizeEditorRenderButton"]').click();
cy.waitForLoader();
cy.get('[data-test-subj="globalToastList"]')
.find('[data-test-subj="errorToastMessage"]')
.contains('Timeline request error: undefined Error: Unknown argument to es: kibana1');
});

it('.elasticsearch(*, kibana=true) should not report search error', () => {
cy.get('[class="view-line"]').type(
'{selectall}{backspace}{backspace}{backspace}{backspace}{backspace}elasticsearch(*, kibana=true)'
);
cy.get('[class="view-line"]').type('.elasticsearch(*, kibana=true)');
cy.get('[data-test-subj="visualizeEditorRenderButton"]').click();
cy.waitForLoader();
cy.get('[data-test-subj="globalToastList"]')
.find('[data-test-subj="errorToastMessage"]')
.should('not.exist');
});

it('.elasticsearch(*, opensearchDashboards=true) should not report search error', () => {
cy.get('[class="view-line"]').type(
'{selectall}{backspace}{backspace}{backspace}{backspace}{backspace}elasticsearch(*, opensearchDashboards=true)'
);
cy.get('[class="view-line"]').type('.elasticsearch(*, opensearchDashboards=true)');
cy.get('[data-test-subj="visualizeEditorRenderButton"]').click();
cy.waitForLoader();
cy.get('[data-test-subj="globalToastList"]')
.find('[data-test-subj="errorToastMessage"]')
.should('not.exist');
});

it('.opensearch(*, kibana1=true) should report search error', () => {
cy.get('[class="view-line"]').type(
'{selectall}{backspace}{backspace}{backspace}{backspace}{backspace}opensearch(*, kibana1=true)'
);
cy.get('[class="view-line"]').type('.opensearch(*, kibana1=true)');
cy.get('[data-test-subj="visualizeEditorRenderButton"]').click();
cy.waitForLoader();
cy.get('[data-test-subj="globalToastList"]')
.find('[data-test-subj="errorToastMessage"]')
.contains('Timeline request error: undefined Error: Unknown argument to es: kibana1');
});

it('.opensearch(*, kibana=true) should not report search error', () => {
cy.get('[class="view-line"]').type(
'{selectall}{backspace}{backspace}{backspace}{backspace}{backspace}opensearch(*, kibana=true)'
);
cy.get('[class="view-line"]').type('.opensearch(*, kibana=true)');
cy.get('[data-test-subj="visualizeEditorRenderButton"]').click();
cy.waitForLoader();
cy.get('[data-test-subj="globalToastList"]')
.find('[data-test-subj="errorToastMessage"]')
.should('not.exist');
});

it('.opensearch(*, opensearchDashboards=true) should not report search error', () => {
cy.get('[class="view-line"]').type(
'{selectall}{backspace}{backspace}{backspace}{backspace}{backspace}opensearch(*, opensearchDashboards=true)'
);
cy.get('[class="view-line"]').type('.opensearch(*, opensearchDashboards=true)');
cy.get('[data-test-subj="visualizeEditorRenderButton"]').click();
cy.waitForLoader();
cy.get('[data-test-subj="globalToastList"]')
.find('[data-test-subj="errorToastMessage"]')
.should('not.exist');
Expand Down
8 changes: 8 additions & 0 deletions cypress/integration/with-security/helpers/generate_data.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,14 @@ describe('Generating BWC test data with security', () => {
.find('[data-test-subj="newItemButton"]')
.click();
cy.get('[data-test-subj="visType-timelion"]').click();
// Because monaco editor doesn't use a contenteditable, input, or textarea, .clear() or .type('{selectall}') won't work. To clear, we just backspace for each character instead.
cy.get('[class="view-line"]')
.invoke('text')
.then((expressionText) => {
cy.get('[class="view-line"]').type('{backspace}'.repeat(expressionText.length));
});
// update default expression to use `.es(*)` instead of `.opensearch(*)` for bwc
Copy link
Member

Choose a reason for hiding this comment

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

NIT: do you think we could create a common function

Cypress.Commands.add('clearWithBackspaces', (selector, count) => {
  cy.get(selector).then($element => {
    const backspaces = '{backspace}'.repeat(count);
    $element.type(`{selectAll}${backspaces}`);
  });
});

So this could simplifies to

cy.clearWithBackspaces('[class="view-line"]', 15).type('.es(*)');

and this function could also be imported in other tests. I see there are 2 more tests using similar function. maybe put it in a until or common folder?

Also curious why we need these many backspaces

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I meant to call this out in a comment. For some reason, the {selectAll} cypress sequence doesn't work in this field the way it's supposed to (or at least the way I understand that it's supposed to work). Previously, when the default expression was .es(*), there were 6 total characters - you'll see that some tests "cleared" the input by repeating five backspaces (and reusing the .). Now that we've made the default expression opensearch(*), there are 14 total characters that need deleting, if you delete them one by one. I didn't try .clear() either, but because it's supposed to just alias type({selectAll}{backspace}), I suspect it won't work either.

I do like the simplification idea, although I don't think we need a common function for it.

Copy link
Member Author

Choose a reason for hiding this comment

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

See if you like my latest approach better! I sure do 😁

Copy link
Member

Choose a reason for hiding this comment

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

kk I like it

cy.get('[class="view-line"]').type('.es(*)');
cy.get('[data-test-subj="visualizeSaveButton"]').click();
cy.get('[data-test-subj="savedObjectTitle"]').type('test-timeline');
cy.get('[data-test-subj="confirmSaveSavedObjectButton"]').click();
Expand Down
45 changes: 24 additions & 21 deletions cypress/integration/without-security/check_timeline.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,87 +28,90 @@ describe('check timeline visualization', () => {
.find('[data-test-subj="newItemButton"]')
.click();
cy.get('[data-test-subj="visType-timelion"]').click();
// Because monaco editor doesn't use a contenteditable, input, or textarea, .clear() or .type('{selectall}') won't work. To clear, we just backspace for each character instead.
cy.get('[class="view-line"]')
.invoke('text')
.then((expressionText) => {
cy.get('[class="view-line"]').type('{backspace}'.repeat(expressionText.length));
});
});

it('.es(*, kibana1=true) should report search error', () => {
cy.get('[class="view-line"]').type('{selectall}{backspace}, kibana1=true)');
cy.get('[class="view-line"]').type('.es(*, kibana1=true)');
cy.get('[data-test-subj="visualizeEditorRenderButton"]').click();
cy.waitForLoader();
cy.get('[data-test-subj="globalToastList"]')
.find('[data-test-subj="errorToastMessage"]')
.contains('Timeline request error: undefined Error: Unknown argument to es: kibana1');
});

it('.es(*, kibana=true) should not report search error', () => {
cy.get('[class="view-line"]').type('{selectall}{backspace}, kibana=true)');
cy.get('[class="view-line"]').type('.es(*, kibana=true)');
cy.get('[data-test-subj="visualizeEditorRenderButton"]').click();
cy.waitForLoader();
cy.get('[data-test-subj="globalToastList"]')
.find('[data-test-subj="errorToastMessage"]')
.should('not.exist');
});

it('.es(*, opensearchDashboards=true) should not report search error', () => {
cy.get('[class="view-line"]').type('{selectall}{backspace}, opensearchDashboards=true)');
cy.get('[class="view-line"]').type('.es(*, opensearchDashboards=true)');
cy.get('[data-test-subj="visualizeEditorRenderButton"]').click();
cy.waitForLoader();
cy.get('[data-test-subj="globalToastList"]')
.find('[data-test-subj="errorToastMessage"]')
.should('not.exist');
});

it('.elasticsearch(*, kibana1=true) should report search error', () => {
cy.get('[class="view-line"]').type(
'{selectall}{backspace}{backspace}{backspace}{backspace}{backspace}elasticsearch(*, kibana1=true)'
);
cy.get('[class="view-line"]').type('.elasticsearch(*, kibana1=true)');
cy.get('[data-test-subj="visualizeEditorRenderButton"]').click();
cy.waitForLoader();
cy.get('[data-test-subj="globalToastList"]')
.find('[data-test-subj="errorToastMessage"]')
.contains('Timeline request error: undefined Error: Unknown argument to es: kibana1');
});

it('.elasticsearch(*, kibana=true) should not report search error', () => {
cy.get('[class="view-line"]').type(
'{selectall}{backspace}{backspace}{backspace}{backspace}{backspace}elasticsearch(*, kibana=true)'
);
cy.get('[class="view-line"]').type('.elasticsearch(*, kibana=true)');
cy.get('[data-test-subj="visualizeEditorRenderButton"]').click();
cy.waitForLoader();
cy.get('[data-test-subj="globalToastList"]')
.find('[data-test-subj="errorToastMessage"]')
.should('not.exist');
});

it('.elasticsearch(*, opensearchDashboards=true) should not report search error', () => {
cy.get('[class="view-line"]').type(
'{selectall}{backspace}{backspace}{backspace}{backspace}{backspace}elasticsearch(*, opensearchDashboards=true)'
);
cy.get('[class="view-line"]').type('.elasticsearch(*, opensearchDashboards=true)');
cy.get('[data-test-subj="visualizeEditorRenderButton"]').click();
cy.waitForLoader();
cy.get('[data-test-subj="globalToastList"]')
.find('[data-test-subj="errorToastMessage"]')
.should('not.exist');
});

it('.opensearch(*, kibana1=true) should report search error', () => {
cy.get('[class="view-line"]').type(
'{selectall}{backspace}{backspace}{backspace}{backspace}{backspace}opensearch(*, kibana1=true)'
);
cy.get('[class="view-line"]').type('.opensearch(*, kibana1=true)');
cy.get('[data-test-subj="visualizeEditorRenderButton"]').click();
cy.waitForLoader();
cy.get('[data-test-subj="globalToastList"]')
.find('[data-test-subj="errorToastMessage"]')
.contains('Timeline request error: undefined Error: Unknown argument to es: kibana1');
});

it('.opensearch(*, kibana=true) should not report search error', () => {
cy.get('[class="view-line"]').type(
'{selectall}{backspace}{backspace}{backspace}{backspace}{backspace}opensearch(*, kibana=true)'
);
cy.get('[class="view-line"]').type('.opensearch(*, kibana=true)');
cy.get('[data-test-subj="visualizeEditorRenderButton"]').click();
cy.waitForLoader();
cy.get('[data-test-subj="globalToastList"]')
.find('[data-test-subj="errorToastMessage"]')
.should('not.exist');
});

it('.opensearch(*, opensearchDashboards=true) should not report search error', () => {
cy.get('[class="view-line"]').type(
'{selectall}{backspace}{backspace}{backspace}{backspace}{backspace}opensearch(*, opensearchDashboards=true)'
);
cy.get('[class="view-line"]').type('.opensearch(*, opensearchDashboards=true)');
cy.get('[data-test-subj="visualizeEditorRenderButton"]').click();
cy.waitForLoader();
cy.get('[data-test-subj="globalToastList"]')
.find('[data-test-subj="errorToastMessage"]')
.should('not.exist');
Expand Down
8 changes: 8 additions & 0 deletions cypress/integration/without-security/helpers/generate_data.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,14 @@ describe('Generating BWC test data without security', () => {
.find('[data-test-subj="newItemButton"]')
.click();
cy.get('[data-test-subj="visType-timelion"]').click();
// Because monaco editor doesn't use a contenteditable, input, or textarea, .clear() or .type('{selectall}') won't work. To clear, we just backspace for each character instead.
cy.get('[class="view-line"]')
.invoke('text')
.then((expressionText) => {
cy.get('[class="view-line"]').type('{backspace}'.repeat(expressionText.length));
});
// update default expression to use `.es(*)` instead of `.opensearch(*)` for bwc
cy.get('[class="view-line"]').type('.es(*)');
cy.get('[data-test-subj="visualizeSaveButton"]').click();
cy.get('[data-test-subj="savedObjectTitle"]').type('test-timeline');
cy.get('[data-test-subj="confirmSaveSavedObjectButton"]').click();
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/timeline/public/app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ export function initTimelineApp(app, deps) {
const savedVisualizations = deps.plugins.visualizations.savedVisualizationsLoader;
const timezone = getTimezone(deps.core.uiSettings);

const defaultExpression = '.es(*)';
const defaultExpression = '.opensearch(*)';

$scope.topNavMenu = getTopNavMenu();

Expand Down
4 changes: 2 additions & 2 deletions src/plugins/timeline/public/directives/timeline_expression_input.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
For some reasons it doesn't work without it (even though the default role of
the element is textbox anyway). -->
<textarea data-expression-input role="textbox" rows="{{ rows }}" class="timExpressionInput kuiTextArea fullWidth"
placeholder="{{ ::'timeline.expressionInputPlaceholder' | i18n: { defaultMessage: 'Try a query with {opensearchQuery}', values: { opensearchQuery: '.es(*)' } } }}"
placeholder="{{ ::'timeline.expressionInputPlaceholder' | i18n: { defaultMessage: 'Try a query with {opensearchQuery}', values: { opensearchQuery: '.opensearch(*)' } } }}"
ng-model="sheet" ng-focus="onFocusInput()" ng-keydown="onKeyDownInput($event)" ng-keyup="onKeyUpInput($event)"
ng-blur="onBlurInput()" ng-mousedown="onMouseDownInput()" ng-mouseup="onMouseUpInput()"
ng-click="onClickExpression()"
Expand All @@ -16,4 +16,4 @@
suggestions-type="suggestions.type" selected-index="suggestions.index"
on-click-suggestion="onClickSuggestion(suggestionIndex)" should-popover="shouldPopoverSuggestions">
</timeline-expression-suggestions>
</div>
</div>
Loading