Skip to content

[ticket/10910] Function build_cfg_template() allow $size for $tpl_type = select#1317

Closed
Skouat wants to merge 2 commits into
phpbb:developfrom
Skouat:ticket/10910
Closed

[ticket/10910] Function build_cfg_template() allow $size for $tpl_type = select#1317
Skouat wants to merge 2 commits into
phpbb:developfrom
Skouat:ticket/10910

Conversation

@Skouat

@Skouat Skouat commented Mar 23, 2013

Copy link
Copy Markdown
Contributor

Function build_cfg_template() allow $size for $tpl_type = select

_How to use it ?_
In ACP file (eg: includes/acp/acp_foo.php) use the select type as below :
For example define the size to 8

$display_vars = array(
    'title' => 'FOO_CONFIG',
    'vars'  => array(
        'legend1'                       => 'FOO_SETTINGS',
        'foo_group_id'  => array('lang' => 'FOO_GROUP',     'validate' => 'int',    'type' => 'select:8', 'function' => 'group_select_options', 'params' => array('{CONFIG_VALUE}', false, false), 'explain' => true),

        'legend4'               => 'ACP_SUBMIT_CHANGES',
    )
);

After, on submit, we check if no value is selected by using the following code

// We go compare $display_vars with $cfg_array to determine if the type "select" does not have a choice selected.
if ($submit)
{
    $display_vars_diff = array_diff_key($display_vars['vars'] , $cfg_array);

    foreach ($display_vars_diff as $config_name_diff => $vars_diff)
    {
        if (strpos($vars_diff['type'], 'select') === false)
        {
            continue;
        }

        $cfg_array[$config_name_diff] = '';
    }
    unset($config_name_diff);
}

Ticket PHPBB3-10910

@bantu

bantu commented Mar 24, 2013

Copy link
Copy Markdown
Collaborator

Please add unit tests to "tests/functions_acp/build_cfg_template_test.php".

@bantu

bantu commented Mar 24, 2013

Copy link
Copy Markdown
Collaborator

Please remove the " for phpBB 3.1.x" from the last line of the commit message.

@Skouat

Skouat commented Mar 25, 2013

Copy link
Copy Markdown
Contributor Author

Hi bantu,

It's my first PR and i'm not familiar with GitHub and your process

For your first comment, do I create two functions on the basis of existing ones?
eg :

public function build_cfg_template_select_data()
public function test_build_cfg_template_select($tpl_type, $key, $new, $config_key, $vars, $expected)

For your second comment, how can I edit the commit message?
Do I create a new branch and submit a new PR with the correct commit message ?
Because, i tried to amend the last commit, but as it has already been published on GitHub, it seems it can not be edited.

Regards.

@imkingdavid

Copy link
Copy Markdown
Contributor

For your second comment, how can I edit the commit message?
Do I create a new branch and submit a new PR with the correct commit message ?
Because, i tried to amend the last commit, but as it has already been published on GitHub, it seems it can not be edited.

If you are using the command line, you can do an interactive rebase like so:
git rebase -i develop
Next to your commit, change pick to edit or reword.
Once you change the commit message, allow the rebase to finish. You will need to force push to overwrite the commit that is already there:
git push origin ticket/10910 -f
(assuming origin is your fork's remote name; otherwise, change it to the correct name)

@Skouat

Skouat commented Mar 27, 2013

Copy link
Copy Markdown
Contributor Author

Thanks imkingdavid.
Comment changed.

It only remains to add the unit tests.
I will do that ASAP

@Skouat

Skouat commented Mar 30, 2013

Copy link
Copy Markdown
Contributor Author

Hi guys !

I don't know if i've well coded for the test unit. So, here is the code and let me know if this is right or wrong.

Regards.

    public function build_cfg_template_select_data()
    {
        return array(
            array(
                array('select', 3),
                'key_name',
                array('config_key_name' => '2'),
                'config_key_name',
                array(),
                '<select name="config[config_key_name]" id="key_name" size="3"><option value="1">First_Option</option><option value="2" selected="selected">Second_Option</option><option value="3">Third_Option</option></select>',
            ),
            array(
                'select',
                'key_name',
                array('config_key_name' => '2'),
                'config_key_name',
                array(),
                '<select name="config[config_key_name]" id="key_name"><option value="1">First_Option</option><option value="2" selected="selected">Second_Option</option><option value="3">Third_Option</option></select>',
            ),
        );
    }

    /**
    * @dataProvider build_cfg_template_select_data
    */
    public function test_build_cfg_template_select($tpl_type, $key, $new, $config_key, $vars, $expected)
    {
        global $user, $phpbb_dispatcher;

        $phpbb_dispatcher = new phpbb_mock_event_dispatcher();
        $user->lang = new phpbb_mock_lang();

        $this->assertEquals($expected, build_cfg_template($tpl_type, $key, $new, $config_key, $vars));
    }

@nickvergessen

Copy link
Copy Markdown
Contributor

In the second case, you need to replace 'select', with array('select'),

Also while at it, I'd like to add multiple support here.

        array(
            array('select', 3, 1),
            'key_name',
            array('config_key_name' => '2'),
            'config_key_name',
            array(),
            '<select name="config[config_key_name]" id="key_name" size="3" multiple="multiple">...',
        ),

But adding the tests is not so easy btw, but I will help you with that, when you implemented the multiple support

@Skouat

Skouat commented Mar 31, 2013

Copy link
Copy Markdown
Contributor Author

But adding the tests is not so easy btw, but I will help you with that,

Indeed, adding the test units is not so easy. I've tried, i've failed :)

when you implemented the multiple support

Given my level of coding, it is not easy. But i'm working on it.
I need to find a way to retrieve data from a form with a multidimensional array and convert them into a string value. Order to be able to store it in $config.

Regards

@nickvergessen

Copy link
Copy Markdown
Contributor

Replaced by #1887

@Skouat Skouat deleted the ticket/10910 branch March 30, 2014 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants