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

[Logs Explorer] Rename test subjects and page objects #175711

Merged
merged 7 commits into from
Jan 30, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
2 changes: 1 addition & 1 deletion .buildkite/ftr_configs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ enabled:
- x-pack/test/functional/apps/ml/short_tests/config.ts
- x-pack/test/functional/apps/ml/stack_management_jobs/config.ts
- x-pack/test/functional/apps/monitoring/config.ts
- x-pack/test/functional/apps/observability_log_explorer/config.ts
- x-pack/test/functional/apps/observability_logs_explorer/config.ts
- x-pack/test/functional/apps/painless_lab/config.ts
- x-pack/test/functional/apps/remote_clusters/config.ts
- x-pack/test/functional/apps/reporting_management/config.ts
Expand Down
4 changes: 2 additions & 2 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -1055,7 +1055,7 @@ x-pack/plugins/infra/server/lib/alerting @elastic/obs-ux-management-team
/x-pack/test/functional/apps/monitoring @elastic/obs-ux-infra_services-team @elastic/stack-monitoring
/x-pack/test/api_integration/apis/monitoring @elastic/obs-ux-infra_services-team @elastic/stack-monitoring
/x-pack/test/api_integration/apis/monitoring_collection @elastic/obs-ux-infra_services-team @elastic/stack-monitoring
/x-pack/test_serverless/functional/test_suites/observability/observability_log_explorer @elastic/obs-ux-logs-team
/x-pack/test_serverless/functional/test_suites/observability/observability_logs_explorer @elastic/obs-ux-logs-team

# Fleet
/fleet_packages.json @elastic/fleet
Expand Down Expand Up @@ -1086,7 +1086,7 @@ x-pack/plugins/infra/server/lib/alerting @elastic/obs-ux-management-team

# Logs
/x-pack/test/api_integration/apis/logs_ui @elastic/obs-ux-logs-team
/x-pack/test/functional/apps/observability_log_explorer @elastic/obs-ux-logs-team
/x-pack/test/functional/apps/observability_logs_explorer @elastic/obs-ux-logs-team

# Observability onboarding tour
/x-pack/plugins/observability_shared/public/components/tour @elastic/platform-onboarding
Expand Down
2 changes: 1 addition & 1 deletion docs/developer/plugin-list.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -648,7 +648,7 @@ using the CURL scripts in the scripts folder.
|{kib-repo}blob/{branch}/x-pack/plugins/observability_solution/logs_explorer/README.md[logsExplorer]
|This plugin is home to the <LogExplorer /> component and related types. It implements several of the underlying concepts that the Observability Log Explorer app builds upon.
|This plugin is home to the <LogExplorer /> component and related types. It implements several of the underlying concepts that the Observability Logs Explorer app builds upon.
|{kib-repo}blob/{branch}/x-pack/plugins/logs_shared/README.md[logsShared]
Expand Down
2 changes: 1 addition & 1 deletion packages/deeplinks/observability/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

export const LOGS_APP_ID = 'logs';

export const OBSERVABILITY_LOG_EXPLORER_APP_ID = 'observability-log-explorer';
export const OBSERVABILITY_LOGS_EXPLORER_APP_ID = 'observability-logs-explorer';

export const OBSERVABILITY_OVERVIEW_APP_ID = 'observability-overview';

Expand Down
4 changes: 2 additions & 2 deletions packages/deeplinks/observability/deep_links.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@ import {
APM_APP_ID,
LOGS_APP_ID,
METRICS_APP_ID,
OBSERVABILITY_LOG_EXPLORER_APP_ID,
OBSERVABILITY_LOGS_EXPLORER_APP_ID,
OBSERVABILITY_ONBOARDING_APP_ID,
OBSERVABILITY_OVERVIEW_APP_ID,
SYNTHETICS_APP_ID,
} from './constants';

type LogsApp = typeof LOGS_APP_ID;
type ObservabilityLogExplorerApp = typeof OBSERVABILITY_LOG_EXPLORER_APP_ID;
type ObservabilityLogExplorerApp = typeof OBSERVABILITY_LOGS_EXPLORER_APP_ID;
type ObservabilityOverviewApp = typeof OBSERVABILITY_OVERVIEW_APP_ID;
type MetricsApp = typeof METRICS_APP_ID;
type ApmApp = typeof APM_APP_ID;
Expand Down
2 changes: 1 addition & 1 deletion packages/deeplinks/observability/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

export {
LOGS_APP_ID,
OBSERVABILITY_LOG_EXPLORER_APP_ID,
OBSERVABILITY_LOGS_EXPLORER_APP_ID,
OBSERVABILITY_ONBOARDING_APP_ID,
OBSERVABILITY_OVERVIEW_APP_ID,
} from './constants';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,8 @@ describe('DiscoverTopNavServerless', () => {
});
});

