Skip to content

regression: ABAC table & form fixes #37782

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.

Sign up for GitHub

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

Merged
tassoevan merged 7 commits into from
Dec 15, 2025
Merged

regression: ABAC table & form fixes #37782

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ffb9a05
regression: truncate abac table cells
MartinSchoeler Dec 11, 2025
b0cc02b
regression: add validation for duplicated attribute values
MartinSchoeler Dec 11, 2025
9851abe
regression: form resetting on error
MartinSchoeler Dec 11, 2025
a5f188d
snap
MartinSchoeler Dec 11, 2025
c4bffe0
fix: don't render empty fieldError
MartinSchoeler Dec 12, 2025
81da92a
fix: ty to protect selects from big values
MartinSchoeler Dec 12, 2025
308ad7c
snaps
MartinSchoeler Dec 12, 2025
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
3 changes: 2 additions & 1 deletion apps/meteor/client/views/admin/ABAC/ABACAttributesTab/AttributesContextualBar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ const AttributesContextualBar = ({ attributeData, onClose }: AttributesContextua
attributeValues: [{ value: '' }],
lockedAttributes: [],
},
mode: 'onChange',
});

const { getValues } = methods;
Expand Down Expand Up @@ -74,7 +75,7 @@ const AttributesContextualBar = ({ attributeData, onClose }: AttributesContextua
dispatchToastMessage({ type: 'error', message: error });
},
onSettled: () => {
queryClient.invalidateQueries({ queryKey: ABACQueryKeys.roomAttributes.all() });
queryClient.invalidateQueries({ queryKey: ABACQueryKeys.roomAttributes.list() });
},
});

Expand Down
78 changes: 52 additions & 26 deletions apps/meteor/client/views/admin/ABAC/ABACAttributesTab/AttributesForm.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
TextInput,
} from '@rocket.chat/fuselage';
import { ContextualbarScrollableContent } from '@rocket.chat/ui-client';
import { useCallback, useId, useMemo } from 'react';
import { useCallback, useId, useMemo, Fragment } from 'react';
import { useFieldArray, useFormContext } from 'react-hook-form';
import { useTranslation } from 'react-i18next';

Expand All @@ -35,29 +35,42 @@ const AttributesForm = ({ onSave, onCancel, description }: AttributesFormProps)
watch,
} = useFormContext<AttributesFormFormData>();

const { t } = useTranslation();

const attributeValues = watch('attributeValues');
const lockedAttributes = watch('lockedAttributes');

const { fields: lockedAttributesFields, remove: removeLockedAttribute } = useFieldArray({
name: 'lockedAttributes',
});

const validateRepeatedValues = useCallback(
(value: string) => {
// Only one instance of the same attribute value is allowed to be in the form at a time
const repeatedAttributes = [...lockedAttributes, ...attributeValues].filter((attribute) => attribute.value === value).length > 1;
return repeatedAttributes ? t('ABAC_No_repeated_values') : undefined;
},
[lockedAttributes, attributeValues, t],
);

const { fields, append, remove } = useFieldArray({
name: 'attributeValues',
rules: {
minLength: 1,
},
});
const { t } = useTranslation();

const formId = useId();
const nameField = useId();
const valuesField = useId();
const attributeValues = watch('attributeValues');

const getAttributeValuesError = useCallback(() => {
if (errors.attributeValues?.length && errors.attributeValues?.length > 0) {
return t('Required_field', { field: t('Values') });
return errors.attributeValues[0]?.value?.message;
}

return '';
}, [errors.attributeValues, t]);
}, [errors.attributeValues]);

