Browse Source

Maintenance: Improve form id schema.

Co-authored-by: Martin Gruner <mg@zammad.com>
Rolf Schmidt 1 year ago
parent
commit
4f56011498

+ 4 - 2
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

+ 1 - 6
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: () => {

+ 3 - 5
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':

+ 2 - 3
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

+ 1 - 1
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,

+ 1 - 1
app/models/store.rb

@@ -59,7 +59,7 @@ returns
   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

+ 1 - 1
db/migrate/20120101000001_create_base.rb

@@ -481,7 +481,7 @@ class CreateBase < ActiveRecord::Migration[4.2]
     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

+ 11 - 0
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

+ 1 - 2
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.

+ 2 - 2
spec/db/migrate/template_migration_spec.rb

@@ -7,7 +7,7 @@ RSpec.describe TemplateMigration, type: :db_migration do
     {
       '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 @@ RSpec.describe TemplateMigration, type: :db_migration do
                      {
                        '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>',

Some files were not shown because too many files changed in this diff