search

tbar0970 / jethro-pmm

Jethro Pastoral Ministry Manager
GNU General Public License v3.0
35 stars 25 forks source link

roster_reminder.php warnings in 2.35.1 if new 2FA constants set #1038

Closed jefft closed 3 months ago

jefft commented 3 months ago

After upgrading from 2.35.0-RC3 to 2.35.1 I'm seeing warnings when running roster_reminder.php:

# php /srv/www/jethro/current/app/scripts/roster_reminder.php sms.ini 
SUCCESS: The setting 2FA_SENDER_ID has now been migrated to the database and should be removed from conf.php
PHP Warning:  session_set_cookie_params(): Cannot change session cookie parameters when headers already sent in /srv/www/jethro/2.35.1/app/include/init.php on line 60

Warning: session_set_cookie_params(): Cannot change session cookie parameters when headers already sent in /srv/www/jethro/2.35.1/app/include/init.php on line 60
PHP Warning:  session_name(): Cannot change session name when headers already sent in /srv/www/jethro/2.35.1/app/include/init.php on line 62

Warning: session_name(): Cannot change session name when headers already sent in /srv/www/jethro/2.35.1/app/include/init.php on line 62
PHP Warning:  session_start(): Cannot start session when headers already sent in /srv/www/jethro/2.35.1/app/include/init.php on line 63

Warning: session_start(): Cannot start session when headers already sent in /srv/www/jethro/2.35.1/app/include/init.php on line 63
Hello! You are rostered on for serving this Sunday. Here is a link to the run sheet for this week. If you need to swap with someone on your team then please do so and let your Team Leader and myself know by 12pm on Friday. 

Debug mode - no messages sent

The script still runs, it's just ugly and unexpected.

jefft commented 3 months ago

The problem is due to my adding to conf.php:

define('2FA_SENDER_ID','Jethro');

Jethro sees this and migrates the setting to the database: https://github.com/tbar0970/jethro-pmm/blob/d52deebdb73a51c0c5fcbc98271e652b40bfce68/include/config_manager.class.php#L19

resulting in the output:

SUCCESS: The setting 2FA_SENDER_ID has now been migrated to the database and should be removed from conf.php

However, apparently we're not allowed to emit content yet, as headers haven't all been set. Perhaps Config_Manager::init(); needs to defer its call a bit later in https://github.com/tbar0970/jethro-pmm/blob/master/include/init.php

Anyhow, I think 2FA_* variables are like SMS_* variables, in that they should be settable in conf.php and not overrideable in the System Configuration. See linked PR.