Maintenance: Improve form id schema.

Co-authored-by: Martin Gruner <mg@zammad.com>

app/assets/javascripts/app/controllers/_application_controller/form.coffee

@@ -725,8 +725,10 @@ class App.ControllerForm extends App.Controller
     param
 
   @formId: ->
-    formId = new Date().getTime() + Math.floor( Math.random() * 99999 )
-    formId.toString().substr formId.toString().length-9, 9
+    try
+      return crypto.randomUUID()
+    catch e
+      return URL.createObjectURL(new Blob()).substr(-36)
 
   @findForm: (form) ->
     # check jquery event

app/frontend/shared/components/Form/Form.vue

@@ -118,12 +118,7 @@ export interface Props {
   ) => Promise<void | (() => void)> | void | (() => void)
 }
 
-// Zammad currently expects formIds to be BigInts. Maybe convert to UUIDs later.
-// const formId = `form-${getUuid()}`
-
-// This is the formId generation logic from the legacy desktop app.
-let formId = new Date().getTime() + Math.floor(Math.random() * 99999).toString()
-formId = formId.substr(formId.length - 9, 9)
+const formId = getUuid()
 
 const props = withDefaults(defineProps<Props>(), {
   schema: () => {

app/frontend/tests/graphql/builders/index.ts

@@ -20,6 +20,7 @@ import {
 import { createRequire } from 'node:module'
 import type { DeepPartial, DeepRequired } from '#shared/types/utils.ts'
 import { uniqBy } from 'lodash-es'
+import getUuid from '#shared/utils/getUuid.ts'
 import { generateGraphqlMockId, hasNodeParent, setNodeParent } from './utils.ts'
 import logger from './logger.ts'
 
@@ -184,11 +185,8 @@ const getScalarValue = (
       return faker.number.float()
     case 'BinaryString':
       return faker.image.dataUri()
-    case 'FormId': {
-      const formId =
-        faker.date.recent() + Math.floor(Math.random() * 99999).toString()
-      return formId.substring(formId.length - 9, 9)
-    }
+    case 'FormId':
+      return getUuid()
     case 'ISO8601Date':
       return faker.date.recent().toISOString().substring(0, 10)
     case 'ISO8601DateTime':

app/graphql/gql/types/form_id_type.rb

@@ -1,8 +1,7 @@
 # Copyright (C) 2012-2024 Zammad Foundation, https://zammad-foundation.org/
 
 module Gql::Types
-  class FormIdType < GraphQL::Types::BigInt
-    # Zammad currenly requires FormIDs to be BigInts. Maybe this could be changed to UUIDs later on.
-    description 'BigInt to identify a form.'
+  class FormIdType < GraphQL::Types::String
+    description 'UUID to identify a form.'
   end
 end

app/graphql/graphql_introspection.json

@@ -4159,7 +4159,7 @@
         {
           "kind": "SCALAR",
           "name": "FormId",
-          "description": "BigInt to identify a form.",
+          "description": "UUID to identify a form.",
           "fields": null,
           "inputFields": null,
           "interfaces": null,

app/models/store.rb

@@ -59,7 +59,7 @@ def set_store_file
   def self.list(data)
     # search
     store_object_id = Store::Object.lookup(name: data[:object])
-    Store.where(store_object_id: store_object_id, o_id: data[:o_id].to_i)
+    Store.where(store_object_id: store_object_id, o_id: data[:o_id])
                   .reorder(created_at: :asc)
 
   end

db/migrate/20120101000001_create_base.rb

@@ -481,7 +481,7 @@ def up
     create_table :stores do |t|
       t.references :store_object,               null: false
       t.references :store_file,                 null: false
-      t.integer :o_id,              limit: 8,   null: false
+      t.string :o_id,               limit: 255, null: false
       t.string :preferences,        limit: 2500, null: true
       t.string :size,               limit: 50,  null: true
       t.string :filename,           limit: 250, null: false

db/migrate/20240227104106_update_upload_cache_object_id.rb

@@ -0,0 +1,11 @@
+# Copyright (C) 2012-2024 Zammad Foundation, https://zammad-foundation.org/
+
+class UpdateUploadCacheObjectId < ActiveRecord::Migration[7.0]
+  def change
+    # return if it's a new setup
+    return if !Setting.exists?(name: 'system_init_done')
+
+    change_column :stores, :o_id, :string, limit: 255
+    Store.reset_column_information
+  end
+end

lib/upload_cache.rb

@@ -13,8 +13,7 @@ class UploadCache
   #
   # @return [UploadCache]
   def initialize(id)
-    # conversion to Integer is required for proper Store#o_id comparsion
-    @id = id.to_i
+    @id = id
   end
 
   # Adds a Store item with the given attributes for the form_id.

spec/db/migrate/template_migration_spec.rb

@@ -7,7 +7,7 @@
     {
       'article.body'                  => 'twet 23123',
       'ticket.formSenderType'         => 'phone-in',
-      'article.form_id'               => '187440978',
+      'article.form_id'               => '19d8d2a2-8af3-4992-add0-353ee32bcb5f',
       'ticket.title'                  => 'aaa',
       'ticket.customer_id'            => '2',
       'ticket.customer_id_completion' => 'Nicole Braun <nicole.braun@example.com>',
@@ -35,7 +35,7 @@
                      {
                        'body'                   => 'twet 23123',
                        'formSenderType'         => 'phone-in',
-                       'form_id'                => '187440978',
+                       'form_id'                => '19d8d2a2-8af3-4992-add0-353ee32bcb5f',
                        'title'                  => 'aaa',
                        'customer_id'            => '2',
                        'customer_id_completion' => 'Nicole Braun <nicole.braun@example.com>',

spec/graphql/gql/mutations/form/upload_cache/add_spec.rb

@@ -19,7 +19,7 @@
         }
       QUERY
     end
-    let(:form_id)      { 12_345 }
+    let(:form_id)      { SecureRandom.uuid }
     let(:file_name)    { 'my_testfile.pdf' }
     let(:file_type)    { 'application/pdf' }
     let(:file_content) { 'some test content' }

spec/graphql/gql/mutations/form/upload_cache/remove_spec.rb

@@ -15,7 +15,7 @@
         }
       QUERY
     end
-    let(:form_id)           { 12_345 }
+    let(:form_id)           { SecureRandom.uuid }
     let(:upload_cache_file) { UploadCache.new(form_id).add(filename: file_name, data: file_content, created_by_id: 1) }
     let(:file_name)         { 'my_testfile.pdf' }
     let(:file_type)         { 'application/pdf' }

spec/graphql/gql/mutations/knowledge_base/answer/suggestion/content/transform_spec.rb

@@ -37,14 +37,14 @@
       MUTATION
     end
 
-    let(:variables) { { translationId: Gql::ZammadSchema.id_from_object(knowledge_base_answer_translation), formId: 12_345 } }
+    let(:variables) { { translationId: Gql::ZammadSchema.id_from_object(knowledge_base_answer_translation), formId: '5570fac8-8868-40b7-89e7-1cdabbd954ba' } }
 
     before do
       gql.execute(mutation, variables: variables)
     end
 
     context 'with authenticated session' do
-      let(:copied_attachments) { Store.list(object: 'UploadCache', o_id: 12_345) }
+      let(:copied_attachments) { Store.list(object: 'UploadCache', o_id: '5570fac8-8868-40b7-89e7-1cdabbd954ba') }
 
       it 'converts inline images to base64 data' do
         expect(gql.result.data['body']).to include('src="data:image/jpeg;base64,')
@@ -63,7 +63,7 @@
       end
 
       context 'with not existing translation' do
-        let(:variables) { { translationId: Gql::ZammadSchema.id_from_object(knowledge_base_answer), formId: 12_345 } }
+        let(:variables) { { translationId: Gql::ZammadSchema.id_from_object(knowledge_base_answer), formId: '5570fac8-8868-40b7-89e7-1cdabbd954ba' } }
 
         it 'raises an error' do
           expect(gql.result.error_type).to eq(ActiveRecord::RecordNotFound)

spec/graphql/gql/mutations/ticket/article/email_forward_reply_spec.rb

@@ -6,7 +6,7 @@
   let(:article)  { create(:ticket_article, :inbound_email, :with_attachment, from: customer.email) }
   let(:agent)    { create(:agent, groups: [article.ticket.group]) }
   let(:customer) { create(:customer) }
-  let(:form_id)  { 12_345 }
+  let(:form_id)  { SecureRandom.uuid }
 
   let(:query) do
     <<~QUERY

spec/graphql/gql/mutations/ticket/create_spec.rb

@@ -196,7 +196,7 @@ def it_fails_to_create_ticket
 
         context 'with attachments' do
           let(:article_payload) do
-            form_id = 12_345
+            form_id = SecureRandom.uuid
 
             file_name    = 'file1.txt'
             file_type    = 'text/plain'
@@ -243,7 +243,7 @@ def it_fails_to_create_ticket
           end
 
           let(:article_payload) do
-            form_id = 12_345
+            form_id = SecureRandom.uuid
 
             file_name    = 'file1.txt'
             file_type    = 'text/plain'

spec/graphql/gql/queries/form_updater_spec.rb

@@ -13,7 +13,7 @@
         }
       QUERY
     end
-    let(:variables) { { formUpdaterId: 'FormUpdater__Updater__Ticket__Create', meta: { formId: 12_345 }, data: {}, relationFields: relation_fields } }
+    let(:variables) { { formUpdaterId: 'FormUpdater__Updater__Ticket__Create', meta: { formId: '5570fac8-8868-40b7-89e7-1cdabbd954ba' }, data: {}, relationFields: relation_fields } }
     let(:relation_fields) do
       [
         {

spec/jobs/upload_cache_cleanup_job_spec.rb

@@ -4,7 +4,7 @@
 
 RSpec.describe UploadCacheCleanupJob, type: :job do
   context 'when upload cache exists' do
-    let(:upload_cache) { UploadCache.new(1337) }
+    let(:upload_cache) { UploadCache.new(SecureRandom.uuid) }
 
     before do
       UserInfo.current_user_id = 1
@@ -20,8 +20,9 @@
       travel_to 1.month.ago
 
       # create one taskbar and related upload cache entry, which should not be deleted
-      create(:taskbar, state: { form_id: 9999 })
-      UploadCache.new(9999).add(
+      taskbar_form_id = SecureRandom.uuid
+      create(:taskbar, state: { form_id: taskbar_form_id })
+      UploadCache.new(taskbar_form_id).add(
         data:        'Some Example with related Taskbar',
         filename:    'another_example_with_taskbar.txt',
         preferences: {

spec/lib/upload_cache_spec.rb

@@ -3,19 +3,13 @@
 require 'rails_helper'
 
 RSpec.describe UploadCache do
+  subject(:upload_cache) { described_class.new(form_id) }
 
-  subject(:upload_cache) { described_class.new(1337) }
+  let(:form_id) { SecureRandom.uuid }
 
   # required for adding items to the Store
   before { UserInfo.current_user_id = 1 }
 
-  describe '#initialize' do
-
-    it 'converts given (form_)id to an Integer' do
-      expect(described_class.new('1337').id).to eq(1337)
-    end
-  end
-
   describe '#add' do
 
     it 'adds a Store item' do
@@ -109,7 +103,7 @@
     end
 
     it 'fails for non existing UploadCache Store items' do
-      expect { upload_cache.remove_item(1337) }.to raise_error(ActiveRecord::RecordNotFound)
+      expect { upload_cache.remove_item(form_id) }.to raise_error(ActiveRecord::RecordNotFound)
     end
   end
 end

spec/models/core_workflow/conditions_spec.rb

@@ -755,7 +755,7 @@
                            body:            'hello',
                            type:            'note',
                            internal:        true,
-                           form_id:         '210458899',
+                           form_id:         SecureRandom.uuid,
                            shared_draft_id: '',
                            subtype:         '',
                            in_reply_to:     '',

spec/models/form_updater/updater/organization/edit_spec.rb

@@ -17,7 +17,7 @@
 
   let(:user)    { create(:agent) }
   let(:context) { { current_user: user } }
-  let(:meta)    { { initial: true, form_id: 12_345 } }
+  let(:meta)    { { initial: true, form_id: SecureRandom.uuid } }
   let(:data)    { {} }
   let(:relation_fields) do
     []

spec/models/form_updater/updater/ticket/create_spec.rb

@@ -23,7 +23,7 @@
   let(:level2_group)  { create(:group, name: 'Depth 2-G1', parent: level1_group) }
   let(:user)          { create(:agent, groups: [group, group2, level1_group2, level2_group]) }
   let(:context)       { { current_user: user } }
-  let(:meta)          { { initial: true, form_id: 12_345 } }
+  let(:meta)          { { initial: true, form_id: SecureRandom.uuid } }
   let(:data)          { {} }
 
   let(:relation_fields) do

spec/models/form_updater/updater/ticket/edit_spec.rb

@@ -19,7 +19,7 @@
   let(:group)   { create(:group) }
   let(:user)    { create(:agent, groups: [group]) }
   let(:context) { { current_user: user } }
-  let(:meta)    { { initial: true, form_id: 12_345 } }
+  let(:meta)    { { initial: true, form_id: SecureRandom.uuid } }
   let(:data)    { {} }
   let(:id)      { nil }
 

spec/models/form_updater/updater/user/create_spec.rb

@@ -17,7 +17,7 @@
 
   let(:user)    { create(:agent) }
   let(:context) { { current_user: user } }
-  let(:meta)    { { initial: true, form_id: 12_345 } }
+  let(:meta)    { { initial: true, form_id: SecureRandom.uuid } }
   let(:data)    { {} }
   let(:relation_fields) do
     []

spec/models/form_updater/updater/user/edit_spec.rb

@@ -17,7 +17,7 @@
 
   let(:user)                    { create(:agent) }
   let(:context)                 { { current_user: user } }
-  let(:meta)                    { { initial: true, form_id: 12_345 } }
+  let(:meta)                    { { initial: true, form_id: SecureRandom.uuid } }
   let(:data)                    { {} }
   let(:organization)            { create(:organization) }
   let(:secondary_organizations) { create_list(:organization, 5) }

spec/models/form_updater/updater/user/invite_spec.rb

@@ -16,7 +16,7 @@
 
   let(:user)                    { create(:admin) }
   let(:context)                 { { current_user: user } }
-  let(:meta)                    { { initial: true, form_id: 123 } }
+  let(:meta)                    { { initial: true, form_id: SecureRandom.uuid } }
   let(:data)                    { {} }
 
   describe '#resolve' do

spec/models/taskbar/has_attachments_examples.rb

@@ -3,14 +3,16 @@
 require 'rails_helper'
 
 RSpec.shared_examples 'Taskbar::HasAttachments' do
+  let(:form_id) { SecureRandom.uuid }
+
   describe '.with_form_id' do
     before do
       create(:taskbar)
-      create_list(:taskbar, 2, state: { form_id: 1337 })
+      create_list(:taskbar, 2, state: { form_id: form_id })
     end
 
     it 'get list of all form ids' do
-      expect(described_class.with_form_id.filter_map(&:persisted_form_id)).to eq([1337, 1337])
+      expect(described_class.with_form_id.filter_map(&:persisted_form_id)).to eq([form_id, form_id])
     end
   end
 
@@ -19,7 +21,7 @@
 
     let(:taskbar) do
       taskbar = create(:taskbar, state: state)
-      UploadCache.new(1337).add(
+      UploadCache.new(form_id).add(
         data:        'Some Example',
         filename:    'another_example.txt',
         preferences: {
@@ -39,7 +41,7 @@
 
     context 'when ticket create' do
       let(:state) do
-        { form_id: 1337 }
+        { form_id: form_id }
       end
 
       it 'delete attachments in upload cache after destroy' do
@@ -49,7 +51,7 @@
 
     context 'when ticket zoom' do
       let(:state) do
-        { ticket: {}, article: { form_id: 1337 } }
+        { ticket: {}, article: { form_id: form_id } }
       end
 
       it 'delete attachments in upload cache after destroy' do