Scc 4346/non roman 2 #423
Scc 4346/non roman 2 #423
Conversation
| // This builds a filter cause from the value: | ||
| const buildClause = (value) => { | ||
| // If filtering on a packed field and value isn't a packed value: | ||
| if (config.operator === 'match' && value.indexOf('||') < 0 && config.field.match(/_packed$/)) { |
There was a problem hiding this comment.
I removed these references to config.operator === 'match' because we are passing in an array that is already filtered on that condition.
| } | ||
|
|
||
| // This builds a filter cause from the value: | ||
| buildClause (value, field) { |
There was a problem hiding this comment.
Seems too general a name for the specific thing it's doing? Could it be:
buildFilterClause (value, fieldConfig)
lib/elasticsearch/config.js
Outdated
| issuance: { operator: 'match', field: 'issuance_packed', repeatable: true }, | ||
| createdYear: { operator: 'match', field: 'createdYear', repeatable: true }, | ||
| recordType: { operator: 'match', field: ['recordTypeId'], repeatable: true }, | ||
| owner: { operator: 'match', field: ['items.owner_packed'], repeatable: true, path: 'items' }, |
There was a problem hiding this comment.
Doesn't need to happen as part of this, but because this enables matching on multiple fields, you could change these _packed configs to match explicitly on each entity subfield. For example, above could use field ['items.owner.id', 'items.owner.label'], since that's how the clause ultimately manifests, right?
There was a problem hiding this comment.
yess I like that. I knew there was something there because their structure was so similar.
test/elastic-query-builder.test.js
Outdated
| buildMultiFieldClause: ElasticQueryBuilder.prototype.buildMultiFieldClause, | ||
| buildMatchOperatorFilterQueries: ElasticQueryBuilder.prototype.buildMatchOperatorFilterQueries, | ||
| buildFilterClause: ElasticQueryBuilder.prototype.buildFilterClause, | ||
| buildPackedFieldClause: ElasticQueryBuilder.prototype.buildPackedFieldClause |
I did some a bunch of refactoring to allow for some fine grained unit testing. I tested this on an integration level by hitting
/discovery/resources?q=&filters[creatorLiteral]=순끼on a local invocation. Not sure if I should add some higher level tests anywhere.