Skip to content

Commit

Permalink
[Managed content] readonly in library views (elastic#176263)
Browse files Browse the repository at this point in the history
## Summary

Close elastic#172387

This PR stops users from doing two things in the library views
1. deleting managed content
2. editing metadata of managed content

It also includes a refactor to the `TableListView` interface to replace
the `isItemEditable` callback with a row-level action.

<img width="1596" alt="Screenshot 2024-02-06 at 3 03 06 PM"
src="https://github.com/elastic/kibana/assets/315764/add84572-d4d7-4d69-baa8-a298a0ca7b83">

<img width="553" alt="Screenshot 2024-02-06 at 3 03 24 PM"
src="https://github.com/elastic/kibana/assets/315764/9bbdb6d5-a030-4c17-8f6d-daec4f5232a2">


### Checklist

- [x] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials — will be
completed in elastic#175150
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed —
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5087
- [x] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Stratoula Kalafateli <[email protected]>
  • Loading branch information
3 people authored and fkanout committed Mar 4, 2024
1 parent d5cc659 commit 227e6d3
Show file tree
Hide file tree
Showing 16 changed files with 137 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export interface Props {
item: Item;
entityName: string;
isReadonly?: boolean;
readonlyReason?: string;
services: Pick<Services, 'TagSelector' | 'TagList' | 'notifyError'>;
onSave?: (args: {
id: string;
Expand All @@ -62,6 +63,7 @@ export const ContentEditorFlyoutContent: FC<Props> = ({
item,
entityName,
isReadonly = true,
readonlyReason,
services: { TagSelector, TagList, notifyError },
onSave,
onCancel,
Expand Down Expand Up @@ -136,6 +138,12 @@ export const ContentEditorFlyoutContent: FC<Props> = ({
<MetadataForm
form={{ ...form, isSubmitted }}
isReadonly={isReadonly}
readonlyReason={
readonlyReason ||
i18n.translate('contentManagement.contentEditor.metadataForm.readOnlyToolTip', {
defaultMessage: 'To edit these details, contact your administrator for access.',
})
}
tagsReferences={item.tags}
TagList={TagList}
TagSelector={TagSelector}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,14 @@ import type { Props as ContentEditorFlyoutContentProps } from './editor_flyout_c

type CommonProps = Pick<
ContentEditorFlyoutContentProps,
'item' | 'isReadonly' | 'services' | 'onSave' | 'onCancel' | 'entityName' | 'customValidators'
| 'item'
| 'isReadonly'
| 'readonlyReason'
| 'services'
| 'onSave'
| 'onCancel'
| 'entityName'
| 'customValidators'
>;

export type Props = CommonProps;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ interface Props {
isSubmitted: boolean;
};
isReadonly: boolean;
readonlyReason: string;
tagsReferences: SavedObjectsReference[];
TagList?: Services['TagList'];
TagSelector?: Services['TagSelector'];
Expand All @@ -40,6 +41,7 @@ export const MetadataForm: FC<Props> = ({
TagList,
TagSelector,
isReadonly,
readonlyReason,
}) => {
const {
title,
Expand All @@ -54,11 +56,6 @@ export const MetadataForm: FC<Props> = ({
getWarnings,
} = form;

const readOnlyToolTip = i18n.translate(
'contentManagement.contentEditor.metadataForm.readOnlyToolTip',
{ defaultMessage: 'To edit these details, contact your administrator for access.' }
);

return (
<EuiForm isInvalid={isSubmitted && !isValid} error={getErrors()} data-test-subj="metadataForm">
<ContentEditorFlyoutWarningsCallOut warningMessages={getWarnings()} />
Expand All @@ -73,7 +70,7 @@ export const MetadataForm: FC<Props> = ({
>
<EuiToolTip
position="top"
content={isReadonly ? readOnlyToolTip : undefined}
content={isReadonly ? readonlyReason : undefined}
display="block"
>
<EuiFieldText
Expand Down Expand Up @@ -104,7 +101,7 @@ export const MetadataForm: FC<Props> = ({
>
<EuiToolTip
position="top"
content={isReadonly ? readOnlyToolTip : undefined}
content={isReadonly ? readonlyReason : undefined}
display="block"
>
<EuiTextArea
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import type { ContentEditorFlyoutContentContainerProps } from './components';

export type OpenContentEditorParams = Pick<
ContentEditorFlyoutContentContainerProps,
'item' | 'onSave' | 'isReadonly' | 'entityName' | 'customValidators'
'item' | 'onSave' | 'isReadonly' | 'readonlyReason' | 'entityName' | 'customValidators'
>;

export function useOpenContentEditor() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ export type TableListViewProps<T extends UserContentCommonSchema = UserContentCo
| 'contentEditor'
| 'titleColumnName'
| 'withoutPageTemplateWrapper'
| 'itemIsEditable'
> & {
title: string;
description?: string;
Expand Down Expand Up @@ -74,7 +73,6 @@ export const TableListView = <T extends UserContentCommonSchema>({
titleColumnName,
additionalRightSideActions,
withoutPageTemplateWrapper,
itemIsEditable,
}: TableListViewProps<T>) => {
const PageTemplate = withoutPageTemplateWrapper
? (React.Fragment as unknown as typeof KibanaPageTemplate)
Expand Down Expand Up @@ -120,7 +118,6 @@ export const TableListView = <T extends UserContentCommonSchema>({
id={listingId}
contentEditor={contentEditor}
titleColumnName={titleColumnName}
itemIsEditable={itemIsEditable}
withoutPageTemplateWrapper={withoutPageTemplateWrapper}
onFetchSuccess={onFetchSuccess}
setPageDataTestSubject={setPageDataTestSubject}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,6 @@ export interface TableListViewTableProps<
*/
editItem?(item: T): void;

/**
* Handler to set edit action visiblity, and content editor readonly state per item. If not provided all non-managed items are considered editable. Note: Items with the managed property set to true will always be non-editable.
*/
itemIsEditable?(item: T): boolean;

/**
* Name for the column containing the "title" value.
*/
Expand Down Expand Up @@ -259,7 +254,6 @@ function TableListViewTableComp<T extends UserContentCommonSchema>({
findItems,
createItem,
editItem,
itemIsEditable,
deleteItems,
getDetailViewLink,
onClickTitle,
Expand Down Expand Up @@ -440,14 +434,34 @@ function TableListViewTableComp<T extends UserContentCommonSchema>({
items,
});

const isEditable = useCallback(
(item: T) => {
// If the So is `managed` it is never editable.
if (item.managed) return false;
return itemIsEditable?.(item) ?? true;
},
[itemIsEditable]
);
const tableItemsRowActions = useMemo(() => {
return items.reduce<TableItemsRowActions>((acc, item) => {
const ret = {
...acc,
[item.id]: rowItemActions ? rowItemActions(item) : undefined,
};

if (item.managed) {
ret[item.id] = {
...ret[item.id],
delete: {
enabled: false,
reason: i18n.translate('contentManagement.tableList.managedItemNoDelete', {
defaultMessage: 'This item is managed by Elastic. It cannot be deleted.',
}),
},
edit: {
enabled: false,
reason: i18n.translate('contentManagement.tableList.managedItemNoEdit', {
defaultMessage: 'This item is managed by Elastic. Clone it before making changes.',
}),
},
};
}

return ret;
}, {});
}, [items, rowItemActions]);

const inspectItem = useCallback(
(item: T) => {
Expand All @@ -464,7 +478,9 @@ function TableListViewTableComp<T extends UserContentCommonSchema>({
},
entityName,
...contentEditor,
isReadonly: contentEditor.isReadonly || !isEditable(item),
isReadonly:
contentEditor.isReadonly || tableItemsRowActions[item.id]?.edit?.enabled === false,
readonlyReason: tableItemsRowActions[item.id]?.edit?.reason,
onSave:
contentEditor.onSave &&
(async (args) => {
Expand All @@ -475,7 +491,14 @@ function TableListViewTableComp<T extends UserContentCommonSchema>({
}),
});
},
[getTagIdsFromReferences, openContentEditor, entityName, contentEditor, isEditable, fetchItems]
[
getTagIdsFromReferences,
openContentEditor,
entityName,
contentEditor,
tableItemsRowActions,
fetchItems,
]
);

const tableColumns = useMemo(() => {
Expand Down Expand Up @@ -549,7 +572,7 @@ function TableListViewTableComp<T extends UserContentCommonSchema>({
),
icon: 'pencil',
type: 'icon',
available: (item) => isEditable(item),
available: (item) => Boolean(tableItemsRowActions[item.id]?.edit?.enabled),
enabled: (v) => !(v as unknown as { error: string })?.error,
onClick: editItem,
'data-test-subj': `edit-action`,
Expand Down Expand Up @@ -605,7 +628,7 @@ function TableListViewTableComp<T extends UserContentCommonSchema>({
addOrRemoveExcludeTagFilter,
addOrRemoveIncludeTagFilter,
DateFormatterComp,
isEditable,
tableItemsRowActions,
inspectItem,
]);

Expand All @@ -617,15 +640,6 @@ function TableListViewTableComp<T extends UserContentCommonSchema>({
return selectedIds.map((selectedId) => itemsById[selectedId]);
}, [selectedIds, itemsById]);

const tableItemsRowActions = useMemo(() => {
return items.reduce<TableItemsRowActions>((acc, item) => {
return {
...acc,
[item.id]: rowItemActions ? rowItemActions(item) : undefined,
};
}, {});
}, [items, rowItemActions]);

// ------------
// Callbacks
// ------------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export interface Tag {
color: string;
}

export type TableRowAction = 'delete';
export type TableRowAction = 'delete' | 'edit';

export type RowActions = {
[action in TableRowAction]?: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ test('when showWriteControls is true, table list view is passed editing function
createItem: expect.any(Function),
deleteItems: expect.any(Function),
editItem: expect.any(Function),
itemIsEditable: expect.any(Function),
}),
expect.any(Object) // react context
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,6 @@ describe('useDashboardListingTable', () => {
initialPageSize: 5,
listingLimit: 20,
onFetchSuccess: expect.any(Function),
itemIsEditable: expect.any(Function),
setPageDataTestSubject: expect.any(Function),
title: 'Dashboard List',
urlStateEnabled: false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,6 @@ export const useDashboardListingTable = ({
createItem: !showWriteControls || !showCreateDashboardButton ? undefined : createItem,
deleteItems: !showWriteControls ? undefined : deleteItems,
editItem: !showWriteControls ? undefined : editItem,
itemIsEditable: () => showWriteControls,
emptyPrompt,
entityName,
entityNamePlural,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,16 +61,19 @@ export function mapHitSource(
id,
references,
updatedAt,
managed,
}: {
attributes: SavedObjectAttributes;
id: string;
references: SavedObjectReference[];
updatedAt?: string;
managed?: boolean;
}
) {
const newAttributes: {
id: string;
references: SavedObjectReference[];
managed?: boolean;
url: string;
savedObjectType?: string;
editor?: { editUrl?: string };
Expand All @@ -86,6 +89,7 @@ export function mapHitSource(
references,
url: urlFor(id),
updatedAt,
managed,
...attributes,
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ const toTableListViewSavedObject = (savedObject: Record<string, unknown>): Visua
return {
id: savedObject.id as string,
updatedAt: savedObject.updatedAt as string,
managed: savedObject.managed as boolean,
references: savedObject.references as Array<{ id: string; type: string; name: string }>,
type: savedObject.savedObjectType as string,
icon: savedObject.icon as string,
Expand All @@ -90,7 +91,7 @@ type CustomTableViewProps = Pick<
| 'editItem'
| 'contentEditor'
| 'emptyPrompt'
| 'itemIsEditable'
| 'rowItemActions'
>;

const useTableListViewProps = (
Expand Down Expand Up @@ -260,7 +261,8 @@ const useTableListViewProps = (
editItem,
emptyPrompt: noItemsFragment,
createItem: createNewVis,
itemIsEditable: ({ attributes: { readOnly } }) => visualizeCapabilities.save && !readOnly,
rowItemActions: ({ attributes: { readOnly } }) =>
!visualizeCapabilities.save || readOnly ? { edit: { enabled: false } } : undefined,
};

return props;
Expand Down
24 changes: 21 additions & 3 deletions test/functional/services/listing_table.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,14 @@ export class ListingTableService extends FtrService {

private async getAllSelectableItemsNamesOnCurrentPage(): Promise<string[]> {
const visualizationNames = [];
const links = await this.find.allByCssSelector('.euiTableRow-isSelectable .euiLink');
for (let i = 0; i < links.length; i++) {
visualizationNames.push(await links[i].getVisibleText());
// TODO - use .euiTableRow-isSelectable when it's working again (https://github.com/elastic/eui/issues/7515)
const rows = await this.find.allByCssSelector('.euiTableRow');
for (let i = 0; i < rows.length; i++) {
const checkbox = await rows[i].findByCssSelector('.euiCheckbox__input');
if (await checkbox.isEnabled()) {
const link = await rows[i].findByCssSelector('.euiLink');
visualizationNames.push(await link.getVisibleText());
}
}
this.log.debug(`Found ${visualizationNames.length} selectable visualizations on current page`);
return visualizationNames;
Expand Down Expand Up @@ -165,6 +170,19 @@ export class ListingTableService extends FtrService {
await inspectButtons[index].click();
}

public async inspectorFieldsReadonly() {
const disabledValues = await Promise.all([
this.testSubjects.getAttribute('nameInput', 'readonly'),
this.testSubjects.getAttribute('descriptionInput', 'readonly'),
]);

return disabledValues.every((value) => value === 'true');
}

public async closeInspector() {
await this.testSubjects.click('closeFlyoutButton');
}

/**
* Edit Visualization title and description in the flyout
*/
Expand Down
3 changes: 2 additions & 1 deletion x-pack/plugins/lens/public/vis_type_alias.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,14 @@ export const getLensAliasConfig = (): VisTypeAlias => ({
clientOptions: { update: { overwrite: true } },
client: getLensClient,
toListItem(savedObject) {
const { id, type, updatedAt, attributes } = savedObject;
const { id, type, updatedAt, attributes, managed } = savedObject;
const { title, description } = attributes as { title: string; description?: string };
return {
id,
title,
description,
updatedAt,
managed,
editor: { editUrl: getEditPath(id), editApp: 'lens' },
icon: 'lensApp',
stage: 'production',
Expand Down
Loading

0 comments on commit 227e6d3

Please sign in to comment.