Skip to:

Opened 13 years ago

Closed 13 years ago

#3153 closed defect (bug) (fixed)

Upgrades from 1.2 should be recognized as upgrades

Reported by: boonebgorges's profile boonebgorges Owned by: cnorris23's profile cnorris23
Milestone: 1.5 Priority: critical
Severity: Version: 1.5
Component: Core Keywords: has-patch


Currently, if you upgrade from an existing 1.2.x install of BP to the trunk version, the install wizard calls it a "new installation", since you don't have bp-pages and some other metadata that BP uses to recognize existing 1.3 installs. I think that, in this case, it actually *is* like a new install in a sense (since bp-pages, etc need to be created), but the language should probably be different, so as not to freak out users. Also, there's no need for a Theme or Permalinks step in this case (though it doesn't hurt anything to have them).

Attachments (7)

aperture-science.patch (1.4 KB) - added by DJPaul 13 years ago.
3153.002.diff (6.7 KB) - added by cnorris23 13 years ago.
3153.003.diff (2.3 KB) - added by cnorris23 13 years ago.
3153.004.diff (12.7 KB) - added by cnorris23 13 years ago.
3153.005.diff (13.2 KB) - added by DJPaul 13 years ago.
3153.005.2.diff (13.2 KB) - added by DJPaul 13 years ago.
3153.006.diff (14.4 KB) - added by cnorris23 13 years ago.

Download all attachments as: .zip

Change History (26)

#1 @cnorris23
13 years ago

This already appears to be done. When I perform an upgrade it uses the term "update," there's no theme or permalinks options.

#2 @DJPaul
13 years ago

  • Priority changed from major to critical

Sort of, but there's an issue in the installer when you upgrade from BP 1.2.x to 1.3. On the "2. Pages" screen, it doesn't figure out which components you had active in 1.2.x.

Attached patch is a pretty rough attempt at how to sort this, but I think if there's a flag we could set that says "this is an upgrade from 1.2" vs "this is an upgrade from another version of 1.3", it'll avoid this stuff getting run twice if we don't clear up the deprecated DB key.

#3 @cnorris23
13 years ago

If it's an update, we can just snag $bp->active_components. I wrote a patch and have tested multiple times, and everything came out alright.

#4 @cnorris23
13 years ago

Something like this.

13 years ago

#5 @cnorris23
13 years ago

  • Keywords has-patch added

Actually here's the original patch I was talking about. It's kind of big because it turned out to be a big rabbit hole. Here's what the patch does.

1) Typo fixes
2) Steal from the $bp global to set $bp_wizard global values so we don't duplicate work that was just done
3) Replace the $bp global with the $bp_wizard global where applicable since it's smaller
4) Fixes thee active_components routine in bp-core-loader.php to account for a previous installation that had no deactivated components.
5) Use $bp->active_components where possible so we don't have to duplicate work and we get the benefit of already having deactivated components accounted for
6) Update database version numbers in two spots that kept 1.2.x version from being updated properly
7) Whitespace cleanup

This patch should actually fix #3154 also.

13 years ago

#6 @boonebgorges
13 years ago

Thanks for the patches, cnorris23. Just to be clear - these patches are intended to be applied together, right? It looks like they do different things.

#7 @cnorris23
13 years ago

Well 3153.003.diff is meant to be the "final" patch, so you can ignore 3153.002.diff.

#8 @DJPaul
13 years ago

  • Keywords needs-patch added; has-patch removed

3153.003.diff doesn't work, and makes BuddyPress upgrader seem to think I have the bp-blogs component enabled (I don't, it's non-ms).

#9 @cnorris23
13 years ago

My patch isn't what's causing the issue with blogs being active. It just exposed the issue. The problem is that the optional_components global doesn't factor in ms vs. non-ms. Other than that, what else isn't working for you? I'll be happy to work up a new patch fixing the optional_components issue and anything else.

#10 @DJPaul
13 years ago

