Make compatible with ad module

You mean you didn't rush out and install it right away? I can't believe it ;)

Anyway, the module provides a single registered content type 'ad' so far as the node (or was it just menu?) module knows. However, there is actually no 'ad' type. Instead, it is a link to a page where you can select from the actual various ad types. In other words, node/add/ad takes you to a selection page and does not serve up a content type creation page of any kind. The default two ad types are (1) image and (2) text. In the main menu's 'create content' sub-menu, 'Advertisement' appears as a parent menu item, and clicking it brings up the selection page as well as exposes the two available types as menu children, which you can select at that time (a la node/add/ad/image and node/add/ad/text links).

The problem with the og_user_roles module installed is that inside groups, the 'Create advertisement' link doesn't take one to the ad type choice page. Instead, there is a strange (can't seem to get there any other way) content creation form where a radio button allows one to select the ad type, but never shows the form sections specific to the type you select. Also, the 'Create advertisement' link in the group context menu is not a parent to any children.

Naturally, I first thought this was an issue with the ad module itself. However, an odd sequence of function calls is only called when og_user_roles is enabled and the ognodeadd is in the URL. In other words, creation of ads within groups works as expected (by displaying the selection page) when og_user_roles is disabled. I thought I wrote down that sequence, but I can't find it.

I did post an issue over at the ad module here: http://drupal.org/node/183064. Although his method of content type selection via choice page instead of menu item directly is odd, he mentions at least one other popular(?) module that does it as well. And there may be more. So a solution to this here might solve any similar issues that would come up.

Exemption, now that's a good idea. I'll test ASAP. Thanks for your trouble.

I didn't apply the patch yet. I have a concern with the version lines at the top and the dates. The new one has a smaller version number and older date. They don't seem right, and I want to make sure this patch is made against the correct version.

Applied patch. Got an error for line 916. Something didn't like you using a variable named $this, so I changed mine to read $this_thingy on that line and the next.

The page with the choices is properly shown, and the links seem okay. However, clicking a link results in a malformed URL, like http://www.sitename.com//node/add/ad/text?gids[]=1. Notice the double slash before node. The form is generated correctly, but submission tries to go to http://node/add/ad/text?gids[]=1 which fails.

I was just about to suggest the ltrim. I'll try the new patch now.

BTW, would there be an advantage to getting the ognodeadd feature to work with ad's sub-types? If so, let me know and I'll try to figure it out.

All seems to work, including the submission guidelines for all types both in and out of groups.

Well done.

Since this does work, I'm not changing this away from 'ready to be committed'. However, I have a comment about this line:
if (arg(0) == 'node' && arg(1) == 'add' && arg(2) == 'ad' && (arg(3) == 'image' || arg(3) == 'text') && $_SESSION['og_last'] && (!isset($_REQUEST['gids'])) ) {

In the conditional using arg(3), is there a way to do this generically? I tried using && arg(3), or even isset(arg(3)) [which is a wrong usage of isset it turns out], but neither method worked. The reason this interests me is because there are other sub-modules in the works. For example, someone has contributed a module to display flash ads. Someone else has added one for adsense. And so on. I don't mind adding these cases by hand if I decide to use them (which I haven't yet), but thought that it might be better to make that part of the 'if' generic.

Any ideas? I thought for sure that && arg(3) would work because it returns FALSE when arg(3) isn't set. I figured the combo of arg(2) being 'ad' and arg(3) returning anything not FALSE would do the trick.

If the 'if' you just provided works, then that's all I'm thinking of. You see, every bit of that long 'if' is obviously needed to make sure we're looking at the right kind of URL. In other words, rather than looking for 'image' or 'text', I thought that it would be possible/good enough to know that arg(3) has SOME value in it along with arg(2) == 'ad'.

What I didn't mean was generic code for handling all other (non-ad) modules if that's what you thought I meant. I'm happy with the results you've worked out as-is.

Thanks. I'll see about plugging in the above 'if' to see what happens.