pkp / pkp-lib

The library used by PKP's applications OJS, OMP and OPS, open source software for scholarly publishing.
https://pkp.sfu.ca
GNU General Public License v3.0
304 stars 444 forks source link

PLUGIN_GALLERY_ALL_CATEGORY_SEARCH_VALUE is defined in the wrong context #7080

Open ctgraham opened 3 years ago

ctgraham commented 3 years ago

Describe the bug Constant PLUGIN_GALLERY_ALL_CATEGORY_SEARCH_VALUE is defined in PluginGalleryGridHandler.inc.php, but it is used in PluginGalleryDAO. It is also referenced in PluginGridHandler

It doesn't make sense for PluginGalleryDAO to require PluginGalleryGridHandler, and PluginGalleryGridHandler uses the constant in contexts where PluginGalleryDAO has not necessarily been loaded, so PluginGalleryDAO doesn't seem the right home either.

Perhaps the best solution is for PluginGalleryGridHander to detect the case where this constant is going to be passed to PluginGalleryDAO::getNewestCompatible(), and pass in null instead.

To Reproduce Steps to reproduce the behavior:

  1. Instanciate PluginGalleryDAO outside of PluginGalleryGridHandler and call getNewestCompatible() with any $category value.
  2. See error:
    PHP Warning:  Use of undefined constant PLUGIN_GALLERY_ALL_CATEGORY_SEARCH_VALUE - assumed 'PLUGIN_GALLERY_ALL_CATEGORY_SEARCH_VALUE' (this will throw an Error in a future version of PHP) in lib/pkp/classes/plugins/PluginGalleryDAO.inc.php on line 41
    PHP Stack trace:
    PHP   1. {main}() lib/pkp/tools/pluginGallery.php:0
    PHP   2. PluginGalleryTool->execute() lib/pkp/tools/pluginGallery.php:134
    PHP   3. PluginGalleryDAO->getNewestCompatible() lib/pkp/tools/pluginGallery.php:89
    PHP Warning:  Use of undefined constant PLUGIN_GALLERY_ALL_CATEGORY_SEARCH_VALUE - assumed 'PLUGIN_GALLERY_ALL_CATEGORY_SEARCH_VALUE' (this will throw an Error in a future version of PHP) in lib/pkp/classes/plugins/PluginGalleryDAO.inc.php on line 41

What application are you using? OJS 3.2.x

asmecher commented 3 years ago

(See also https://github.com/pkp/pkp-lib/issues/6092, in which constants are being moved into classes. This won't necessarily mean the constant is moved from A to B, but it will mean include statements are no longer needed to ensure constants are loaded -- only the appropriate use statements.)