Skip to content

Commit b17fa0e

Browse files
TackAdamAdam Tackett
andauthored
[Bug] Side-nav applications (#12260)
* fix side nav Signed-off-by: Adam Tackett <tackadam@amazon.com> * address comment Signed-off-by: Adam Tackett <tackadam@amazon.com> --------- Signed-off-by: Adam Tackett <tackadam@amazon.com> Co-authored-by: Adam Tackett <tackadam@amazon.com>
1 parent 33c302d commit b17fa0e

3 files changed

Lines changed: 172 additions & 5 deletions

File tree

src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,11 @@ import { WorkspaceClient } from '../../workspace_client';
2828
import { DataSourceManagementPluginSetup } from '../../../../../plugins/data_source_management/public';
2929
import { DataPublicPluginStart } from '../../../../data/public';
3030
import { WorkspaceUseCase } from '../../types';
31-
import { getFirstUseCaseOfFeatureConfigs } from '../../utils';
31+
import {
32+
getApplicationsSnapshot,
33+
getFirstUseCaseOfFeatureConfigs,
34+
pickUseCaseLandingAppId,
35+
} from '../../utils';
3236
import { useFormAvailableUseCases } from '../workspace_form/use_form_available_use_cases';
3337
import { NavigationPublicPluginStart } from '../../../../../plugins/navigation/public';
3438
import { DataSourceConnectionType } from '../../../common/types';
@@ -144,8 +148,9 @@ export const WorkspaceCreator = (props: WorkspaceCreatorProps) => {
144148
if (application && http) {
145149
const newWorkspaceId = result.result.id;
146150
const useCaseId = getFirstUseCaseOfFeatureConfigs(attributes.features);
147-
const useCaseLandingAppId = availableUseCases?.find(({ id }) => useCaseId === id)
148-
?.features[0].id;
151+
const matchedUseCase = availableUseCases?.find(({ id }) => useCaseId === id);
152+
const apps = getApplicationsSnapshot(application);
153+
const useCaseLandingAppId = pickUseCaseLandingAppId(matchedUseCase?.features, apps);
149154

150155
// For observability workspaces, run trace detection and create datasets if found
151156
const isObservabilityWorkspace = useCaseId === 'observability';

src/plugins/workspace/public/utils.test.ts

Lines changed: 104 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@
33
* SPDX-License-Identifier: Apache-2.0
44
*/
55

6-
import { AppNavLinkStatus, NavGroupType, PublicAppInfo } from '../../../core/public';
6+
import { BehaviorSubject } from 'rxjs';
7+
import { AppNavLinkStatus, AppStatus, NavGroupType, PublicAppInfo } from '../../../core/public';
78
import {
89
featureMatchesConfig,
910
filterWorkspaceConfigurableApps,
@@ -12,6 +13,7 @@ import {
1213
getDataSourcesList,
1314
convertNavGroupToWorkspaceUseCase,
1415
isEqualWorkspaceUseCase,
16+
pickUseCaseLandingAppId,
1517
prependWorkspaceToBreadcrumbs,
1618
mergeDataSourcesWithConnections,
1719
fetchDataSourceConnections,
@@ -967,6 +969,81 @@ describe('workspace utils: mergeDataSourcesWithConnections', () => {
967969
});
968970
});
969971

972+
describe('workspace utils: pickUseCaseLandingAppId', () => {
973+
const accessibleVisible = ({
974+
status: AppStatus.accessible,
975+
navLinkStatus: AppNavLinkStatus.default,
976+
} as Partial<PublicAppInfo>) as PublicAppInfo;
977+
const featureFlagDisabled = ({
978+
status: AppStatus.accessible,
979+
navLinkStatus: AppNavLinkStatus.hidden,
980+
} as Partial<PublicAppInfo>) as PublicAppInfo;
981+
const outsideWorkspaceHidden = ({
982+
// Mirrors the state read on the workspace creator page for an
983+
// `insideWorkspace`-only app: workspace plugin pushed `inaccessible`,
984+
// some other path pushed `navLinkStatus: hidden` — both flip back
985+
// once the user enters the workspace, so the picker must keep them.
986+
status: AppStatus.inaccessible,
987+
navLinkStatus: AppNavLinkStatus.hidden,
988+
} as Partial<PublicAppInfo>) as PublicAppInfo;
989+
990+
it('returns undefined when the use case has no features', () => {
991+
expect(pickUseCaseLandingAppId(undefined, new Map())).toBeUndefined();
992+
expect(pickUseCaseLandingAppId([], new Map())).toBeUndefined();
993+
});
994+
995+
it('falls back to features[0] when no apps snapshot is provided', () => {
996+
expect(pickUseCaseLandingAppId([{ id: 'first' }, { id: 'second' }], undefined)).toBe('first');
997+
});
998+
999+
it('skips feature-flag-disabled apps (hidden + accessible)', () => {
1000+
const apps = new Map<string, PublicAppInfo>([
1001+
['alerting', featureFlagDisabled],
1002+
['dashboards', accessibleVisible],
1003+
]);
1004+
expect(pickUseCaseLandingAppId([{ id: 'alerting' }, { id: 'dashboards' }], apps)).toBe(
1005+
'dashboards'
1006+
);
1007+
});
1008+
1009+
it('keeps apps that are transiently hidden outside a workspace (hidden + inaccessible)', () => {
1010+
// Repro of the bug we shipped this for: workspace creator runs outside
1011+
// any workspace, so `insideWorkspace` apps look hidden — but we must
1012+
// still pick the first one as the landing target, because it'll be
1013+
// accessible immediately after the redirect.
1014+
const apps = new Map<string, PublicAppInfo>([
1015+
['dashboards', outsideWorkspaceHidden],
1016+
['explore/logs', outsideWorkspaceHidden],
1017+
]);
1018+
expect(pickUseCaseLandingAppId([{ id: 'dashboards' }, { id: 'explore/logs' }], apps)).toBe(
1019+
'dashboards'
1020+
);
1021+
});
1022+
1023+
it('falls back to features[0] when every feature is feature-flag-disabled', () => {
1024+
const apps = new Map<string, PublicAppInfo>([
1025+
['a', featureFlagDisabled],
1026+
['b', featureFlagDisabled],
1027+
]);
1028+
expect(pickUseCaseLandingAppId([{ id: 'a' }, { id: 'b' }], apps)).toBe('a');
1029+
});
1030+
1031+
it('treats a feature id absent from the apps map as selectable', () => {
1032+
// Load-bearing for the transient-load case: feature ids come from
1033+
// `convertNavGroupToWorkspaceUseCase` over real nav links, so an
1034+
// absent lookup means the apps snapshot hasn't propagated yet, not
1035+
// that the app is missing. Skipping such features would silently
1036+
// skip the entire list during early page load.
1037+
const apps = new Map<string, PublicAppInfo>([['known-feature-flag-off', featureFlagDisabled]]);
1038+
expect(
1039+
pickUseCaseLandingAppId(
1040+
[{ id: 'known-feature-flag-off' }, { id: 'not-yet-in-apps-map' }],
1041+
apps
1042+
)
1043+
).toBe('not-yet-in-apps-map');
1044+
});
1045+
});
1046+
9701047
describe('workspace utils: getUseCaseUrl', () => {
9711048
it('should get use case url', () => {
9721049
startMock.application.getUrlForApp.mockImplementation((id) => `http://localhost/${id}`);
@@ -979,6 +1056,32 @@ describe('workspace utils: getUseCaseUrl', () => {
9791056
const url = getUseCaseUrl(undefined, 'foo', startMock.application, startMock.http);
9801057
expect(url).toEqual('http://localhost/w/foo/workspace_detail');
9811058
});
1059+
1060+
it('should skip feature-flag-disabled features when picking the landing app', () => {
1061+
startMock.application.getUrlForApp.mockImplementation((id) => `http://localhost/${id}`);
1062+
(startMock.application.applications$ as BehaviorSubject<Map<string, PublicAppInfo>>).next(
1063+
new Map<string, PublicAppInfo>([
1064+
// `bar` is the first feature of `useCaseMock`. Flag it off so the
1065+
// picker has to fall through to the next feature.
1066+
[
1067+
'bar',
1068+
({
1069+
status: AppStatus.accessible,
1070+
navLinkStatus: AppNavLinkStatus.hidden,
1071+
} as Partial<PublicAppInfo>) as PublicAppInfo,
1072+
],
1073+
[
1074+
'baz',
1075+
({
1076+
status: AppStatus.accessible,
1077+
navLinkStatus: AppNavLinkStatus.default,
1078+
} as Partial<PublicAppInfo>) as PublicAppInfo,
1079+
],
1080+
])
1081+
);
1082+
const url = getUseCaseUrl(useCaseMock, 'foo', startMock.application, startMock.http);
1083+
expect(url).toEqual('http://localhost/w/foo/baz');
1084+
});
9821085
});
9831086

9841087
describe('workspace utils: fetchDataSourceConnections', () => {

src/plugins/workspace/public/utils.ts

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
AppCategory,
1212
ApplicationStart,
1313
AppNavLinkStatus,
14+
AppStatus,
1415
ChromeBreadcrumb,
1516
CoreStart,
1617
DEFAULT_APP_CATEGORIES,
@@ -526,13 +527,71 @@ export function prependWorkspaceToBreadcrumbs(
526527
}
527528
}
528529

530+
/**
531+
* Read the current value of `application.applications$` synchronously.
532+
*
533+
* Load-bearing assumption: `applications$` is wrapped in `shareReplay(1)`
534+
* upstream (see `application_service.tsx`), so subscribing then immediately
535+
* unsubscribing emits the latest cached value without leaking the
536+
* subscription. If a future refactor drops `shareReplay`, this returns
537+
* `undefined` instead of throwing — callers that branch on snapshot
538+
* presence (e.g. `pickUseCaseLandingAppId`) will silently regress to
539+
* pre-fix behavior. Keep this helper as the single source of truth so the
540+
* regression has one place to surface rather than many.
541+
*/
542+
export const getApplicationsSnapshot = (
543+
application: ApplicationStart
544+
): ReadonlyMap<string, PublicAppInfo> | undefined => {
545+
let apps: ReadonlyMap<string, PublicAppInfo> | undefined;
546+
const sub = application.applications$.subscribe((value) => {
547+
apps = value;
548+
});
549+
sub.unsubscribe();
550+
return apps;
551+
};
552+
553+
/**
554+
* Pick the landing app for a use case. Skips features that are
555+
* **feature-flag-disabled** — i.e. `navLinkStatus === hidden` *and*
556+
* `status === accessible`. An app gated by a feature flag is still
557+
* accessible in principle, the plugin just hides the nav link to suppress
558+
* the UI; an app that's transiently inaccessible (e.g. an
559+
* `insideWorkspace`-only app read from outside any workspace, which the
560+
* workspace plugin marks `inaccessible` and which therefore renders as
561+
* `hidden` too) will become available once the user enters the workspace
562+
* and must not be filtered out here.
563+
*
564+
* Reading both fields lets us distinguish "hidden by config" (skip) from
565+
* "hidden because of where we are" (keep — it'll come back). A feature id
566+
* with no entry in `apps` is treated as selectable: feature ids come from
567+
* `convertNavGroupToWorkspaceUseCase` over real registered nav links, so
568+
* an absent lookup means the applications snapshot hasn't propagated yet,
569+
* not that the app doesn't exist.
570+
*/
571+
export const pickUseCaseLandingAppId = (
572+
features: WorkspaceUseCaseFeature[] | undefined,
573+
apps: ReadonlyMap<string, PublicAppInfo> | undefined
574+
): string | undefined => {
575+
if (!features?.length) {
576+
return undefined;
577+
}
578+
if (!apps) {
579+
return features[0].id;
580+
}
581+
const isFeatureFlagDisabled = (app: PublicAppInfo | undefined) =>
582+
app?.navLinkStatus === AppNavLinkStatus.hidden && app?.status === AppStatus.accessible;
583+
const firstSelectable = features.find((feature) => !isFeatureFlagDisabled(apps.get(feature.id)));
584+
return (firstSelectable ?? features[0]).id;
585+
};
586+
529587
export const getUseCaseUrl = (
530588
useCase: WorkspaceUseCase | undefined,
531589
workspaceId: string,
532590
application: ApplicationStart,
533591
http: HttpSetup
534592
): string => {
535-
const appId = useCase?.features?.[0]?.id || WORKSPACE_DETAIL_APP_ID;
593+
const apps = getApplicationsSnapshot(application);
594+
const appId = pickUseCaseLandingAppId(useCase?.features, apps) || WORKSPACE_DETAIL_APP_ID;
536595
const useCaseURL = formatUrlWithWorkspaceId(
537596
application.getUrlForApp(appId, {
538597
absolute: false,

0 commit comments

Comments
 (0)