Code enhancement - detecting required login


hey :slight_smile:
When a login is required and the requested date is “today”, the code goes to a very long journey to detect a login required and unfortunately it does so by handling an exception which is thrown by Piwik_Date::factory. Maze ha? this is too confusing because it doesn’t imply what it does at all. Even when testing that exception variable I found it has this message: Date format must be …
which is caught to express a needed login. Totally confusing.
Methods, exceptions and class names should imply what they do. Otherwise this will hinder Piwik thirdparty extension development.

(Matthieu Aubry) #2

I agree, I remember finding this confusing. is the following code the issue, or something else?

Index: core/Controller.php
--- core/Controller.php	(revision 4315)
+++ core/Controller.php	(working copy)
@@ -358,6 +358,7 @@
 		} catch(Exception $e) {
 			//TODO here display std error message rather than redirect
+			echo $e->getMessage();exit;
 			self::redirectToIndex( Piwik::getLoginPluginName(), $action = 'index' );

(Matthieu Aubry) #3

also please use the new RC, I think I improved some messages around this issue (not sure if it’s the same as yours though),74492


because the strDate member variable not set below, this causes Piwik_Date::factory to throw exception of “Incorrect date”, and as handled below, this sends user to login.

core/Controller.php: class Piwik_Controller

protected function setGeneralVariablesView($view)
		$view->date = $this->strDate;
		try {
			$date = Piwik_Date::factory($this->strDate);  
		} catch(Exception $e) {
			self::redirectToIndex( Piwik::getLoginPluginName(), $action = 'index' );

(Matthieu Aubry) #5

I think this will help and fix the ‘confusing’ aspect of this error:

let us know if you have more feedback, we are very keen on making the code, docs, etc. more developer friendly, as we like to support all apps & tools built on top of Piwik.


(Matthieu Aubry) #6

another changeset that is related


I finally could create a plugin that login users from other table (aMember in this case). No code changes are made in core, it’s just plugged and works, and I just like to contribute it.

But after that struggle, I found another problem when the user login and UsersManagers tries to get related stuff to this “transparent” user. I cannot override the UsersManagers plugin method for fetching users that throws exceptions when user doesn’t exist in local table (it doesn’t since users come form other table now: the aMember users). All because the code calls this class method Piwik_UsersManager_API::getInstance statically, and the member variable $instance is private.:

class Piwik_UsersManager_Controller[130]:

$user = Piwik_UsersManager_API::getInstance()->getUser($userLogin);

Also this could use the Zend::Registry method you used with the ‘auth’ which helped me accomplish my login plugin which is thus useless with this collateral issue…

EDIT: The above means I have to alter the core somewhere… No?

(Matthieu Aubry) #8

I think we will need a new hook for this use case. See more info in:

the best way to get it done, is that you try and understand how hooks work, then create a ticket on trac with the new hooks that are necessary for your use case, and we will comment and add them in core after reviewing.


Okay done