Skip to content

Commit

Permalink
Gracefully disable favorites if profile is not available (elastic#204397
Browse files Browse the repository at this point in the history
)

## Summary

When a user profile is not available, the favorites (starred) service
can't be used. On UI user profile can be not available if security is
disabled or for an anonymous user.

This PR improves the handling of starred features for rare cases when a
profile is missing:

- No unnecessary `GET favorites` requests that would fail with error and
add noise to console/networks
- No unhandled errors are thrown
- Starred tab in esql is hidden
- The Dashboard Starred tab isn't flickering on each attempt to fetch
favorites

For this needed to expose `userProfile.enabled$` from core, also created
elastic#204570



### Testing 

```
node scripts/functional_tests_server.js --config test/functional/apps/dashboard/group4/config.ts

localhost:5620
```

another way is by configuring an anonymous user
https://www.elastic.co/guide/en/elasticsearch/reference/current/anonymous-access.html
  • Loading branch information
Dosant authored and benakansara committed Jan 2, 2025
1 parent c28a4b2 commit b583207
Show file tree
Hide file tree
Showing 23 changed files with 150 additions and 54 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@

import type { HttpStart } from '@kbn/core-http-browser';
import type { UsageCollectionStart } from '@kbn/usage-collection-plugin/public';
import type { UserProfileServiceStart } from '@kbn/core-user-profile-browser';
import type {
GetFavoritesResponse as GetFavoritesResponseServer,
AddFavoriteResponse,
GetFavoritesResponse as GetFavoritesResponseServer,
RemoveFavoriteResponse,
} from '@kbn/content-management-favorites-server';
import { firstValueFrom } from 'rxjs';

