og_forum overwrites Views page title if set for a page type View

setvik - October 9, 2007 - 09:36
Project:OG Forum
Version:5.x-2.x-dev
Component:Code
Category:bug report
Priority:critical
Assigned:rconstantine
Status:won't fix
Description

The drupal_set_title() function call in the og_forum_nodeapi "case 'view':" section overwrites the page title for any View that has "node: body" as one of the fields.

I temporarily fixed it on my site by moving all of the case 'view' code inside of the

if ($page && $node->og_forum_nid) {}
statement there.

I don't know if that's the best or correct solution, however.

Any thoughts?

#1

setvik - October 9, 2007 - 11:15

To be a bit more specific, og_forum overwrites the View page title with the title of the final node returned by the view (regardless of whether the node-type is a forum post or not).

e.g. Let's say you have a page View with the "Title" set to "Upcoming Events" that displays all Upcoming Events and lets say it

returns the following 3 events:

10/4/2007 Wine and Cheese Party
10/18/2007 Charity Bike Ride
10/31/2007 Halloween Party at the pub

Instead of "Upcoming Events", which is what the View title has been set to, the page title becomes: "Halloween Party at the pub"

#2

rconstantine - October 9, 2007 - 19:05

OK, this clearly needs to be fixed. Do you have site-wide forums as well as group forums on your site? If so, how are the titles of site-wide forum topics affected by this? Could you test that for me please? It seems to me that they wouldn't have a title since they don't have a gid and therefore wouldn't pass the second condition of that if statement.

What other conditions might we check to set the title? Is this problem only occurring when views are viewed?

Does Views expose any global variable we can check to see if it's the one making requesting the content bodies?

#3

douggreen - October 13, 2007 - 14:38
Status:active» patch (code needs review)

I think that all you need to do is check for $teaser. See attached patch.

AttachmentSize
181945.patch710 bytes

#4

rconstantine - October 19, 2007 - 19:50

This looks good. I'm testing now...

Didn't quite work for me. Try this instead (in og_forum_nodapi, change the view case to):

<?php
case 'view':
     
// Don't do anything when handling teasers
     
if ($teaser) {
        break;
      }
     
// If we're viewing a forum post in a group forum, set that
      // group as the context.
     
if ($page) {
        if (
$node->og_forum_nid) {
         
og_set_group_context(node_load($node->og_forum_nid));
        }
        if (
is_numeric($node->tid)) {
         
$tid = $node->tid;
        }
       
$parents = taxonomy_get_parents_all($tid);
       
// Breadcrumb navigation:
       
global $_menu;
        if (isset(
$_menu)) {
         
$breadcrumb = array();
         
$breadcrumb[] = array('path' => "og", 'title' => t('Groups'));
         
$breadcrumb[] = array('path' => "node/$group_node->nid", 'title' => $group_node->title);
          if (
$tid) {
           
$breadcrumb[] = array('path' => 'forum', 'title' => $title);
          }
          if (
$parents) {
           
$parents = array_reverse($parents);
            foreach (
$parents as $p) {
             
$breadcrumb[] = array('path' => 'forum/' .$p->tid, 'title' => $p->name);         
            }       
          }
         
$breadcrumb[] = array('path' => "node/$node->nid", 'title' => $node->title);
         
$breadcrumb[] = array('path' => $_GET['q']);
         
menu_set_location($breadcrumb);
        }
       
drupal_set_title(check_plain($node->title));
        break;
      }
?>

This seems to work for me. An example is a simple search results page. Before -> was setting title to last result on page's title. After -> Title is 'Search'

#5

douggreen - October 19, 2007 - 21:05

Can you submit your change as a patch file? Without it, it's difficult to determine what you've changed.

#6

rconstantine - October 19, 2007 - 21:45

Maybe later. If you don't want to wait, replace the whole views case in og_forum_nodeapi. I've made a bunch of changes since the last release, so I'll have to figure out which revision to patch against.

#7

rconstantine - October 22, 2007 - 17:37

Sorry, still no patch. Here's another mod of that part of the og_forum_nodeapi function. Replace the entire 'view' case. Sorry, you'll have to cut/paste for now. Will roll a patch later I hope.

<?php
case 'view':
     
// Don't do anything when handling teasers
     
if ($teaser) {
        break;
      }
     
// If we're viewing a forum post in a group forum, set that
      // group as the context.
     
if ($page && $node->og_groups) {
       
og_set_group_context(node_load($node->og_groups[0]));
       
        if (
is_numeric($node->tid)) {
         
$tid = $node->tid;
        }
       
$parents = taxonomy_get_parents_all($tid);
       
// Breadcrumb navigation:
       
global $_menu;
        if (isset(
$_menu)) {
         
$breadcrumb = array();
         
$breadcrumb[] = array('path' => "og", 'title' => t('Groups'));
         
$breadcrumb[] = array('path' => "node/". $node->og_groups[0], 'title' => $node->og_groups_names[0]);
          if (
$tid) {
           
$breadcrumb[] = array('path' => 'forum', 'title' => $title);
          }
          if (
$parents) {
           
$parents = array_reverse($parents);
            foreach (
$parents as $p) {
             
$breadcrumb[] = array('path' => 'forum/' .$p->tid, 'title' => $p->name);         
            }       
          }
         
$breadcrumb[] = array('path' => "node/$node->nid", 'title' => $node->title);
         
$breadcrumb[] = array('path' => $_GET['q']);
         
menu_set_location($breadcrumb);
        }
       
drupal_set_title(check_plain($node->title));
        break;
      }
?>

#8

agn507 - October 22, 2007 - 20:03

This seems to have fixed the problem for me.

#9

phasmaphobic - October 23, 2007 - 18:12

I applied this change to the file, and receive the following error now:
Parse error: syntax error, unexpected $end in /modules/og_forum/og_forum.module on line 1024

I'm using OG forum 5.x-2.x-dev.

#10

phasmaphobic - October 23, 2007 - 18:17

Both the code snippets from #4 and #7 have the same effect, although the one from #4 gives a line value of 1025.

Currently using:
Drupal 5.2
Organic groups 5.x-4.0-rc7
OG forum 5.x-2.x-dev

#11

rconstantine - October 23, 2007 - 19:09

Did you clear all of your caches after the code change? If not, both cache_views and cahce_pages, and maybe other caches could mess things up.

#12

phasmaphobic - October 23, 2007 - 19:42

Hm, not sure. I tested it with the performance cache turned off completely, and I'm not entirely sure how to clear the others you mention. Not finding any documentation at the moment on how to go about that either...

#13

phasmaphobic - October 23, 2007 - 20:02

Update: Okay, after remembering I had already set up a way to do just that, I cleared the caches and tried again, same error when loading anything.

#14

ebeyrent - October 29, 2007 - 21:56

This code fixed the issue for me as well.

How long before this patch can be rolled into the dev version?

#15

rconstantine - October 30, 2007 - 00:35
Status:patch (code needs review)» fixed

The code is in the latest of both official and dev releases. Use the official.

@phasmaphobic, I am thinking that perhaps you applied the code change incorrectly. That's my fault for not rolling a proper patch. Try the new official version.

#16

phasmaphobic - October 30, 2007 - 16:43

That's entirely possible. All I did was replace the text from the module with the text you supplied, and I have done this a few times before in the past with other modules without issue. But ah well, a new version is preferable anyway.

Thanks!

#17

Anonymous - November 13, 2007 - 16:51
Status:fixed» closed

Automatically closed -- issue fixed for two weeks with no activity.

#18

Mark Theunissen - December 11, 2007 - 13:01
Status:closed» active

Hey everyone,

Just to let you know that this issue caused problems with my code too.

og_forum version 2.2.

The problem is that the 'view' case of hook_nodeapi will override the title of ANY node that belongs to a group, because it uses the if statement:

if ($page && $node->og_groups) {

What it should do it check to ensure that the node type is a forum, as well as belonging to a group!

if ($page && $node->og_groups && $node->type == 'forum') {

Hope that can help someone...

Cheers
Mark

#19

rconstantine - December 11, 2007 - 17:30

Could some of you folks be so kind as to post some simple examples of the Views that are getting screwed up? Frankly, I only use views a little and haven't needed interaction with this module myself yet. Just export the View and post it here. Make sure you haven't done anything too exotic so that it will actually work on my (or other) generic setup.

I'm sure you can understand that I'd just like to see all of this first hand, including this potential fix.

And as always, if anyone wants to extend the Views support of this module, I am very open to patches - same with my other modules.

Cheers.

#20

rconstantine - December 11, 2007 - 19:04
Status:active» patch (code needs review)

#21

rconstantine - December 11, 2007 - 19:06
Status:patch (code needs review)» fixed

Ah, forget it. I reviewed the surrounding code and see no problem with the proposed addition. So I'm adding it to the code base.

I'm making some other major changes, so expect a new release soon!!!

#22

rconstantine - December 11, 2007 - 23:08
Assigned to:Anonymous» rconstantine

#23

Mark Theunissen - December 13, 2007 - 07:36

rconstantine, apologies for not getting back sooner. I'll reply to your question in case anyone else can benefit.

I have a custom node type which belongs to a group. In my code, I use the function drupal_set_title() to override the title of the node, but what is happening is that the nodeapi() of og_forum is called later in the stack and overrides the title again.

To reproduce, you could for example create a new CCK type called "DVD" with an artist, title, etc. Create a node of type DVD in a group (that's important), and then create a module that implements hook_nodeapi() and overrides the title of the DVD node to something else.

You will find that the changes to the title are being overridden, because og_forum does not ensure that a node is of type 'forum' before it runs drupal_set_title() again.

One workaround would be to change the weight of the modules in the system table, but that's a bit of a hack when the og_forum nodeapi hook should only operate on nodes of type forum!

Hope that clarifys things.

Mark

#24

Anonymous - December 27, 2007 - 07:41
Status:fixed» closed

Automatically closed -- issue fixed for two weeks with no activity.

#25

vpiotr - March 30, 2008 - 09:21
Status:closed» won't fix

I have had a stupid situation when a standard "page" node has been added to OG group as a topic.
This was not my intention, but it happened.

Result was far from expected - on every page (including "admin" pages) title has been changed to this latest page title - probably because this page was displayed on every page as a block.

The first tip, althought I'm using the latest version of og_forum - 5.x-2.1, fixed the situation:

<?php
if ($page && $node->og_forum_nid)
?>

 
 

Drupal is a registered trademark of Dries Buytaert.