There is some obvious additional cleanup to do, plus for the d.o redesign it would be nice to support field aliases for sorts.

Damien proposes that all the params for the query should live in the object, not in a separate array and that (since you must pass a Solr object into the query when you instantiate it) that the query even have some kind of execute() type method to actually fetch the result.

Feedback especially on this latter idea appreciated.

CommentFileSizeAuthor
#11 query-more-436074-11.patch9.79 KBpwolanin
#10 query-more-436074-10.patch8.95 KBpwolanin
#2 query-more-436074-2.patch6.91 KBpwolanin

Comments

Scott Reynolds’s picture

Scott Reynolds commented

With the views query class there are four stages that happen. init, build, alter, execute. I think that lends itself to a very nice flow.

pwolanin’s picture

pwolanin commented
Status: Active » Needs work
StatusFileSize
new query-more-436074-2.patch6.91 KB

here's a start on more complete handling of negative queries by parsing the leading '-' and taking it as a separate param for make_filter()

(my favorite kind of patch - more lines removed than added)

pwolanin’s picture

pwolanin commented

idea fmor Damien: http://drupalbin.com/8844

Scott Reynolds’s picture

Scott Reynolds commented

shouldn't there be a build() method does all that code (variable_gets() etc) instead of making the hook_search() unreadable. That would be preferable.

pwolanin’s picture

pwolanin commented

@Scott - no, I don't think most of that should be in the class itself - for example, the mlt module does not use or need most of this setup.

However, that current is certainly a monster that should be broken up a bit.

Scott Reynolds’s picture

Scott Reynolds commented

ahh good point. Moving it to a separate function though would be a huge bonus. With localsolr and the Views implementation both basically copy and paste that code in there. If we could just call apachesolr_build_params() that would be advantageous

David Lesieur’s picture

David Lesieur commented

Regarding comments #4 to #6, #436792: Modular handling of params might be of interest.

Scott Reynolds’s picture

Scott Reynolds commented

They apachesolr_views project does something very similar to whats in this patch for handling negative filters.

/**
   * Add in a facet string
   *
   * @param string $type
   * The type of facet. use apachesolr_index_key() for dynamic fields
   *
   * @param string $value
   * The value of the facet. Can be "story OR page" to filter multiple
   *
   * @param boolean $exclude
   * Whether or not to exclude the value from the results
   *
   * @return none
   */
  public function add_filter($type, $value, $exclude = FALSE) {
    $this->_facets[$type][] = array(
      'value' => $value,
      'exclude' => $exclude,
    );
  }
pwolanin’s picture

pwolanin commented

ok, I could even roll it as a boolean like:

if ($exclude) {
  $filter['#prefix'] = '-';
}
else {
  $filter['#prefix'] = '';
}

if that abstraction is more useful.

pwolanin’s picture

pwolanin commented
Status: Needs work » Needs review
StatusFileSize
new query-more-436074-10.patch8.95 KB
pwolanin’s picture

pwolanin commented
Status: Needs review » Fixed
StatusFileSize
new query-more-436074-11.patch9.79 KB

A couple more little fixes - committing to 6.x

note - without this the current search block is a bit broken for negative queries: (-) [* TO *] since it's looking for a function using a name containing '-'.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.