backdrop-contrib / facetapi

An abstracted facet API that can be used by various search backends.
GNU General Public License v2.0
0 stars 3 forks source link

Support for deprecated function drupal_sort_weight() #2

Closed earlyburg closed 3 years ago

earlyburg commented 3 years ago

In D7 located in /includes/common.inc there is a function named drupal_sort_weight(). This function was not moved into Backdrop, but it's needed. My pull request places this functionality as facetapi_sort_weight() in facetapi.module. This fixes the following PHP Warning:

Warning: uasort() expects parameter 2 to be a valid callback, function 'backdrop_sort_weight' not found or invalid function name in uasort() (line 662 of /modules/facetapi/facetapi.module).

Original function is documented here: https://api.drupal.org/api/drupal/includes!common.inc/function/drupal_sort_weight/7.x

docwilmot commented 3 years ago

FYI https://api.backdropcms.org/change-records/generic-sorting-function-replace-implementations-uasort

earlyburg commented 3 years ago

FYI https://api.backdropcms.org/change-records/generic-sorting-function-replace-implementations-uasort

@docwilmot this is helpful. I now understand why this was changed. The new function may still be necessary because as mentioned in the pull request :

docwilmot commented 3 years ago

You should be able to use backdrop_sort() with any appropriate key in the array. backdrop_sort($element, array('#weight' => SORT_STRING)); backdrop_sort($array, array('weight' => SORT_STRING)); See https://api.backdropcms.org/api/backdrop/core%21includes%21common.inc/function/calls/backdrop_sort/1

earlyburg commented 3 years ago

I can see how that would be cleaner. Let me do a bit of testing -- I may have to do another pull request. Thanks

earlyburg commented 3 years ago

This is still not anywhere close to ready but I have fixed this issue: https://github.com/backdrop-contrib/search_api/issues/12 More soon...

docwilmot commented 3 years ago

There seems to be many empty file changes. Usually that's due to EOL change. Was this intentional?

earlyburg commented 3 years ago

@docwilmot It's file permission changes on my dev box. I'm developing on Debian so if I want to save changes in Atom to files in my webroot I have to adjust my file permissions accordingly. Sorry my dev branch / approach is so messy - I will re-roll the pull request when I get done fixing the module to keep the project code shiny. Thanks for doing the grunt work on this and helping to bury the bones of ctools forever. I just wanted to keep the thread going as I speculate there is some community interest in getting this module functioning.

earlyburg commented 3 years ago

My last code push renders facetapi usable with search api and search api solr and search api pages. One thing that I noticed is that facet blocks do not show up in the custom blocks section here: /admin/structure/block/list but you can add them to a layout here: /admin/structure/layouts/manage/default/blocks (they show up in the dropdown selects) I'm not sure if this is ideal behavior but I welcome feedback. The current_search_blocks sub-module is next for repair which is pretty dependant on old ctools functions. More soon... (note: I have not tested this with backdrops native search functionality yet)

docwilmot commented 3 years ago

One thing that I noticed is that facet blocks do not show up in the custom blocks section here: /admin/structure/block/list

That list in Backdrop is just for reusable text blocks. All other blocks are placed via layouts. So this is all expected behaviour

I can review the PR once the permissions changes are fixed.

earlyburg commented 3 years ago

@docwilmot Thank you. As promised I am closing this pull request and re-creating it to make it shiny.