- Issue created by @sujan shrestha
- First commit to issue fork.
- last update
over 1 year ago 15 pass, 7 fail - Status changed to Needs review
over 1 year ago 5:40pm 5 July 2023 - ๐จ๐ฆCanada jigarius Montrรฉal
Here's a PR that resolves the warning message. There seems to be a new concept of "destinations[]" now and I've not handled this for now.
- ๐ณ๐ฑNetherlands ecvandenberg
I tried the patch in a Drupal 9.5.10 site with autologout 8.1.4 and php 8.1.21.
The warnings are gone. - ๐ฌ๐งUnited Kingdom Chris--S
This seems to be a duplicate of https://www.drupal.org/project/autologout/issues/3301642 ๐ Throws Notice on Login when destination is set Needs work
- Status changed to Needs work
over 1 year ago 5:01pm 5 September 2023 - ๐ธ๐ฎSlovenia deaom
The tests that are failing seem to stem from a core issue ๐ฑ [META] Serialization issues in Migration tests Active . So either this issue can be blocked until the core issue is fixed, or based on the reviews that it's working can be merged. Leaving it up to maintainers and leaving the status as is.
- ๐ธ๐ฎSlovenia deaom
It also seems these are duplicates ๐ Throws Notice on Login when destination is set Needs work and ๐ Autologout does not handle multiple destinations Needs work .
- Status changed to Needs review
about 1 year ago 10:51am 25 September 2023 - ๐ธ๐ฎSlovenia deaom
Corrected the CS issue for missing doc comment for the new getDestination function, testing is now done via GitLab CI, so test are passing now. Changing status to needs review so it can be re-tested.
- last update
about 1 year ago Patch Failed to Apply - First commit to issue fork.
Cleaned up the code a bit, since it's partially related to this issue, I think it's better to just fix it now. Otherwise, everything works as expected, and looking at the other two related issues, this one is cleaner and should probably be merged. Leaving the other two issues open so maintainers can credit them.
Hiding the issue patch file, since the test there will always fail now that we are switching to GitlabCI. @sandipta this is probably why you thought that the MR was falling, it's not. Just integration with GitlabCI is different and doesn't display passing and failing tests the same way DrupalCI did.
Leaving the issue to "needs review", because of the code changes I made.
- ๐ธ๐ฎSlovenia deaom
The getDestination() function which was getting the destination from referrer was always returning an empty string, which means the if block was always skipped. It did solve the warning but also broke the functionality of redirecting user back to its own profile (if for example user/2 was logged in and then logged out and user/3 logged in after, the user/3 was redirected to the user/2 profile, because of the destination). Adjusted the condition to basically always redirect to its own user profile, so we do not need to explicitly check if user and user id match based on destination, getting ride of a chunk of code. Also added a test that checks that the destination is set and after user/2 is logged out and user/3 is logged in, the user/3 is redirected to the correct user profile. Ready for review.
Tests were passing over gitlabCI but were not passing locally. This was down to /web/ being used in path checking for the destination query parameter. Changed it so it now dynamically gets the correct path depending on the Drupal configuration. Otherwise, the fix works, testing by hand if I get logged out on an account with UID 2 and login using an account with UID 3 it redirects me to the currently logged-in user profile.
All that said, there is an issue if this should even be the functionality in the first place, for now, this should be merged since it fixes a bug in the current implementation of the module. But it would be nice if people contributed their opinions on this issue #3301938: Consider Backing out User Destination Manipulation (3101732) โ .
Because of the change to test, this still needs review.
- Status changed to RTBC
about 1 year ago 11:53am 28 September 2023 - ๐ธ๐ฎSlovenia deaom
Tests are passing and I agree with the change to get the URL of user login page via route as this then guarantees tests are working everywhere.
- ๐ง๐ชBelgium borisson_ Mechelen, ๐ง๐ช
Can we copy over credit from ๐ Throws Notice on Login when destination is set Needs work please?
- First commit to issue fork.
- ๐ช๐ธSpain unstatu
I have applied a small change on the MR in order to improve the detection of the logic that should happen when destination parameter is found in the URL.
Instead of using strpos (that is too generic), I have changed the implementation to use a regex.
I have done that because I found a problem with the following situation:
- Create a page with the following alias /user/my-user-info
- Login with the following URL /user/login?destination=/user/my-user-info
- After login, Drupal redirects to /user/ instead of /user/my-user-info because `strpos` detects the "user" word in the destination parameter - Status changed to Needs review
12 months ago 5:12pm 26 December 2023 i am getting the same warnings; hope there will be release with fix soon (:
- ๐ท๐ดRomania claudiu.cristea Arad ๐ท๐ด
claudiu.cristea โ made their first commit to this issueโs fork.
- Status changed to RTBC
7 months ago 1:10pm 23 May 2024 - ๐ท๐ดRomania claudiu.cristea Arad ๐ท๐ด
Moving to 2.x but I have no permission to change the base branch in MR
- ๐ฎ๐ณIndia krishna mishra
Patch is not working with 8.x-1.4, so generated new patch file (no logical change, only generated compatible patch) .
Here is patch. - ๐จ๐ฆCanada gwvoigt London, ON ๐จ๐ฆ
Previous patches are not working with 8.x-1.5, so generated new patch file.
also +1 to release this fix... thank you!
- First commit to issue fork.
- Merge request !68Issue #3372010: Fix: Undefined array key: destination โ (Merged) created by tame4tex
- ๐จ๐ฆCanada tame4tex
tame4tex โ changed the visibility of the branch 2.x to hidden.
- ๐จ๐ฆCanada tame4tex
Updated the MR to fix the failing test. The only change was to the test itself.
Also added a new MR https://git.drupalcode.org/project/autologout/-/merge_requests/68 for the 2.x branch, which is exactly the same as the previous MR.
Rather than using one of the uploaded patch files, obtain the patch from the MR by following the instructions on https://www.drupal.org/docs/develop/git/using-gitlab-to-contribute-to-dr... โ . This should apply to the latest 1.x and 2.x versions. If it doesn't please comment below or update the MR.
- First commit to issue fork.
-
japerry โ
committed 7ec5fc0e on 2.x authored by
tame4tex โ
Issue #3372010 by DeaOm, admirlju, jigarius: Warning: Undefined array...
-
japerry โ
committed 7ec5fc0e on 2.x authored by
tame4tex โ
- ๐บ๐ธUnited States japerry KVUO
Thanks tame4tex! Tests are passing, committed to both 8.x-1.x and 2.x
Note, there were many comments on this long standing issue (as well as the closed dup), some valid for credit, others not so much. I'm sure some credits were missed -- However, I prioritized the original patch creators for this issue.
-
japerry โ
committed 7ec5fc0e on 8.x-1.x authored by
tame4tex โ
Issue #3372010 by DeaOm, admirlju, jigarius: Warning: Undefined array...
-
japerry โ
committed 7ec5fc0e on 8.x-1.x authored by
tame4tex โ
- ๐ฆ๐บAustralia VladimirAus Brisbane, Australia
vladimiraus โ changed the visibility of the branch 3372010-warning-undefined-array to hidden.
Automatically closed - issue fixed for 2 weeks with no activity.