Skip to content
This repository has been archived by the owner on Nov 30, 2022. It is now read-only.

Removed !! from the full_page_redirect breaking authenticate route #1222

Merged
merged 2 commits into from
Oct 4, 2022

Conversation

Kyon147
Copy link
Collaborator

@Kyon147 Kyon147 commented Sep 28, 2022

Currently the authenticated route can break usually on a proper visit to /authenticate because of the {{!! !!}} formatting in the blade template for the api key, shop origin and host.

@Kyon147 Kyon147 changed the title Removed !! from the blade template as it is outputting the braces i… Removed !! from the full_page_redirect breaking authenticate route Sep 28, 2022
@Kyon147
Copy link
Collaborator Author

Kyon147 commented Sep 28, 2022

@osiset a little bit of a hotfix as the auth route is currently broken when accessing it directly.

@Kyon147 Kyon147 requested a review from gnikyt September 28, 2022 12:09
@gnikyt
Copy link
Owner

gnikyt commented Sep 28, 2022

@Kyon147 It seems testbench-core introduces static properties. Specifically here, but its not declared in the class head, resulting in and error for PHP 8.

Edit: It was literally added 10 hours ago. Seems like its trying to set lastResponse, but its not defined anywhere. Do we need to define it in TestCase.php of our package? Like... public static $latestResponse = null; ?

@Kyon147
Copy link
Collaborator Author

Kyon147 commented Sep 29, 2022

It was literally added 10 hours ago. Seems like its trying to set lastResponse, but its not defined anywhere. Do we need to define it in TestCase.php of our package? Like... public static $latestResponse = null; ?

Yeah that is totally weird, I'll take a look but defining it in our test case sounds like a good idea for now!

@Kyon147
Copy link
Collaborator Author

Kyon147 commented Oct 1, 2022

@osiset that has fixed the error for now in the tests - not sure why codecov is complaining about files not even touched in my PR though....

@Kyon147 Kyon147 marked this pull request as ready for review October 1, 2022 08:39
Copy link
Owner

@gnikyt gnikyt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Kyon147 Good to go, this will fix up other PR tests for PHP8. Codecov is fine :)

@Kyon147 Kyon147 merged commit 00f5958 into master Oct 4, 2022
@gnikyt gnikyt deleted the hotfix/fix_full_page_redirect branch October 4, 2022 15:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants