Skip to:
Content

BuddyPress.org

Opened 2 years ago

Closed 16 months ago

#8687 closed defect (bug) (fixed)

Deprecated code is never loaded

Reported by: imath's profile imath Owned by: imath's profile imath
Milestone: 11.0.0 Priority: high
Severity: major Version: 2.7
Component: Core Keywords: has-patch
Cc:

Description (last modified by imath)

Since [11105], BuddyPress is no more loading deprecated code if you installed BuddyPress when BuddyPress version was upper than 2.7.

That's because the bp_add_option( '_bp_ignore_deprecated_code', false ); code was only added into the function to upgrade to 2.7.

You can reproduce the issue installing BP 9.2.0, than update to current stable and you'll see deprecated code is never loaded.

That's really annoying because, we can never be sure plugins stopped using deprecated code. We should at least make sure to load latest deprecated functions.

I believe we should take a decision about it, before our next major upgrade.

PS: working on codex/documentation update made me find this bug 😉.

Attachments (5)

8687.branch-10.0.patch (4.9 KB) - added by imath 23 months ago.
8687.trunk.patch (8.0 KB) - added by imath 23 months ago.
8687.patch (6.0 KB) - added by imath 21 months ago.
8687.2.patch (10.7 KB) - added by imath 21 months ago.
8687.3.patch (11.5 KB) - added by imath 20 months ago.

Download all attachments as: .zip

Change History (22)

#1 @imath
2 years ago

  • Description modified (diff)

This ticket was mentioned in Slack in #buddypress by imath. View the logs.


23 months ago

#3 @DJPaul
23 months ago

Good catch! I haven't verified the behaviour, but on the other hand, this has been around 6 years. Either there are low numbers of BuddyPress sites being updated, plugin maintainers are keeping up to date, we're moving stuff out that no-one uses, or I think we would have heard about a lot about fatal errors after upgrades.

Remember to maintain that deprecated files are only loaded for upgrades, not new installs.

Last edited 23 months ago by DJPaul (previous) (diff)

@imath
23 months ago

#4 @imath
23 months ago

  • Keywords has-patch added; needs-patch removed

Thanks a lot for your feedback @DJPaul 👍. I'm suggesting to only load deprecated functions when the BuddyPress version when it was first installed is lower or equals the list of previous BuddyPress major versions.

I've prepared 2 patches, one if we want to make this move in branch 10.0: the downside would be that people installing 10.3.0 would load deprecated functions as well as those upgrading. See 8687.branch-10.0.patch.

As we've waited for a bunch of versions, we can also wait for 11.0 and only use 8687.trunk.patch.

#5 @imath
23 months ago

Sometimes including deprecated code might cause issues, see #8702

This ticket was mentioned in Slack in #buddypress by imath. View the logs.


23 months ago

#7 @imath
23 months ago

  • Keywords needs-patch added; has-patch removed
  • Milestone changed from 10.3.0 to 11.0.0

After more thoughts about it, I think we should only load current version -2 versions deprecated code. Like if we are 11.0, then load 10.0 and 9.0 deprecated code.

Let's move this ticket to 11.0 to build a "risk free" 10.3.0 maintenance release.

@imath
21 months ago

#8 @imath
21 months ago

  • Keywords has-patch added; needs-patch removed

The above patch is changing the way we load deprecated functions.

When BuddyPress is installed, it stores a new option to save the initial BP Version. It uses this option to check if it's lower than the current version. If so it loads the deprecated functions of 2 last versions.

This means we keep what was put in place in 2.8.0: new installs won't load deprecated functions.

if we define( 'BP_IGNORE_DEPRECATED', false ); all deprecated functions are loaded.

This ticket was mentioned in Slack in #buddypress by imath. View the logs.


21 months ago

@imath
21 months ago

#10 @imath
21 months ago

I've just improved the patch (see 8687.2.patch:

  • If the previous version is current version - 1, it will load only the previous version deprecated functions.
  • If the previous version is <= current version - 2, it will load only the the last 2 versions deprecated functions.
  • In new installs or when define( 'BP_IGNORE_DEPRECATED', true );, no deprecated functions are loaded.
  • if define( 'BP_IGNORE_DEPRECATED', false );, all versions deprecated functions are loaded.

I've also included a PHPUnit test which also performs a check for us in case we forget to update the list of versions with deprecated functions checking the existing files inside src/bp-core/deprecated.

I believe this is the right way of dealing with this.

This ticket was mentioned in Slack in #buddypress by imath. View the logs.


20 months ago

@imath
20 months ago

#12 @imath
20 months ago

  • Keywords commit added; dev-feedback removed

I've just simulated an upgrade from 10.3.0 to 11.0.0-alpha and I can confirm this updated version loaded last 2 deprecated version functions.

I've updated the patch according to latest trunk version and added a check to avoid loading deprecated functions when the development version of BuddyPress is used.

I'll commit this last patch soon.

#13 @imath
20 months ago

  • Owner set to imath
  • Resolution set to fixed
  • Status changed from new to closed

In 13304:

Start using a new strategy about deprecated functions management

The main goal of this commit is to change the way we load the files containing deprecated functions. This files are located into the src/bp-core/deprecated/ directory and there's one file for each version of BuddyPress we created to store the deprecate functions.

We noticed that these files were not necessarily loaded although they should have. This case happens to all users who firstly installed BuddyPress when its version was upper than 2.7.

From now on, here's how deprecated code will eventually load into your BuddyPress installation:

  • First installs won't load deprecated code.
  • Updated installs will load the 2 latest BuddyPress versions deprecated code.
  • Defining the BP_IGNORE_DEPRECATED constant to true will change the 2 above points behavior ignoring any BuddyPress versions deprecated code.
  • Defining the BP_IGNORE_DEPRECATED constant to false will change the 2 first points behavior loading all BuddyPress versions deprecated code.

Props djpaul, dcavins.

Fixes #8687

This ticket was mentioned in Slack in #buddypress by imath. View the logs.


17 months ago

#15 @imath
17 months ago

  • Keywords needs-patch added; has-patch commit removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

As discussed during dev-chat, let's avoid double negative and change define( 'BP_IGNORE_DEPRECATED', false ) in favor of define( 'BP_LOAD_DEPRECATED', true )

This ticket was mentioned in PR #36 on buddypress/buddypress by @imath.


16 months ago
#16

  • Keywords has-patch added; needs-patch removed

Introduce the BP_LOAD_DEPRECATED constant

Setting this constant to true will force all deprecated code to load. This replaces the previous double negation approach where we needed to define BP_IGNORE_DEPRECATED to false to load all deprecated code.

Trac ticket: https://buddypress.trac.wordpress.org/ticket/8687

#17 @imath
16 months ago

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

In 13363:

Introduce the BP_LOAD_DEPRECATED constant

Setting this constant to true will force all deprecated code to load. This replaces the previous double negation approach where we needed to define BP_IGNORE_DEPRECATED to false to load all deprecated code.

props johnjamesjacoby

Closes https://github.com/buddypress/buddypress/pull/36
Fixes #8687

Note: See TracTickets for help on using tickets.