Skip to content

[18.0][IMP] server_environment: Preserve not env managed data - #264

Merged
OCA-git-bot merged 2 commits into
OCA:18.0from
acsone:18.0-server_env_preserve_data-qgr
Jul 28, 2026
Merged

[18.0][IMP] server_environment: Preserve not env managed data#264
OCA-git-bot merged 2 commits into
OCA:18.0from
acsone:18.0-server_env_preserve_data-qgr

Conversation

@qgroulard

Copy link
Copy Markdown
Contributor

Heavily inspired from #219, thank you @rven for your contribution.

I wanted to make @rven script more generic because this problem potentially happens every time we add the server.env.mixin to a model or when we add fields to the _server_env_fields() list.


Helper function typically used for hooks and migration scripts.
Restores database values for fields transitioning to 'server env managed'.

When a field is defined as managed by the server environment, Odoo ignores the value stored in the database, prioritizing the environment configuration instead. If no environment configuration exists, the field may effectively lose its previous value.

This method forces to 'persist' these values if they are not explicitly overridden by the current environment configuration.

@qgroulard
qgroulard force-pushed the 18.0-server_env_preserve_data-qgr branch from 65f05d6 to f772104 Compare April 2, 2026 10:07
@qgroulard
qgroulard marked this pull request as ready for review April 2, 2026 10:09
return

@api.model
def preserve_not_env_managed_data(self):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def preserve_not_env_managed_data(self):
def _preserve_not_env_managed_data(self):

If this is to be used in migration scripts, maybe it's safer to pass the list of fields/columns to handle explicitly?

@qgroulard
qgroulard force-pushed the 18.0-server_env_preserve_data-qgr branch from f772104 to 51885bd Compare April 2, 2026 13:06
Helper function typically used for hooks and migration scripts.
Restores database values for fields transitioning to 'server env managed'.

When a field is defined as managed by the server environment, Odoo
ignores the value stored in the database, prioritizing the environment
configuration instead. If no environment configuration exists, the field
may effectively lose its previous value.

This method forces to 'persist' these values if they are not
explicitly overridden by the current environment configuration.
@qgroulard
qgroulard force-pushed the 18.0-server_env_preserve_data-qgr branch from 51885bd to 7bec03f Compare April 2, 2026 13:07
…cation"

To preserve the value of field "smtp_authentication" if we don't set it from
the environment.
Which may happen to anyone updating the sources of mail_environment for its project.

@rousseldenis rousseldenis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sbidoul

sbidoul commented Jul 28, 2026

Copy link
Copy Markdown
Member

/ocabot merge patch

@OCA-git-bot

Copy link
Copy Markdown
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 18.0-ocabot-merge-pr-264-by-sbidoul-bump-patch, awaiting test results.

@OCA-git-bot
OCA-git-bot merged commit f688701 into OCA:18.0 Jul 28, 2026
5 of 7 checks passed
@OCA-git-bot

Copy link
Copy Markdown
Contributor

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

@sbidoul
sbidoul deleted the 18.0-server_env_preserve_data-qgr branch July 28, 2026 08:43
record_values = {}
for field_name in field_name_list:
if field_name in row:
record_values[field_name] = row[field_name]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be complete, this may need conversion based on the field type. But this is good enough for practical use cases of server env fields.

@yankinmax

yankinmax commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

I'm working on the issue:

Therefore, I've opened several PR's to remove server_environment dependency from OCA/storage and OCA/web-api modules.

For example:

Only on that v18 split PR the current PR issue was raised in the tests and the logical fix was to add a _preserve_not_env_managed_data post init hook on the impacted modules.

Why do we need this hook there:

Before this split (storage_backend to storage_backend and storage_backend_environment), configuration fields such as storage.backend::backend_type and storage.backend::filename_strategy were normal stored database fields.
The new environment module apply server.env.mixin to these fields. The mixin changes how their values are obtained:

  1. If an environment key exists, its value is used.
  2. Otherwise, a value stored in server_env_defaults is used as a fallback.

When the environment module is installed on an existing database, the old value is still present in the original SQL column, but it is not automatically copied into server_env_defaults.
As a result, Odoo can return False instead of the existing value.
This caused errors such as component usage False and empty filename strategies.
storage_backend_environment module therefore will have a post_init_hook calling _preserve_not_env_managed_data.
The helper reads the original column values and stores them as environment fallbacks after the module is installed.
The preserved database value is used only when the corresponding environment key is absent.

The question which now comes to me:

  1. Do we need to backport this helper to v16 and v17, forward port to v19?
  2. Should we reuse it on each branch starting from v16?

From what I see it impacts all the versions.

As for now we've decided to cover server_environment dependency removal starting from v16:

OCA/storage PR's that can be updated with _preserve_not_env_managed_data post init hook:

OCA/web-api PR's that can be updated with _preserve_not_env_managed_data post init hook:

@lmignon @sbidoul @etobella @rousseldenis @ivantodorovich WDYT?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants