Use current Login auth plugin instead of hard-coded 'Login' on the error page #8808
Labels
Task
Indicates an issue is neither a feature nor a bug and it's purely a "technical" change.
Milestone
Issue
When an exception page is shown, it has following links underneath the exception message:
Go Back | Go to Piwik | Login
The issue is that that
Login
link is hard-coded, as can be seen in https://github.com/piwik/piwik/blob/2.15.0-b7/core/testMinimumPhpVersion.php#L163 (using a released version here, just so that the line number does not change). That is a problem for the many third-party login plugins already in the Plugin Marketplace. Since if the user has a different login plugin installed and clicks on the aboveLogin
link, another exception page will be shown saying that the Login plugin is not active. (And so on in a circle, until the user clicks onGo to Piwik
.) So, basically, that link is not usable with other login plugins and only leads to user confusion.Solution
There are following two options:
a. Replace the hard-coded
Login
name withPiwik::getLoginPluginName()
, which is currently used across the platform specifically to find out the name of the currently active login module.The disadvantage of the above approach (a) is that it makes the exception page dependent on more things which could possibly fail. I haven't checked, but that could result in the exception page failure to appear, if something goes wrong on login plugin name acquisition, throwing another exception (cascading exceptions resulting in an uncaught error output?).
So, here is a simple and safe option:
b. Remove the
Login
link from the page.What also speaks for this solution is the fact that the Login link is not really necessary. Because when a user clicks on
Go to Piwik
, they will:(i) if logged in: return to their dashboard
(ii) if not logged in: be gently escorted to the login page
Either option is trivial to implement. I could submit a pull request, if it's decided which one to go with. BUT: I guess that that would cause a Travis "expected screenshot" test to fail due to the changed page appearance. And I won't be able to get a hold of the new image - see #8781 (comment).
Additional Point
Since there is also another
Back to Piwik
link at the very bottom of the exception page, which links exactly where the aboveGo to Piwik
does, one of them could be removed. Or not. I'm not a designer. :)The text was updated successfully, but these errors were encountered: