[17.0][ADD] server_environment_autocreate: add possibility of auto creating records#188
Conversation
3c8a812 to
69c34fd
Compare
|
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. |
69c34fd to
f320932
Compare
|
@OCA/server-environment-maintainers Reviews welcome |
simahawk
left a comment
There was a problem hiding this comment.
Hello, thanks for your contrib.
TBH Is not a feature that I would include in our projs but can be interesting for others.
However:
- you should move this feature to a separate module
- you should take care of failure: some create can fail (eg: missing required fields)
- it can be very tricky to handle because you might have models that are not fully loaded yet
| for model_name in self.env: | ||
| model = self.env[model_name] | ||
| if hasattr(model, "_server_env_global_section_name"): | ||
| global_section_name = model._server_env_global_section_name() | ||
| for section in serv_config: | ||
| if section.startswith(f"{global_section_name}."): | ||
| if serv_config[section].get("__autocreate", None): | ||
| name_value = section[len(global_section_name) + 1 :] | ||
| domain = [ | ||
| (model._server_env_section_name_field, "=", name_value) | ||
| ] | ||
| count = model.with_context(active_test=False).search_count( | ||
| domain | ||
| ) | ||
| if count == 0: | ||
| _logger.debug("Automatic creation of %s", section) | ||
| values = literal_eval( | ||
| serv_config[section].get("__autocreate") | ||
| ) | ||
| values[ | ||
| model._server_env_section_name_field | ||
| ] = name_value | ||
| records = model.create([values]) | ||
| self.env["ir.model.data"].create( | ||
| [ | ||
| { | ||
| "name": section, | ||
| "model": model_name, | ||
| "module": "__server_environment__", | ||
| "res_id": record.id, | ||
| } | ||
| for record in records | ||
| ] | ||
| ) |
There was a problem hiding this comment.
Why would you loop on all the models? This mixin will be inherited by a specific model and that model only should be set up...
| for model_name in self.env: | |
| model = self.env[model_name] | |
| if hasattr(model, "_server_env_global_section_name"): | |
| global_section_name = model._server_env_global_section_name() | |
| for section in serv_config: | |
| if section.startswith(f"{global_section_name}."): | |
| if serv_config[section].get("__autocreate", None): | |
| name_value = section[len(global_section_name) + 1 :] | |
| domain = [ | |
| (model._server_env_section_name_field, "=", name_value) | |
| ] | |
| count = model.with_context(active_test=False).search_count( | |
| domain | |
| ) | |
| if count == 0: | |
| _logger.debug("Automatic creation of %s", section) | |
| values = literal_eval( | |
| serv_config[section].get("__autocreate") | |
| ) | |
| values[ | |
| model._server_env_section_name_field | |
| ] = name_value | |
| records = model.create([values]) | |
| self.env["ir.model.data"].create( | |
| [ | |
| { | |
| "name": section, | |
| "model": model_name, | |
| "module": "__server_environment__", | |
| "res_id": record.id, | |
| } | |
| for record in records | |
| ] | |
| ) | |
| global_section_name = self._server_env_global_section_name() | |
| for section in serv_config: | |
| if not section.startswith(f"{global_section_name}."): | |
| continue | |
| if not serv_config[section].get("__autocreate", None): | |
| continue | |
| name_value = section[len(global_section_name) + 1 :] | |
| domain = [ | |
| (model._server_env_section_name_field, "=", name_value) | |
| ] | |
| count = model.with_context(active_test=False).search_count( | |
| domain | |
| ) | |
| if not count: | |
| _logger.info("Automatic creation of %s", section) | |
| values = literal_eval( | |
| serv_config[section].get("__autocreate") | |
| ) | |
| values[ | |
| model._server_env_section_name_field | |
| ] = name_value | |
| # TODO: handle failure | |
| records = model.create([values]) | |
| self.env["ir.model.data"].create( | |
| [ | |
| { | |
| "name": section, | |
| "model": model_name, | |
| "module": "__server_environment__", | |
| "res_id": record.id, | |
| } | |
| for record in records | |
| ] | |
| ) |
Something like this would be better.
|
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. |
This is a feature that is optional, so I did not see the need for a separate module. Have you any reason for doing it in a separate module? Concerning your other point, I do not expect all cases to work out of the box. Basic cases (outgoing mail servers, fs storage), it works and the current version is already in production. |
That's exactly the main reason to move it to its own module. This is not something we want to maintain OOTB ;) |
a4493d7 to
869a0f2
Compare
|
I changed the feature to a new module as suggested. |
869a0f2 to
27b37b2
Compare
simahawk
left a comment
There was a problem hiding this comment.
LG overall, minor comments
27b37b2 to
00bc821
Compare
00bc821 to
ae59432
Compare
e243177 to
1f5006f
Compare
1f5006f to
f3c31a4
Compare
|
/ocabot merge nobump |
|
This PR looks fantastic, let's merge it! |
|
Congratulations, your PR was merged at b31f61e. Thanks a lot for contributing to OCA. ❤️ |
We need to create mail server or fs.storage in the interface to match those configured with server_environment. This enhancement proposes to let server_environment creates it automatically instead.