List: Update LABKEY.List.create utility (#154)

labkey-nicka · web-flow · commit 928b5776060a · 2023-05-23T09:24:53.000-07:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,10 @@
+## 1.21.0 - 2023-05-23
+- Fixes the implementation of `List.create()` to support `keyName` and `keyType` as they were originally documented.
+- Deprecate `keyName` in favor of specifying `options.keyName` as specified on `Domain.create()`.
+- Deprecate `keyType` in favor of specifying `kind` as specified on `Domain.create()`.
+- Rename `ICreateOptions` to `ListCreateOptions`.
+- Update `ListCreateOptions` to extend `CreateDomainOptions` and only support an extension of a subset of `DomainDesign` properties in favor of using `domainDesign` directly.
+
 ## 1.20.0 - 2023-05-02
 - Add `renameContainer()` to `Security` API
 
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@labkey/api",
-  "version": "1.20.0",
+  "version": "1.21.0",
   "description": "JavaScript client API for LabKey Server",
   "scripts": {
     "build": "npm run build:dist && npm run build:docs",
diff --git a/src/labkey/List.spec.ts b/src/labkey/List.spec.ts
@@ -0,0 +1,171 @@
+/*
+ * Copyright (c) 2023 LabKey Corporation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+import * as Domain from './Domain';
+
+import { create } from './List';
+
+describe('List', () => {
+    describe('create', () => {
+        it('should require list name', () => {
+            expect(() => {
+                create({});
+            }).toThrowError('List name required');
+            expect(() => {
+                create({ name: undefined });
+            }).toThrowError('List name required');
+        });
+
+        it('should require kind or valid keyType', () => {
+            expect(() => {
+                create({ name: 'myList' });
+            }).toThrowError('List kind or keyType required');
+            expect(() => {
+                create({ name: 'myList', keyType: 'invalidKeyType' });
+            }).toThrowError('List kind or keyType required');
+        });
+
+        it('should require keyName or options.keyName', () => {
+            expect(() => {
+                create({ name: 'myList', kind: 'IntList' });
+            }).toThrowError('List keyName or options.keyName required');
+            expect(() => {
+                create({ name: 'myList', keyName: undefined, kind: 'IntList' });
+            }).toThrowError('List keyName or options.keyName required');
+        });
+
+        it('should support deprecated keyName', () => {
+            const createSpy = jest.spyOn(Domain, 'create').mockImplementation();
+
+            create({ name: 'myList', keyName: 'keyField', kind: 'IntList' });
+            expect(createSpy).toHaveBeenCalledWith(expect.objectContaining({ options: { keyName: 'keyField' } }));
+        });
+
+        it('should support deprecated keyType', () => {
+            const createSpy = jest.spyOn(Domain, 'create').mockImplementation();
+
+            // keyType == 'int'
+            create({ name: 'myList', keyName: 'keyField', keyType: 'int' });
+            expect(createSpy).toHaveBeenCalledWith(expect.objectContaining({ kind: 'IntList' }));
+
+            // keyType == 'IntList'
+            create({ name: 'myList', keyName: 'keyField', keyType: 'IntList' });
+            expect(createSpy).toHaveBeenCalledWith(expect.objectContaining({ kind: 'IntList' }));
+
+            // keyType == 'string'
+            create({ name: 'myList', keyName: 'keyField', keyType: 'string' });
+            expect(createSpy).toHaveBeenCalledWith(
+                expect.objectContaining({ kind: 'VarList', options: { keyName: 'keyField', keyType: 'Varchar' } })
+            );
+
+            // keyType == 'VarList'
+            create({ name: 'myList', keyName: 'keyField', keyType: 'VarList' });
+            expect(createSpy).toHaveBeenCalledWith(
+                expect.objectContaining({ kind: 'VarList', options: { keyName: 'keyField', keyType: 'Varchar' } })
+            );
+        });
+
+        it('should give domainDesign precedence', () => {
+            const createSpy = jest.spyOn(Domain, 'create').mockImplementation();
+
+            const name = 'myList';
+            const domainDesign = { name: 'myListDomain' };
+            const kind = 'IntList';
+            const options = { keyName: 'rowId' };
+
+            create({ domainDesign, kind, name, options });
+            expect(createSpy).toHaveBeenCalledWith({
+                domainDesign,
+                kind,
+                options,
+            });
+        });
+
+        it('should support old docs example', () => {
+            const createSpy = jest.spyOn(Domain, 'create').mockImplementation();
+
+            const description = 'my first list';
+            const fields = [
+                {
+                    name: 'one',
+                    rangeURI: 'int',
+                },
+                {
+                    name: 'two',
+                    rangeURI: 'multiLine',
+                    required: true,
+                },
+                {
+                    name: 'three',
+                    rangeURI: 'Attachment',
+                },
+            ];
+            const keyName = 'one';
+            const name = 'mylist';
+
+            create({ description, fields, keyName, keyType: 'int', name });
+            expect(createSpy).toHaveBeenCalledWith({
+                domainDesign: {
+                    description,
+                    fields,
+                    name,
+                },
+                kind: 'IntList',
+                options: {
+                    keyName,
+                },
+            });
+        });
+
+        it('should support auto-increment docs example', () => {
+            const createSpy = jest.spyOn(Domain, 'create').mockImplementation();
+
+            const description = 'teams in the league';
+            const fields = [
+                {
+                    name: 'rowId',
+                    rangeURI: 'int',
+                },
+                {
+                    name: 'name',
+                    rangeURI: 'string',
+                    required: true,
+                },
+                {
+                    name: 'slogan',
+                    rangeURI: 'multiLine',
+                },
+                {
+                    name: 'logo',
+                    rangeURI: 'Attachment',
+                },
+            ];
+            const kind = 'IntList';
+            const name = 'Teams';
+            const options = { keyName: 'rowId', keyType: 'AutoIncrementInteger' };
+
+            create({ description, fields, kind, name, options });
+            expect(createSpy).toHaveBeenCalledWith({
+                domainDesign: {
+                    description,
+                    fields,
+                    name,
+                },
+                kind,
+                options,
+            });
+        });
+    });
+});
diff --git a/src/labkey/List.ts b/src/labkey/List.ts
@@ -15,65 +15,119 @@
  */
 import { create as createDomain, CreateDomainOptions, DomainDesign } from './Domain';
 
-export interface ICreateOptions {
-    domainDesign: DomainDesign;
-    /** The name of the key column.*/
-    keyName: string;
-    /** The type of the key column.  Either "int" or "string". */
+// The configuration options intermix the DomainDesign configuration with the CreateDomainOptions.
+// This can be very confusing, so we've limited the scope of which domain design configuration options are supported.
+// Users can specify all available DomainDesign configuration options by specifying `domainDesign` explicitly.
+export type DomainDesignOptions = Pick<DomainDesign, 'description' | 'fields' | 'indices' | 'name'>;
+
+export interface ListCreateOptions extends DomainDesignOptions, CreateDomainOptions {
+    /**
+     * @deprecated Use `options.keyName` instead.
+     * The name of the key column. The `options.keyName` takes precedence if both are specified.
+     */
+    keyName?: string;
+    /**
+     * @deprecated Use `kind` instead.
+     * The type of the key column. Can be `IntList`, `VarList`, or `AutoIncrementInteger`.
+     * The `kind` takes precedence if both are specified.
+     * Note: If the `AutoIncrementInteger` configuration is desired and you're specifying `kind` then the
+     * `kind` should be set to `IntList` and the `options.keyType` should be set to `AutoIncrementInteger`.
+     */
     keyType?: string;
-    kind?: string;
-    options?: any;
 }
 
 /**
- * Create a new list.
- * A primary key column must be specified with the properties 'keyName' and 'keyType'. If the key is not
- * provided in the domain design's array of fields, it will be automatically added to the domain.
+ * Create a new List.
+ * Lists support three types of primary key configurations; `IntList`, `VarList`, or `AutoIncrementInteger`.
+ * These key configurations determine what "kind" of List is created.
+ * If the key field configuration is not provided in the domain design's array of fields,
+ * it will be automatically added to the domain.
+ * Note: If a `domainDesign` is specified then it's configuration will take precedence over other `DomainDesign`
+ * properties that are specified.
  *
  * ```
+ *  // Creates a List with a string primary key.
+ *  LABKEY.List.create({
+ *      fields: [{
+ *          name: 'partCode', rangeURI: 'string',
+ *      },{
+ *          name: 'manufacturer', rangeURI: 'string',
+ *      },{
+ *          name: 'purchaseDate', rangeURI: 'date',
+ *      }],
+ *      kind: 'VarList',
+ *      name: 'Parts',
+ *      options: {
+ *          keyName: 'partCode',
+ *          keyType: 'Varchar',
+ *      }
+ *  });
+ *
+ *  // Creates a List with an auto-incrementing primary key.
  *  LABKEY.List.create({
- *      name: "mylist",
- *      keyType: "int",
- *      keyName: "one",
- *      description: "my first list",
+ *      description: 'teams in the league',
+ *      kind: 'IntList',
  *      fields: [{
- *          name: "one", rangeURI: "int"
- *          },{
- *          name: "two", rangeURI: "multiLine", required: true
- *          },{
- *          name: "three", rangeURI: "Attachment"
- *      }]
+ *          name: 'rowId', rangeURI: 'int',
+ *      },{
+ *          name: 'name', rangeURI: 'string', required: true,
+ *      },{
+ *          name: 'slogan', rangeURI: 'multiLine',
+ *      },{
+ *          name: 'logo', rangeURI: 'Attachment',
+ *      }],
+ *      name: 'Teams',
+ *      options: {
+ *          keyName: 'rowId',
+ *          keyType: 'AutoIncrementInteger',
+ *      }
  *  });
  * ```
  */
-export function create(config: ICreateOptions) {
+export function create(config: ListCreateOptions) {
+    // Separate out `ListCreateOptions` configuration
+    const { keyName, keyType, ...options } = config;
+
+    // Separate out `DomainDesignOptions` configuration
+    const { description, fields, indices, name, ...createDomainOptions } = options;
+
     const domainOptions: CreateDomainOptions = {
-        // not really awesome...intermixing interface with domain design. Separate these concerns.
-        domainDesign: config as Partial<DomainDesign>,
-        options: {},
+        ...createDomainOptions,
+        options: createDomainOptions.options ?? {},
     };
 
+    // If a `domainDesign` is not explicitly provided then fallback to `DomainDesignOptions` options
+    if (!domainOptions.domainDesign) {
+        domainOptions.domainDesign = { description, fields, indices, name };
+    }
+
     if (!domainOptions.domainDesign.name) {
         throw new Error('List name required');
     }
 
-    if (!config.kind) {
-        if (config.keyType == 'int') {
-            config.kind = 'IntList';
-        } else if (config.keyType == 'string') {
-            config.kind = 'VarList';
+    // Handle `keyType` only if `kind` is not specified
+    if (!domainOptions.kind) {
+        if (keyType === 'int' || keyType === 'IntList') {
+            domainOptions.kind = 'IntList';
+        } else if (keyType === 'string' || keyType === 'VarList') {
+            domainOptions.kind = 'VarList';
+            domainOptions.options.keyType = 'Varchar';
+        } else if (keyType === 'AutoIncrementInteger') {
+            domainOptions.kind = 'IntList';
+            domainOptions.options.keyType = 'AutoIncrementInteger';
+        } else {
+            throw new Error('List kind or keyType required');
         }
     }
 
-    if (config.kind != 'IntList' && config.kind != 'VarList') {
-        throw new Error('Domain kind or keyType required');
-    }
-    domainOptions.kind = config.kind;
-
-    if (!config.keyName) {
-        throw new Error('List keyName required');
+    // Handle `keyName` only if `options.keyName` is not specified
+    if (!domainOptions.options.keyName) {
+        if (keyName) {
+            domainOptions.options.keyName = keyName;
+        } else {
+            throw new Error('List keyName or options.keyName required');
+        }
     }
-    domainOptions.options.keyName = config.keyName;
 
     createDomain(domainOptions);
 }

Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"name": "@labkey/api",`
`3`		`- "version": "1.20.0",`
	`3`	`+ "version": "1.21.0",`
`4`	`4`	`"description": "JavaScript client API for LabKey Server",`
`5`	`5`	`"scripts": {`
`6`	`6`	`"build": "npm run build:dist && npm run build:docs",`