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

Upgrade legacy modSessionHandler class references in settings #16379

Merged

Conversation

opengeek
Copy link
Member

@opengeek opengeek commented Feb 3, 2023

What does it do?

Upgrades legacy references to modSessionHandler in system and context settings to MODX\Revolution\modSessionHandler

Why is it needed?

Sites upgraded from 2.x still had the legacy modSessionHandler class referenced in the session_handler_class setting.

How to test

Find a 2.x site with session_handler_class set to modSessionHandler, upgrade it to 3.0.4-dev, and confirm that the value is updated to MODX\Revolution\modSessionHandler

Related issue(s)/PR(s)

n/a

@opengeek opengeek added the bug The issue in the code or project, which should be addressed. label Feb 3, 2023
@opengeek opengeek added this to the v3.0.4 milestone Feb 3, 2023
@opengeek opengeek requested a review from Mark-H as a code owner February 3, 2023 18:05
@cla-bot cla-bot bot added the cla-signed CLA confirmed for contributors to this PR. label Feb 3, 2023
@opengeek opengeek added the pr/port-to-3 Pull request has to be ported to 3.x. label Feb 3, 2023
@codecov-commenter
Copy link

codecov-commenter commented Feb 3, 2023

Codecov Report

Base: 17.96% // Head: 17.96% // Decreases project coverage by -0.01% ⚠️

Coverage data is based on head (352653b) compared to base (af6a209).
Patch coverage: 0.00% of modified lines in pull request are covered.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@             Coverage Diff              @@
##              3.0.x   #16379      +/-   ##
============================================
- Coverage     17.96%   17.96%   -0.01%     
  Complexity    10437    10437              
============================================
  Files           561      561              
  Lines         39040    39041       +1     
============================================
  Hits           7013     7013              
- Misses        32027    32028       +1     
Impacted Files Coverage Δ
core/src/Revolution/Sources/modMediaSource.php 41.80% <0.00%> (-0.04%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@rthrash
Copy link
Member

rthrash commented Feb 3, 2023

This pull request has been mentioned on MODX Community. There might be relevant details there:

https://community.modx.com/t/error-flush-sessions-not-supported-when-trying-to-logout-all-users-in-modx-3/6361/13

@JoshuaLuckers
Copy link
Contributor

After approving I came across two lexicons that use modSessionHandler and not the FQC:

$_lang['setting_session_gc_maxlifetime'] = 'Session Garbage Collector Max Lifetime';
$_lang['setting_session_gc_maxlifetime_desc'] = 'Allows customization of the session.gc_maxlifetime PHP ini setting when using \'modSessionHandler\'.';
$_lang['setting_session_handler_class'] = 'Session Handler Class Name';
$_lang['setting_session_handler_class_desc'] = 'For database managed sessions, use \'modSessionHandler\'. Leave this blank to use standard PHP session management.';

@opengeek opengeek merged commit aba9c70 into modxcms:3.0.x Feb 7, 2023
@opengeek opengeek deleted the 3.0.x-upgrade-session-handler-class branch February 7, 2023 17:43
opengeek added a commit that referenced this pull request Feb 7, 2023
* Upgrade legacy modSessionHandler class references in settings

* Update related lexicons with FQCN
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The issue in the code or project, which should be addressed. cla-signed CLA confirmed for contributors to this PR. pr/port-to-3 Pull request has to be ported to 3.x.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants