[17.0][FIX] web_m2x_options_manager: safe_eval() ValueError

[vvrossem]

safe_eval() raises a ValueError when evaluating node options containing boolean values written in lowercase (true or false).

Fixes

ValueError: <class 'NameError'>: "name 'true' is not defined" while evaluating
"{'<option_name>': [true|false}"

[imlopes]

It could be interesting to update tests to demonstrate what you are changing here.

[SilvioC2C]

Not blocking for such a minor fix, but could you please update the tests to include this case?

[OCA-git-bot]

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

[SilvioC2C]

@OCA/web-maintainers ping for an easy review and possibly merge?

[vvrossem]

Not blocking for such a minor fix, but could you please update the tests to include this case?

I chose to not update the test as they currently use safe_eval independently: https://github.com/OCA/web/blob/18.0/web_m2x_options_manager/tests/common.py#L32-L37

    @staticmethod
    def _eval_node_options(node):
        opt = node.attrib.get("options")
        if opt:
            return safe_eval(opt, nocopy=True)
        return {}

[SilvioC2C]

@vvrossem I think you have 2 options:

Option 1

You could update the test method to this:

    @staticmethod
    def _eval_node_options(node, **safe_eval_kwargs):
        opt = node.attrib.get("options")
        if opt:
            safe_eval_kwargs = dict(safe_eval_kwargs or {})
            if "nocopy" not in safe_eval_kwargs:
                safe_eval_kwargs["nocopy"] = True  # Default value if missing
            return safe_eval(opt, **safe_eval_kwargs)
        return {}

and then, in tests themselves, you can evaluate a node that contains options={'some-key': 'true'} by calling:

self._eval_node_options(node, globals_dict={"true": True, "false": False})

Option 2

At the very beginning of test method test_apply_options(), you could create m2x.create.edit.option records that change nothing on the node themselves:

        opt_partner_title = self._create_opt(
            "res.partner",
            "title",
            {
                "option_create": "none",
                "option_create_edit": "none",
            },
        )
        opt_partner_parent = self._create_opt(
            "res.partner",
            "parent_id",
            {
                "option_create": "none",
                "option_create_edit": "none",
            },
        )
        opt_partner_category = self._create_opt(
            "res.partner",
            "category_id",
            {
                "option_create": "none",
                "option_create_edit": "none",
            },
        )

then, you can use these records to do the node's evaluation for you:

        form_doc = self._get_test_view_parsed()
        self.assertEqual(
            opt_partner_title._read_node_options(form_doc.xpath("//field[@name='title']")[0]), {}
        )
        self.assertEqual(
            opt_partner_parent._read_node_options(form_doc.xpath("//field[@name='parent_id']")[0]),
            {"create": False, "create_edit": False},
        )
        self.assertEqual(
            opt_partner_category._read_node_options(form_doc.xpath("//field[@name='category_id']")[0]),
            {"create": False, "create_edit": False},
        )

and after the first check, you can update those options to override the attributes (as defined here: https://github.com/OCA/web/blob/17.0/web_m2x_options_manager/tests/test_m2x_create_edit_option.py#L30-L54).

[ivs-cetmix]

Hi @vvrossem thank you for your contribution! Could you please check the comments and reply/address them?

[vvrossem]

Hello, tests are updated.
My apologies for the delay.

[ivs-cetmix]

Hello, tests are updated. My apologies for the delay.

Nice, thank you for your contribution @vvrossem !
/ocabot merge patch

[OCA-git-bot]

This PR looks fantastic, let's merge it!
Prepared branch 17.0-ocabot-merge-pr-3388-by-ivs-cetmix-bump-patch, awaiting test results.

[OCA-git-bot]

Congratulations, your PR was merged at d6b6880. Thanks a lot for contributing to OCA. ❤️