Be careful with drupal_get_form() in theme layer

When you create Drupal code for 7 years, it’s easy to mistakenly assume that you know everything regarding such basic things as theme_preprocess_node() and drupal_get_form() functions.
And that is a wrong feeling. Today I’ve discovered that you shouldn’t mix these functions!

The bug description:
I was creating “join” form which was rendered in node.tpl.php of organic group.
When the form was submitted, validation/status messages were shown only after additional page refresh. So, after form submit page was reloading without any messages, and all the messages were appearing on the screen after additional page reload.

That was a weird bug which I never encountered earlier. After multiple-hour review of all custom modules of the website (I thought some module was doing magic things with $_SESSION array, where all the messages are stored at by drupal_set_message()) I realized that I’m looking in the wrong place.
The real issue was in our theme variables preparation.

It was a cool custom “join this group” form (we create only cool forms here in Pixeljets, no kidding), so that is how the code was looking like:

mymodule.module:

<?php
...
function
mymodule_preprocess_node(&$vars) {
  if (
$vars['node']->type == 'mygroup') {
    
$vars['join_form'] = drupal_get_form('mycoolform', $vars['node']);
  }
}
?>

the theme, node—group.tpl.php:

<?php
echo render($join_form);
?>

Everything was looking good at first sight, but no validation messages were appearing after form submit.

The reason is easy (when you know it): with such code organisation, form validation happens too late, so all messages are put to $_SESSION _after_ the moment when Drupal grabs it to show it to user on his screen.
Interesting thing is that form was working good in node teaser, but not in full node! that’s because preprocess_node() was run earlier than template_process_page() on the page where we rendered node teasers.

So, I’ve fixed the bug by moving the form generation code from _preprocess_node to hook_node_view like this:

<?php
function mymodule_node_view($node, $view_mode) {
  
$node->content['join_form'] = drupal_get_form('mycoolform', $node);
}
?>

Now, everything is working like a charm.

Comments

Hi,

I would like to know why did you not use drupal_get_form via a callback inside hook_menu? Is there is a specific use case?

Thanks

07 March, 2012

Hi Vikky,
we needed form as part of node.tpl.php, so having separate menu callback won’t help, I think.

07 March, 2012

Why didn't you use hook_node_view()?

07 March, 2012

I’ve used it - after the _preprocess approach failed :)
but the preprocess approach is more flexible, e.g. you can set some bool variables there (node_view is not good for such type of things - you should generate only renderable things there)

07 March, 2012

Oh, crap... I should read the entire article before wondering why you didn't do it the proper (and MVC :P ) way!

Btw, why don't you generate the things you want to render depending on the bool variables during node_view? That way, you can pass only renderable things to your templates, making your designer / designers all the happier. It also keeps all data processing in one spot, instead of spread out over node_view and hook_preprocess_*. That creates some happy maintainers ;)

07 March, 2012

so, do you think that $teaser or $is_front bool variables are bad practice? :)
they make tpl files much more readable sometimes, isn’t it?

regarding designers - that is a long story :)
I guess, you mean html coders (frontend devs), since regular designer will get buried in Drupal tpls - Photoshop and .psd is the area where he usually works.
I never saw html coder without PHP knowledge, who was happy looking into Drupal tpl files! no matter if bool variables are there or not, there are still a lot of “if” conditions in tpls, if you create complex real-world website in Drupal. Trying to move this logic to programmatic conditions in node_view makes the maintenance even more difficult and the theming process slows down.

07 March, 2012

I've never thought of having to change anything to $teaser. I mean, the designer I usually work with, will place $teaser in his .tpl files, and if it doesn't display the way he wanted it to, he'll complain to me and I'll have to make sure he receives a $teaser variable he can just place in his HTML. That is a HTML coder that knows some PHP and does add it to his tpl files now and then. He mainly wants to focus on the frontend dev. though, not on data processing.

Another HTML coder I've worked with, doesn't know any PHP at all. Even working with PHP variables she could place anywhere, was alot to take in. She's a very good designer, though, and has no problems with HTML and CSS. I did my best to help and guide her, but she felt as if Drupal was for developers and that it didn't offer what she wanted: something where she can just change some HTML and CSS to make her site look the way she wants. She ended up tossing out all my dev. work and going with Smugmug instead. Ever since then, I've tried to take as much logic to my side, so I can offer designers bite-sized vars they can just place into their HTML.

Yet another designer I've worked with, does HTML and CSS, but gave me raw HTML / CSS files which I had to turn into tpls (hurray! Like I didn't have enough to do already!). I could then pretty much do whatever I wanted. That designer was also intimidated by writing logic and minor PHP code, but couldn't say no to using Drupal cause the entire company was based on using it.

If you got designers in your team that prefer to work with bools and such, so much the better for you. To my experience, though, you should present them with as little programming as possible, so they can focus on what they know and do best.

Oh, and... I don't even know $is_front. Never used it, never seen it being used, never heard of it before, yet I've worked on some pretty damn complex far too real-world websites in Drupal and have done plenty of theming myself :P

07 March, 2012

You're so super cool, Raf! Thanks for proving how super cool you are. I was starting to think you were only really cool.

07 March, 2012

You're welcome! But I'm only proving how super cool keeping to the MVC principle is, super cool to everybody involved in the project!

07 March, 2012

@raf Please... Drupal is nothing like using a strong MVC model. Look at PAC.

Making mistake is the only way to learn. Anton, made a mistake a blog about it so other can learn. Comment like "why did you don't use the proper way directly" is quiet disturbing... But "Why you don't use MVC patern direclty" is simply stupid.

05 July, 2012

Excellent blog here! Also your website loads up very fast!
What host are you using? Can I get your affiliate link
to your host? I wish my website loaded up as quickly as yours lol

Here is my website italkonline blackberry executive

25 November, 2014

TҺanks very nice blog!

Have a look aat my web site - web site (Juan)

24 November, 2014

Aah I see, makes sense then.

08 March, 2012

Fantastic post! I was ready completely change how form errors were to be displayed before reading this. Thank you for saving me from my own impatience. :)

07 April, 2012

you’re welcome ;)

07 April, 2012

This piece is not working when I embed this code in the function.php file. Please help me in getting me out of trouble. As I am new to Drupal, and I really want this “join this group” page to my website.

16 April, 2012

This saved my day! Thank you very much!

Now to something completely different: I need a Beer. print render($beer);

17 December, 2013

Hello would you mind stating which blog platform you're working with?
I'm planning to start my own blog in the near future but I'm
having a difficult time deciding between BlogEngine/Wordpress/B2evolution and Drupal.
The reason I ask is because your design and style seems different then most blogs and I'm looking
for something unique. P.S Sorry for being off-topic but I had to ask!

25 October, 2014

Post new comment

Private and will not be shown publicly.
  • Web page addresses and e-mail addresses turn into links automatically.
  • Allowed HTML tags: <a> <em> <strong> <cite> <code> <ul> <ol> <li> <dl> <dt> <dd>
  • Lines and paragraphs break automatically.

More information about formatting options

Note for potential spammers: all links in your comment will not be indexed by search engines.

Anton Sidashin

Anton Sidashin senior developer, Pixeljets co-founder

I'm a web developer specializing in PHP and Javascript, and Drupal, of course. I'm building Drupal projects since 2005, and I was working as full-time senior engineer in CS-Cart for a while, building revolutionary e-commerce software. In my free time, I enjoy playing soccer, building my body in gym, and playing guitar.

Drupal.org ID: restyler
Drupal association member