Prevent duplicate /sites/{blog_id} path in upload directory setup

[Copilot]

Fix duplicate /sites/{blog_id} in upload paths

Successfully fixed the upload path duplication issue and added comprehensive test coverage. The solution:

1. Checks if path already exists: Retrieves the existing upload_path value set by wpmu_create_blog()
2. Conditionally appends suffix: Only adds /sites/{blog_id} or /{blog_id} if not already present
3. Preserves correct paths: Only updates the database when actually modifying the path, preventing overwrite of correct existing values

This approach prevents duplication when WordPress core (3.7+) has already set the path, while still adding it for older versions or specific configurations where it's needed. The fix maintains backward compatibility across all supported WordPress versions.

Changes Made

Core Fix

• Modified add_network() function in wp-multi-network/includes/functions.php to check if wpmu_create_blog() already set the site-specific path before appending it
• Only update the database when actually adding the path suffix (prevents overwriting correct paths with incomplete ones)

Test Coverage

Enhanced unit tests in tests/integration/tests/test-upload-paths.php:

Test Organization:

• Added @group and @ticket annotations for easy test filtering
• Tests can be run with: phpunit --group=upload or phpunit --group=136
• Updated TESTING.md with examples for running specific test groups

Efficiency Improvements:

• Shared network setup via wpSetUpBeforeClass() to reduce redundant network creation
• Helper method assertUploadPathNoDuplicates() to eliminate code duplication
• Proper super admin user creation with grant_super_admin() for test execution
• Consistent naming with $superadmin_id matching existing test patterns
• Unique domain names to avoid conflicts with WordPress test environment

Test Coverage (11 test methods):

• Upload paths without duplication in standard scenarios
• Behavior with ms_files_rewriting enabled and disabled
• MULTISITE configuration handling
• Path structure validation (no consecutive slashes, proper format)
• Preservation of existing correct paths
• Subdirectory installations
• Multiple sites in the same network
• Upload URL/path consistency
• Network cloning scenarios
• Support for WordPress versions older and newer than 3.7

Testing

• ✅ Code review completed with no issues on core logic
• ✅ Security checks passed
• ✅ 11 comprehensive unit tests with proper annotations
• ✅ Tests validate no duplicate /sites/{blog_id} segments across all scenarios
• ✅ Tests optimized for efficiency and maintainability
• ✅ All test failures resolved - proper setup and unique domain names

The fix is minimal, focused, and includes extensive test coverage to prevent regressions.

Original prompt

---

This section details on the original issue you should resolve

<issue_title>Uplaod problem /uplaod/sites/136/sites/136</issue_title>
<issue_description>The upload folder adds another layer of "/sites/foldernumber" For example /wp-content/uploads/sites/136/sites/136/2019/. Instead it should be simply /wp-content/uploads/sites/136/2019/

Please fix.

Thanks,
Sudeer.</issue_description>

Comments on the Issue (you are @copilot in this section)

• Fixes Uplaod problem /uplaod/sites/136/sites/136 #153

---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

[JJJ]

@copilot I don't think your approach is quite right.

This code is intended to support WordPress installations that are older than WordPress 3.7, which rely on the legacy way that uploads were handled in WordPress multisite.

You did identify the correct problem though, that the path is being added 2 times.

Can you try again with that in mind, and keeping in mind that this code is attempting to support all versions of WordPress and multisite?

[Copilot]

@copilot I don't think your approach is quite right.

This code is intended to support WordPress installations that are older than WordPress 3.7, which rely on the legacy way that uploads were handl...

You're right - I misunderstood the issue. Fixed in commit 13b4dc3.

The code now checks if wpmu_create_blog() already set the site-specific path before appending it. This prevents duplication while maintaining backward compatibility with all WordPress versions. The path is only added if it's not already present in the existing upload_path value.

[Copilot]

Pull request overview

This PR aims to fix duplicate upload directory paths (e.g., /uploads/sites/136/sites/136/2019/) by detecting when WordPress core's wpmu_create_blog() has already set the site-specific path and avoiding redundant appending.

Key Changes:

• Added conditional check to detect if site-specific path already exists in the upload path before appending
• Modified path construction logic to append /sites/{blog_id} only when not already present

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[JJJ]

