Allow the list of SearchIDs to be alphabetically sorted.

CommentFileSizeAuthor
#8 2570879-facetapi-integration-sort-applicable-searchids-8.patch1.96 KBthePanz
#7 2570879-facetapi-integration-sort-applicable-searchids-7.patch2 KBthePanz
#3 2570879-facetapi-integration-sort-applicable-searchids-3.patch1.7 KBthePanz
#2 2570879-facetapi-integration-sort-applicable-searchids.patch671 bytesthePanz
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

thePanz created an issue. See original summary.

thePanz’s picture

thePanz CreditAttribution: thePanz at Liip for FREITAG lab. AG commented
Status: Active » Needs review
FileSize
2570879-facetapi-integration-sort-applicable-searchids.patch671 bytes
thePanz’s picture

thePanz CreditAttribution: thePanz at Liip for FREITAG lab. AG commented
FileSize
2570879-facetapi-integration-sort-applicable-searchids-3.patch1.7 KB

Reworked patch: ensure that the SearchID is always captured even if it's already in the facet configuration.

Reason: in some (weird) cases the variable 'search_api_facets_search_ids' lost the tracked SearchID, but since the SearchID was present in the faced configuration, it will not be pushed into 'search_api_facets_search_ids' anymore.

thePanz’s picture

thePanz CreditAttribution: thePanz at Liip for FREITAG lab. AG commented
drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
CreditAttribution: drunken monkey as a volunteer commented

Thanks for creating this issue, seems like a good idea! The patch also looks good, just one thing:

+++ b/contrib/search_api_facetapi/plugins/facetapi/adapter.inc
@@ -104,6 +101,19 @@ class SearchApiFacetapiAdapter extends FacetapiAdapter {
   /**
+   * @param $index_id
+   * @param $search_id
+   */

This needs proper documentation.

But apart from that, the patch looks great, thanks!

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
CreditAttribution: drunken monkey as a volunteer commented
Status: Needs review » Needs work
thePanz’s picture

thePanz CreditAttribution: thePanz at Liip for FREITAG lab. AG commented
Status: Needs work » Needs review
FileSize
2570879-facetapi-integration-sort-applicable-searchids-7.patch2 KB

Documentation added, mostly taken from previous code.

thePanz’s picture

thePanz CreditAttribution: thePanz at Liip for FREITAG lab. AG commented
FileSize
2570879-facetapi-integration-sort-applicable-searchids-8.patch1.96 KB

Re-based patch against current 7.x-1.x branch

The last submitted patch, 7: 2570879-facetapi-integration-sort-applicable-searchids-7.patch, failed testing.

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
CreditAttribution: drunken monkey as a volunteer commented
Status: Needs review » Fixed

Thanks for the revision!
It's still not perfect, the first sentence of a doc comment should always be on a single line (and, thus, shorter than 80 characters), and you forgot the type hint for the $search_id parameter, but I just fixed that and committed the patch.

Thanks again for your work!

Status: Fixed » Closed (fixed)

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