pr comments addressed. adding validation for different properties fro…

…m annotations to make sure only valid values from each one are set

docs/user-docs/annotation.md

@@ -469,7 +469,7 @@ Supported display _option_ syntax:
 - `"show_foreign_key_link": true`: Override the inherited behavior of foreign key display and add a link to the referred row.
 - `"show_foreign_key_link": false`: Override the inherited behavior of foreign key display by not adding any the extra.
 - `"selector_ux_mode"`: The display mode for the recordedit input field when this foreign key relationship is part of the visible columns. Supported values are `"facet-search-popup"` and `"simple-search-dropdown"`, with `"facet-search-popup"` being the default. Currently only supported in `entry` contexts.
-- `"bulk_create_foreign_key"`:  Use this property to control the bulk selection of foreign key values in `entry/create` context when there is a prefill query parameter. Supported values are a foreign key `name` in the format of `['schema_name', 'foreign_key_name']` from the schema document, `false`, and `null`. Using a foreign key name will use that foreign key as the one being bulk selected if that foreign key is in the visible columns list.  `false` turns off the heuristics that trigger this feature. `null` will override inheritance for this property and use the default heuristics. This will override the `bulk_create_foreign_key_candidates` property defined in the table-display annotation. Currently only supported in `entry/create` context.
+- `"bulk_create_foreign_key"`:  Use this property to control the bulk selection of foreign key values in `entry/create` context when there is a prefill query parameter. Supported values are a foreign key `name` in the format of `['schema_name', 'foreign_key_name']` from the schema document, `false`, or `null`. Using a foreign key name will use that foreign key as the one being bulk selected if that foreign key is in the visible columns list.  `false` turns off the heuristics that trigger this feature. `null` will override inheritance for this property and use the default heuristics. This will override the `bulk_create_foreign_key_candidates` property defined in the table-display annotation. Currently only supported in `entry/create` context.
 
 Supported _columnorder_key_ syntax:
 
@@ -1167,7 +1167,7 @@ List of _context_ names that are used in ERMrest:
       - `"compact/select/association/link"`: A sub-context of `compact/select/association` used for selecting entities to link to the main record.
       - `"compact/select/association/unlink"`: A sub-context of `compact/select/association` used for selecting entities to unlink from the main record.
     - `"compact/select/foreign_key"`: A sub-context of `compact/select` used for selecting entities for a foreign key value in an `entry` context.
-    - `"compactSelectBulkForeignKey"`: a sub-context of `compact/select` used for selecting multiple entities to fill in foreign key values in multiple forms in `entry/create` context.
+    - `"compact/select/foreign_key/bulk"`: a sub-context of `compact/select` used for selecting multiple entities to fill in foreign key values in multiple forms in `entry/create` context.
     - `"compact/select/saved_queries"`: A sub-context of `compact/select` used for selecting a saved query to apply in `compact` context.
     - `"compact/select/show_more"`: A sub-context of `compact/select` used for selecting entities as a facet value.
 - `"detailed"`: Any detailed read-only, entity-level presentation context.
@@ -1345,7 +1345,7 @@ The following attributes can be used to manipulate the presentation settings of
       - `csv` for comma-seperated values.
       - `raw` for space-seperated values.
   - `"selector_ux_mode"`: The display mode for the recordedit input field when this column directive is a foreign key relationship. Supported values are `"facet-search-popup"` and `"simple-search-dropdown"`, with `"facet-search-popup"` being the default. Currently only supported in `entry` contexts.
-  - `"bulk_create_foreign_key"`: Use this property to control the bulk selection of foreign key values in `entry/create` context when there is a prefill query parameter. Supported values are a foreign key `name` in the format of `['schema_name', 'foreign_key_name']` from the schema document, `false`, and `null`. Using a foreign key name will use that foreign key as the one being bulk selected if that foreign key is in the visible columns list. `false` turns off the heuristics that trigger this feature. `null` will override inheritance for this property and use the default heuristics. This will override the `bulk_create_foreign_key` property defined in the display property of the foreign-key annotation. Currently only supported in `entry/create` context.
+  - `"bulk_create_foreign_key"`: Use this property to control the bulk selection of foreign key values in `entry/create` context when there is a prefill query parameter. Supported values are a foreign key `name` in the format of `['schema_name', 'foreign_key_name']` from the schema document, `false`, or `null`. Using a foreign key name will use that foreign key as the one being bulk selected if that foreign key is in the visible columns list. `false` turns off the heuristics that trigger this feature. `null` will override inheritance for this property and use the default heuristics. This will override the `bulk_create_foreign_key` property defined in the display property of the foreign-key annotation. Currently only supported in `entry/create` context.
 - `array_display`: This property is _deprecated_. It is the same as `array_ux_mode` that is defined above under `display` property.
 - `array_options`: Applicaple only to read-only non-filter context of `visible-columns` annotation. This property is meant to be an object of properties that control the display of `array` or `array_d` aggregate column. These options will only affect the display (and templating environment) and have no effect on the generated ERMrest query. The available options are:
     - `order`: An alternative sort method to apply when a client wants to semantically sort by key values. It follows the same syntax as `column_order`. In scalar array aggregate, you cannot sort based on other columns values, you can only sort based on the scalar value of the column.

docs/user-docs/column-directive.md