Great. Well, to clarify so that we're on the same wavelength: this ticket as I understand it is about the problem when you upgrade from BP 1.2.x to 1.3 and, on the "2. Pages" screen, it doesn't figure out which components you had active (or disabled) in 1.2.x. It should pre-set the checkboxes appropriately.

Seeing the blogs option on non-ms was unexpected, and certainly that can be addressed in this patch, too.

#11 @cnorris23
13 years ago

I didn't test it before writing here, so thanks for the clarification. Yeah, I add a 'default to all' because new installs had nothing checked. Weird that upgrades aren't selecting previously activated components properly. They were when I patched. I must have missed a change that would have broken that patch. I'll dig into it and get something in the next day or two.

#12 @boonebgorges
13 years ago

  • Owner set to cnorris23
  • Status changed from new to assigned

Thanks, cnorris23. Assigning to you for the moment.

#13 @DJPaul
13 years ago

See also #3154

#14 @cnorris23
13 years ago

  • Keywords has-patch added; needs-patch removed

So... I don't know what I was thinking before, but 3153.002.diff and 3153.003.diff should have been combined to make one meaty patch. @DJPaul, now that there's not some missing code, the patch should work now :)

13 years ago

13 years ago

#15 @DJPaul
13 years ago

Tweaked code style a bit. Question: What's up with renaming bp->maintenence_mode to bp_wizard->setup_type? What about the remaining bp->maintenence_mode references in BP?

13 years ago

#16 @cnorris23
13 years ago

The idea behind switch from bp->maintenance_mode to bp_wizard->setup_type, was for consistency. Since we're doing setup, might as well use the bp_wizard global, and also, while it may be negligible performance wise, it is a smaller global than bp. As to why the others didn't get changed, at the moment, I couldn't tell you, because I first did the patch two months ago. I'll definitely look into it tonight when I've left my day job. Otherwise, did it work this time?

#17 @DJPaul
13 years ago

Not sure; wasn't able to get far enough to test. On an upgrade, we're getting two BuddyPress menus added in wp-admin (one is the upgrade, the other is the regular items). Also we're getting a number of PHP warnings:

[16-Jun-2011 20:40:16] PHP Notice: Undefined index: bp-xprofile-base-group-name in /Users/paul/Sites/ on line 73
[16-Jun-2011 20:40:16] PHP Notice: Undefined index: bp-xprofile-fullname-field-name in /Users/paul/Sites/ on line 74
[16-Jun-2011 20:40:16] PHP Notice: Undefined index: bp-xprofile-fullname-field-name in /Users/paul/Sites/ on line 395


[16-Jun-2011 20:40:16] WordPress database error Table 'wordpress.wp_bp_user_blogs' doesn't exist for query SELECT COUNT(DISTINCT b.blog_id) FROM wp_bp_user_blogs b LEFT JOIN wp_blogs wb ON b.blog_id = wb.blog_id WHERE wb.deleted = 0 AND wb.spam = 0 AND wb.mature = 0 AND wb.archived = '0' AND user_id = 1 made by require_once, require_once, require_once, require_once, do_action, call_user_func_array, bp_init, do_action, call_user_func_array, bp_setup_nav, do_action, call_user_func_array, BP_Blogs_Component->_setup_nav, bp_blogs_total_blogs_for_user, BP_Blogs_Blog->total_blog_count_for_user

(I appreciate those may not have been caused by this patch, but I'm putting them here so we don't overlook them).

#18 @cnorris23
13 years ago

Okay, I've fixed the double admin menu issue. The other PHP notices I can't replicate. Also, the new patch fixes an issue where updates weren't saving the active components to the database because they skip the step_components_save() stage.

13 years ago

#19 @boonebgorges
13 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [4556]) Ensures that upgrades from BP 1.2.x are recognized as upgrades, and previous component settings are saved. Fixes #3153. Props cnorris23

Note: See TracTickets for help on using tickets.