fix: we're not allow to change tagtype if it is a checkbox (#224)

* fix: we're not allowing change tagtype if it is a checkbox

* refactoring: deletes console.log

* refactoring: delets comment

* + not allow to change tag type to "checkbox"

* refactoring: deletes extra brackets

* fix: let  -> const

* fix prev review issues + added warning toast

* fix: failing pipeline test

* refactoring

* refactor: spelling "service"

* refactor: switching to is empty(temporary), disabling options instead of removing, updates state on change

* refactor: renaming fix test on pipelines

* refactor: now with documentCount

* style: deletes empty line

* refactor: delete 'isEmpty'

* style: fix variable to camelcase

* refactor

* refactor: switching tag types to enum

* refactor: switch statement

* Stew ro/check tag type and region catagory compatibility (#241)

* refactor: check tag type and region catagory compatibility

* refactor and spelling

Co-authored-by: alex-krasn <v-alexkr@microsoft.com>

* feature: no more than one checkbox per tag

* refactor: add compatibilty method and refactor

* refactor

* spelling

* style: missing trailing comma

* refactor: isCompatible

Co-authored-by: stew-ro <60453211+stew-ro@users.noreply.github.com>
Co-authored-by: kunzheng <58841788+kunzms@users.noreply.github.com>

src/common/localization/en-us.ts

@@ -172,6 +172,8 @@ export const english: IAppStrings = {
             existingName: "Tag name already exists. Choose another name",
             emptyName: "Cannot have an empty tag name",
             unknownTagName: "Unknown",
+            notCompatibleTagType: "Tag type is not compatible with this feature",
+            checkboxPerTagLimit: "Cannot assign more than one checkbox per tag",
         },
         toolbar: {
             add: "Add new tag",

src/common/localization/es-cl.ts

@@ -173,6 +173,8 @@ export const spanish: IAppStrings = {
             existingName: "Nombre de etiqueta ya existe. Elige otro nombre",
             emptyName: "El nombre de etiqueta no puede ser vacío",
             unknownTagName: "Desconocido",
+            notCompatibleTagType: "El tipo de etiqueta no es compatible con esta función",
+            checkboxPerTagLimit: "No se puede asignar más de una casilla de verificación por etiqueta",
         },
         toolbar: {
             add: "Agregar nueva etiqueta",

src/common/mockFactory.ts

@@ -400,7 +400,7 @@ export default class MockFactory {
 
     /**
      * Creates an array of test regions
-     * @param count The number of regions to create (deafult: 5)
+     * @param count The number of regions to create (default: 5)
      */
     public static createTestRegions(count: number = 5) {
         const regions: IRegion[] = [];
@@ -561,7 +561,7 @@ export default class MockFactory {
     }
 
     /**
-     * Runs and waits for a condidtion to be met and resolves a promise
+     * Runs and waits for a condition to be met and resolves a promise
      * @param predicate The predicate to evaluate the condition
      * @param interval The interval to check the value
      */
@@ -729,7 +729,7 @@ export default class MockFactory {
 
     /**
      * Gets StorageType for asset providers
-     * @param providerType Asset Providet type
+     * @param providerType Asset Provider type
      */
     private static getStorageType(providerType: string): StorageType {
         switch (providerType) {

src/common/strings.ts

@@ -186,6 +186,8 @@ export interface IAppStrings {
             existingName: string;
             emptyName: string;
             unknownTagName: string;
+            notCompatibleTagType: string;
+            checkboxPerTagLimit: string;
         }
     };
     connections: {

src/react/components/common/tagInput/tagInput.tsx

@@ -463,19 +463,41 @@ export class TagInput extends React.Component<ITagInputProps, ITagInputState> {
             let deselect = selected && oldTagOperation === TagOperationMode.None;
 
             // Only fire click event if a region is selected
-            if (this.props.selectedRegions &&
-                this.props.selectedRegions.length > 0 &&
-                this.props.onTagClick) {
-                deselect = false;
-                this.props.onTagClick(tag);
+            const { selectedRegions, onTagClick, labels } = this.props;
+            if (selectedRegions && selectedRegions.length && onTagClick) {
+                const { category } = selectedRegions[0];
+                const { format, type, documentCount, name } = tag;
+                const tagCategory = this.getTagCategory(type);
+                if (tagCategory === category ||
+                    (documentCount === 0 && type === FieldType.String && format === FieldFormat.NotSpecified)) {
+                    if (category === "checkbox" && this.labelAssigned(labels, name)) {
+                        toast.warn(strings.tags.warnings.checkboxPerTagLimit);
+                        return;
+                    }
+                    onTagClick(tag);
+                    deselect = false;
+                } else {
+                    toast.warn(strings.tags.warnings.notCompatibleTagType);
+                }
             }
-
             this.setState({
                 selectedTag: deselect ? null : tag,
                 tagOperation,
             });
+        }
+    }
 
-       }
+    private labelAssigned = (labels, name): boolean => {
+         return labels.find((label) => label.label === name ? true : false);
+    }
+
+    private getTagCategory = (tagType: string) => {
+        switch (tagType) {
+            case FieldType.SelectionMark:
+                return "checkbox";
+            default:
+                return "text";
+        }
     }
 
     private onSearchKeyDown = (event: KeyboardEvent): void => {
@@ -611,17 +633,28 @@ export class TagInput extends React.Component<ITagInputProps, ITagInputState> {
         return menuItems;
     }
 
+    private isTypeCompatibleWithTag = (tag, type) => {
+        // If free tag we can assign any type
+        if (tag && tag.documentCount <= 0) {
+            return true;
+        }
+        const tagType = this.getTagCategory(tag.type);
+        const menuItemType = this.getTagCategory(type);
+        return tagType === menuItemType;
+    }
+
     private getTypeSubMenuItems = (): IContextualMenuItem[] => {
         const tag = this.state.selectedTag;
         const types = Object.values(FieldType);
-
         return types.map((type) => {
+            const isCompatible = this.isTypeCompatibleWithTag(tag, type);
             return {
                 key: type,
                 text: type,
-                canCheck: true,
+                canCheck: isCompatible,
                 isChecked: type === tag.type,
                 onClick: this.onTypeSelect,
+                disabled: !isCompatible,
             } as IContextualMenuItem;
         });
     }

src/services/projectService.test.ts

@@ -11,7 +11,7 @@ import { generateKey } from "../common/crypto";
 import { encryptProject, decryptProject } from "../common/utils";
 
 describe("Project Service", () => {
-    let projectSerivce: IProjectService = null;
+    let projectService: IProjectService = null;
     let testProject: IProject = null;
     let projectList: IProject[] = null;
     let securityToken: ISecurityToken = null;
@@ -29,21 +29,21 @@ describe("Project Service", () => {
             key: generateKey(),
         };
         testProject = MockFactory.createTestProject("TestProject");
-        projectSerivce = new ProjectService();
+        projectService = new ProjectService();
 
         storageProviderMock.writeText.mockClear();
         storageProviderMock.deleteFile.mockClear();
     });
 
     it("Load decrypts any project settings using the specified key", async () => {
         const encryptedProject = await encryptProject(testProject, securityToken);
-        const decryptedProject = await projectSerivce.load(encryptedProject, securityToken);
+        const decryptedProject = await projectService.load(encryptedProject, securityToken);
 
         expect(decryptedProject).toEqual(testProject);
     });
 
     it("Saves calls project storage provider to write project", async () => {
-        const result = await projectSerivce.save(testProject, securityToken);
+        const result = await projectService.save(testProject, securityToken);
 
         const encryptedProject: IProject = {
             ...testProject,
@@ -69,27 +69,27 @@ describe("Project Service", () => {
 
     it("initializes tags to empty array if not defined", async () => {
         testProject.tags = null;
-        const result = await projectSerivce.save(testProject, securityToken);
+        const result = await projectService.save(testProject, securityToken);
 
         expect(result.tags).toEqual([]);
     });
 
     it("Save throws error if writing to storage provider fails", async () => {
         const expectedError = "Error writing to storage provider";
         storageProviderMock.writeText.mockImplementationOnce(() => Promise.reject(expectedError));
-        await expect(projectSerivce.save(testProject, securityToken)).rejects.toEqual(expectedError);
+        await expect(projectService.save(testProject, securityToken)).rejects.toEqual(expectedError);
     });
 
     it("Save throws error if storage provider cannot be created", async () => {
         const expectedError = new Error("Error creating storage provider");
         const createMock = StorageProviderFactory.create as jest.Mock;
         createMock.mockImplementationOnce(() => { throw expectedError; });
 
-        await expect(projectSerivce.save(testProject, securityToken)).rejects.toEqual(expectedError);
+        await expect(projectService.save(testProject, securityToken)).rejects.toEqual(expectedError);
     });
 
     it("Delete calls project storage provider to delete project", async () => {
-        await projectSerivce.delete(testProject);
+        await projectService.delete(testProject);
 
         expect(StorageProviderFactory.create).toBeCalledWith(
             testProject.sourceConnection.providerType,
@@ -104,28 +104,28 @@ describe("Project Service", () => {
         storageProviderMock.deleteFile
             .mockImplementationOnce(() => Promise.reject(expectedError));
 
-        await expect(projectSerivce.delete(testProject)).rejects.toEqual(expectedError);
+        await expect(projectService.delete(testProject)).rejects.toEqual(expectedError);
     });
 
     it("Delete call fails if storage provider cannot be created", async () => {
         const expectedError = new Error("Error creating storage provider");
         const createMock = StorageProviderFactory.create as jest.Mock;
         createMock.mockImplementationOnce(() => { throw expectedError; });
 
-        await expect(projectSerivce.delete(testProject)).rejects.toEqual(expectedError);
+        await expect(projectService.delete(testProject)).rejects.toEqual(expectedError);
     });
 
     it("isDuplicate returns false when called with a unique project", async () => {
         testProject = MockFactory.createTestProject("TestProject");
         projectList = MockFactory.createTestProjects();
-        expect(projectSerivce.isDuplicate(testProject, projectList)).toEqual(false);
+        expect(projectService.isDuplicate(testProject, projectList)).toEqual(false);
     });
 
     it("isDuplicate returns true when called with a duplicate project", async () => {
         testProject = MockFactory.createTestProject("1");
         testProject.id = undefined;
         projectList = MockFactory.createTestProjects();
-        expect(projectSerivce.isDuplicate(testProject, projectList)).toEqual(true);
+        expect(projectService.isDuplicate(testProject, projectList)).toEqual(true);
     });
 
     it("deletes all asset metadata files when project is deleted", async () => {
@@ -136,7 +136,7 @@ describe("Project Service", () => {
 
         testProject.assets = _.keyBy(assets, (asset) => asset.id);
 
-        await projectSerivce.delete(testProject);
+        await projectService.delete(testProject);
         expect(storageProviderMock.deleteFile.mock.calls).toHaveLength(assets.length + 1);
     });
 });

src/services/projectService.ts

@@ -211,7 +211,7 @@ export default class ProjectService implements IProjectService {
         asset?: string) {
         const tags: ITag[] = [];
         const tagNameSet = new Set<string>();
-        const tagdocumentCount = {};
+        const tagDocumentCount = {};
         try {
             const blobs = new Set<string>(await storageProvider.listFiles(project.folderPath));
             const assetLabel = asset ? asset + constants.labelFileExtension : undefined;
@@ -225,10 +225,10 @@ export default class ProjectService implements IProjectService {
                             const content = JSON.parse(await storageProvider.readText(blob));
                             content.labels.forEach((label) => {
                                 tagNameSet.add(label.label);
-                                if (tagdocumentCount[label.label]) {
-                                    tagdocumentCount[label.label] += 1;
+                                if (tagDocumentCount[label.label]) {
+                                    tagDocumentCount[label.label] += 1;
                                 } else {
-                                    tagdocumentCount[label.label] = 1;
+                                    tagDocumentCount[label.label] = 1;
                                 }
                             });
                         }
@@ -255,11 +255,11 @@ export default class ProjectService implements IProjectService {
                     // use default type
                     type: FieldType.String,
                     format: FieldFormat.NotSpecified,
-                    documentCount: tagdocumentCount[name],
+                    documentCount: tagDocumentCount[name],
                 } as ITag);
             });
             if (project.tags) {
-                await this.addMissingTagsAndUpdatedocumentCount(project, tags, tagdocumentCount);
+                await this.addMissingTagsAndUpdateDocumentCount(project, tags, tagDocumentCount);
             } else {
                 project.tags = tags;
             }
@@ -293,7 +293,7 @@ export default class ProjectService implements IProjectService {
             });
             if (project.tags) {
                 project.tags = patch(project.tags, tags, "name", ["type", "format"]);
-                await this.addMissingTagsAndUpdatedocumentCount(project, tags);
+                await this.addMissingTagsAndUpdateDocumentCount(project, tags);
             } else {
                 project.tags = tags;
             }
@@ -328,21 +328,22 @@ export default class ProjectService implements IProjectService {
         updatedProject.tags = existingTags;
     }
 
-    private async addMissingTagsAndUpdatedocumentCount(project: IProject, tags: ITag[], tagdocumentCount?: any) {
+    private async addMissingTagsAndUpdateDocumentCount(project: IProject, tags: ITag[], tagDocumentCount?: any) {
         const missingTags = tags.filter((fileTag) => {
             const foundExistingTag = project.tags.find((tag) => fileTag.name === tag.name );
             if (!foundExistingTag) {
                 return true;
             } else {
-                if (tagdocumentCount) {
-                    foundExistingTag.documentCount =  tagdocumentCount[foundExistingTag.name];
+                if (tagDocumentCount) {
+                    foundExistingTag.documentCount =  tagDocumentCount[foundExistingTag.name];
                 }
                 return false;
             }
         });
         project.tags = [...project.tags, ...missingTags];
     }
 
+    // private async getAllTagsInProjectCount(project: IProject, tags: ITag[]) {}
     /**
      * Save fields.json
      * @param project the project we're trying to create