Facet within facet (#1017)

* support max_facet_depth display annot + chaise-config maxFacetDepth

* fix the hideNullChoice API to properly check for null on any levels

* fix parser issues related to multi-layered facets:

  - fix typos in ermrestCompactPath and _createParsedJoinFromStr
  - make sure Location constructor uses the correct join. the function
    was assuming that the first portion of the url always has a facet,
    but that might not be true.
  - fix an issue in the addJoin regarding multi-level pathParts with joins.
    The previous code was only working with one level of facets and joins,
    and was breaking when we changed chaise to allow facets within facets.

docs/user-docs/annotation.md

@@ -100,6 +100,7 @@ Supported JSON payload patterns:
 - `{`... `"show_foreign_key_link":` `{` _context_ `:` _fklink_ `,` ... `}`: Whether default display of foreign keys should include link to the row.
 - `{`... `"hide_row_count":` `{` _context_ `:` _rowcount_ `,` ... `}`: Whether we should display the total row count. Since the request to fetch total row count is expensive, you can use this to signal to client to skip the request (and therefore do not display it to users.)
 - `{`... `"show_saved_query":` _savedquery_ ...`}`: Whether we want to display the saved query UI features or not. By default, this feature is turned off (set to false).
+- `{`... `"max_facet_depth":` _maxfacetdepth_ ...`}`: How many levels of facet popups we should allow.
 
 Supported JSON _ccomment_ patterns:
 
@@ -157,6 +158,13 @@ Supported JSON _savedquery_ patterns:
 - `true`: Display the saved query UI features.
 - `false`: Don't display the saved query UI features.
 
+Supported JSON _maxfacetdepth_ patterns:
+
+- `0`: Disable the faceting feature.
+- `1`: Facet panel is only displayed for one level. The facet panel is not displayed as part of the facet popups.
+- `2`: Facet panel is displayed for two levels. Both once the main table is opened and when the facet popup is opened for any of the facets.
+- Any number bigger than 2 will be treated the same as defining `2`.
+
 Supported JSON _context_ patterns:
 - See [Context Names](#context-names) section for the list of supported JSON _context_ patterns.
 

js/column.js

@@ -3467,7 +3467,7 @@ FacetColumn.prototype = {
                 }
 
                 // G3
-                var othersHaveNull = self.reference.facetColumns.some(function (fc, index) {
+                var othersHaveNull = self.reference.location.isUsingRightJoin || self.reference.facetColumns.some(function (fc, index) {
                     return index !== self.index && fc.hasNullFilter && fc.hasPath;
                 });
 

js/parser.js

@@ -268,14 +268,13 @@
         }
 
         // pathParts: <joins/facet/cfacet/filter/>
-        var aliasJoinRegExp = /\$.*/,
-            joinRegExp = /(?:left|right|full|^)\((.*)\)=\((.*:.*:.*)\)/,
+        var joinRegExp = /(?:left|right|full|^)\((.*)\)=\((.*:.*:.*)\)/,
             facetsRegExp = /\*::facets::(.+)/,
             customFacetsRegExp = /\*::cfacets::(.+)/;
 
-        var schemaTable = parts[0], table = this._rootTableName, schema = this._rootSchemaName;
-        var self = this, pathParts = [], alias, match, prevJoin = false;
-        var facets, cfacets, filter, filtersString, searchTerm, join, joins = [];
+        var table = this._rootTableName, schema = this._rootSchemaName;
+        var self = this, pathParts = [], alias, match, prevJoin = false, startWithT1 = false, aliasNumber;
+        var facets, cfacets, filter, filtersString, join, joins = [];
 
         // go through each of the parts
         parts.forEach(function (part, index) {
@@ -291,9 +290,23 @@
                 // there wasn't any join before, so this is the start of new path,
                 // so we should create an object for the previous one.
                 if (!prevJoin && index !== 1) {
-                    // we're creating this for the previous section, so we should use the previous index
-                    // Alias will be T, T1, T2, ... (notice we don't have T0)
-                    alias = module._parserAliases.JOIN_TABLE_PREFIX + (pathParts.length > 0 ? pathParts.length : "");
+                    /**
+                     * we're creating this alias for the previous section, so we should use the previous index
+                     * Alias will be T, T1, T2, ... (notice we don't have T0)
+                     */
+                    aliasNumber = pathParts.length;
+
+                    /**
+                     * T is always preserved for the initial schema:table. if the first pathPart doesn't have any joins
+                     * and has facet/filters, then it's for the schema:table and so we should start with T.
+                     * but if has join, then it means that the schema:table didn't have any filter/facets. so we have to start from T1,
+                     * and the rest of the parts should just add one more
+                     */
+                    if (pathParts.length === 0 && joins.length > 0) {
+                        startWithT1 = true;
+                    }
+                    if (startWithT1) aliasNumber++;
+                    alias = module._parserAliases.JOIN_TABLE_PREFIX + (aliasNumber > 0 ? aliasNumber : "");
                     pathParts.push(new PathPart(alias, joins, schema, table, facets, cfacets, filter, filtersString));
                     filter = undefined; filtersString = undefined; cfacets = undefined; facets = undefined; join = undefined; joins = [];
                 }
@@ -485,6 +498,7 @@
          * @returns {Object} an object wit the following properties:
          *   - `path`: Path without modifiers or queries for ermrest
          *   - `pathPrefixAliasMapping`: alias mapping that are used in the url
+         *   - `isUsingRightJoin`: whether we've used right outer join for this path.
          */
         computeERMrestCompactPath: function (usedSourceObjects) {
             var self = this;
@@ -671,7 +685,7 @@
                 };
             });
 
-            // no rightJoin: s:t/<parths>
+            // no rightJoin: s:t/<parts>
             if (rightJoinIndex === -1) {
                 uri = self.rootTableAlias + ":=";
                 if (self.rootSchemaName) {
@@ -692,14 +706,14 @@
                     if (self.pathParts[i].joins.length > 0) {
                         // since we're reversing, we have to make sure we're using the
                         // alias of the previous pathpart
-                        alias = i > 0 ? self.pathPart[i-1].alias : self.rootTableAlias;
+                        alias = i > 0 ? self.pathParts[i-1].alias : self.rootTableAlias;
                         uri += "/" + joinsStr(self.pathParts[i], alias, i, true);
                     }
                 }
 
                 // if there was pathParts before facet with null, change back to the facet with null
                 if (self.pathParts[rightJoinIndex].joins.length > 0) {
-                    uri += "/$" + self.pathParts[i].alias;
+                    uri += "/$" + self.pathParts[rightJoinIndex].alias;
                 }
             }
 
@@ -723,7 +737,8 @@
             return {
                 path: uri,
                 // TODO could be replaced with the function
-                pathPrefixAliasMapping: lastPathPartAliasMapping
+                pathPrefixAliasMapping: lastPathPartAliasMapping,
+                isUsingRightJoin: rightJoinIndex !== -1,
             };
         },
 
@@ -732,7 +747,7 @@
                 var res = this.computeERMrestCompactPath();
                 this._ermrestCompactPath = res.path;
                 this._pathPrefixAliasMapping = res.pathPrefixAliasMapping;
-
+                this._isUsingRightJoin = res.isUsingRightJoin;
             }
             return this._ermrestCompactPath;
         },
