-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
FAIL #250
FAIL #250
Conversation
I don't know why your tests have failed, but it's working here: page move, page rename etc. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As the tests show, this is not a simple refactoring but breaks the plugin. Please fix the code so the tests pass again.
helper/handler.php
Outdated
@@ -66,7 +69,7 @@ public function resolveMoves($old, $type) { | |||
|
|||
if($type != 'media' && $type != 'page') throw new Exception('Not a valid type'); | |||
|
|||
$old = resolve_id($this->origNS, $old, false); | |||
$old = (new PageResolver($this->origNS))->resolveId($old); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolve_id
is not the same as PageResolver
. I assume, but I'm not sure, that this breaks the tests as you can see from the checks. The test breakages look quite strange, as if there was a quite fundamental error somewhere. I don't know where it comes from but please run the tests and debug where the differences come from.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed from PageResolver to FileResolver.
The failing tests concern the adaptation of the links after the move, not the actual moving of the page. |
Not working. Don't know how to fix. Closing. |
This PR fix deprecation warnings in PHP 8+ and Jack Jackrum: PageResolver and MediaResolver.
Related to issue #241