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

Fixing "Undefined variable $backURL" in enforeLogin function #100

Merged
merged 2 commits into from
Apr 30, 2023
Merged

Fixing "Undefined variable $backURL" in enforeLogin function #100

merged 2 commits into from
Apr 30, 2023

Conversation

LABCAT
Copy link
Contributor

@LABCAT LABCAT commented Apr 19, 2023

Note, there is already a pull request for this issue which looks to be abandoned:
#81

This pull request is a simpler fix which doesn't remove the call to validSiteURL.

Unfortunately, I also do not have time to contribute a unit test to the repository.

Copy link
Member

@GuySartorelli GuySartorelli left a comment

Choose a reason for hiding this comment

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

Please just swap all instances of $backURL to $backUrl (or vice versa) - it seems clear they were all meant to be the same variable.

@LABCAT
Copy link
Contributor Author

LABCAT commented Apr 28, 2023

@GuySartorelli I have updated the merge request as suggested.

@GuySartorelli GuySartorelli changed the base branch from 4 to 4.4 April 28, 2023 03:10
@GuySartorelli
Copy link
Member

Nice, thanks! I've retargetted this PR to the 4.4 branch so it can be released as a patch. Can you please reset the PR to not include the extra commits retargetted it has picked up?
Easiest way is probably to do this:

git branch -D patch-1
git checkout 4.4
git checkout -b patch-1
git cherry-pick 3a544fb 6618825
git push patch-1 --force

This will delete your current branch locally, then recreate a new branch with the same name, but this time based on the 4.4 branch. It'll then cherry-pick in your original commits, and push them to the remote, which will update this PR.

@LABCAT
Copy link
Contributor Author

LABCAT commented Apr 30, 2023

@GuySartorelli branch has now been rebased onto 4.4 and ready to merge.

@GuySartorelli
Copy link
Member

Awesome, thanks for that. Just letting CI run and will merge on green.

@GuySartorelli GuySartorelli merged commit aca95a9 into silverstripe:4.4 Apr 30, 2023
@GuySartorelli
Copy link
Member

Released as 4.4.1

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

Successfully merging this pull request may close these issues.

2 participants