[18.0][ADD] server_action_input_box: Adds a server action wizard input utility

[jesanmor]

This module adds a feature that extends server actions by displaying an input dialog before execution.

It allows passing parameters to the action context, such as:

• Text (char)
• Integer
• Float
• Boolean (checkbox)

This can be useful for dynamically filtering or modifying the behavior of the server action based on user input.

The module is named server_action_input_box and is compatible with Odoo version 18.0.

[gdgellatly]

Pretty exciting new module tbh. Take these with a grain of salt, I could be wrong on a lot, just a very quick look, initial thoughts and I feel like I need to really test it locally to work out

[gdgellatly]

This is called unique name, but has no check to verify that is the case. I am presuming (although early in review) that uniqueness is only within the action itself.

[jesanmor]

Thank you very much for the suggestions and insights!

Regarding the name field: you're right — it's labeled as "Unique Name", but its purpose could be clearer. It's not meant to be globally unique across the model, but rather to serve as a valid Python variable name. This name is used in the server action code to retrieve the input value.

That’s why I validate it using isidentifier() in both create and write, ensuring it conforms to Python syntax rules (e.g., no spaces, special characters, or starting with a digit).

For this reason, a global SQL uniqueness constraint wouldn't be appropriate. The same name can appear in different server actions — uniqueness is only enforced within the same action, and the key point is the validity of the variable name.

Admittedly, "Unique Name" may not be the best label; something like "Variable Name" would be more accurate.

Each server.action.input.box.line record represents an input box shown when executing a server action:

parameter_label: label shown to the user in the input box.