@@ -757,6 +772,18 @@
             return this._pathPrefixAliasMapping;
         },
 
+        /**
+         * whether we're using right outer join for parsing the location
+         * @type {boolean}
+         */
+        get isUsingRightJoin() {
+            if (this._isUsingRightJoin === undefined) {
+                // this API will populate this
+                var dummy = this.ermrestCompactPath;
+            }
+            return this._isUsingRightJoin;
+        },
+
         /**
          * Array of path parts
          * @type {PathPart[]}
@@ -1381,8 +1408,31 @@
             var loc = clone ? this._clone() : this;
             // change alias of the previous part
             var lastPart = loc.lastPathPart;
+
+            /**
+             * since we're adding a path part, we have to change the alias of previous part.
+             * this alias is for the projected table. if there are joins, it will be added to the last
+             * join and if there aren't, it will be attached to the schema:table.
+             *
+             * when there aren't any joins, this could be the schema:table, so we have to start with T.
+             * but if there are joins, we can start with T1, as T alias is always the first schema:table.
+             *
+             * this is how we're using the alias property:
+             * - if it doesn't have any joins, it's the alias of the previous path or root (so facet/)
+             * - if it has join, it's the alias that we're using to name the projected table that the join represents.
+             */
             if (lastPart) {
-                var alias = module._parserAliases.JOIN_TABLE_PREFIX + (loc.pathParts.length > 1 ? (loc.pathParts.length-1) : "");
+                var aliasNumber = "";
+                if (lastPart.joins && lastPart.joins.length > 0) {
+                    if (loc.pathParts.length > 0) {
+                        aliasNumber = loc.pathParts.length;
+                    }
+                }
+                else if (loc.pathParts.length > 1) {
+                    aliasNumber = loc.pathParts.length - 1;
+                }
+
+                var alias = module._parserAliases.JOIN_TABLE_PREFIX + aliasNumber;
                 lastPart.alias = alias;
             }
 
@@ -1416,6 +1466,8 @@
      * Container for the path part, It will have the following attributes:
      * - joins: an array of ParsedJoin objects.
      * - alias: The alias that the facets should refer to
+     *   - if there aren't any joins, it's the alias of the previous path or root.
+     *   - otherwise, it's the alias that we're using to name the projected table that the join represents.
      * - schema: The schema that the join ends up with
      * - table: the table that the joins end up with
      * - facets: the facets object