@@ -31,6 +31,8 @@ Column directive allows instruction of a data source and modification of its pre
       - [markdown\_pattern](#markdown_pattern)
       - [wait\_for](#wait_for)
       - [show\_foreign\_key\_link](#show_foreign_key_link)
+      - [selector\_ux\_mode](#selector_ux_mode)
+      - [bulk\_create\_foreign\_key](#bulk_create_foreign_key)
       - [show\_key\_link](#show_key_link)
       - [array\_ux\_mode](#array_ux_mode)
     - [array\_options](#array_options)
@@ -425,6 +427,10 @@ While generating a default presentation for all outbound foreign key paths, ERMr
 
 While generating a default presentation in `entry` mode for single outbound foreign key paths, Chaise will show a modal popup dialog for selecting rows. Using this attribute, you can modify this behavior. If this attribute is missing, we are going to use the inherited behavior from the [foreign key](annotation.md#tag-2016-foreign-key) annotation defined on the foreign key relationship. If that one is missing too, [table display](annotation.md#tag-2016-table-display) annotation will be applied. Supported values are `"facet-search-popup"` and `"simple-search-dropdown"`, with `"facet-search-popup"` being the default.
 
+##### bulk_create_foreign_key
+
+Use this property to control the bulk selection of foreign key values in `entry/create` context when there is a prefill query parameter. Supported values are a foreign key `name` in the format of `['schema_name', 'foreign_key_name']` from the schema document, `false`, or `null`. Using a foreign key name will use that foreign key as the one being bulk selected if that foreign key is in the visible columns list. `false` turns off the heuristics that trigger this feature. `null` will override inheritance for this property and use the default heuristics. This will override the `bulk_create_foreign_key` property defined in the display property of the foreign-key annotation. Currently only supported in `entry/create` context.
+
 ##### show_key_link
 
 While generating a default presentation for key column directives (self link), ERMrestJS will add a link to the referred row. Using this attribute you can modify this behavior. If this attribute is missing, we are going to use the inherited behavior from the [key display](annotation.md#tag-2017-key-display) annotation. If that one is missing too, [display annotation](annotation.md#tag-2015-display) will be applied.

js/column.js

@@ -2166,8 +2166,8 @@ Object.defineProperty(ForeignKeyPseudoColumn.prototype, "display", {
                 }
 
                 // check visible-columns annotation display property and set to override property from fk annotation or display annotation
-                // make sure `false` is not ignored since that turns this feature off
-                if (displ.bulk_create_foreign_key !== null) {
+                // make sure `false` and `null` are not ignored since that turns this feature off
+                if (_isValidBulkCreateForeignKey(displ.bulk_create_foreign_key)) {
                     sourceDisplay.bulkForeignKeyCreateConstraintName = displ.bulk_create_foreign_key;
                 }
             }

js/core.js

@@ -4804,14 +4804,15 @@
                 if (this.table.annotations.contains(module._annotations.TABLE_DISPLAY)) {
                     var tableAnnotation = module._getRecursiveAnnotationValue(context, this.table.annotations.get(module._annotations.TABLE_DISPLAY).content);
 
-                    if (tableAnnotation.bulk_create_foreign_key_candidates !== null) {
+                    // check the value is an array since only `[['schema_name', 'foreignkey_name'], ... ]` is allowed for this property
+                    // each individual array element is validated when reading this annotation value
+                    if (Array.isArray(tableAnnotation.bulk_create_foreign_key_candidates)) {
                         bulkCreateConstraintName = tableAnnotation.bulk_create_foreign_key_candidates;
                     }
                 }
 
                 // check foreign key annotation
-                // make sure `false` is not ignored since that turns this feature off
-                if (displayAnnot.bulk_create_foreign_key !== undefined && displayAnnot.bulk_create_foreign_key !== null) {
+                if (_isValidBulkCreateForeignKey(displayAnnot.bulk_create_foreign_key)) {
                     bulkCreateConstraintName = displayAnnot.bulk_create_foreign_key;
                 }
 

js/reference.js

@@ -4140,14 +4140,16 @@
                         for (i = 0; i < self.columns.length; i++) {
                             var column = self.columns[i];
 
-                            // column should be a foreignkey pseudo column
-                            if (!column.isForeignKey) continue;
+                            // column should be a simple foreignkey pseudo column
+                            // return if it's not a foreign key or the column is a foreign key but it's not simple
+                            if (!column.isForeignKey || !column.foreignKey.simple) continue;
 
                             // if constraintNameProp is string[][], it's from bulk_create_foreign_key_candidates
                             // we need to iterate over the set to find the first matching column
                             if (Array.isArray(constraintNameProp) && Array.isArray(constraintNameProp[0])) {
                                 for (j = 0; j < constraintNameProp.length; j++) {
-                                    leafCol = findLeafColumn(column, constraintNameProp[j].join("_"));
+                                    var name = constraintNameProp[j];
+                                    if (_isValidForeignKeyName(name)) leafCol = findLeafColumn(column, name.join("_"));
 
                                     if (leafCol) break;
                                 }

js/utils/helpers.js

@@ -895,6 +895,28 @@
         return typeof display === "string" && module._commentDisplayModes[display] !== -1;
     };
 
+    /**
+     * Given a foreign key name, will return true or false depending if the name value is of a valid type and value
+     *    - if =['', ''] : returns true
+     *    - otherwise returns false
+     *
+     * @private
+     */
+    _isValidForeignKeyName = function (fkName) {
+        return Array.isArray(fkName) && fkName.length === 2 && typeof fkName[0] === 'string' && typeof fkName[1] === 'string';
+    }
+
+    /**
+     * Given input value for bulk_create_foreign_key, will return true or false depending if the value is of a valid type and value for the bulk_create_foreign_key
+     *   - if =false | =null | =['', ''] : returns true
+     *   - otherwise returns false
+     *
+     * @private
+     */
+    _isValidBulkCreateForeignKey = function (bulkCreateProp) {
+        return bulkCreateProp === false || bulkCreateProp === null || _isValidForeignKeyName(bulkCreateProp);
+    }
+
     /**
      * @function
      * @param {ERMrest.Table} table The object that we want the formatted values for.

test/specs/reference/conf/bulk_create_foreign_keys/schema.json

@@ -1588,6 +1588,135 @@
         }
       }
     },
+    "association_table_w_false_fk_annotation_null_viz_columns": {
+      "kind": "table",
+      "schema_name": "bulk_create_foreign_keys",
+      "table_name": "association_table_w_false_fk_annotation_null_viz_columns",
+      "column_definitions": [
+        {
+          "name": "static_col1",
+          "type": {
+            "typename": "int4"
+          }
+        },
+        {
+          "name": "static_col2",
+          "type": {
+            "typename": "text"
+          }
+        },
+        {
+          "name": "main_fk_col",
+          "nullok": false,
+          "type": {
+            "typename": "text"
+          }
+        },
+        {
+          "name": "leaf_fk_col",
+          "nullok": false,
+          "type": {
+            "typename": "text"
+          }
+        }
+      ],
+      "keys": [
+        {
+          "unique_columns": [
+            "RID"
+          ]
+        },
+        {
+          "unique_columns": [
+            "main_fk_col",
+            "leaf_fk_col"
+          ]
+        }
+      ],
+      "foreign_keys": [
+        {
+          "foreign_key_columns": [
+            {
+              "schema_name": "bulk_create_foreign_keys",
+              "table_name": "association_table_w_false_fk_annotation_null_viz_columns",
+              "column_name": "leaf_fk_col"
+            }
+          ],
+          "referenced_columns": [
+            {
+              "schema_name": "bulk_create_foreign_keys",
+              "table_name": "leaf_table_for_false_fk_annotation_null_viz_columns",
+              "column_name": "RID"
+            }
+          ],
+          "names": [
+            [
+              "bulk_create_foreign_keys",
+              "false_fk_annotation_null_viz_columns_to_leaf_fkey"
+            ]
+          ]
+        },
+        {
+          "foreign_key_columns": [
+            {
+              "schema_name": "bulk_create_foreign_keys",
+              "table_name": "association_table_w_false_fk_annotation_null_viz_columns",
+              "column_name": "main_fk_col"
+            }
+          ],
+          "referenced_columns": [
+            {
+              "schema_name": "bulk_create_foreign_keys",
+              "table_name": "main_table_for_annotations",
+              "column_name": "id"
+            }
+          ],
+          "names": [
+            [
+              "bulk_create_foreign_keys",
+              "false_fk_annotation_null_viz_columns_to_main_fkey"
+            ]
+          ],
+          "annotations": {
+            "tag:isrd.isi.edu,2016:foreign-key": {
+              "display": {
+                "entry/create": {
+                  "bulk_create_foreign_key": false
+                }
+              }
+            }
+          }
+        }
+      ],
+      "annotations": {
+        "tag:isrd.isi.edu,2016:visible-columns": {
+          "entry": [
+            "static_col1",
+            "static_col2",
+            {
+              "source": [
+                {
+                  "outbound": [
+                    "bulk_create_foreign_keys",
+                    "false_fk_annotation_null_viz_columns_to_main_fkey"
+                  ]
+                },
+                "RID"
+              ],
+              "display": {
+                "bulk_create_foreign_key": null
+              }
+            },
+            [
+              "bulk_create_foreign_keys",
+              "false_fk_annotation_null_viz_columns_to_leaf_fkey"
+            ]
+          ],
+          "compact": "entry",
+          "detailed": "entry"
+        }
+      }
+    },
     "leaf_table_for_viz_columns": {
       "comment": "leaf table for testing an association with visible columns display property",
       "kind": "table",
@@ -1794,6 +1923,29 @@
         }
       ],
       "annotations": {}
+    },
+    "leaf_table_for_false_fk_annotation_null_viz_columns": {
+      "comment": "leaf table for testing an association with null in visible columns source definition that overrides false in display property of foreign key annotation",
+      "kind": "table",
+      "table_name": "leaf_table_for_false_fk_annotation_null_viz_columns",
+      "schema_name": "bulk_create_foreign_keys",
+      "keys": [
+        {
+          "unique_columns": [
+            "RID"
+          ]
+        }
+      ],
+      "foreign_keys": [],
+      "column_definitions": [
+        {
+          "name": "details",
+          "type": {
+            "typename": "text"
+          }
+        }
+      ],
+      "annotations": {}
     }
   },
   "table_names": [
@@ -1817,6 +1969,7 @@
     "association_table_w_display_annotation",
     "association_table_w_false_fk_annotation",
     "association_table_w_false_viz_columns",
+    "association_table_w_false_fk_annotation_null_viz_columns",
     "leaf_table_for_viz_columns",
     "third_table_for_viz_columns",
     "leaf_table_for_fk_annotation",
@@ -1825,6 +1978,7 @@
     "third_table_for_table_display_annotation",
     "leaf_table_for_display_annotation",
     "leaf_table_for_false_fk_annotation",
-    "leaf_table_for_false_viz_columns"
+    "leaf_table_for_false_viz_columns",
+    "leaf_table_for_false_fk_annotation_null_viz_columns"
   ]
 }