name ("unique name": the name of the variable that collects the value of the input box to then be used in the server action code.

data_type: used to validate user input and convert the value accordingly.

There’s currently some redundancy in the validation logic across create and write, which I’ll refactor. I’ll also improve the field help text and docstrings to clarify all this further.

Lastly, I really appreciate your help and feedback. I’m still quite new to this and have a lot to learn.
I’ll go through the rest of your suggestions carefully and get back to you with my thoughts.

[gdgellatly]

Well you can still have a unique sql constraint, it would be something like (name, server_action_input_box_id). Personally I don't use explicit SQL constraints much, especially for something like this where performance is not a concern, I would use @api.constrains and make it a python constraint. As below, it is also better to have those isidentifier checks inside api.constrains rather than overriding create/write so you will finish with something like

@api.constrains("name, "server_action_input_box_id")
def _check_name(self):
  for action ins self:
     perform validation logic (isidentifier, uniqueness)- raise ValidationError if wrong.

[jesanmor]

Thanks a lot for the suggestion regarding the use of @api.constrains. I've applied it to validate both the uniqueness of name within the same server_action_input_box_id and to ensure it's a valid Python identifier.

This allowed me to remove the custom create and write methods entirely, simplifying the code significantly.

Here is the updated constraint method:

@api.constrains("name", "server_action_input_box_id")
    def _check_name(self):
        for line in self:
            if not line.name.isidentifier():
                raise ValidationError(
                    _(
                        "'%s' is not a valid parameter name. \
                    Remove spaces and special characters or numbers at the beginning.",
                        line.name,
                    )
                )
            domain = [
                ("server_action_input_box_id", "=", line.server_action_input_box_id.id),
                ("name", "=", line.name),
                ("id", "!=", line.id),
            ]
            if self.search_count(domain):
                raise ValidationError(
                    _("the unique name '%s' cannot be duplicated", line.name)
                )

Thanks again for your guidance — it's been really helpful to improve the quality and maintainability of the code.

[gdgellatly]

Better as constraint on name.

[gdgellatly]

Better as constraint on name (same as above)

[gdgellatly]

This should be outside loop, it is a dispatcher that applies universally. Also the dispatcher itself IMO should be populated from a separate function to allow extension to other field types.

Also, as it stands the dispatcher is not really necessary if string selection is renamed str because you could simply eval data_type(raw_data) but I'd leave it in.

[jesanmor]

Thanks a lot for your observations and suggestions.

I’ve refactored the parsed_parameters method according to your feedback:

Moved the data_type_conversion dispatcher out of the loop and into a separate _get_data_type_conversion method to make it easier to extend.

Separated the raw value handling logic into a _get_raw_data method, to decouple it from the main loop and allow further customization per data type.

The try/except block now only catches ValueError, as you recommended.

Here’s the updated code:

   def _get_data_type_conversion(self):
        return {
            "string": lambda s: s,
            "int": int,
            "float": float,
            "bool": lambda b: b,
        }
    
    def _get_raw_data(self, line, parameters):
        raw_value = parameters.get(line.name)
        return raw_value or 0 if line.data_type != "string" else raw_value

    def parsed_parameters(self, parameters):
        parsed_parameters_dict = {}
        data_type_conversion = self._get_data_type_conversion()

        for line in self.server_action_input_box_line_ids:
            raw_data = self._get_raw_data(line, parameters)
            try:
                convert_data = data_type_conversion[line.data_type]
                data = convert_data(raw_data)
            except ValueError as e:
                raise UserError(
                    _(
                        "The value '%(param_value)s' for '%(param_name)s' "
                        "is not of type '%(data_type)s'."
                    )
                    % {
                        "param_value": parameters.get(line.name),
                        "param_name": line.name,
                        "data_type": line.data_type,
                    }
                ) from e

            parsed_parameters_dict[line.name] = data

        return parsed_parameters_dict

Thanks again for your guidance—I'm reviewing more of your suggestions before pushing all the changes.

[gdgellatly]

for me this try/except is way too big and broad. It should only be concerned with ValueError from line 164.

[gdgellatly]

See above

[gdgellatly]

Probably a way from this. This is a very interesting new module, likely to get a lot of attention. Consider Beta

[jesanmor]

Thanks for the suggestion.

You're right — since this is a new module and could still evolve with community feedback and testing, it makes sense to mark it as "Beta" for now. I've updated the manifest accordingly.

[gdgellatly]

String concat

[jesanmor]

I've fixed the implicit string concatenation and now it's properly written as a single string:

"author": "Jesús Sánchez - jesanmor, Odoo Community Association (OCA)",

[gdgellatly]

Remove this file

[gdgellatly]

Will comment later when doing functional tests, but initial feel is more complex than needed.

[jesanmor]

Thanks for your initial feedback! I understand your point about the complexity — I tried to ensure flexibility and robustness, but I’m open to suggestions on simplifying it.
I’ll look forward to your further comments after your functional tests.

[jesanmor]

Hi gdgellatly,

I've updated the module's documentation, and the README.rst now includes the full content I originally prepared, with visual examples, screenshots, and a detailed explanation of how the module works—including the configuration and purpose of the "unique name" field.

The confusion earlier was due to the fact that the complete documentation was only in the manually written README.rst, and not split into the individual files under the readme/ folder. So when odoo-gen-readme generated the new README.rst, much of that content was missing, making it harder to understand the module’s logic.

Now everything is correctly structured and visible directly in the generated README.rst, making it much easier to understand the module's behavior and configuration at a glance.

You can test the application locally and use the examples included in the readme to verify it works correctly.

Thank you for your time.

[jesanmor]

Hi @gdgellatly,

Thank you for all the suggestions you made — they really helped me improve the quality of the code.
Have you had a chance to review it again? If there’s anything else that needs to be adjusted, I’m happy to work on it.
Or, if everything looks good to you, would you consider approving the pull request?

Thanks again for your time and guidance — it’s been very helpful, especially as this is my first contribution to the OCA. 😊

Best regards,
Jesús (jesanmor)

[thomaspaulb]

@jesanmor It's sad to see a nice module not get reviewed, have you considered doing review swaps with other PR's? Review someone else's to get yours reviewed

[jesanmor]

@jesanmor It's sad to see a nice module not get reviewed, have you considered doing review swaps with other PR's? Review someone else's to get yours reviewed

@thomas Paulb thanks for your kind words about the module.

And apologies for the late reply — I’ve just seen your message. A few months ago I started a new job and things have been quite busy, so I haven’t been very active on GitHub lately.

This contribution was mainly my way of giving something back to the community, since I’ve benefited a lot from OCA projects in the past. To be honest, this was my first contribution and I wasn’t very familiar with how the review process usually works.

I’m also very grateful to @gdgellatly for the time he took to review the code and for his valuable suggestions — they really helped improve the quality of the module.

Your suggestion about reviewing other PRs makes a lot of sense. I’ll definitely keep it in mind, although I’m not entirely sure where to start yet.

Thanks again for taking the time to comment and for the encouragement!

[jesanmor]

I accidentally closed the PR while reviewing the conversation.
I've reopened it and will continue addressing the feedback.