@@ -1684,7 +1736,7 @@
      * @returns {ParsedJoin}
      */
     function _createParsedJoinFromStr (linking, table, schema) {
-        var fromSchemaTable = schema ? [table,schema].join(":") : table;
+        var fromSchemaTable = schema ? [schema,table].join(":") : table;
         var fromCols = linking[1].split(",");
         var toParts = linking[2].match(/([^:]*):([^:]*):([^\)]*)/);
         var toCols = toParts[3].split(",");

js/reference.js

@@ -2303,11 +2303,14 @@
 
         /**
          * An object which contains row display properties for this reference.
-         * It is determined based on the `table-display` annotation. It has the
+         * It is determined based on the `table-display`, `display`, and 'chaise-config' annotations. It has the
          * following properties:
          *
          *   - `rowOrder`: `[{ column: '`_column object_`', descending:` {`true` | `false` } `}`...`]` or `undefined`,
          *   - `type`: {`'table'` | `'markdown'` | `'module'`} (default: `'table'`)
+         *   - `showFaceting`: A boolean indicating whether we should show the faceting feature or not.
+         *   - `maxFacetDepth`: A number indicating the facet depth.
+         *   - `facetPanelOpen`: Whether the facet panel should be opened by default or not.
          *
          * If type is `'markdown'`, the object will also these additional
          * properties:
@@ -2445,7 +2448,7 @@
                 }
 
                 // if facetpanel won't be used or the _clientConfig.facetPanelDisplay doesn't include the current context or a parent context, set to null
-                var fpo = null;
+                var fpo = null, maxFacetDepth = null;
                 // NOTE: clientConfig should always be defined if used by a client, some ermrestJS tests don't always set it, so don't calculate this for those tests
                 if (this._context && module._clientConfig) {
                     // _clientConfig.facetPanelDisplay will be defined from configuration or based on chaise defaults
@@ -2454,6 +2457,20 @@
 
                     // facet panel is not available in COMPACT_BRIEF and it's subcontexts, and other non-compact contexts
                     if (context.startsWith(module._contexts.COMPACT) && !context.startsWith(module._contexts.COMPACT_BRIEF)) {
+
+                        // get it from the display annotation
+                        maxFacetDepth = module._getHierarchicalDisplayAnnotationValue(this.table, context, "max_facet_depth", true);
+                        // missing from the annotation, so get it from the chaise-config
+                        if (maxFacetDepth === -1 && typeof ccFacetDisplay.maxFacetDepth === 'number') {
+                            maxFacetDepth = ccFacetDisplay.maxFacetDepth;
+                        }
+                        // validate the value
+                        if (!isInteger(maxFacetDepth) || maxFacetDepth < 0) {
+                            maxFacetDepth = 1
+                        } else if (maxFacetDepth > 2) {
+                            maxFacetDepth = 2;
+                        }
+
                         if (ccFacetDisplay.closed && ccFacetDisplay.closed.includes("*")) fpo = false;
                         if (ccFacetDisplay.open && ccFacetDisplay.open.includes("*")) fpo = true;
                         // check inheritence
@@ -2472,6 +2489,12 @@
                         }
                     }
                 }
+                // for other contexts that don't allow facet
+                maxFacetDepth = maxFacetDepth === null ? 0 : maxFacetDepth;
+
+                this._display.maxFacetDepth = maxFacetDepth;
+                this._display.showFaceting = maxFacetDepth > 0;
+
                 this._display.facetPanelOpen = fpo;
 
                 /**

js/utils/helpers.js

@@ -631,6 +631,8 @@
     };
 
     /**
+     * retun the value that should be used for the display setting. If missing, it will return "-1".
+     *
     * @param {ERMrest.Table|ERMrest.Column|ERMrest.ForeignKeyRef} obj either table object, or an object that has `.table`
     * @param {String} context the context string
     * @param {String} annotKey the annotation key that you want the annotation value for

test/specs/annotation/conf/table_display/schema.json

@@ -671,6 +671,37 @@
                 }
             }
         }
+    },
+    "table_w_max_facet_depth": {
+      "table_name": "table_w_max_facet_depth",
+        "schema_name": "schema_table_display",
+        "kind": "table",
+        "keys": [
+            {"unique_columns": ["id"]}
+        ],
+        "foreign_keys": [],
+        "column_definitions": [
+            {
+                "name": "id",
+                "nullok": false,
+                "type": {
+                    "typename": "integer"
+                }
+            },
+            {
+                "name": "text_col",
+                "type": {
+                    "typename": "text"
+                }
+            }
+        ],
+        "annotations": {
+            "tag:misd.isi.edu,2015:display": {
+              "max_facet_depth": {
+                  "compact/select/association/link": 0
+              }
+            }
+        }
     }
   },
   "schema_name": "schema_table_display",
@@ -679,6 +710,9 @@
           "hide_row_count": {
               "compact": true,
               "compact/select": false
+          },
+          "max_facet_depth": {
+            "compact/select/association/unlink": 1
           }
       }
   }