[Data Explorer] Allow render from View directly, not from Data Explorer

[ananzh]

trafficstars

Description

This PR avoids re-rendering the entire AppContainer when data services update. The re-render is caused by history object which is in AppMountParameters. This history object is part of the application's routing context and can change frequently as the url is updated by query, filter or time range. Each change in the history object could potentially trigger a re-render of the AppContainer and its child components.

With this PR, the AppContainer will not be re-loaded. Instead, each component, like Canvas and Panel, will be re-rendered based on the updates from data.

Issues Resolved

NA

Screenshot

• new workflow

https://github.com/opensearch-project/OpenSearch-Dashboards/assets/79961084/ab505ca5-4045-4813-8f88-16b8df2733e1

Check List

• [x] All tests pass
  • [ ] yarn test:jest
  • [ ] yarn test:jest_integration
• [ ] New functionality includes testing.
• [ ] New functionality has been documented.
• [x] Update CHANGELOG.md
• [x] Commits are signed per the DCO using --signoff

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 80.00000% with 4 lines in your changes missing coverage. Please review.

Project coverage is 67.42%. Comparing base (c6820db) to head (56a33c7). Report is 5 commits behind head on main.

Files	Patch %	Lines
.../data_explorer/public/components/app_container.tsx	66.66%	3 Missing and 1 partial :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6167      +/-   ##
==========================================
- Coverage   67.44%   67.42%   -0.03%     
==========================================
  Files        3444     3445       +1     
  Lines       67849    67791      -58     
  Branches    11035    11026       -9     
==========================================
- Hits        45764    45705      -59     
  Misses      19418    19418              
- Partials     2667     2668       +1     

Flag	Coverage Δ
Linux_1	33.11% <ø> (+0.02%)	:arrow_up:
Linux_2	55.04% <ø> (-0.09%)	:arrow_down:
Linux_3	?
Linux_4	?
Windows_1	33.13% <ø> (+0.02%)	:arrow_up:
Windows_2	55.00% <ø> (-0.09%)	:arrow_down:
Windows_3	45.27% <80.00%> (-0.03%)	:arrow_down:
Windows_4	34.85% <ø> (+0.03%)	:arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[ashwin-pc]

the issue with this approach is that when the user changes the query, the current context is lost. In 2.9 the lable once rendered does not change until eother the new result comes in or there are no results

[ashwin-pc]

if history is the reason for the rerender, cant we just move it out of the props?

[ananzh]

the issue with this approach is that when the user changes the query, the current context is lost. In 2.9 the lable once rendered does not change until eother the new result comes in or there are no results

Let me evaluate whether Discover needs history passed from Data Explorer.

[kavilla]

@ananzh, double check if this still relevant and next steps.

[ananzh]

if history is the reason for the rerender, cant we just move it out of the props?

We can't. History is used in Data explorer and it is not easy to remove.

[ashwin-pc]

the issue with this approach is that when the user changes the query, the current context is lost. In 2.9 the lable once rendered does not change until eother the new result comes in or there are no results

@ananzh did we resolve this?

[ananzh]

the issue with this approach is that when the user changes the query, the current context is lost. In 2.9 the lable once rendered does not change until eother the new result comes in or there are no results

@ananzh did we resolve this?

I have used a diff approach and don't have that issue. Discover can still use its own history.

[kavilla]

@ananzh I think one of the tests are failing for refresh interval after re-running still. Mind checking it?

[github-actions[bot]]

❌ Invalid Changelog Heading

The '## Changelog' heading in your PR description is either missing or malformed. Please make sure that your PR description includes a '## Changelog' heading with proper spelling, capitalization, spacing, and Markdown syntax.

[ananzh]

@ananzh I think one of the tests are failing for refresh interval after re-running still. Mind checking it?

@kavilla I have fixed the test in https://github.com/opensearch-project/opensearch-dashboards-functional-test/pull/1358

Since we don't have double render, the tmp solution

Cypress.Commands.add('makeDatePickerMenuOpen', () => {
  cy.get(
    '[class="euiFormControlLayout euiFormControlLayout--group euiSuperDatePicker"]'
  ).then(($popover) => {
    // Check if the popover does not have the 'euiPopover-isOpen' class
    if (!$popover.hasClass('euiPopover-isOpen')) {
      // If not open, click the button to open the quick menu
      cy.getElementByTestId('superDatePickerToggleQuickMenuButton').click();
    }
  });
})

, which is just to re-click the DatePickerToggle to make the menu re-open, is not required. We could just simply remove this tmp fix.

https://github.com/opensearch-project/OpenSearch-Dashboards/assets/79961084/b5551487-21b5-44e7-a4d4-d959fc372b24

[github-actions[bot]]

[MANUAL CYPRESS TEST RUN RESULTS]

:white_check_mark: Cypress test run succeeded!

Inputs:

Test repo: 'ananzh/opensearch-dashboards-functional-test'
Test branch: 'update-test'

Test spec:
''

Link to results:

https://github.com/opensearch-project/OpenSearch-Dashboards/actions/runs/9389954391