From c0cb5a87e7af42bc4369ceff027532f0296dba9f Mon Sep 17 00:00:00 2001
From: Sergei Ogorelkov <sergei.ogorelkov@icloud.com>
Date: Tue, 30 May 2023 10:28:36 +0400
Subject: [PATCH] [UBER-196] Fix duplicated project ids when creating a project
 (#3247)

Signed-off-by: Sergei Ogorelkov <sergei.ogorelkov@icloud.com>
---
 models/core/src/tx.ts                         |   2 +
 packages/core/src/operations.ts               |   8 +-
 packages/core/src/tx.ts                       |   5 +
 .../components/projects/ChangeIdentity.svelte |  24 ++--
 .../components/projects/CreateProject.svelte  | 103 +++++++++++++++---
 server/core/src/storage.ts                    |   9 ++
 6 files changed, 118 insertions(+), 33 deletions(-)

diff --git a/models/core/src/tx.ts b/models/core/src/tx.ts
index 37333bb2a0..51d0f794b9 100644
--- a/models/core/src/tx.ts
+++ b/models/core/src/tx.ts
@@ -96,5 +96,7 @@ export class TTxApplyIf extends TTx implements TxApplyIf {
 
   // All matches should be true with at least one document.
   match!: DocumentClassQuery<Doc>[]
+  // All matches should be false for all documents.
+  notMatch!: DocumentClassQuery<Doc>[]
   txes!: TxCUD<Doc>[]
 }
diff --git a/packages/core/src/operations.ts b/packages/core/src/operations.ts
index 44228ca7eb..0296a7e07c 100644
--- a/packages/core/src/operations.ts
+++ b/packages/core/src/operations.ts
@@ -281,6 +281,7 @@ export class TxOperations implements Omit<Client, 'notify'> {
 export class ApplyOperations extends TxOperations {
   txes: TxCUD<Doc>[] = []
   matches: DocumentClassQuery<Doc>[] = []
+  notMatches: DocumentClassQuery<Doc>[] = []
   constructor (readonly ops: TxOperations, readonly scope: string) {
     const txClient: Client = {
       getHierarchy: () => ops.client.getHierarchy(),
@@ -303,10 +304,15 @@ export class ApplyOperations extends TxOperations {
     return this
   }
 
+  notMatch<T extends Doc>(_class: Ref<Class<T>>, query: DocumentQuery<T>): ApplyOperations {
+    this.notMatches.push({ _class, query })
+    return this
+  }
+
   async commit (): Promise<boolean> {
     if (this.txes.length > 0) {
       return await ((await this.ops.tx(
-        this.ops.txFactory.createTxApplyIf(core.space.Tx, this.scope, this.matches, this.txes)
+        this.ops.txFactory.createTxApplyIf(core.space.Tx, this.scope, this.matches, this.notMatches, this.txes)
       )) as Promise<boolean>)
     }
     return true
diff --git a/packages/core/src/tx.ts b/packages/core/src/tx.ts
index d351e853d2..540bcfb00f 100644
--- a/packages/core/src/tx.ts
+++ b/packages/core/src/tx.ts
@@ -120,6 +120,9 @@ export interface TxApplyIf extends Tx {
   // All matches should be true with at least one document.
   match: DocumentClassQuery<Doc>[]
 
+  // All matches should be false for all documents.
+  notMatch: DocumentClassQuery<Doc>[]
+
   // If all matched execute following transactions.
   txes: TxCUD<Doc>[]
 }
@@ -586,6 +589,7 @@ export class TxFactory {
     space: Ref<Space>,
     scope: string,
     match: DocumentClassQuery<Doc>[],
+    notMatch: DocumentClassQuery<Doc>[],
     txes: TxCUD<Doc>[],
     modifiedOn?: Timestamp,
     modifiedBy?: Ref<Account>
@@ -599,6 +603,7 @@ export class TxFactory {
       objectSpace: space,
       scope,
       match,
+      notMatch,
       txes
     }
   }
diff --git a/plugins/tracker-resources/src/components/projects/ChangeIdentity.svelte b/plugins/tracker-resources/src/components/projects/ChangeIdentity.svelte
index 795a9475f0..551c527020 100644
--- a/plugins/tracker-resources/src/components/projects/ChangeIdentity.svelte
+++ b/plugins/tracker-resources/src/components/projects/ChangeIdentity.svelte
@@ -1,34 +1,26 @@
 <script lang="ts">
-  import presentation, { Card, getClient } from '@hcengineering/presentation'
-  import { Project } from '@hcengineering/tracker'
+  import presentation, { Card } from '@hcengineering/presentation'
   import EditBox from '@hcengineering/ui/src/components/EditBox.svelte'
   import { createEventDispatcher } from 'svelte'
   import tracker from '../../plugin'
 
-  export let project: Project
+  export let identifier: string
+  export let projectsIdentifiers: Set<string>
 
-  let identifier = project.identifier
+  let newIdentifier = identifier
 
   const dispatch = createEventDispatcher()
 
   function save () {
-    dispatch('close', identifier)
+    dispatch('close', newIdentifier)
   }
-
-  let projects: Set<string> = new Set()
-
-  $: getClient()
-    .findAll(tracker.class.Project, {})
-    .then((pr) => {
-      projects = new Set(pr.map((p) => p.identifier))
-    })
 </script>
 
 <Card
-  label={projects.has(identifier) ? tracker.string.IdentifierExists : tracker.string.ProjectIdentifier}
+  label={projectsIdentifiers.has(newIdentifier) ? tracker.string.IdentifierExists : tracker.string.ProjectIdentifier}
   okLabel={presentation.string.Save}
   okAction={save}
-  canSave={identifier !== project.identifier && !projects.has(identifier)}
+  canSave={!!newIdentifier && newIdentifier !== identifier && !projectsIdentifiers.has(newIdentifier)}
   on:close={() => {
     dispatch('close')
   }}
@@ -36,7 +28,7 @@
 >
   <div class="float-left-box">
     <div class="float-left p-2">
-      <EditBox bind:value={identifier} />
+      <EditBox bind:value={newIdentifier} />
     </div>
   </div>
 </Card>
diff --git a/plugins/tracker-resources/src/components/projects/CreateProject.svelte b/plugins/tracker-resources/src/components/projects/CreateProject.svelte
index d9c29f0e31..b192e95558 100644
--- a/plugins/tracker-resources/src/components/projects/CreateProject.svelte
+++ b/plugins/tracker-resources/src/components/projects/CreateProject.svelte
@@ -15,9 +15,17 @@
 <script lang="ts">
   import { Employee } from '@hcengineering/contact'
   import { AccountArrayEditor, AssigneeBox } from '@hcengineering/contact-resources'
-  import core, { Account, DocumentUpdate, Ref, SortingOrder, generateId, getCurrentAccount } from '@hcengineering/core'
+  import core, {
+    Account,
+    ApplyOperations,
+    DocumentUpdate,
+    Ref,
+    SortingOrder,
+    generateId,
+    getCurrentAccount
+  } from '@hcengineering/core'
   import { Asset } from '@hcengineering/platform'
-  import presentation, { Card, getClient } from '@hcengineering/presentation'
+  import presentation, { Card, createQuery, getClient } from '@hcengineering/presentation'
   import { StyledTextBox } from '@hcengineering/text-editor'
   import { IssueStatus, Project, TimeReportDayType, genRanks } from '@hcengineering/tracker'
   import {
@@ -43,6 +51,7 @@
 
   const client = getClient()
   const hierarchy = client.getHierarchy()
+  const projectsQuery = createQuery()
 
   let name: string = project?.name ?? ''
   let description: string = project?.description ?? ''
@@ -52,13 +61,21 @@
   let defaultAssignee: Ref<Employee> | null | undefined = project?.defaultAssignee ?? null
   let members: Ref<Account>[] =
     project?.members !== undefined ? hierarchy.clone(project.members) : [getCurrentAccount()._id]
+  let projectsIdentifiers: Set<string> = new Set()
+  let isSaving = false
+
+  let changeIdentityRef: HTMLElement
 
   const dispatch = createEventDispatcher()
 
   $: isNew = !project
 
   async function handleSave () {
-    isNew ? createProject() : updateProject()
+    if (isNew) {
+      await createProject()
+    } else {
+      await updateProject()
+    }
   }
 
   let identifier: string = project?.identifier ?? 'TSK'
@@ -84,6 +101,10 @@
   }
 
   async function updateProject () {
+    if (!project) {
+      return
+    }
+
     const { sequence, issueStatuses, defaultIssueStatus, ...projectData } = getProjectData()
     const update: DocumentUpdate<Project> = {}
     if (projectData.name !== project?.name) {
@@ -121,21 +142,46 @@
       }
     }
     if (Object.keys(update).length > 0) {
-      await client.update(project!, update)
+      const ops = client.apply(project._id).notMatch(tracker.class.Project, { identifier: projectData.identifier })
+
+      isSaving = true
+      await ops.update(project, update)
+      const succeeded = await ops.commit()
+      isSaving = false
+
+      if (succeeded) {
+        close()
+      } else {
+        changeIdentity(changeIdentityRef)
+      }
     }
   }
 
   async function createProject () {
-    const id = await client.createDoc(tracker.class.Project, core.space.Space, getProjectData())
-    await createProjectIssueStatuses(id, defaultStatusId)
+    const projectId = generateId<Project>()
+    const projectData = getProjectData()
+    const ops = client.apply(projectId).notMatch(tracker.class.Project, { identifier: projectData.identifier })
+
+    isSaving = true
+    await ops.createDoc(tracker.class.Project, core.space.Space, projectData, projectId)
+    await createProjectIssueStatuses(ops, projectId, defaultStatusId)
+    const succeeded = await ops.commit()
+    isSaving = false
+
+    if (succeeded) {
+      close()
+    } else {
+      changeIdentity(changeIdentityRef)
+    }
   }
 
   async function createProjectIssueStatuses (
+    ops: ApplyOperations,
     projectId: Ref<Project>,
     defaultStatusId: Ref<IssueStatus>,
     defaultCategoryId = tracker.issueStatusCategory.Backlog
   ): Promise<void> {
-    const categories = await client.findAll(
+    const categories = await ops.findAll(
       core.class.StatusCategory,
       { ofAttribute: tracker.attribute.IssueStatus },
       { sort: { order: SortingOrder.Ascending } }
@@ -147,7 +193,7 @@
       const rank = issueStatusRanks[i]
 
       if (defaultStatusName !== undefined) {
-        await client.createDoc(
+        await ops.createDoc(
           tracker.class.IssueStatus,
           projectId,
           {
@@ -170,26 +216,37 @@
       }
     })
   }
-  function changeIdentity (ev: MouseEvent) {
-    showPopup(ChangeIdentity, { project }, eventToHTMLElement(ev), (result) => {
+  function changeIdentity (element: HTMLElement) {
+    showPopup(ChangeIdentity, { identifier, projectsIdentifiers }, element, (result) => {
       if (result != null) {
         identifier = result
       }
     })
   }
+
+  function close () {
+    dispatch('close')
+  }
+
+  $: projectsQuery.query(
+    tracker.class.Project,
+    { _id: { $nin: project ? [project._id] : [] } },
+    (res) => (projectsIdentifiers = new Set(res.map(({ identifier }) => identifier)))
+  )
 </script>
 
 <Card
   label={isNew ? tracker.string.NewProject : tracker.string.EditProject}
   okLabel={isNew ? presentation.string.Create : presentation.string.Save}
   okAction={handleSave}
-  canSave={name.length > 0 && !(members.length === 0 && isPrivate)}
+  canSave={name.length > 0 &&
+    identifier.length > 0 &&
+    !projectsIdentifiers.has(identifier) &&
+    !(members.length === 0 && isPrivate)}
   accentHeader
   width={'medium'}
   gap={'gapV-6'}
-  on:close={() => {
-    dispatch('close')
-  }}
+  onCancel={close}
   on:changeContent
 >
   <div class="antiGrid">
@@ -217,7 +274,7 @@
         <Label label={tracker.string.Identifier} />
         <span><Label label={tracker.string.UsedInIssueIDs} /></span>
       </div>
-      <div class="padding flex-row-center">
+      <div bind:this={changeIdentityRef} class="padding flex-row-center relative">
         <EditBox
           bind:value={identifier}
           disabled={!isNew}
@@ -226,7 +283,13 @@
           uppercase
         />
         {#if !isNew}
-          <Button size={'small'} icon={IconEdit} on:click={changeIdentity} />
+          <div class="ml-1">
+            <Button size={'small'} icon={IconEdit} on:click={(ev) => changeIdentity(eventToHTMLElement(ev))} />
+          </div>
+        {:else if !isSaving && projectsIdentifiers.has(identifier)}
+          <div class="absolute overflow-label duplicated-identifier">
+            <Label label={tracker.string.IdentifierExists} />
+          </div>
         {/if}
       </div>
     </div>
@@ -305,3 +368,11 @@
     </div>
   </div>
 </Card>
+
+<style lang="scss">
+  .duplicated-identifier {
+    left: 0;
+    bottom: -0.25rem;
+    color: var(--theme-warning-color);
+  }
+</style>
diff --git a/server/core/src/storage.ts b/server/core/src/storage.ts
index 9a795d4a6e..1352e040b1 100644
--- a/server/core/src/storage.ts
+++ b/server/core/src/storage.ts
@@ -594,6 +594,15 @@ class TServerStorage implements ServerStorage {
         break
       }
     }
+    if (passed) {
+      for (const { _class, query } of applyIf.notMatch) {
+        const res = await findAll(ctx, _class, query, { limit: 1 })
+        if (res.length > 0) {
+          passed = false
+          break
+        }
+      }
+    }
     return { passed, onEnd }
   }