Remove excluded links from total broken count and fix setting defaults.#257
Conversation
…ble link checking and auto archiving wtih default value. This deals with the issue where we dont have a saved value due to choosing false and the default being true. We now have a optional default, so the wizard can be default to true and the settings to false.
WalkthroughThis pull request modifies link counting and settings initialization across the codebase. Dashboard_Page.php now excludes explicitly excluded links from the all_broken count. Two Settings methods (is_link_processing_enabled and add_own_links) have been updated to accept an optional $default_value parameter for controlling option retrieval defaults. The wizard step templates have been updated to pass true to these Settings methods when initializing checkbox states. Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
templates/admin/wizard/step-3.php (1)
67-67: Consider consistent default parameter usage.Line 39 explicitly passes
trueto initialize the main checkbox, but this checkbox on line 67 doesn't pass a parameter toSettings::is_link_processing_enabled(), so it will default tofalse. This inconsistency might not reflect the intended wizard UX.If the scheduled archiving checkbox should also default to checked in the wizard, consider:
-<input type="checkbox" name="iawmlf_wizard_recurring_backup" value="1" <?php checked( Settings::is_link_processing_enabled() ); ?> /> +<input type="checkbox" name="iawmlf_wizard_recurring_backup" value="1" <?php checked( Settings::is_link_processing_enabled( true ) ); ?> />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/Dashboard/Dashboard_Page.php(1 hunks)src/Settings/Settings.php(2 hunks)templates/admin/wizard/step-2.php(1 hunks)templates/admin/wizard/step-3.php(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: gin0115
Repo: a8cteam51/internet-archive-wayback-machine-link-fixer PR: 250
File: src/Settings/Settings.php:301-311
Timestamp: 2025-11-17T23:43:45.310Z
Learning: In the Internet Archive Wayback Machine Link Fixer plugin (file: src/Settings/Settings.php), the `iawmlf_add_own_content_to_wayback_machine` filter is intentionally applied after the production environment check in the `add_own_links()` method. This prevents the filter from overriding the hard block on archiving in non-production environments (staging, development, local), which is expected behavior to ensure staging/dev sites are never archived.
📚 Learning: 2025-11-17T23:43:45.310Z
Learnt from: gin0115
Repo: a8cteam51/internet-archive-wayback-machine-link-fixer PR: 250
File: src/Settings/Settings.php:301-311
Timestamp: 2025-11-17T23:43:45.310Z
Learning: In the Internet Archive Wayback Machine Link Fixer plugin (file: src/Settings/Settings.php), the `iawmlf_add_own_content_to_wayback_machine` filter is intentionally applied after the production environment check in the `add_own_links()` method. This prevents the filter from overriding the hard block on archiving in non-production environments (staging, development, local), which is expected behavior to ensure staging/dev sites are never archived.
Applied to files:
templates/admin/wizard/step-2.phptemplates/admin/wizard/step-3.php
🧬 Code graph analysis (3)
src/Dashboard/Dashboard_Page.php (1)
src/Link/Link.php (1)
is_broken(180-182)
templates/admin/wizard/step-2.php (1)
src/Settings/Settings.php (2)
Settings(22-464)is_link_processing_enabled(82-84)
templates/admin/wizard/step-3.php (1)
src/Settings/Settings.php (2)
Settings(22-464)add_own_links(304-316)
🔇 Additional comments (3)
src/Dashboard/Dashboard_Page.php (1)
209-211: LGTM! Correctly excludes explicitly excluded links from broken count.This change ensures that links marked as excluded are not counted in the all_broken statistics, which aligns with the PR objective and is consistent with the filtering already applied to redirected_broken links on line 213.
templates/admin/wizard/step-3.php (1)
39-39: LGTM! Explicit default ensures checkbox is checked in wizard.Passing
trueensures the checkbox appears checked by default for new users going through the wizard setup, which aligns with the intended onboarding experience.templates/admin/wizard/step-2.php (1)
39-39: LGTM! Consistent with wizard UX pattern.Explicitly passing
trueensures the Link Fixer checkbox appears checked by default during initial setup, matching the pattern used in step-3.php for the Auto Archiver checkbox.
| * @param boolean $default_value Optional default value if not set. Default false. | ||
| * | ||
| * @return boolean | ||
| */ | ||
| public static function is_link_processing_enabled(): bool { | ||
| return (bool) get_option( self::PROCESS_LINKS, true ); | ||
| public static function is_link_processing_enabled( bool $default_value = false ): bool { | ||
| return (bool) get_option( self::PROCESS_LINKS, $default_value ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the breaking change impact on existing installations.
The default value for these methods changed from true to false. For existing installations where these options were never explicitly set (relying on the implicit true default), these features will now be disabled after upgrade.
Impact example: should_scan_existing_posts() (line 203) calls is_link_processing_enabled() without arguments, so installations that never saved this setting will see scanning disabled.
Run the following script to check all call sites and assess impact:
Consider either:
- Adding a migration that explicitly sets these options to
truefor existing installations - Documenting this as a breaking change in release notes
- Changing the parameter default to
trueto maintain backward compatibility and requiring explicitfalsewhen that's desired
Based on learnings
Also applies to: 300-316
🏁 Script executed:
#!/bin/bash
# Find all calls to is_link_processing_enabled and add_own_links to assess default value usage
echo "=== Calls to is_link_processing_enabled ==="
rg -n -B2 -A2 'is_link_processing_enabled\(' --type=php
echo ""
echo "=== Calls to add_own_links ==="
rg -n -B2 -A2 'add_own_links\(' --type=phpLength of output: 11590
Verify database migration and initialization to confirm breaking change handling.
The script confirms a significant breaking change: most call sites (13 out of 14 for is_link_processing_enabled() and 11 out of 12 for add_own_links()) omit the argument and will now default to false instead of true. The wizard templates explicitly pass true, indicating developers expected true as the default.
This affects critical functionality checks in:
should_scan_existing_posts()(line 203)WP_Post_Controllerlink processing (line 88)WP_Post_Controllerown links (line 116)Scan_Own_Posts_Event(line 64)- Dashboard visibility and data gathering
For existing installations where these options were never explicitly saved, all features will become disabled after upgrade. Before finalizing, I need to check whether a migration or initialization handles setting these options to true for existing sites.
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for database migration, plugin activation hooks, or initialization logic
# that might set default values for PROCESS_LINKS or ALLOW_OWN_CONTENT_SUBMISSIONS
echo "=== Searching for activation/migration hooks ==="
rg -n 'register_activation_hook|add_action.*activate' --type=php
echo ""
echo "=== Searching for option initialization ==="
rg -n 'add_option.*PROCESS_LINKS|add_option.*ALLOW_OWN_CONTENT_SUBMISSIONS' --type=php
echo ""
echo "=== Checking Settings.php for constructor or static initialization ==="
rg -n -A10 'class Settings' --type=php | head -50
Length of output: 1829
---
🏁 Script executed:
```shell
#!/bin/bash
# Find and examine the activation hook function
echo "=== Searching for iawmlf_activate function ==="
rg -n -A20 'function iawmlf_activate' --type=php
echo ""
echo "=== Also check for any update/migration functions ==="
rg -n -A10 'function.*migration|function.*update' --type=php | head -80
Length of output: 6430
---
🏁 Script executed:
```shell
#!/bin/bash
# Find the Migrations class and examine up() method
echo "=== Searching for Migrations class ==="
rg -n 'class Migrations' --type=php
echo ""
echo "=== Find and examine the up() method ==="
rg -n -A30 'public static function up' --type=php | grep -A30 'Migrations' | head -60
Length of output: 2028
---
🏁 Script executed:
```shell
#!/bin/bash
# Find migrations array definition and migration classes
echo "=== Search for \$migrations array ==="
rg -n 'self::\$migrations|private static.*migrations' --type=php -A5
echo ""
echo "=== List all migration files ==="
fd -e php 'Migration' --type f
echo ""
echo "=== Check individual migration up() methods for option setting ==="
rg -n 'add_option|update_option' src/Migration/ --type=php -B2 -A2
Length of output: 1157
---
🏁 Script executed:
```shell
#!/bin/bash
# Examine Migrations.php for $migrations array definition
echo "=== Full Migrations.php file ==="
cat src/Migration/Migrations.php
echo ""
echo "=== Migration_1.php file ==="
cat migrations/Migration_1.php
Length of output: 3614
Address the unmitigated breaking change in default behavior.
Verification confirms the breaking change is real and unhandled by migrations or initialization:
self::$migrationsis empty (no migrations registered)Migration_1.phponly creates the database table—it does NOT initializePROCESS_LINKSorALLOW_OWN_CONTENT_SUBMISSIONS- No activation hook initializes these options to
true
For existing installations where these options were never explicitly set and fresh installations, get_option() will now return false instead of the old implicit true, silently disabling link processing, scanning, and auto-archiving for all 24+ call sites.
Fix required:
- Add a migration that sets these options to
truefor existing installations, OR - Change default parameter from
falsetotrueto maintain backward compatibility, OR - Document this as a breaking change with migration instructions in release notes
Lines 82 and 304 in src/Settings/Settings.php need updating.
🤖 Prompt for AI Agents
In src/Settings/Settings.php around lines 78-84 (and also update the similar
signature at line ~304), the method default causes a breaking behavioral change
by returning false when options were previously implicitly true; update the
method signatures to use true as the default parameter to preserve backward
compatibility, and add a new migration that sets PROCESS_LINKS and
ALLOW_OWN_CONTENT_SUBMISSIONS to true for existing installations (or
alternatively document the breaking change in release notes if you choose not to
migrate); ensure the migration is registered in self::$migrations so it runs on
upgrade.
1 participant
Changes proposed in this Pull Request
This pull request makes improvements to how link processing and auto archiving options are handled in the plugin, focusing on more flexible default values and more accurate statistics. The changes also update the wizard UI to ensure the correct default behaviors are reflected.
Settings and Option Handling Improvements:
Settings::is_link_processing_enabledandSettings::add_own_linksto accept an optional$default_valueparameter, allowing callers to specify a default if the option is not set, instead of always usingtrue. [1] [2]Wizard UI Updates:
trueas the default value toSettings::is_link_processing_enabledandSettings::add_own_links, ensuring the UI reflects the intended default behavior. (templates/admin/wizard/step-2.php, [1];templates/admin/wizard/step-3.php, [2]Statistics Calculation Accuracy:
Dashboard_Page.phpto exclude links marked as "excluded" from the broken links count, resulting in more accurate reporting. (src/Dashboard/Dashboard_Page.php, src/Dashboard/Dashboard_Page.phpL209-R209)Testing instructions
Mentions #
Summary by CodeRabbit
Bug Fixes
Changes