describe('LogExplorerTabs', () => {
it('should render when showLogExplorerTabs is true', async () => {
describe('LogsExplorerTabs', () => {
it('should render when showLogsExplorerTabs is true', async () => {
mockUseKibana.mockReturnValue({
services: {
...mockDiscoverService,
Expand All @@ -148,12 +148,12 @@ describe('DiscoverTopNavServerless', () => {
const topNav = screen.queryByTestId('discoverTopNavServerless');
expect(topNav).not.toBeNull();
await waitFor(() => {
const logExplorerTabs = screen.queryByTestId('logExplorerTabs');
const logExplorerTabs = screen.queryByTestId('logsExplorerTabs');
expect(logExplorerTabs).not.toBeNull();
});
});

it('should not render when showLogExplorerTabs is false', async () => {
it('should not render when showLogsExplorerTabs is false', async () => {
mockUseKibana.mockReturnValue({
services: {
...mockDiscoverService,
Expand All @@ -169,7 +169,7 @@ describe('DiscoverTopNavServerless', () => {
const topNav = screen.queryByTestId('discoverTopNavServerless');
expect(topNav).not.toBeNull();
await waitFor(() => {
const logExplorerTabs = screen.queryByTestId('logExplorerTabs');
const logExplorerTabs = screen.queryByTestId('logsExplorerTabs');
expect(logExplorerTabs).toBeNull();
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,15 @@ export const LogExplorerTabs = ({ services, selectedTab }: LogExplorerTabsProps)
});

return (
<EuiTabs bottomBorder={false} data-test-subj="logExplorerTabs">
<EuiTabs bottomBorder={false} data-test-subj="logsExplorerTabs">
<EuiTab
isSelected={selectedTab === 'discover'}
href={discoverUrl}
onClick={navigateToDiscover}
css={{ '.euiTab__content': { lineHeight: euiTheme.size.xxxl } }}
data-test-subj="discoverTab"
>
{i18n.translate('discover.logExplorerTabs.discover', {
{i18n.translate('discover.logsExplorerTabs.discover', {
defaultMessage: 'Discover',
})}
</EuiTab>
Expand All @@ -58,9 +58,9 @@ export const LogExplorerTabs = ({ services, selectedTab }: LogExplorerTabsProps)
href={logExplorerUrl}
onClick={navigateToLogExplorer}
css={{ '.euiTab__content': { lineHeight: euiTheme.size.xxxl } }}
data-test-subj="logExplorerTab"
data-test-subj="logsExplorerTab"
>
{i18n.translate('discover.logExplorerTabs.logExplorer', {
{i18n.translate('discover.logsExplorerTabs.logsExplorer', {
defaultMessage: 'Logs Explorer',
})}
</EuiTab>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ export const applicationUsageSchema = {
ml: commonSchema,
monitoring: commonSchema,
'observability-log-explorer': commonSchema,
'observability-logs-explorer': commonSchema,
'observability-overview': commonSchema,
observabilityOnboarding: commonSchema,
observabilityAIAssistant: commonSchema,
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/telemetry/schema/oss_plugins.json
Original file line number Diff line number Diff line change
Expand Up @@ -4587,7 +4587,7 @@
}
}
},
"observability-log-explorer": {
"observability-logs-explorer": {
"properties": {
"appId": {
"type": "keyword",
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/infra/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ export class Plugin implements InfraClientPluginClass {
entries: [
{
label: 'Explorer',
app: 'observability-log-explorer',
app: 'observability-logs-explorer',
path: '/',
isBetaFeature: true,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -618,11 +618,11 @@ describe('[Logs onboarding] Custom logs - install elastic agent', () => {
.should('exist');
});

it('when user clicks on Explore Logs it navigates to observability log explorer', () => {
it('when user clicks on Explore Logs it navigates to observability logs explorer', () => {
cy.wait('@checkOnboardingProgress');
cy.getByTestSubj('obltOnboardingExploreLogs').should('exist').click();

cy.url().should('include', '/app/observability-log-explorer');
cy.url().should('include', '/app/observability-logs-explorer');
cy.get('[data-test-subj="datasetSelectorPopoverButton"]')
.contains('[Mylogs] mylogs', { matchCase: false })
.should('exist');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -673,11 +673,11 @@ describe('[Logs onboarding] System logs', () => {
cy.visitKibana('/app/observabilityOnboarding/systemLogs');
});

it('when users clicks on Explore logs they navigate to log explorer - All logs', () => {
it('when users clicks on Explore logs they navigate to logs explorer - All logs', () => {
cy.wait('@systemIntegrationInstall');
cy.getByTestSubj('obltOnboardingExploreLogs').should('exist').click();

cy.url().should('include', '/app/observability-log-explorer');
cy.url().should('include', '/app/observability-logs-explorer');
cy.get('button').contains('All logs').should('exist');
});
});
Expand All @@ -692,11 +692,11 @@ describe('[Logs onboarding] System logs', () => {
cy.visitKibana('/app/observabilityOnboarding/systemLogs');
});

it('when users clicks on Explore logs they navigate to log explorer and System integration is selected', () => {
it('when users clicks on Explore logs they navigate to logs explorer and System integration is selected', () => {
cy.wait('@systemIntegrationInstall');
cy.getByTestSubj('obltOnboardingExploreLogs').should('exist').click();

cy.url().should('include', '/app/observability-log-explorer');
cy.url().should('include', '/app/observability-logs-explorer');
cy.get('button').contains('[System] syslog').should('exist');
});
});
Expand Down
14 changes: 7 additions & 7 deletions x-pack/plugins/observability_solution/logs_explorer/README.md
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
# Log Explorer
# Logs Explorer

This plugin is home to the `<LogExplorer />` component and related types. It implements several of the underlying concepts that the [Observability Log Explorer app](../observability_solution/observability_logs_explorer) builds upon.
This plugin is home to the `<LogExplorer />` component and related types. It implements several of the underlying concepts that the [Observability Logs Explorer app](../observability_solution/observability_logs_explorer) builds upon.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: shall we rename also the component here and in the implementation to LogsExplorer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Definitely, there are quite a few things that still have the singular name, I have started renaming these in batches to avoid major conflicts and long-running PRs. This PR is mostly for tests and the app url.


## Developing the `<LogExplorer />` component

⚠ The Log Explorer is in early stages of development, so the following partly describes the current situation and partly the intended future scenario.
⚠ The Logs Explorer is in early stages of development, so the following partly describes the current situation and partly the intended future scenario.

### Dependencies

Expand All @@ -20,11 +20,11 @@ While not fully realized yet, the dependency graph would roughly resemble the fo
```mermaid
flowchart TD
obs_log_explorer_app(Observability Log Explorer app)
obs_log_explorer_app(Observability Logs Explorer app)
obs_apps(Other Observability apps)
obs_log_explorer_component(Observability Log Explorer component)
obs_log_explorer_component(Observability Logs Explorer component)
other_apps(Other non-Observability apps)
log_explorer_component(Log Explorer component)
log_explorer_component(Logs Explorer component)
platform(Kibana Platform)
discover(Discover Main container)
fleet(Fleet / EPM)
Expand All @@ -47,7 +47,7 @@ When designing the API we face two conflicting goals:
- It should be easy to consume by any non-observability app. This means...
- its API needs to be relatively stable and straightforward.
- it should not perform any page-wide changes that could interfere with consuming app's page state (such as URL changes).
- It should be extensible so the Observability Log Explorer can build on top of this.
- It should be extensible so the Observability Logs Explorer can build on top of this.

In its current state the `<LogExplorer />` achieves neither goal. To resolve the tension in the future we could export two variants with different sets of properties.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

export const LOG_EXPLORER_PROFILE_ID = 'log-explorer';
export const LOG_EXPLORER_PROFILE_ID = 'logs-explorer';

// Fields constants
export const TIMESTAMP_FIELD = '@timestamp';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export function FlyoutHeader({ doc }: { doc: FlyoutDoc }) {
<LogLevel
level={doc[constants.LOG_LEVEL_FIELD]}
renderInFlyout={true}
dataTestSubj="logExplorerFlyoutLogLevel"
dataTestSubj="logsExplorerFlyoutLogLevel"
/>
</EuiFlexItem>
</HoverActionPopover>
Expand All @@ -71,7 +71,7 @@ export function FlyoutHeader({ doc }: { doc: FlyoutDoc }) {

const contentField = hasMessageField && (
<EuiFlexItem grow={false}>
<EuiFlexGroup direction="column" gutterSize="s" data-test-subj="logExplorerFlyoutLogMessage">
<EuiFlexGroup direction="column" gutterSize="s" data-test-subj="logsExplorerFlyoutLogMessage">
<EuiFlexItem>
<EuiFlexGroup alignItems="flexEnd" gutterSize="none" justifyContent="spaceBetween">
<EuiFlexItem grow={false}>
Expand Down Expand Up @@ -110,9 +110,9 @@ export function FlyoutHeader({ doc }: { doc: FlyoutDoc }) {
buttonContent={accordionTitle}
paddingSize="m"
initialIsOpen={true}
data-test-subj={`logExplorerFlyoutHeaderSection${flyoutContentLabel}`}
data-test-subj={`logsExplorerFlyoutHeaderSection${flyoutContentLabel}`}
>
<EuiFlexGroup direction="column" gutterSize="none" data-test-subj="logExplorerFlyoutDetail">
<EuiFlexGroup direction="column" gutterSize="none" data-test-subj="logsExplorerFlyoutDetail">
{hasMessageField ? contentField : logLevelAndTimestamp}
</EuiFlexGroup>
</EuiAccordion>
Expand Down
Loading