dev/drupal#149 Override sessionStart function for Drupal8 using appro… #19044
Conversation
|
(Standard links)
|
CRM/Utils/System/Drupal8.php
Outdated
| */ | ||
| public function sessionStart() { | ||
| if (\Drupal::hasContainer()) { | ||
| $session = \Drupal::request()->getSession(); |
There was a problem hiding this comment.
It's better to grab from session service than the request session. I've seen some weirdness otherwise. Can't fully explain why, as kids usually the same reference
There was a problem hiding this comment.
Okay, the parent class has this:
/**
* Start a new session.
*/
public function sessionStart() {
session_start();
}
If you look at the session manager in Drupal, which is what $session->start() calls, you'll see it's safe for existing started sessions: https://git.drupalcode.org/project/drupal/-/blob/8.8.x/core/lib/Drupal/Core/Session/SessionManager.php#L106
This is regenerate, which migrate calls: https://git.drupalcode.org/project/drupal/-/blob/8.8.x/core/lib/Drupal/Core/Session/SessionManager.php#L207
I don't know if this code actually should call migrate over ->start() directly.
|
@seamuslee001 @mglaman once you are both happy with this PR please merge it @seamuslee001 |
…priate session functions Use Drupal Session service and don't worry about calling migrate
89f11fc to
592e730
Compare
|
@mglaman Matt does this look right to you now? |
|
Looks good to me, @seamuslee001 ! |
|
Merging as per @mglaman comment and @eileenmcnaughton 's comment |
…priate session functions
Overview
This overrides the sessionStart function to fix Session already started errors when running cron.php and as per the Drupal Documentation
Before
deprecated methods from DrupalBase.php used for Drupal8/9
After
Correct Session functions used for Drupal8/9
ping @demeritcowboy @KarinG @mglaman @MikeyMJCO