lonnieezell / Bonfire

Jumpstart your CodeIgniter web applications with a modular, HMVC-ready, backend.
http://cibonfire.com
1.39k stars 524 forks source link

Wrong timezone setting #793

Closed artlantis closed 9 years ago

artlantis commented 11 years ago

There isn't any option in Bonfire Administration section for timezone setting. Therefore, when we want to create an user, Bonfire getting default user timezone setting which is Administrator. We should set global timezone to take appropiate value.

#file: users/views/settings/user_form.php
        <div class="control-group <?php echo form_error('timezone') ? 'error' : '' ?>">
            <label class="control-label" for="timezones"><?php echo lang('bf_timezone') ?></label>
            <div class="controls">
                <?php echo timezone_menu(set_value('timezones', isset($user) ? $user->timezone : $current_user->timezone)); ?>
                <?php if (form_error('timezones')) echo '<span class="help-inline">'. form_error('timezones') .'</span>'; ?>
            </div>
        </div>
artlantis commented 11 years ago

Adding Timezone Setting into Admin Section

#file: /bonfire/migrations/013_Move_settings_into_db.php
        $default_settings = "
            INSERT INTO `{$prefix}settings` (`name`, `module`, `value`) VALUES
             ('site.title', 'core', ''),
             ('site.timezone', 'core', 'UM6') // adding default timezone for sitewide
#file /bonfire/modules/settings/views/settings/index.php
# add the following line after bf_top_numer line
                    <div class="control-group">
                        <label class="control-label" for="timezones"><?php echo lang('bf_site_timezone') ?></label>
                        <div class="controls">
                            <?php echo timezone_menu(set_value('site.timezone', isset($settings['site.timezone']) ? $settings['site.timezone'] : '')) ?>
                            <?php if (form_error('site_timezone')) echo '<span class="help-inline">'. form_error('site_timezone') .'</span>'; ?>
                        </div>
                    </div>
#file: /bonfire/modules/settings/controllers/settings.php
# add date helper into construct
        Template::set('toolbar_title', 'Site Settings');

        $this->load->helper('config_file');
        $this->load->helper('date');
        $this->lang->load('settings');

# modify the $data array, add timezone line
            array('name' => 'site.title', 'value' => $this->input->post('title')),
            array('name' => 'site.timezone', 'value' => $this->input->post('timezones')),
            array('name' => 'site.system_email', 'value' => $this->input->post('system_email')),
mwhitneysdsu commented 11 years ago

CodeIgniter uses the $config['time_reference'] setting in config.php to determine the timezone used when calling the date helper's now() method. Setting an additional timezone setting in Bonfire's site settings is likely to add to the potential confusion when time-based functions don't behave as expected.

If the existing behavior of the timezone menu in the users module is not desired, it should probably be changed to use the value from the config file as the fallback.

Also, if you want to add a new setting to the database you would usually create a new migration file, rather than changing an old one. The old migration files would usually only be changed to fix installer issues.

artlantis commented 11 years ago

Dear @mwhitneysdsu Mat,

I understood your concern but the reason is to add timezone setting into admin section is because of the "$current_user->timezone" variable. When installation finished, Bonfire automatically setting admin user timezone. If, admin wasn't change the himself/herself timezone, Bonfire automatically adding admin timezone to the created new users timezone is because of the above variable.

About your suggestion, it's a good way, just adding a paremeter in config.php then use instead of $current_user->timezone variable.

mwhitneysdsu commented 11 years ago

My suggestion is that if the $current_user->timezone is an issue you should probably replace it with the setting that is already in the config.php (i.e. $this->config->item('time_reference')). The setting is already there and is actively being used by CodeIgniter; you don't have to add it, just make sure it's set correctly. Any time you use the date helper for something other than generating a list of time zones you will likely end up using the setting in the config.php file in one way or another.

If a new setting is added to the database and the two are in conflict, it will likely cause confusion. Personally, I use a slightly modified version of CodeIgniter's date helper (that's been sitting in their develop branch for 10 months or so) to handle some other issues, then use an overridden version of the user form to call it. However, I try to avoid modifying CI in the Bonfire project itself, as it makes it difficult to keep CI up to date, and developers familiar with CI do tend to like it to behave as they expect, especially since HMVC and Bonfire add so much to learn.

Ideally, I think it would be a good idea to put a system time zone setting into the database and get CI to use it as well. Maybe one of the other developers, like @sourcejedi would have a better idea of a way to do that without replacing the actual CI files or requiring extra attention every time CI is updated (unless they make a significant change, obviously).

Also, you can override the default user form by copying the existing form into your admin theme under themes/{admin_theme}/settings/user_form.php or override the whole users module by copying it into your application's modules folder. The public pages in the users module can be overridden by copying them into your public theme in a users folder, for example, the login.php file would go into themes/{theme_name}/users/users/{login.php}.

Unfortunately, the admin themes (as opposed to the public themes) do have some limitations and can result in unexpected behavior with common file names (like index.php). Hopefully we can figure out a way to fix that at some point.