Skip to content
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

Error in ArchiveRestoreAction.php on line 92 where 2nd arg is invalid type #330

Closed
4 tasks done
ssmarco opened this issue Mar 10, 2024 · 4 comments
Closed
4 tasks done

Comments

@ssmarco
Copy link
Contributor

ssmarco commented Mar 10, 2024

Module version(s) affected

2.1.2

Description

When restoring an archived Elemental block, as shown in the screenshot below, an error occurs due to invalid argument passed to the 2nd parameter of redirect function. Instead if an int a string is passed, hence the redirection did not work to the target page.

image

How to reproduce

The method, doRestore of ArchivedRestoreAction class is calling redirect method of the controller but passing a string as the 2nd argument. This is now incorrect due to PHP 8.2 strict typing applied to CMS 5.1 hence creating an error.

Line 92: return $controller->redirect($controller->Link(), 'index');

Possible Solution

The fix should be to remove the 2nd argument since index is default behaviour.

Line 92: return $controller->redirect($controller->Link());

Additional Context

No response

Validations

  • Check that there isn't already an issue that reports the same bug
  • Double check that your reproduction steps work in a fresh installation of silverstripe/installer (with any code examples you've provided)

Acceptance criteria

  • No errors are thrown when restoring a Page or Content block.
  • Restoration of page or content block is covered by some PHPunit test and at least some Behat test.

PRs

@GuySartorelli
Copy link
Member

PR merged. This will be automatically tagged by GitHub actions

@MichAlkhouri
Copy link

Does this fix cover archiving a page?
From my testing I can see archiving a page behave the same way and produces the same error.

@maxime-rainville
Copy link
Contributor

Good job on the fix ... but this looks like something our CI should have pick it up. I'll reopen to cover this functionality with some test.

@GuySartorelli
Copy link
Member

PRs merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants