test(#3616606): cover the active updating skip path by Decipher · Pull Request #25 · Decipher/filefield_paths

Decipher · 2026-08-28T01:28:33Z

Refs https://www.drupal.org/project/filefield_paths/issues/3616606

Summary

Test-only. Adds coverage for the active_updating skip path, which nothing exercised before, and records that the reported bug does not reproduce.

Why there is no fix here

dshields reports that files are renamed on every save even with active_updating off, and attributes it to property_exists($entity, 'original') missing a magic property on Drupal 10. That mechanism does not hold on any supported core:

On 10.x, 11.0 and 11.1, original is a real dynamic property set by EntityStorageBase::doPreSave(), and property_exists() returns TRUE for those.
On 11.2+, original became magic, but the code takes the getOriginal() branch there, and core clears the original only after invoking the update hook.

Even the released isset($entity->original) form is sound on 11.2+, because core's __isset() special-cases original.

Verification

testFileIsNotMovedOnUpdateWhenActiveUpdatingIsOff performs the reported sequence: save with a file, change the path pattern, save again. The file stays put.
testFileIsMovedOnUpdateWhenActiveUpdatingIsOn is the control. Without it, the first test could pass simply because the hook never ran.

The issue needs the reporter's core version and entity type before it can go further.

Summary by CodeRabbit

Bug Fixes
- Improved file handling when processing entities across supported Drupal versions.
- Prevented errors caused by incompatible access to an entity’s original revision.
- Ensured file locations update correctly when active updating is enabled, while remaining unchanged when it is disabled.
Tests
- Added coverage for file path behavior during entity updates.

coderabbitai · 2026-08-28T01:28:43Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: a94d814f-0189-4925-af92-4b237e878588

📥 Commits

Reviewing files that changed from the base of the PR and between 509a32e and 6b97eb7.

📒 Files selected for processing (3)

rector.php
src/Hook/FileFieldPathsProcessFileLegacy.php
tests/src/Kernel/ActiveUpdatingTest.php

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

Walkthrough

The hook now resolves an entity’s original revision once with Drupal-version compatibility. Rector excludes this legacy access from automated rewriting. A new kernel test verifies file movement when active updating is disabled or enabled.

Changes

Active updating behavior

Layer / File(s)	Summary
Original entity compatibility `rector.php`, `src/Hook/FileFieldPathsProcessFileLegacy.php`	The hook resolves the original entity once through the Drupal 11.2+ accessor or the legacy property. Rector skips this file during `ReplaceEntityOriginalPropertyRector` processing.
Active updating kernel coverage `tests/src/Kernel/ActiveUpdatingTest.php`	The kernel test verifies that files remain in the original directory when active updating is off and move to the new path when it is on.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 6b97e

The PR adds focused coverage for the active-updating behavior and keeps existing files in place when updating is disabled without introducing new file-operation capabilities. No actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 3 files.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the primary change: adding test coverage for the active updating skip path described in the PR objectives.

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

Create stacked PR
Commit on current branch

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch feature/3616606-active-updating

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

codecov · 2026-08-28T01:36:38Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.39%. Comparing base (d23f47e) to head (75b9e99).
⚠️ Report is 4 commits behind head on 8.x-1.x.

Additional details and impacted files

@@           Coverage Diff            @@
##           8.x-1.x      #25   +/-   ##
========================================
  Coverage    83.39%   83.39%           
========================================
  Files           20       20           
  Lines          783      783           
========================================
  Hits           653      653           
  Misses         130      130

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

github-actions Bot assigned Decipher Aug 28, 2026

Decipher force-pushed the feature/3616606-active-updating branch from a5c2a2c to 6b97eb7 Compare August 28, 2026 02:34

fix(#3616606): detect the original entity without property_exists

b434b12

Decipher force-pushed the feature/3616606-active-updating branch from 6b97eb7 to e2afb3f Compare August 28, 2026 02:48

Decipher changed the base branch from 8.x-1.x to chore/renovate-automerge August 28, 2026 03:16

Decipher changed the base branch from chore/renovate-automerge to 8.x-1.x August 28, 2026 03:17

Decipher closed this Aug 28, 2026

Decipher reopened this Aug 28, 2026

test(#3616606): cover the active updating skip path

75b9e99

Decipher force-pushed the feature/3616606-active-updating branch from e2afb3f to 75b9e99 Compare August 28, 2026 03:38

Navigation Menu

test(#3616606): cover the active updating skip path - #25

test(#3616606): cover the active updating skip path#25
Decipher wants to merge 2 commits into
8.x-1.xfrom
feature/3616606-active-updating

Decipher commented Aug 28, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Aug 28, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

codecov Bot commented Aug 28, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

Decipher commented Aug 28, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why there is no fix here

Verification

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

codecov Bot commented Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Decipher commented Aug 28, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Aug 28, 2026 •

edited

Loading

codecov Bot commented Aug 28, 2026 •

edited

Loading