[ACS-8824] Disable node properties save button if values are invalid (#10560)

* [ACS-8824] Disable node properties save button if values are invalid

* [ACS-8824] remove commented code

* [ACS-8824] unit tests [ci:force]

* [ACS-8824] cr fixes
This commit is contained in:
Mykyta Maliarchuk
2025-01-28 15:49:32 +01:00
committed by GitHub
parent 1634716bbe
commit eaa1008018
7 changed files with 160 additions and 34 deletions

View File

@@ -38,7 +38,7 @@
(click)="saveChanges($event)"
color="primary"
data-automation-id="save-general-info-metadata"
[disabled]="!hasMetadataChanged">
[disabled]="!hasMetadataChanged || invalidProperties.size > 0">
<mat-icon>check</mat-icon>
</button>
</div>
@@ -235,7 +235,7 @@
(click)="saveChanges($event)"
color="primary"
data-automation-id="save-metadata"
[disabled]="!hasMetadataChanged">
[disabled]="!hasMetadataChanged || invalidProperties.size > 0">
<mat-icon>check</mat-icon>
</button>
</div>

View File

@@ -114,7 +114,8 @@ describe('ContentMetadataComponent', () => {
fixture.detectChanges();
};
const clickOnGroupSave = () => fixture.debugElement.query(By.css('[data-automation-id="save-metadata"]')).nativeElement.click();
const getGroupSaveButton = () => fixture.debugElement.query(By.css('[data-automation-id="save-metadata"]')).nativeElement;
const clickOnGroupSaveButton = () => getGroupSaveButton().click();
const findTagsCreator = (): TagsCreatorComponent => fixture.debugElement.query(By.directive(TagsCreatorComponent))?.componentInstance;
const getToggleEditButton = () => fixture.debugElement.query(By.css('[data-automation-id="meta-data-general-info-edit"]'));
@@ -288,6 +289,135 @@ describe('ContentMetadataComponent', () => {
expect(contentMetadataService.getGroupedProperties).toHaveBeenCalled();
}));
describe('Save button - Grouped Properties', () => {
beforeEach(() => {
getGroupedPropertiesSpy.and.returnValue(
of([
{
editable: true,
title: 'test',
properties: []
}
])
);
component.ngOnInit();
component.readOnly = false;
});
it('should disable the save button if metadata has not changed', () => {
fixture.detectChanges();
toggleEditModeForGroup();
expect(getGroupSaveButton().disabled).toBeTrue();
});
it('should disable the save button if there are invalid properties', fakeAsync(() => {
updateService.update(
{
key: 'properties.property-key',
isValidValue: false
} as CardViewBaseItemModel,
'updated-value'
);
tick(500);
fixture.detectChanges();
toggleEditModeForGroup();
expect(getGroupSaveButton().disabled).toBeTrue();
}));
it('should enable the save button if metadata has changed and there are no invalid properties', fakeAsync(() => {
updateService.update(
{
key: 'properties.property-key',
isValidValue: true
} as CardViewBaseItemModel,
'updated-value'
);
tick(500);
fixture.detectChanges();
toggleEditModeForGroup();
expect(getGroupSaveButton().disabled).toBeFalse();
}));
});
describe('Save button - Basic Properties', () => {
beforeEach(fakeAsync(() => {
spyOn(contentMetadataService, 'getBasicProperties').and.returnValue(of([]));
component.ngOnInit();
component.readOnly = false;
}));
it('should disable the save button if metadata has not changed', fakeAsync(() => {
fixture.detectChanges();
toggleEditModeForGeneralInfo();
expect(findSaveGeneralInfoButton().disabled).toBeTrue();
}));
it('should enable the save button if metadata has changed and there are no invalid properties', fakeAsync(() => {
updateService.update(
{
key: 'properties.property-key',
isValidValue: true
} as CardViewBaseItemModel,
'updated-value'
);
tick(500);
fixture.detectChanges();
toggleEditModeForGeneralInfo();
expect(findSaveGeneralInfoButton().disabled).toBeFalse();
}));
it('should disable the save button if there are invalid properties', fakeAsync(() => {
updateService.update(
{
key: 'properties.property-key',
isValidValue: false
} as CardViewBaseItemModel,
'updated-value'
);
tick(500);
fixture.detectChanges();
toggleEditModeForGeneralInfo();
expect(findSaveGeneralInfoButton().disabled).toBeTrue();
}));
});
describe('updateInvalidProperties', () => {
it('should add the property key to invalidProperties if isValidValue is false and key is not present', fakeAsync(() => {
const property = { key: 'properties.property-key', isValidValue: false } as CardViewBaseItemModel;
expect(component.invalidProperties.size).toBe(0);
updateService.update(property, 'updated-value');
tick(500);
expect(component.invalidProperties.has(property.key)).toBeTrue();
}));
it('should not add the property key to invalidProperties if isValidValue is false and key is already present', fakeAsync(() => {
const property = { key: 'properties.property-key', isValidValue: false } as CardViewBaseItemModel;
updateService.update(property, 'updated-value-1');
tick(500);
updateService.update(property, 'updated-value-2');
tick(500);
expect(component.invalidProperties.size).toBe(1);
expect(component.invalidProperties.has(property.key)).toBeTrue();
}));
it('should remove the property key from invalidProperties if isValidValue is true and key is present', fakeAsync(() => {
updateService.update({ key: 'properties.property-key', isValidValue: false } as CardViewBaseItemModel, 'updated-value');
tick(500);
expect(component.invalidProperties).toContain('properties.property-key');
updateService.update({ key: 'properties.property-key', isValidValue: true } as CardViewBaseItemModel, 'updated-value');
tick(500);
expect(component.invalidProperties.has('properties.property-key')).toBeFalse();
}));
it('should not change invalidProperties if isValidValue is true and key is not present', fakeAsync(() => {
expect(component.invalidProperties.size).toBe(0);
updateService.update({ key: 'properties.property-key', isValidValue: true } as CardViewBaseItemModel, 'updated-value');
tick(500);
expect(component.invalidProperties.size).toBe(0);
}));
});
it('should save changedProperties on save click', fakeAsync(() => {
getGroupedPropertiesSpy.and.returnValue(
of([
@@ -309,7 +439,7 @@ describe('ContentMetadataComponent', () => {
spyOn(nodesApiService, 'updateNode').and.returnValue(of(expectedNode));
toggleEditModeForGroup();
clickOnGroupSave();
clickOnGroupSaveButton();
expect(nodesApiService.updateNode).toHaveBeenCalled();
expect(component.node).toEqual(expectedNode);
}));

View File

@@ -164,6 +164,7 @@ export class ContentMetadataComponent implements OnChanges, OnInit {
categoriesManagementMode = CategoriesManagementMode.ASSIGN;
classifiableChanged = this.classifiableChangedSubject.asObservable();
editing = false;
invalidProperties = new Set<string>();
editedPanelTitle = '';
currentPanel: ContentMetadataPanel = {
expanded: false,
@@ -194,6 +195,7 @@ export class ContentMetadataComponent implements OnChanges, OnInit {
.subscribe((updatedNode: UpdateNotification) => {
this.hasMetadataChanged = true;
this.targetProperty = updatedNode.target;
this.updateInvalidProperties();
this.updateChanges(updatedNode.changed);
});
@@ -549,4 +551,12 @@ export class ContentMetadataComponent implements OnChanges, OnInit {
}
return observables;
}
private updateInvalidProperties() {
if (this.targetProperty?.isValidValue === false) {
this.invalidProperties.add(this.targetProperty.key);
} else if (this.targetProperty?.isValidValue === true) {
this.invalidProperties.delete(this.targetProperty.key);
}
}
}

View File

@@ -17,13 +17,7 @@
import { inject, Injectable } from '@angular/core';
import { Node } from '@alfresco/js-api';
import {
CardViewDateItemModel,
CardViewItemMatchValidator,
CardViewTextItemModel,
FileSizePipe,
TranslationService
} from '@alfresco/adf-core';
import { CardViewDateItemModel, CardViewItemMatchValidator, CardViewTextItemModel, FileSizePipe, TranslationService } from '@alfresco/adf-core';
@Injectable({
providedIn: 'root'
@@ -44,9 +38,7 @@ export class BasicPropertiesService {
value: node.name,
key: 'properties.cm:name',
editable: true,
validators: [
new CardViewItemMatchValidator('[\\/\\*\\\\"\\\\:]')
]
validators: [new CardViewItemMatchValidator('[\\/\\*\\\\"\\\\:|?<>]')]
}),
new CardViewTextItemModel({
label: 'CORE.METADATA.BASIC.TITLE',

View File

@@ -505,7 +505,7 @@ describe('CardViewTextItemComponent', () => {
await fixture.whenStable();
expect(itemUpdatedSpy).toHaveBeenCalledWith({
target: { ...component.property },
target: { ...component.property, isValidValue: true },
changed: {
textkey: expectedText
}
@@ -580,11 +580,11 @@ describe('CardViewTextItemComponent', () => {
await updateTextField(component.property.key, 'updated-value');
await fixture.whenStable();
const property = { ...component.property };
const property = { ...component.property, isValidValue: true };
expect(cardViewUpdateService.update).toHaveBeenCalledWith(property, 'updated-value');
});
it('should NOT trigger the update event if the editedValue is invalid', async () => {
it('should trigger the update event if the editedValue is NOT valid', async () => {
const cardViewUpdateService = TestBed.inject(CardViewUpdateService);
spyOn(cardViewUpdateService, 'update');
component.property.isValid = () => false;
@@ -592,7 +592,8 @@ describe('CardViewTextItemComponent', () => {
await updateTextField(component.property.key, '@invalid-value');
await fixture.whenStable();
expect(cardViewUpdateService.update).not.toHaveBeenCalled();
const property = { ...component.property, isValidValue: false };
expect(cardViewUpdateService.update).toHaveBeenCalledWith(property, '@invalid-value');
});
it('should trigger the update event if the editedValue is valid', async () => {
@@ -666,7 +667,7 @@ describe('CardViewTextItemComponent', () => {
await fixture.whenStable();
expect(itemUpdatedSpy).toHaveBeenCalledWith({
target: { ...component.property },
target: { ...component.property, isValidValue: true },
changed: {
textkey: expectedText
}
@@ -711,7 +712,7 @@ describe('CardViewTextItemComponent', () => {
await fixture.whenStable();
expect(itemUpdatedSpy).toHaveBeenCalledWith({
target: { ...component.property },
target: { ...component.property, isValidValue: true },
changed: {
textkey: expectedText
}
@@ -826,7 +827,7 @@ describe('CardViewTextItemComponent', () => {
await fixture.whenStable();
expect(itemUpdatedSpy).toHaveBeenCalledWith({
target: { ...component.property },
target: { ...component.property, isValidValue: true },
changed: {
textkey: expectedNumber.toString()
}
@@ -882,7 +883,7 @@ describe('CardViewTextItemComponent', () => {
await fixture.whenStable();
expect(itemUpdatedSpy).toHaveBeenCalledWith({
target: { ...component.property },
target: { ...component.property, isValidValue: true },
changed: {
textkey: expectedNumber.toString()
}

View File

@@ -15,16 +15,7 @@
* limitations under the License.
*/
import {
ChangeDetectorRef,
Component,
DestroyRef,
inject,
Input,
OnChanges,
SimpleChanges,
ViewEncapsulation
} from '@angular/core';
import { ChangeDetectorRef, Component, DestroyRef, inject, Input, OnChanges, SimpleChanges, ViewEncapsulation } from '@angular/core';
import { CardViewTextItemModel } from '../../models/card-view-textitem.model';
import { BaseCardView } from '../base-card-view';
import { MatChipInputEvent, MatChipsModule } from '@angular/material/chips';
@@ -158,9 +149,10 @@ export class CardViewTextItemComponent extends BaseCardView<CardViewTextItemMode
this.resetErrorMessages();
if (this.property.isValid(this.editedValue)) {
this.property.value = this.prepareValueForUpload(this.property, this.editedValue);
this.cardViewUpdateService.update({ ...this.property } as CardViewTextItemModel, this.property.value);
this.cardViewUpdateService.update({ ...this.property, isValidValue: true } as CardViewTextItemModel, this.property.value);
} else {
this.errors = this.property.getValidationErrors(this.editedValue);
this.cardViewUpdateService.update({ ...this.property, isValidValue: false } as CardViewTextItemModel, this.editedValue);
}
}
}

View File

@@ -31,6 +31,7 @@ export abstract class CardViewBaseItemModel<T = any> {
data?: any;
type?: string;
multivalued?: boolean;
isValidValue?: boolean;
constructor(props: CardViewItemProperties) {
this.label = props.label || '';