Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#1152 closed Bug (fixed)

Deprecate redundant use of plugin getName()

Reported by: matt Owned by: vipsoft
Priority: major Milestone: Piwik 0.6.3
Component: Core Keywords:
Cc: Sensitive: no

Description

Must it be the same as the class name?
Is it used anywhere?

let's clarify this and/or remove it if not useful (if we display the class name only, we don't need a human readable plugin name)

Change History (10)

comment:1 Changed 4 years ago by vipsoft (robocoder)

getName() isn't the same as the class name, e.g., 'CoreHome' vs 'Piwik_CoreHome'

getName() is used in two places:

  • in core/PluginsManager.php where it is used in the exception message if installPlugin() fails
  • in core/View.php where it is assigned to currentPluginName and displayed (human readable) next to the Piwik logo (in plugins/CoreHome/templates/logo.tpl)

This is not used by CorePluginsAdmin where it displays the name of each plugin. Here it uses readPluginsDirectory().

Given the above, we could enforce this consistency by adding a check to CorePluginsAdmin/Controller.php's index() -- skip any plugins where these don't match up.

comment:2 Changed 4 years ago by vipsoft (robocoder)

  • Owner set to vipsoft

Oh, I see what you mean. Yes, it would seem that getName() and getClassName() duplicate each other.

Ok. Revised suggestion:

  • document that getName() is the human readable plugin name (may contain spaces, punctuation; similarity to the plugin name is encouraged; translateable?)
  • in CorePluginsAdmin/Controller.php -- use getName() instead of the plugin's directory name when displaying the list of plugins
  • add a check to ensure all the human readable plugin names are unique, and that getClassName() matches the plugin's directory name; should be sufficient to check this constraint at installation, updates, and when installing new plugins

comment:3 Changed 4 years ago by matt (mattab)

Sounds good

comment:4 Changed 4 years ago by matt (mattab)

actually, why do we need getClassName() if it must return the directory name? maybe we can get rid of it and parse it from the dir name.

comment:6 Changed 4 years ago by vipsoft (robocoder)

  • Milestone changed from 2 - Piwik 0.8 - A Web Analytics platform to 0 - Piwik 0.6.3

comment:7 Changed 4 years ago by vipsoft (robocoder)

(In [2263]) refs #1152 -deprecate "name" in getInformation() array;use getClassName() instead of getName(); remove unused getName()

comment:8 Changed 4 years ago by vipsoft (robocoder)

  • Summary changed from Clarify the use of the plugin getName to Deprecate redundant use of plugin getName()

comment:9 Changed 4 years ago by vipsoft (robocoder)

  • Resolution set to fixed
  • Status changed from new to closed

(In [2264]) fixes #1152 - remove deprecated "name" from getInformation() array

comment:10 Changed 4 years ago by vipsoft (robocoder)

(In [2272]) refs #1152 - add "final" keyword to getClassName() because we dont want subclasses (plugins) to redefine this; remove "name" from phpdocs

Note: See TracTickets for help on using tickets.