Opened 8 years ago

Closed 7 years ago

#13380 closed task (fixed)

ComboBoxMixin.query documentation is wrong

Reported by: jameyg Owned by: haysmark
Priority: high Milestone: 1.8
Component: Documentation Version: 1.7.0b1
Keywords: Cc:
Blocked By: Blocking:

Description

The docs for ComboBoxMixin? suggest that the query parameter represents a "query...passed to 'store' to initially filter the items, before doing further filtering based on searchAttr and the key". This differs from the current behavior that simply mixes the 'searchAttr' into query (which, by convention, ANDs the criteria together in a single query). The impact of the current documentation is that it implies there is such a concept as a "pre-filter" and perhaps more importantly that the pre-filter query can be any form of filter supported by the underlying store (when in fact the query must be the object {attr:valu*} form).

The StoreAPI's enhancements to query (vs. IFRS) increase the need to distinguish.

Since pre-filter implies some sort of "subquery" support in the store api (i.e. a query of a resultset) and since such support seems unlikely, I'll qualify this as a doc-bug (athough actual support of the described behavior...especially for FilteringSelect?...would be nice).

Change History (7)

comment:1 Changed 8 years ago by Douglas Hays

Do you have a suggestion for the replacement text?

comment:2 Changed 8 years ago by jameyg

I asked a question on the mailing list that didn't generate much discussion re: query and an (undocumented?) constraint on the expected form, but before I suggest alternate text I guess I need to ask another question...was the 1.7 change to searchAttr from a string to a regExp (w/ a toString()) considered a possible point of regression? Or re: documentation, what are the expectations/constaints of/on the backing store re: this query and is it as strong as "must confirm to the capabilities documented in SimpleQueryEngine?"?

I think the answer ends up affecting how this is written. If CBMixin limits the types of stores that are supported (which is feels like it does), then those docs probably belong on the "store" prop (i.e. "the store must be able to accept an object hash of properties for its query; the searchAttr property will be passed in the object hash as a regExp w/ a toString() method that will return the resolved value of queryExpr"). The query doc would then be something like "an object hash of properties that, along with the control managed searchAttr, is passed into the backing store".

Combined this seems to hit on the main issues I had using CB and trying to wire CBs up to some custom Stores (which ended up actually being wiring up custom stores to the CB)

  1. the query MUST be an object hash; it specifically can't be a Function (a "cool new" feature of the Store API which is also implemented in Memory); the exact structure of this hash can be whatever the store needs/wants, but...
  2. the searchAttr is always added to a clone of the query in name:regExp(.toString()) form

comment:3 Changed 8 years ago by bill

See also #13416. I'm not sure what you mean by "considered a possible point of regression". The change seemed to work because of the toString() method, but I didn't know about issues with dojox stores.

comment:4 Changed 8 years ago by bill

FYI, I'll switch ComboBox back so it passes in a string rather than a regex when talking to a store with the old dojo.data API. Seems better for backwards compatibility.

I think jameyg's description/documentation suggestion is accurate. (Especially when connecting to a new API store, in which case it does pass in a regex.)

comment:5 Changed 8 years ago by bill

Oops, mistyped the ticket number in checkin comment for [25771]: switch ComboBox back so it passes in a string rather than a regex when talking to a store with the old dojo.data API. Seems better for backwards compatibility.

Refs #12373, #13416, #13880 !strict.

comment:6 Changed 7 years ago by Douglas Hays

Component: Dijit - FormDocumentation
Milestone: tbd1.8
Owner: changed from Douglas Hays to haysmark
Status: newassigned

comment:7 Changed 7 years ago by haysmark

Resolution: fixed
Status: assignedclosed

In [28576]:

Clarify ComboBox? doc in light of recent store changes. Fixes #13380.

Note: See TracTickets for help on using tickets.