export interface GetFavoritesResponse<Metadata extends object | void = void>
extends GetFavoritesResponseServer {
Expand All @@ -29,6 +31,7 @@ export interface FavoritesClientPublic<Metadata extends object | void = void> {
addFavorite(params: AddFavoriteRequest<Metadata>): Promise<AddFavoriteResponse>;
removeFavorite(params: { id: string }): Promise<RemoveFavoriteResponse>;

isAvailable(): Promise<boolean>;
getFavoriteType(): string;
reportAddFavoriteClick(): void;
reportRemoveFavoriteClick(): void;
Expand All @@ -40,21 +43,37 @@ export class FavoritesClient<Metadata extends object | void = void>
constructor(
private readonly appName: string,
private readonly favoriteObjectType: string,
private readonly deps: { http: HttpStart; usageCollection?: UsageCollectionStart }
private readonly deps: {
http: HttpStart;
userProfile: UserProfileServiceStart;
usageCollection?: UsageCollectionStart;
}
) {}

public async isAvailable(): Promise<boolean> {
return firstValueFrom(this.deps.userProfile.getEnabled$());
}

private async ifAvailablePreCheck() {
if (!(await this.isAvailable()))
throw new Error('Favorites service is not available for current user');
}

public async getFavorites(): Promise<GetFavoritesResponse<Metadata>> {
await this.ifAvailablePreCheck();
return this.deps.http.get(`/internal/content_management/favorites/${this.favoriteObjectType}`);
}

public async addFavorite(params: AddFavoriteRequest<Metadata>): Promise<AddFavoriteResponse> {
await this.ifAvailablePreCheck();
return this.deps.http.post(
`/internal/content_management/favorites/${this.favoriteObjectType}/${params.id}/favorite`,
{ body: 'metadata' in params ? JSON.stringify({ metadata: params.metadata }) : undefined }
);
}

public async removeFavorite({ id }: { id: string }): Promise<RemoveFavoriteResponse> {
await this.ifAvailablePreCheck();
return this.deps.http.post(
`/internal/content_management/favorites/${this.favoriteObjectType}/${id}/unfavorite`
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,6 @@
"@kbn/content-management-favorites-server",
"@kbn/i18n-react",
"@kbn/usage-collection-plugin",
"@kbn/core-user-profile-browser",
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export const getMockServices = (overrides?: Partial<Services & UserProfilesServi
getTagManagementUrl: () => '',
getTagIdsFromReferences: () => [],
isTaggingEnabled: () => true,
isFavoritesEnabled: () => false,
isFavoritesEnabled: () => Promise.resolve(false),
bulkGetUserProfiles: async () => [],
getUserProfile: async () => ({ uid: '', enabled: true, data: {}, user: { username: '' } }),
isKibanaVersioningEnabled: false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export const getStoryServices = (params: Params, action: ActionFn = () => {}) =>
getTagManagementUrl: () => '',
getTagIdsFromReferences: () => [],
isTaggingEnabled: () => true,
isFavoritesEnabled: () => false,
isFavoritesEnabled: () => Promise.resolve(false),
isKibanaVersioningEnabled: false,
...params,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ export interface Services {
/** Predicate to indicate if tagging features is enabled */
isTaggingEnabled: () => boolean;
/** Predicate to indicate if favorites features is enabled */
isFavoritesEnabled: () => boolean;
isFavoritesEnabled: () => Promise<boolean>;
/** Predicate function to indicate if some of the saved object references are tags */
itemHasTags: (references: SavedObjectsReference[]) => boolean;
/** Handler to return the url to navigate to the kibana tags management */
Expand Down Expand Up @@ -288,7 +288,7 @@ export const TableListViewKibanaProvider: FC<
currentAppId$={application.currentAppId$}
navigateToUrl={application.navigateToUrl}
isTaggingEnabled={() => Boolean(savedObjectsTagging)}
isFavoritesEnabled={() => Boolean(services.favorites)}
isFavoritesEnabled={async () => services.favorites?.isAvailable() ?? false}
getTagList={getTagList}
TagList={TagList}
itemHasTags={itemHasTags}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

import React, { useReducer, useCallback, useEffect, useRef, useMemo } from 'react';
import useDebounce from 'react-use/lib/useDebounce';
import useAsync from 'react-use/lib/useAsync';
import {
EuiBasicTableColumn,
EuiButton,
Expand Down Expand Up @@ -379,6 +380,8 @@ function TableListViewTableComp<T extends UserContentCommonSchema>({
isKibanaVersioningEnabled,
} = useServices();

const favoritesEnabled = useAsync(isFavoritesEnabled, [])?.value ?? false;

const openContentEditor = useOpenContentEditor();
const contentInsightsServices = useContentInsightsServices();

Expand Down Expand Up @@ -621,7 +624,7 @@ function TableListViewTableComp<T extends UserContentCommonSchema>({
}
}}
searchTerm={searchQuery.text}
isFavoritesEnabled={isFavoritesEnabled()}
isFavoritesEnabled={favoritesEnabled}
/>
);
},
Expand Down Expand Up @@ -754,7 +757,7 @@ function TableListViewTableComp<T extends UserContentCommonSchema>({
tableItemsRowActions,
inspectItem,
entityName,
isFavoritesEnabled,
favoritesEnabled,
isKibanaVersioningEnabled,
]);

Expand Down Expand Up @@ -1218,7 +1221,7 @@ function TableListViewTableComp<T extends UserContentCommonSchema>({
addOrRemoveExcludeTagFilter={addOrRemoveExcludeTagFilter}
clearTagSelection={clearTagSelection}
createdByEnabled={createdByEnabled}
favoritesEnabled={isFavoritesEnabled()}
favoritesEnabled={favoritesEnabled}
/>

{/* Delete modal */}
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ describe('convertUserProfileAPI', () => {
beforeEach(() => {
source = {
userProfile$: of(null),
enabled$: of(false),
getCurrent: jest.fn(),
bulkGet: jest.fn(),
suggest: jest.fn(),
Expand All @@ -34,6 +35,12 @@ describe('convertUserProfileAPI', () => {
});
});

describe('getEnabled$', () => {
it('returns the observable from the source', () => {
expect(output.getEnabled$()).toBe(source.enabled$);
});
});

describe('getCurrent', () => {
it('calls the API from the source with the correct parameters', () => {
output.getCurrent();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export const convertUserProfileAPI = (
): InternalUserProfileServiceStart => {
return {
getUserProfile$: () => delegate.userProfile$,
getEnabled$: () => delegate.enabled$,
getCurrent: delegate.getCurrent.bind(delegate),
bulkGet: delegate.bulkGet.bind(delegate),
suggest: delegate.suggest.bind(delegate),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { UserProfileData } from '@kbn/core-user-profile-common';
export const getDefaultUserProfileImplementation = (): CoreUserProfileDelegateContract => {
return {
userProfile$: of(null),
enabled$: of(false),
getCurrent: <D extends UserProfileData>() =>
Promise.resolve(null as unknown as GetUserProfileResponse<D>),
bulkGet: () => Promise.resolve([]),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const createSetupMock = () => {
const createStartMock = () => {
const mock: jest.Mocked<UserProfileServiceStart> = {
getUserProfile$: jest.fn().mockReturnValue(of(null)),
getEnabled$: jest.fn().mockReturnValue(of(false)),
getCurrent: jest.fn(),
bulkGet: jest.fn(),
suggest: jest.fn(),
Expand All @@ -49,6 +50,7 @@ const createInternalSetupMock = () => {
const createInternalStartMock = () => {
const mock: jest.Mocked<InternalUserProfileServiceStart> = {
getUserProfile$: jest.fn().mockReturnValue(of(null)),
getEnabled$: jest.fn().mockReturnValue(of(false)),
getCurrent: jest.fn(),
bulkGet: jest.fn(),
suggest: jest.fn(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ import type { Observable } from 'rxjs';
import type { UserProfileData } from '@kbn/core-user-profile-common';
import type { UserProfileService } from './service';

export type CoreUserProfileDelegateContract = Omit<UserProfileService, 'getUserProfile$'> & {
export type CoreUserProfileDelegateContract = Omit<
UserProfileService,
'getUserProfile$' | 'getEnabled$'
> & {
userProfile$: Observable<UserProfileData | null>;
enabled$: Observable<boolean>;
};
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,13 @@ import type {

export interface UserProfileService {
/**
* Retrieve an observable emitting when the user profile is loaded.
* Retrieve an observable emitting the current user profile data.
*/
getUserProfile$(): Observable<UserProfileData | null>;

/** Flag to indicate if the current user has a user profile. Anonymous users don't have user profiles. */
getEnabled$(): Observable<boolean>;

/**
* Retrieves the user profile of the current user. If the profile isn't available, e.g. for the anonymous users or
* users authenticated via authenticating proxies, the `null` value is returned.
Expand Down
Loading

0 comments on commit b583207

Please sign in to comment.