Code enhancement - detecting required login


#1

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 @@
 			$this->setBasicVariablesView($view);
 		} 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)
http://forum.piwik.org/read.php?2,74492


#4

@Matt:
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 {
			$this->setPeriodVariablesView($view);
			$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: http://dev.piwik.org/trac/changeset/4334

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.

Thanks


(Matthieu Aubry) #6

another changeset http://dev.piwik.org/trac/changeset/4337 that is related


#7

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: http://piwik.org/docs/plugins/hooks/#toc-add-a-new-hook

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.


#9

Okay done
http://dev.piwik.org/trac/ticket/2275