Thank you for the great work for VB integration. sun, are you considering for vb 4 support so far?

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

nicrodgers’s picture

We'd love to see vbulletin version 4 support too. We could potentially part-sponsor the required work.

socceronly’s picture

I would sponsor that as well.

Where do I sign up?

psicomante’s picture

i can develop some part of new version. Btw i startin' to see the last 5.x dev version to update it to 6.x core.

sorry for my "chopped" english..

Johnson2’s picture

When are we expecting Drupal for vB 4 release?

DamienMcKenna’s picture

We've hacked DrupalVB to get it to work, I'll see if I can poke someone into making a patch that's configurable to decide which version of FB it uses.

caponey’s picture

so does that mean we can't integrate VB with Drupal?
we just bought 4 dang it lol...jumped the gun a bit in our excitement =(

kwbrown’s picture

We also need an updated version of Drupal vB for vB 4.

codycraven’s picture

FileSize
16.18 KB

Sorry for the long delay, Damien's been on my back about this for over two weeks.

Please report back any issues --- this patch applies against dev!

DamienMcKenna’s picture

Thanks Cody!

Would you mind giving a brief explanation of what had to change to make it work?

codycraven’s picture

Essentially after vB 3.8.4 PL2, vB stopped using the vBulletin license number as a salt for the cookie hashing. This patch changes the admin configuration screen to prompt the user to enter the new value if the version of vB they select is 3.8.4 PL2 or greater (using Ajax).

To accommodate the Ajax, AHAH, a fieldset wrapper needed to be placed around the field. Because of this I did make a few UX changes to the configuration screen while I was at it.

The added drupalvb.css is used to prevent the throbber graphic from breaking the display of the form while it is rendered on the page.

The patch also carries out some todo's listed in drupalvb such as removing the need to add your config.php file to the drupalvb directory. This change was to accommodate our shared repository at Bonnier and would have been difficult to remove for posting the patch to d.o.

I also noticed some buggy issues in drupalvb/login and addressed those also.

codycraven’s picture

Status: Active » Needs review
sun’s picture

Thanks for this patch! We'll have to distill the changes that are actuall required, but anyway, lovely to see a (the first?) more in-depth patch in this queue. ;)

caponey’s picture

I don't know how to patch a module. I tried to understand how to do it, but I am not that good. I have eclipse, and that seemed to be the easiest method, but still couldn't figure it out. Is there a way to get the module with the patch already in it? Any help would be really appreciated.

Mixologic’s picture

I applied this patch and noticed one issue: I tried to login to Drupal as a user with no drupal account, but an existing vbaccount.

For whatever reason, the following code in drupalvb.inc.php changed in as part of this patch:

@@ -166,8 +237,8 @@
 
     // Splice in our validate handler for authentication if user is performing
     // a vBulletin login.
-    if (!empty($form_state['post']['name']) && drupalvb_db_is_valid()) {
-      $username = $form_state['post']['name'];
+    if (!empty($form_state['values']['name']) && drupalvb_db_is_valid()) {
+      $username = $form_state['values']['name'];
       if ($vbuser = db_fetch_array(drupalvb_db_query("SELECT userid, password, salt, email FROM {user} WHERE username = '%s'", drupalvb_htmlspecialchars($username)))) {
         $key = array_search('user_login_final_validate', $form['#validate']);
         if ($key !== FALSE) 

When I step through with the debugger, $form_state['values'] does not exist, but $form_state['post'] does, and after switching this back It creates New drupal accounts like it is supposed to.

ledzepfan2928’s picture

Category: feature » support
Priority: Normal » Critical
Status: Needs review » Active

I'm trying to take a current Vbulletin site and merge it into a new Drupal site. These currently exist on separate servers. Is there any way for the Drupal vB module to allow this type of integration? Or will we need to physically move the Vbulletin site over to our server that has Drupal all setup and ready to go?

norio’s picture

Excellent work, this patch works very well!

lokolo’s picture

I have some problems with the vBulletin 4.0.4 and vbDrupal. Used the patch above. The user synchronisation only works from Drupal to vBulletin, when i register from vBulletin, the users are not included in my Drupal-Site.

OwenMelbz’s picture

well i just ran the patch and got this on my drupal setup now

    * warning: include_once(C:\dev\xampp\htdocs\drupal\modules\drupalvb\drupalvb.module) [function.include-once]: failed to open stream: Permission denied in C:\dev\xampp\htdocs\drupal\includes\bootstrap.inc on line 690.
    * warning: include_once() [function.include]: Failed opening './modules/drupalvb/drupalvb.module' for inclusion (include_path='.;C:\dev\xampp\php\PEAR') in C:\dev\xampp\htdocs\drupal\includes\bootstrap.inc on line 690.
vladgur’s picture

thank you, this patch worked for me with todays purchase of vbulletin 4.0.5(gave up on drupal builtin forum).
One thing to note for those who are installing this for the first time: replacement variables will take care of edit Email/password links in the User Control Panel, but not the Edit Email/Password button in the Edit Profile form.
I had to search for password in the templates and found a place where it was located and replaced it with a link which is now affected by aforementioned replacement variables

toma’s picture

The last dev version work with vbulletin 4 or i have to apply the patch #8 ??

toma’s picture

How can i change the Variables in vbulletin 4, in readme.txt

* log in to vBulletin's AdminCP, go to

    Styles & Templates >> Replacement Variable Manager

  and add the following replacement variables for your template:

  - login.php?do=login           => /drupalvb/login

  - login.php?do=logout          => /drupalvb/logout?

    (The trailing question mark is required.)

  - login.php?do=lostpw          => /user/password

  - register.php                 => /user/register

drupizzle’s picture

FileSize
15.82 KB

@toma you need to put the text to the left (login.php?do=login) in the top line and the text to the right in the bottom line (/drupalvb/login) for each of the 4 items in the documentation. I just did this and can confirm it works.

On another note the patch in #8 does not work with cygwin on windows 7 so I have uploaded an updated patch that I can confirm works.

sun’s picture

Category: support » feature
Priority: Critical » Major
Status: Active » Needs work

Unfortunately, this patch still is a giant beast, mixing several other issues into one black hole. ;) Therefore, 6.x-2.1 still does not support vB 4.

To make progress here, we need to distill the actually required changes from this patch for this issue. And optionally, create separate issues for every other change (preferably also atomic).

codycraven’s picture

Just a note, all logins should be sent to your Drupal login form with the patch in it's current state. For Bonnier I created a vB4 plugin (I don't recall if that's what vB call them, basically it implements hooks) that causes all register, forgot password, and login forms to be sent to the Drupal site.

The reason I did not include the plugin is that I had to custom write it for our implementation, two different domains were sharing one vB site under two different subdomains. This required some extra special modification of the links to send to the correct Drupal site.

rplescia’s picture

FileSize
12.32 KB

I tried to implement patch in #8 but I get a 500 error when clicking on to the admin settings for the module, perhaps someone can look at my drupalvb.admin-pages.inc file (attached) and tell me where I'm going wrong?

Cray Flatline’s picture

subscribe

johannesjvorstermans’s picture

Where are we up to on this? Has the above patch from 20 April 2010 been integreated into any current release (stable or dev)? Almost a year now so appreciate a update of where we are if possible before I go down and try to patch the module?

Great to have this module.

SlayJay’s picture

Priority: Major » Critical

Now that Vbulletin 3 is at end of life (officially no more updates will be made) I strongly feel that VB4 support should be the top critical priority for this mod.

What's it going to take to make this happen? I'm willing to donate some if it's a money issue.

SlayJay’s picture

@JohnV:
I don't believe any VB4 support has been patched into any official release.

vidmarc’s picture

An update with vb4 support is urgently needed for this module.

omnia.ibrahim’s picture

Any news about when the new version that supports vb4 will be released ?

Taxoman’s picture

Subscribing

mnestor’s picture

subscribing

sk33lz’s picture

subscribing

RandyT’s picture

subscribing

it_mohit’s picture

Hi,

Here is the patched module working fine for me with vbulletin 4

http://mohitsharma.net/content/how-integrate-vbulletin-drupal

Hope this will helps

Thanks

Mohit Sharma

http://www.mohitsharma.net

Cray Flatline’s picture

Thanks a lot! Do you plan to port your patched module to Drupal 7?

it_mohit’s picture

I will do that But currently busy in some other project.

Thanks

Mohit Sharma

http://www.mohitsharma.net

edb’s picture

Status: Needs work » Needs review
FileSize
4.22 KB

Hey guys,

I have used the patch supplied in #8 and stripped out all the other modifications and left it with just the addition of VB4 support as requested. My tests have all been successful so far.

Once this patch gets approved I will start the port over to D7.

Ed

sun’s picture

Status: Needs review » Needs work

Thank you! This looks very promising! :)

diff --git a/drupalvb.admin-pages.inc b/drupalvb.admin-pages.inc
old mode 100644
new mode 100755
...
diff --git a/drupalvb.inc b/drupalvb.inc
old mode 100644
new mode 100755
diff --git a/drupalvb.inc.php b/drupalvb.inc.php
old mode 100644
new mode 100755
...
diff --git a/drupalvb.module b/drupalvb.module
old mode 100644
new mode 100755

Watch out - you changed the file permissions of all PHP files in the repo -- needs to be reverted.

+++ b/drupalvb.admin-pages.inc
@@ -20,12 +20,27 @@ function drupalvb_settings_integration() {
+  $form['drupalvb_settings']['drupalvb_version'] = array(

The first 'drupalvb_settings' key seems to be superfluous?

+++ b/drupalvb.admin-pages.inc
@@ -20,12 +20,27 @@ function drupalvb_settings_integration() {
+    '#description' => 'Select the version that most closely matches your vBulletin installation.',
+    '#options' => array(
+      '3.0' => t('3.0'),
+      '3.8' => t('3.8.4 PL2'),
+      '4.0' => t('4.0'),

I wonder whether we couldn't parse the vB version out of the actual vB files?

See #1367112: Determine vB version by parsing config.php - we may want to tackle that first.

+++ b/drupalvb.admin-pages.inc
@@ -20,12 +20,27 @@ function drupalvb_settings_integration() {
-    '#title' => t('vBulletin license number'),
+    '#title' => t('vBulletin license number(up to version 3.8.4 PL2) or cookie salt (version 3.8.4 PL2 up)'),

It would be cleaner to work with an if/else switch (by configured or automatically determined version), so this configuration UI element stays simple.

Ideally, we'd use PHP's version_compare() function to conditionally show the right label, description, and instructions.

According to #734226: COOKIE_SALT is no longer the license number since vB 3.8.4, this change is required for version 3.8.4+.

+++ b/drupalvb.inc.php
@@ -394,4 +391,16 @@ function drupalvb_htmlspecialchars($text) {
 }
+/**

There should be a single newline between functions.

+++ b/drupalvb.inc.php
@@ -394,4 +391,16 @@ function drupalvb_htmlspecialchars($text) {
+  // Version 4 began using an underscore following the prefix

Code comments should be terminated with a period (full-stop), always forming a complete sentence.

+++ b/drupalvb.module
@@ -470,6 +470,12 @@ function drupalvb_user_delete($account) {
 function drupalvb_login() {
   global $user;
+  // If request is coming from vb4 then set values to vb3 values for auth
+  if (variable_get('drupalvb_version', 3) >= 4) {

1) I wonder whether this needs to be dependent on the version, or whether we couldn't simply check whether the vb_login_* POST parameters are set, and if they are, automatically move them over?

It looks like those POST parameters did not exist in earlier versions (only in the administrative login form), so I guess this should be safe to do.

2) The inline code comment could be a bit more clear... e.g.,

"Starting from 4.x, vB uses 'vb_login_*' login form input names, whereas earlier versions used 'name' and 'pass'."

(Wrap at 80 chars)

3) There could be a single newline after global $user to increase readability.

edb’s picture

Thanks for detailed review! I will re-roll the patch once #1367112: Determine vB version by parsing config.php has been resolved.

edb’s picture

FileSize
4.13 KB

Re-rolled patch with comments from #40 and using functions implemented in #1367112: Determine vB version by parsing config.php.

edb’s picture

Status: Needs work » Needs review
sun’s picture

Status: Needs review » Needs work
+++ b/drupalvb.admin-pages.inc
@@ -19,13 +19,29 @@ function drupalvb_settings_integration() {
+  $form['drupalvb_version'] = array(
+    '#type' => 'markup',
+    '#value' => t('Detected vBulletin version: ') . "<b>".$vb_config['version']."</b>",
+  );

I'm not sure whether it is helpful to output the version on the settings page?

If you really think it is, then this element should be of #type 'item', having the label in #title, and the value in #value (and no manual bold formatting).

+++ b/drupalvb.admin-pages.inc
@@ -19,13 +19,29 @@ function drupalvb_settings_integration() {
+  //In version 3.8.4 vBulletin stopped using license number as salt for password
+  //and began using a constant named COOKIE_SALT.

There should be a space between // and the comment text.

See comment formatting standards: http://drupal.org/node/1354

+++ b/drupalvb.admin-pages.inc
@@ -19,13 +19,29 @@ function drupalvb_settings_integration() {
+  if(version_compare($vb_config['version'], '3.8.4', '>=')) {
...
+  } else {

1) There should be a space after the "if".

2) "else" is always written on a separate line.

Make sure to read http://drupal.org/coding-standards, please.

+++ b/drupalvb.admin-pages.inc
@@ -19,13 +19,29 @@ function drupalvb_settings_integration() {
+    $description = 'Enter your vBulletin cookie salt, which can be found defined in the includes/functions.php PHP file of vBulletin. This is required to generate proper session id hashes for cookies. Please note that this is not your customer number.';

In the cookie salt case,

1) weird: "found" and "defined"

2) "in the" should grammatically be followed by a subject; i.e., "in the file [xyz]"

3) when referring to paths and files in the UI, the file path should be wrapped in <code> tags, and the actual path should be a replacement token passed into t(), so translations of the string are not outdated when the path happens to change. I.e., in this case use:

t('[...] which can be found at the top of the vBulletin file @file. [...]', array(
  '@file' => 'includes/functions.php',
));

4) The note about the customer number seems to be unnecessary for the cookie salt case.

+++ b/drupalvb.admin-pages.inc
@@ -19,13 +19,29 @@ function drupalvb_settings_integration() {
+    $title = 'vBulletin license number';
...
+    '#title' => t($title),
...
+    '#description' => t($description),

Localizable UI strings are extracted by a code parser. It is required that the string value is passed as an argument to t().

I.e., the individual strings need to be wrapped in t(), not the variable.

+++ b/drupalvb.admin-pages.inc
@@ -19,13 +19,29 @@ function drupalvb_settings_integration() {
-    '#size' => 20,
...
+    '#size' => 60,

Any reason for changing the #size?

+++ b/drupalvb.module
@@ -471,6 +471,14 @@ function drupalvb_user_delete($account) {
+  //Starting from 4.x, vB uses 'vb_login_*' login form input names, whereas
+  //earlier versions used 'name' and 'pass'.

Missing space after //

+++ b/drupalvb.module
@@ -471,6 +471,14 @@ function drupalvb_user_delete($account) {
+  if(isset($_POST['vb_login_username']) && isset($_POST['vb_login_password'])) {

Missing space after "if".

+++ b/drupalvb.module
@@ -471,6 +471,14 @@ function drupalvb_user_delete($account) {
+  } ¶

Trailing white-space here.

edb’s picture

Status: Needs work » Needs review
FileSize
3.96 KB

Ah, sorry about all the style errors! I ran this patch through the coder module and it passed (apart from else if's should be one word, but that's another patch).

Heres the re-rolled patch :)

EDIT: Oh, the reason for the size of the textfield being changed to 60 is so the cookie salt key is completely visible and does not overflow the text box.

sun’s picture

Status: Needs review » Needs work

Thanks! Only minor remarks remain now. This looks ready to be committed afterwards.

That said, did you test whether this code actually works?

Lastly, in my old copy of 3.6.5, includes/functions.php contains a PHP expression for the value of the COOKIE_SALT constant. You're saying that in 3.8.4+, the constant holds a literal string value that users can copy?

+++ b/drupalvb.admin-pages.inc
@@ -19,13 +19,25 @@ function drupalvb_settings_integration() {
+  // In version 3.8.4 vBulletin stopped using license number as salt for password

Exceeds 80 chars now.

+++ b/drupalvb.admin-pages.inc
@@ -19,13 +19,25 @@ function drupalvb_settings_integration() {
+    $description = t('Enter your vBulletin cookie salt, which can be found in the file <code>@file

of vBulletin. This is required to generate proper session id hashes for cookies.', array('@file' => 'includes/functions.php'));

1) As this is a very long line, it is recommended to notate the replacement token parameters to t() as a multi-line array, so the arguments are easier to locate, read, and possibly adjust.

2) Was already bogus in the old code before, but "ID" should always be in all-uppercase.

+++ b/drupalvb.admin-pages.inc
@@ -19,13 +19,25 @@ function drupalvb_settings_integration() {
+    '#size' => 60,

60 is the default value defined by System module:
http://api.drupal.org/api/drupal/modules--system--system.module/function...

So I guess we can as well drop the explicit #size.

edb’s picture

Status: Needs work » Needs review
FileSize
3.96 KB

That said, did you test whether this code actually works?

This works just fine on my installation of VB4, unfortunately I do not have a version of VB lower than 3.8.4 so I cannot test that out - although I cannot see why any problems should arise from this patch (famous last words).

You're saying that in 3.8.4+, the constant holds a literal string value that users can copy?

Yes it is an alphanumeric string.

Thanks for all your feedback and help, re-rolled patch attached. I wasn't quite sure on the formatting you were expecting on the multi-line array, so feel free to change that before the commit.

sun’s picture

Status: Needs review » Fixed

Excellent, thanks!

Some last nits, which I'll fix before commit:

+++ b/drupalvb.admin-pages.inc
@@ -19,13 +19,26 @@ function drupalvb_settings_integration() {
+  // In version 3.8.4 vBulletin stopped using license number as salt for ¶

Trailing white-space.

btw, you may want to install and use Dreditor, which many in the Drupal community use to review patches.

+++ b/drupalvb.admin-pages.inc
@@ -19,13 +19,26 @@ function drupalvb_settings_integration() {
+    $description = t('Enter your vBulletin cookie salt, which can be found in the file <code>@file

of vBulletin. This is required to generate proper session ID hashes for cookies.', array(
+ '@file' => 'includes/functions.php'
+ ));

That's correctly formatted! :)

However, the Drupal coding standards ask for a trailing comma to be added to multi-line array elements even in cases where it's syntactically optional (i.e., the last element).

We have that rule to decrease the size of diffs and affected lines in possible follow-up patches, as you do not need to touch the preceding line in order to add a new array element.

--
Thanks for reporting, reviewing, and testing! Committed to 6.x-2.x.

A new development snapshot will be available within the next 12 hours. This improvement will be available in the next official release.

murias’s picture

Just started working on a vb drupal integration site. I was having various issues with previous submitted patches. I have tried and tested the Dec12 dev release 6.x-2.x and happily would like to report it is working beautifully. Thank you for the work done.

sun’s picture

Note that there's a relatively essential/important fix for the 6.x-2.x branch in the patch over at #982894: Redirect to originating page when logging in from vBulletin, which could use some extensive testing.

Apparently, some major improvements from 5.x-3.x were never forward-ported to D6.

Chiaki’s picture

So does this expansion also work with Drupal 7?

edb’s picture

I am currently in the process of porting this module to D7, follow #1106950: Port Drupal vB to Drupal 7 for updates on progress.

Status: Fixed » Closed (fixed)

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