Module test: Views

[Alessio Dal Bianco]

Hi all!

I have just created a proof-of-concept Module here:

https://github.com/U...ocesswire-views.

The main idea was started by me in this topic:

http://processwire.c...age/#entry21257

The aim of this module is to extend the number of files associated of a specific Template.

For example: if the template name is "page", the file associated is "page.php".

With Views we can have others visualization of the same Template named in this way:

view_name1-page.php

view_name2-page.php

|

|

|

view_nameN-page.php

Also you can test your view (useful when developing...) in this way:

example: http://example.com/great-page/?view=fullscreen

where "fullscreen" is the name of the view.

IMPORTANT:

Since i'm still learning how to make modules, the module works with a TextField called "view" (where is stored the name of the view), and you must create it manually. Maybe in the next few days i will make the automatic creation of the field and some other functionalities.

Any Feedback is appreciated!

USSliberty

[Soma]

Hey nice start! Thanks for posting. I see where you're going with this, and maybe it can be of use for people.

Some feedback:

You extend class "Process" here, and I'm not sure that's the right one in this case. Process modules are meant for admin pages that are associated with a page in the admin like the page tree or editor. You assign it to it if you create a new page with the template "admin". The execute method is then the default process that's called when you look at the admin page you created. Process modules are usually also not autoloaded because of that reason. I know it works, but it's not meant to be used for hooks and such, as far as I know.

Not sure this is intended but maybe it makes sense for some other module you will want to create in correlation to this.

So you'd extend Wire or WireData. See the wiki artikel by Ryan about modules, that will tell you a lot! http://wiki.processwire.com/index.php/Module_Creation

I'm not sure but I thought you'd have to use addHookAfter to make it work, not sure what addHook really does... not sure on that one atm.

Some syntax you used is a bit strange and to make it short ("fuel") isn't official, and kinda old. It still appears in the core and modules and is a relict. The right and simple way to use API in modules is using $this->pages or wire("pages").

As for the module, I copied and cleaned it up a bit (removed comments for better overview), that will maybe help you out some to see how someone experience would do it. You see some that could help not executing the hook at all if you do some checks in the init() already. You can already check if a GET var is set or permission are correct. That's all for now, I'll let you figure the rest

<?php

class Views extends Wire implements Module{

   public static function getModuleInfo() {

       return array(
           'title' => 'Views',
           'version' => 90,
           'summary' => 'Add the selection of the view of a specific page',
           'href' => '',
           'singular' => true,
           'autoload' => true,
           );
   }

   public function init() {
       if(!$this->user->hasPermission('page-edit')) return;
       //if($this->input->get->view){
           $this->addHookAfter('Page::loaded', $this, 'changeView');
       //}
   }

   public function changeView($event){
       $page = $event->object;
       if("admin" == $page->template) return;
       $view = $this->input->get->view;
       $view = $view ? $this->sanitizer->name($view) : $page->view;
       if($view):
           $tname = $page->template->name;
           $page->template->filename = str_replace($tname, $view . "-" . $tname, $page->template->filename);
       endif;
   }

}

[Alessio Dal Bianco]

Thank you Soma for the guidelines!

I have adjusted the code in this way and all works like expected...

There is only one little error here:

/* NOTICE: where the user is not logged this line return me an error:
2012-12-09 11:19:00 guest http://localhost/?/ Error Call to a member function has() on a non-object (line 50 of /Users/alessiowork/Progetti/luciavenezia.com/www2/wire/core/Role.php) */

$is_preview = $this->input->get->view && $this->user->hasPermission('page-edit');
// -------------------------------

It is not real problem because we don't want to let anyone to change the view, but "$this->user->hasPermission('page-edit')" doesn't should return false ?

<?php
/**
* Views for ProcessWire
*
* Every Template in ProcessWire have a file associated named in this way:
* <templateName>.php
*
* The aim of this module is to extend the number of files associated of
* a specific Template.
*
* For example: if the template name is "page", the file associated is page.php.
* With Views we can have others visualization of the same Template named in this way:
* <view_name1>-page.php
* <view_name2>-page.php
*	 |
*	 |
* |
* <view_nameN>-page.php
*/
class Views extends Wire implements Module{

/**
* getModuleInfo is a module required by all modules to tell ProcessWire about them
*
* @return array
*
*/
public static function getModuleInfo() {
      return array(
           'title' => 'Views',
           'version' => 90,
           'summary' => 'Add the selection of the view of a specific page',
           'href' => '',
           'singular' => true,
           'autoload' => true,
      );
}
/**
* Initialize the module
* We add the hook to "loaded" event of a page.
* So we have the current template file name
*
* Also we add The Tab "Visualization" and we show the list of voiews.
*/
public function init() {

   $this->addHookAfter('Page::loaded', $this, 'changeView');

}
public function changeView($event){

   $page_temp = $event->object;

  // Avoid change view for admin
  if("admin" == $page_temp->template) return;

  // -------------------------------
  // We check if there is the GET parameter "view" set.
  // example: http://example.com/great-page/?view=fullscreen
  // and also we check if the current user have the right permission to
  // view the page.

  /* NOTICE: where the user is not logged this line return me an error:
  2012-12-09 11:19:00 guest http://localhost/?/ Error Call to a member function has() on a non-object (line 50 of    /Users/alessiowork/Progetti/luciavenezia.com/www2/wire/core/Role.php)
  */
   $is_preview = $this->input->get->view && $this->user->hasPermission('page-edit');
  // -------------------------------

  if( $page_temp->view || $is_preview ):

     $view = $is_preview ? $this->sanitizer->name($this->input->get->view) : $page_temp->get("view");

     // if is specificated the view "default" in the preview,
     // for example : http://example.com/great-page/?view=default ...
     // ... we force to use the default template file.
     if( $view == "default" && $is_preview ) return;

     $templateName = (string) $page_temp->template; // We pick the template name...
     $template_obj = &$page_temp->template;		 // ...and the template object associated.

     // ------------
     // Now we change the template file to the one we choose.
      $template_obj->filename = str_replace($templateName, $view . "-" . $templateName, $template_obj->filename);
    // ------------


   endif;

}
public function ___execute(){
    // TODO

}

public function ___getSelectableViews($event){
  // TODO

}
public function ___install(){
  // TODO

}
}

USSliberty

[Soma]

I'm not sure. It's strange indeed. I tested a little and it seems if the user is "guest" (not logged in) it fails to get the persmission check, but ONLY if checked in the hook function. In the init function the check works. Could be a bug.

However I recommend to do a $user->isLoggedin() check to see if user is logged in at all first, and also user $page->editable(); to determine if user has edit right on the requested page.

Also my suggestion is to use the logged in check in the init, so the hook doesn't get executed on guest users int he first place. Right now this hook is executed on every request.

So it would look like this:

   ...
   public function init() {
       if($this->user->isLoggedin()){
           $this->addHookAfter('Page::loaded', $this, 'changeView');
       }
   }

   public function changeView($event){
       $page = $event->object;
       if("admin" == $page->template) return;
       if(!$page->editable()) return;
       $view = $this->input->get->view;
       ...

[Alessio Dal Bianco]

Also my suggestion is to use the logged in check in the init, so the hook doesn't get executed on guest users int he first place. Right now this hook is executed on every request.

Maybe i was not clear... but the hook *must* be executed every request.

The view change is permanent after page save... The GET variable "view" is only for test the view without change it on the database; that's why i need to check in the hook if the user is logged or not.

Anyway i have resolved this little glitch in this way:

/**
* Initialize the module
* We add the hook to "loaded" event of a page.
* So we have the current template file name
* 
* Also we add The Tab "Visualization" and we show the list of voiews.
*/
public function init() {
   // I save the result of permission test, prior to hook, in a private variable...
   // that i check after in the hook.
   $this->has_perms = $this->user->hasPermission('page-edit');

   $this->addHookAfter('Page::loaded', $this, 'changeView');
}

[Soma]

Ah yes, that makes sense.

Still the $page_temp->editable() could be used in that case too. $this->user->hasPermission('page-edit') is not page context aware, so a user could see it for pages he has no edit rights.

Well while testing I found it works with the user->hasPermission() if you do it with a hook on before render.

$this->addHookBefore('Page::render', $this, 'changeView');

[Alessio Dal Bianco]

Ok with "$page_temp->editable()" works fine.

$is_preview = $this->input->get->view && $page_temp->editable();

Thanks Soma!

[teppo]

Hello there!

Just wanted to say that I like what you're doing -- I've actually built something quite similar and found it very useful for (especially) larger projects, so I'm certain that this kind of module has a lot of potential

I'd also like to make some suggestions, if you don't mind:

* First of all, you might want to consider adding a separate directory for view files -- or perhaps making this configurable via module settings.

Currently there's a small but still existing chance that view and template names could conflict. Making this user-configurable would also make this module usable out-of-the-box for those of us who prefer their own folder structure instead of flat templates directory.

* Second suggestion / observation is that the way you're currently handling template names might not work as expected for templates that have been set to use alternative template filename via advanced template settings.

I'm not sure if you would really consider this a problem, but I felt I should mention it anyway, in case you didn't remember that this kind of option existed. Anyway, with this setting in some cases a) multiple templates might have same filename and b) filename might not contain actual template name at all. This might cause problems, since your code seems to expect that it always does:

$template_obj->filename = str_replace($templateName, $view . "-" . $templateName, $template_obj->filename)

To be honest I'm not even sure that I understand why you're using str_replace() there -- wouldn't it better to simply set $template_obj->filename as $view . "-" . $templateName? This way alternative template filename, if given, would simply be ignored and view filename would always be consistent with actual name of that particular template.

* And finally a quick comment on something quite subjective: code readability.

The cleaned up version Some presented above is actually quite a bit easier to read and understand, partly because those comment blocks are (somewhat unnecessarily) taking some serious screen estate, but also because it seems generally speaking more polished. One very specific thing I'm having hard time wrapping my head around is your variable naming convention:

$templateName = (string) $page_temp->template; // We pick the template name...
$template_obj = &$page_temp->template;		 // ...and the template object associated.

In that short code sample you're already using two naming conventions, first camel case and then delimiter separated. Picking one of those and sticking with it will make your code more predictable and readable in the long run. Trust me, I'm really not trying to be condescending here -- this is just one of those little things that will really help other programmers understand (and appreciate) your code.

And that's it for the nagging. All in all it's really nice to see people posting (especially) these kind of proof-of-concept modules here. Pushing boundaries is always admirable, and that's exactly what you're doing here. Keep up the good work!

[Alessio Dal Bianco]

Hi Teppo,

Thanks for all suggestions!

Anyway this attribute:

$template_obj->filename

Returns me an absolute path, that's why i must do a string replace

In the next few days nights i will complete the basic functionalities !

USSliberty