♻️ refactor(features): remove feature hooks left without implementers - #4750
Open
mauretto78 wants to merge 3 commits into
Open
♻️ refactor(features): remove feature hooks left without implementers#4750mauretto78 wants to merge 3 commits into
mauretto78 wants to merge 3 commits into
Conversation
ProjectUrlsEvent, FilterRevisionChangeNotificationListEvent and AnalysisBeforeMTGetContributionEvent were dispatched only for the airbnb plugin, whose handlers were respectively a no-op, inert without a config key, and superseded by the per-project enable_mt_analysis setting. Removing the ProjectUrls dispatch also leaves fillUrls() with no use for the FeatureSet it was handed, so the parameter goes with it.
…aseFeature No plugin or core code ever called it. Plugins that ship static assets register them through their own decorators instead.
Pairs with matecat/airbnb_plugin#122, which removes the plugin-side handlers for the three hooks deleted here. That PR must merge first so this commit is reachable from the plugin's master branch.
🧪 Test-Guard Report✅ PASS — All changed source files have adequate test coverage. Coverage Analysis: ❌ FAILChanged lines: 100.0% covered (threshold: 80%) 📋 9 files: 3 ❌ fail, 6 ✅ pass
Test File Matching: ❌ FAILFile matching: 4 pass, 2 warning, 3 fail 📋 9 files: 3 ❌ fail, 2
|
| File | Verdict | Reason |
|---|---|---|
lib/Controller/API/V2/UrlsController.php |
✅ pass | Test file modified in PR: tests/unit/Core/Controllers/UrlsControllerTest.php |
lib/Model/FeaturesBase/Hook/Event/Filter/AnalysisBeforeMTGetContributionEvent.php |
❌ fail | No matching test file found |
lib/Model/FeaturesBase/Hook/Event/Filter/FilterRevisionChangeNotificationListEvent.php |
❌ fail | No matching test file found |
lib/Model/FeaturesBase/Hook/Event/Filter/ProjectUrlsEvent.php |
❌ fail | No matching test file found |
lib/Plugins/Features/BaseFeature.php |
✅ pass | Test file modified in PR: tests/unit/Core/Plugins/Features/BaseFeatureTest.php |
lib/Plugins/Features/ReviewExtended/ReviewedWordCountModel.php |
Test file exists (tests/unit/Core/Features/ReviewExtended/ReviewedWordCountModelTest.php) but was not modified in this PR | |
lib/Utils/AsyncTasks/Workers/Analysis/TMAnalysis/Service/EngineService.php |
Test file exists (tests/unit/Core/Workers/TMAnalysisV2/EngineServiceTest.php) but was not modified in this PR | |
lib/View/API/V2/Json/Job.php |
✅ pass | Test file modified in PR: tests/unit/Core/View/API/V2/Json/JobTest.php |
lib/View/API/V3/Json/Chunk.php |
✅ pass | Test file modified in PR: tests/unit/Core/View/API/V3/Json/ChunkTest.php |
Per-File Evaluation: ✅ PASS
All files resolved by deterministic shortcuts.
📋 9 files: 2 ✅ pass, 7 ⏭️ skip
| File | Verdict | Reason |
|---|---|---|
lib/Controller/API/V2/UrlsController.php |
⏭️ skip | shortcut → in coverage report, but no executable lines changed |
lib/Model/FeaturesBase/Hook/Event/Filter/AnalysisBeforeMTGetContributionEvent.php |
⏭️ skip | shortcut → deleted file |
lib/Model/FeaturesBase/Hook/Event/Filter/FilterRevisionChangeNotificationListEvent.php |
⏭️ skip | shortcut → deleted file |
lib/Model/FeaturesBase/Hook/Event/Filter/ProjectUrlsEvent.php |
⏭️ skip | shortcut → deleted file |
lib/Plugins/Features/BaseFeature.php |
⏭️ skip | shortcut → in coverage report, but no executable lines changed |
lib/Plugins/Features/ReviewExtended/ReviewedWordCountModel.php |
⏭️ skip | shortcut → in coverage report, but no executable lines changed |
lib/Utils/AsyncTasks/Workers/Analysis/TMAnalysis/Service/EngineService.php |
⏭️ skip | shortcut → in coverage report, but no executable lines changed |
lib/View/API/V2/Json/Job.php |
✅ pass | shortcut → coverage 100% ≥ 80% |
lib/View/API/V3/Json/Chunk.php |
✅ pass | shortcut → coverage 100% ≥ 80% |
Result: ✅ PASS
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three feature-hook event classes were dispatched from core but implemented only by the Airbnb plugin,
whose handlers were a no-op, inert, and superseded by a core setting respectively. With the plugin
handlers gone (matecat/airbnb_plugin#122) the events, their dispatch sites and the plumbing that fed
them have no remaining purpose, so they are removed here.
Also drops
BaseFeature::getBuildFiles(), which no plugin or core code has ever called.Type
feat— new user-facing featurefix— bug fixrefactor— restructure without behavior changechore— build, deps, config, docsperf— performance improvementtest— test coverageChanges
lib/Model/FeaturesBase/Hook/Event/Filter/ProjectUrlsEvent.phplib/Model/FeaturesBase/Hook/Event/Filter/FilterRevisionChangeNotificationListEvent.phplib/Model/FeaturesBase/Hook/Event/Filter/AnalysisBeforeMTGetContributionEvent.phpenable_mt_analysissettinglib/Controller/API/V2/UrlsController.php,lib/View/API/V2/Json/Job.phpProjectUrlsdispatch; render the formatted object directlylib/View/API/V2/Json/Job.php,lib/View/API/V3/Json/Chunk.phpfillUrls()no longer needs theFeatureSetargument — parameter and call sites updatedlib/Plugins/Features/ReviewExtended/ReviewedWordCountModel.php_sendNotificationEmail()lib/Utils/AsyncTasks/Workers/Analysis/TMAnalysis/Service/EngineService.phpgetMTTranslation()lib/Plugins/Features/BaseFeature.phpgetBuildFiles()— no callers anywheretests/unit/Core/**fillUrls()overrides for the new signatureplugins/airbnbTesting
vendor/bin/phpunit --exclude-group=ExternalServices --no-coveragepasses./vendor/bin/phpstanpasses (0 errors, with baseline)PHPStan level 8 over the whole repository → 0 errors.
The directly affected suites —
UrlsControllerTest,BaseFeatureTest,FilterEventSubclassTest—pass (91 tests, 180 assertions), as does
plugins/airbnb/tests(89 tests, 108 assertions).The full suite on the development machine reports pre-existing
SQLSTATE[HY000] [2002] Connection refusederrors because no database is reachable from thecontainer. The only two genuine assertion failures, both in
EngineServiceUnitTest::getMTTranslation_*, were verified to fail identically atHEADwith thesechanges stashed, so they are unrelated.
AI Disclosure
Claude Code (claude-opus-5)
Notes
Merge order. matecat/airbnb_plugin#122 must merge first. Until it does, the submodule gitlink here
points at a commit that lives on the plugin's
remove-obsolete-codebranch rather than itsmaster.One intentional behaviour change, inherited from the plugin side: the Airbnb handler used to flip
the engine's analysis flag back to
falseafterEngineServicehad set it, which made theskip-analysis gate in
AbstractEngine::call()unreachable and forced MMT to be queried duringanalysis regardless of the project setting. Airbnb projects will now honour their own
enable_mt_analysisvalue like every other customer.Follow-up, not in this PR. The plugin's
characterLengthCount()hook stays for now.character_counter_mode(google_ads/exclude_cjk/all_one) already generalises thatbehaviour, but it is read only by
public/js/utils/charsSizeCounterUtil.js—SizeRestriction::getCleanedStringLength()still hardcodesmb_strlen + CJK + emojiand ignores thejob metadata. Teaching the PHP counter to honour the setting would let the last plugin hook and
CharacterLengthCountEventgo too.