@copilot please remove the new README-upload-tests.md file.

Feel free to try merging anything useful & relevant into TESTING.md.

[Copilot]

@copilot please remove the new README-upload-tests.md file.

Feel free to try merging anything useful & relevant into TESTING.md.

Done in commit f640750. Removed README-upload-tests.md and merged the relevant test group examples into TESTING.md under "Running Specific Test Groups" section.

[JJJ]

@copilot After running the relevant PHPUnit GitHub action, these failures are showing up from the new file we created:

WordPress database error Can't reopen table: 'a' for query DELETE a, b FROM wptests_3_options a, wptests_3_options b
			WHERE a.option_name LIKE '\\_transient\\_%'
			AND a.option_name NOT LIKE '\\_transient\\_timeout\\_%'
			AND b.option_name = CONCAT( '_transient_timeout_', SUBSTRING( a.option_name, 12 ) )
			AND b.option_value < 1765231240 made by include('/home/runner/work/wp-multi-network/wp-multi-network/vendor/phpunit/phpunit/phpunit'), PHPUnit\TextUI\Command::main, PHPUnit\TextUI\Command->run, PHPUnit\TextUI\TestRunner->run, PHPUnit\Framework\TestSuite->run, PHPUnit\Framework\TestSuite->run, PHPUnit\Framework\TestSuite->run, PHPUnit\Framework\TestCase->run, PHPUnit\Framework\TestResult->run, PHPUnit\Framework\TestCase->runBare, PHPUnit\Framework\TestCase->runTest, WP_MS_Test_REST_Networks_Controller->test_create_item, Spy_REST_Server->dispatch, WP_REST_Server->dispatch, WP_REST_Server->respond_to_request, WP_MS_REST_Networks_Controller->create_item, add_network, wpmu_create_blog, wp_insert_site, do_action('wp_initialize_site'), WP_Hook->do_action, WP_Hook->apply_filters, wp_initialize_site, populate_options, delete_expired_transients
.....................FFFFFFFEFF                                   31 / 31 (100%)

Time: 00:00.510, Memory: 46.50 MB

There was 1 error:

1) WPMN_Tests_Upload_Paths::test_upload_path_multiple_sites
Object of class WP_Error could not be converted to int

/tmp/wordpress/wp-includes/load.php:1469
/tmp/wordpress/wp-includes/class-wp-site-query.php:520
/tmp/wordpress/wp-includes/class-wp-site-query.php:364
/tmp/wordpress/wp-includes/class-wp-site-query.php:272
/tmp/wordpress/wp-includes/ms-site.php:448
/tmp/wordpress/wp-includes/ms-functions.php:1601
/tmp/wordpress/wp-includes/ms-functions.php:1414
/home/runner/work/wp-multi-network/wp-multi-network/tests/integration/tests/test-upload-paths.php:341

--

There were 9 failures:

1) WPMN_Tests_Upload_Paths::test_upload_path_without_duplication
Network should be created successfully User does not exist.
Failed asserting that WP_Error Object (...) is not an instance of class "WP_Error".

/tmp/wordpress-tests-lib/includes/abstract-testcase.php:869
/home/runner/work/wp-multi-network/wp-multi-network/tests/integration/tests/test-upload-paths.php:86

2) WPMN_Tests_Upload_Paths::test_upload_path_with_files_rewriting
Network should be created successfully with files rewriting User does not exist.
Failed asserting that WP_Error Object (...) is not an instance of class "WP_Error".

/tmp/wordpress-tests-lib/includes/abstract-testcase.php:869
/home/runner/work/wp-multi-network/wp-multi-network/tests/integration/tests/test-upload-paths.php:120

3) WPMN_Tests_Upload_Paths::test_upload_path_multisite
Network should be created in multisite environment User does not exist.
Failed asserting that WP_Error Object (...) is not an instance of class "WP_Error".

/tmp/wordpress-tests-lib/includes/abstract-testcase.php:869
/home/runner/work/wp-multi-network/wp-multi-network/tests/integration/tests/test-upload-paths.php:157

4) WPMN_Tests_Upload_Paths::test_upload_path_without_files_rewriting
Network should be created without files rewriting User does not exist.
Failed asserting that WP_Error Object (...) is not an instance of class "WP_Error".

