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

[REF] dev/drupal#169 Replace other usages of session_id with CRM_Core… #22080

Merged
merged 1 commit into from
Nov 16, 2021

Conversation

seamuslee001
Copy link
Contributor

…_Config::singleton()->userSystem->getSessionId

Overview

As per the ticket this replaces other places in the code that call for session_id with the newly added function

Before

session_id function used

After

UserSystem getSessionId function used

ping @demeritcowboy @MegaphoneJon

…_Config::singleton()->userSystem->getSessionId
@civibot
Copy link

civibot bot commented Nov 15, 2021

(Standard links)

@civibot civibot bot added the 5.44 label Nov 15, 2021
@demeritcowboy
Copy link
Contributor

This seems technically fine but probably doesn't need to be in 5.44. As noted in the ticket the IDS one is only used for logging and the chart one is a broken feature (broken somewhere along with the changes for civicrm/civicrm-packages#267 and then further with buttonrama 14120ba), so I'd consider just removing the chartId stuff.

@seamuslee001 seamuslee001 changed the base branch from 5.44 to master November 15, 2021 23:16
@civibot civibot bot added master and removed 5.44 labels Nov 15, 2021
@seamuslee001
Copy link
Contributor Author

@demeritcowboy this is now against master as well, I think we should still fix these points even tho the chart one may not work just for consistency sake

@demeritcowboy demeritcowboy merged commit 48dbcda into civicrm:master Nov 16, 2021
@seamuslee001 seamuslee001 deleted the dev_drupal_169 branch November 16, 2021 00:57
@seamuslee001
Copy link
Contributor Author

thanks @demeritcowboy

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

Successfully merging this pull request may close these issues.

2 participants