const hasValuesErrors = useMemo(() => {
const attributeValuesErrors = Array.isArray(errors?.attributeValues) && errors.attributeValues.some((error) => !!error?.value?.message);
Expand All @@ -82,36 +95,49 @@ const AttributesForm = ({ onSave, onCancel, description }: AttributesFormProps)
{...register('name', { required: t('Required_field', { field: t('Name') }) })}
/>
</FieldRow>
<FieldError>{errors.name?.message || ''}</FieldError>
{errors.name && <FieldError>{errors.name.message}</FieldError>}
</Field>
<Field mb={16}>
<FieldLabel required id={valuesField}>
{t('Values')}
</FieldLabel>
{lockedAttributesFields.map((field, index) => (
<FieldRow key={field.id}>
<TextInput
disabled
aria-labelledby={valuesField}
error={errors.lockedAttributes?.[index]?.value?.message || ''}
{...register(`lockedAttributes.${index}.value`, { required: t('Required_field', { field: t('Values') }) })}
/>
{index !== 0 && <IconButton title={t('ABAC_Remove_attribute')} icon='trash' onClick={() => removeLockedAttribute(index)} />}
</FieldRow>
<Fragment key={field.id}>
<FieldRow key={field.id}>
<TextInput
disabled
aria-labelledby={valuesField}
error={errors.lockedAttributes?.[index]?.value?.message || ''}
{...register(`lockedAttributes.${index}.value`, {
required: t('Required_field', { field: t('Values') }),
validate: (value: string) => validateRepeatedValues(value),
})}
/>
{index !== 0 && (
<IconButton title={t('ABAC_Remove_attribute')} icon='trash' onClick={() => removeLockedAttribute(index)} />
)}
</FieldRow>
{errors.lockedAttributes?.[index]?.value && <FieldError>{errors.lockedAttributes?.[index]?.value?.message}</FieldError>}
</Fragment>
))}
{fields.map((field, index) => (
<FieldRow key={field.id}>
<TextInput
aria-labelledby={valuesField}
error={errors.attributeValues?.[index]?.value?.message || ''}
{...register(`attributeValues.${index}.value`, { required: t('Required_field', { field: t('Values') }) })}
/>
{(index !== 0 || lockedAttributesFields.length > 0) && (
<IconButton title={t('ABAC_Remove_attribute')} icon='trash' onClick={() => remove(index)} />
)}
</FieldRow>
<Fragment key={field.id}>
<FieldRow>
<TextInput
aria-labelledby={valuesField}
error={errors.attributeValues?.[index]?.value?.message || ''}
{...register(`attributeValues.${index}.value`, {
required: t('Required_field', { field: t('Values') }),
validate: (value: string) => validateRepeatedValues(value),
})}
/>
{(index !== 0 || lockedAttributesFields.length > 0) && (
<IconButton title={t('ABAC_Remove_attribute')} icon='trash' onClick={() => remove(index)} />
)}
</FieldRow>
{errors.attributeValues?.[index]?.value && <FieldError>{errors.attributeValues[index].value.message}</FieldError>}
</Fragment>
))}
<FieldError>{getAttributeValuesError()}</FieldError>
<Button
onClick={() => append({ value: '' })}
// Checking for values since rhf does consider the newly added field as dirty after an append() call
Expand Down
4 changes: 2 additions & 2 deletions apps/meteor/client/views/admin/ABAC/ABACAttributesTab/AttributesPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@ const AttributesPage = () => {
<GenericTableBody>
{data?.attributes?.map((attribute) => (
<GenericTableRow key={attribute._id}>
<GenericTableCell>{attribute.key}</GenericTableCell>
<GenericTableCell>{attribute.values.join(', ')}</GenericTableCell>
<GenericTableCell withTruncatedText>{attribute.key}</GenericTableCell>
<GenericTableCell withTruncatedText>{attribute.values.join(', ')}</GenericTableCell>
<GenericTableCell>
<AttributeMenu attribute={attribute} />
</GenericTableCell>
Expand Down
12 changes: 0 additions & 12 deletions ...teor/client/views/admin/ABAC/ABACAttributesTab/__snapshots__/AttributesForm.spec.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,6 @@ exports[`AttributesForm renders NewAttribute without crashing 1`] = `
type="text"
/>
</span>
<span
class="rcx-box rcx-box--full rcx-field__error"
/>
</div>
<div
class="rcx-box rcx-box--full rcx-field rcx-css-1jagyun"
Expand Down Expand Up @@ -94,9 +91,6 @@ exports[`AttributesForm renders NewAttribute without crashing 1`] = `
type="text"
/>
</span>
<span
class="rcx-box rcx-box--full rcx-field__error"
/>
<button
class="rcx-box rcx-box--full rcx-button"
disabled=""
Expand Down Expand Up @@ -237,9 +231,6 @@ exports[`AttributesForm renders WithLockedAttributes without crashing 1`] = `
type="text"
/>
</span>
<span
class="rcx-box rcx-box--full rcx-field__error"
/>
</div>
<div
class="rcx-box rcx-box--full rcx-field rcx-css-1jagyun"
Expand Down Expand Up @@ -316,9 +307,6 @@ exports[`AttributesForm renders WithLockedAttributes without crashing 1`] = `
</i>
</button>
</span>
<span
class="rcx-box rcx-box--full rcx-field__error"
/>
<button
class="rcx-box rcx-box--full rcx-button"
type="button"
Expand Down
2 changes: 1 addition & 1 deletion apps/meteor/client/views/admin/ABAC/ABACRoomsTab/RoomForm.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ const RoomForm = ({ onClose, onSave, roomInfo, setSelectedRoomLabel }: RoomFormP
/>
)}
</FieldRow>
<FieldError>{errors.room?.message}</FieldError>
{errors.room && <FieldError>{errors.room.message}</FieldError>}
</Field>
{fields.map((field, index) => (
<Field key={field.id} mb={16}>
Expand Down
6 changes: 4 additions & 2 deletions apps/meteor/client/views/admin/ABAC/ABACRoomsTab/RoomFormAttributeField.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,19 +75,21 @@ const RoomFormAttributeField = ({ onRemove, index }: ABACAttributeAutocompletePr
placeholder={t('ABAC_Search_Attribute')}
mbe={4}
error={keyFieldState.error?.message}
withTruncatedText
/>
</FieldRow>
<FieldError>{keyFieldState.error?.message || ''}</FieldError>
{keyFieldState.error && <FieldError>{keyFieldState.error.message}</FieldError>}

<FieldRow>
<MultiSelect
withTruncatedText
{...valuesField}
options={valueOptions}
placeholder={t('ABAC_Select_Attribute_Values')}
error={valuesFieldState.error?.message}
/>
</FieldRow>
<FieldError>{valuesFieldState.error?.message || ''}</FieldError>
{valuesFieldState.error && <FieldError>{valuesFieldState.error.message}</FieldError>}

<Button onClick={onRemove} title={t('Remove')} mbs={4}>
{t('Remove')}
Expand Down
2 changes: 1 addition & 1 deletion apps/meteor/client/views/admin/ABAC/ABACRoomsTab/RoomsContextualBar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ const RoomsContextualBar = ({ roomInfo, attributesData, onClose }: RoomsContextu
dispatchToastMessage({ type: 'error', message: error });
},
onSettled: () => {
queryClient.invalidateQueries({ queryKey: ABACQueryKeys.rooms.all() });
queryClient.invalidateQueries({ queryKey: ABACQueryKeys.rooms.list() });
},
});

Expand Down
8 changes: 6 additions & 2 deletions apps/meteor/client/views/admin/ABAC/ABACRoomsTab/RoomsPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,12 @@ const RoomsPage = () => {
<GenericTableRow key={room._id}>
<GenericTableCell>{room.fname || room.name}</GenericTableCell>
<GenericTableCell>{room.usersCount}</GenericTableCell>
<GenericTableCell>{room.abacAttributes?.flatMap((attribute) => attribute.key ?? []).join(', ')}</GenericTableCell>
<GenericTableCell>{room.abacAttributes?.flatMap((attribute) => attribute.values ?? []).join(', ')}</GenericTableCell>
<GenericTableCell withTruncatedText>
{room.abacAttributes?.flatMap((attribute) => attribute.key ?? []).join(', ')}
</GenericTableCell>
<GenericTableCell withTruncatedText>
{room.abacAttributes?.flatMap((attribute) => attribute.values ?? []).join(', ')}
</GenericTableCell>
<GenericTableCell>
<RoomMenu room={{ rid: room._id, name: room.fname || room.name || room._id }} />
</GenericTableCell>
Expand Down
10 changes: 2 additions & 8 deletions ...r/client/views/admin/ABAC/ABACRoomsTab/__snapshots__/RoomFormAttributeField.spec.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ exports[`RoomFormAttributeField renders Default without crashing 1`] = `
class="rcx-box rcx-box--full rcx-field__row"
>
<div
class="rcx-box rcx-box--full rcx-select rcx-css-18q594f"
class="rcx-box rcx-box--full rcx-select rcx-css-1hx4au7"
name="attributes.0.key"
>
<div
Expand All @@ -38,14 +38,11 @@ exports[`RoomFormAttributeField renders Default without crashing 1`] = `
</div>
</div>
</span>
<span
class="rcx-box rcx-box--full rcx-field__error"
/>
<span
class="rcx-box rcx-box--full rcx-field__row"
>
<div
class="rcx-box rcx-box--full rcx-select"
class="rcx-box rcx-box--full rcx-select rcx-css-1te28na"
name="attributes.0.values"
>
<div
Expand Down Expand Up @@ -76,9 +73,6 @@ exports[`RoomFormAttributeField renders Default without crashing 1`] = `
</div>
</div>
</span>
<span
class="rcx-box rcx-box--full rcx-field__error"
/>
<button
class="rcx-box rcx-box--full rcx-button rcx-css-1qsx7et"
title="Remove"
Expand Down
1 change: 1 addition & 0 deletions packages/i18n/src/locales/en.i18n.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
"ABAC_Edit_Room": "Edit Room",
"ABAC_Add_room": "Add Room",
"ABAC_No_repeated_attributes": "Attribute already added",
"ABAC_No_repeated_values": "Value already added",
"ABAC_Room_created": "Access to {{roomName}} is now restricted to attribute-compliant users",
"ABAC_Room_to_be_managed": "Room to be ABAC-managed",
"ABAC_Room_updated": "{{roomName}} ABAC room updated",
Expand Down
Loading