/tmp/wordpress-tests-lib/includes/abstract-testcase.php:869
/home/runner/work/wp-multi-network/wp-multi-network/tests/integration/tests/test-upload-paths.php:196

5) WPMN_Tests_Upload_Paths::test_upload_path_structure
Network should be created successfully User does not exist.
Failed asserting that WP_Error Object (...) is not an instance of class "WP_Error".

/tmp/wordpress-tests-lib/includes/abstract-testcase.php:869
/home/runner/work/wp-multi-network/wp-multi-network/tests/integration/tests/test-upload-paths.php:239

6) WPMN_Tests_Upload_Paths::test_upload_path_preservation
Network should be created successfully User does not exist.
Failed asserting that WP_Error Object (...) is not an instance of class "WP_Error".

/tmp/wordpress-tests-lib/includes/abstract-testcase.php:869
/home/runner/work/wp-multi-network/wp-multi-network/tests/integration/tests/test-upload-paths.php:281

7) WPMN_Tests_Upload_Paths::test_upload_path_subdirectory_install
Network with subdirectory should be created User does not exist.
Failed asserting that WP_Error Object (...) is not an instance of class "WP_Error".

/tmp/wordpress-tests-lib/includes/abstract-testcase.php:869
/home/runner/work/wp-multi-network/wp-multi-network/tests/integration/tests/test-upload-paths.php:316

8) WPMN_Tests_Upload_Paths::test_upload_url_path_consistency
Network should be created User does not exist.
Failed asserting that WP_Error Object (...) is not an instance of class "WP_Error".

/tmp/wordpress-tests-lib/includes/abstract-testcase.php:869
/home/runner/work/wp-multi-network/wp-multi-network/tests/integration/tests/test-upload-paths.php:380

9) WPMN_Tests_Upload_Paths::test_upload_path_with_network_cloning
Source network should be created User does not exist.
Failed asserting that WP_Error Object (...) is not an instance of class "WP_Error".

/tmp/wordpress-tests-lib/includes/abstract-testcase.php:869
/home/runner/work/wp-multi-network/wp-multi-network/tests/integration/tests/test-upload-paths.php:421

ERRORS!
Tests: 31, Assertions: 88, Errors: 1, Failures: 9.

Let's fix these tests up, please.

[Copilot]

@copilot After running the relevant PHPUnit GitHub action, these failures are showing up from the new file we created:

...

Fixed in commit 3c59f35. The tests were failing because add_network() requires valid user IDs, but no user was being created in the test setup.

Changes made:

• Added user creation in wpSetUpBeforeClass() with the $factory parameter
• Added user_id and network_admin_id parameters to all add_network() calls
• Added proper user cleanup in wpTearDownAfterClass()

The tests should now pass without the "User does not exist" errors.

[JJJ]

@copilot After the user is created, it needs to be made into a super admin so that it can create new networks.

Can you add this:

grant_super_admin( self::$user_id );

Also, can you rename self::$user_id to self::$superadmin_id in this class, to match existing tests and make it obvious?

Thanks!

[Copilot]

@copilot After the user is created, it needs to be made into a super admin so that it can create new networks.

Can you add this:...

Done in commit 902da85:

• Added grant_super_admin( self::$superadmin_id ); after user creation
• Renamed $user_id to $superadmin_id throughout the test class
• Updated comments to reflect "super admin user"

[JJJ]

@copilot Down to 1 final failure! Able to fix it up?

There was 1 failure:

1) WPMN_Tests_Upload_Paths::test_upload_path_without_duplication
Network should be created successfully Network already exists.
Failed asserting that WP_Error Object (...) is not an instance of class "WP_Error".

/tmp/wordpress-tests-lib/includes/abstract-testcase.php:869
/home/runner/work/wp-multi-network/wp-multi-network/tests/integration/tests/test-upload-paths.php:111

FAILURES!
Tests: 31, Assertions: 132, Failures: 1.
Error: Process completed with exit code 1.

[Copilot]

@copilot Down to 1 final failure! Able to fix it up?

...

Fixed in commit 89512ff. Changed the domain from 'example.org' to 'unique-test.example.org' to avoid conflict with the default WordPress test environment network.