Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[10.0] Editable list of many2one does not display correctly #13780

Closed
gdgellatly opened this issue Oct 12, 2016 · 12 comments

Comments

Projects
None yet
5 participants
@gdgellatly
Copy link
Contributor

commented Oct 12, 2016

Impacted versions: 10 (pulled 10 minutes ago)
This is probably related to this old v7 bug which got closed but was never really fixed #3964 .

Steps to reproduce:
While this can be reproduced many ways, the common theme is that you have an editable tree view, inside of an editable tree view.

Setup (or just install testo2m from https://github.com/gdgellatly/odoo_bug_demos.git)

  • Add an m2o field to sale_order_line call testm2o
  • Add an o2m field to testm2o called testo2m
  • Add testm2o to sale order form view
  • Add an editable tree view to testm2o for testo2m

Reproduction

  • Create a sale order and add a line, click create and edit for the m2o relation,
  • Try and enter data in the editable tree

Current behavior:
Everything works as expected except -
The values just disappear as you enter them.
Upon save the values are there.
On edit, changes are not reflected.

Expected behavior:
Values are reflected correctly

Video/Screenshot link (optional):
What it looks like entering
What it looks like after opening after save

@JKE-be

This comment has been minimized.

Copy link
Contributor

commented Oct 13, 2016

@ged-odoo You have probably already spotted the bug in #3964... Can you take a look ?

JKE-be added a commit to odoo-dev/odoo that referenced this issue Oct 13, 2016

[FIX] base_import: use custom date format when importing datetime col…
…umn.

The user date format was ignored for datetime field.
field_date_format was datetime format, but options['date_format'] was date format.

This commit closes odoo#13780
@gdgellatly

This comment has been minimized.

Copy link
Contributor Author

commented Oct 18, 2016

@JKE-be are you sure that commit closes this issue. I'm pretty sure it doesn't.

@gdgellatly

This comment has been minimized.

Copy link
Contributor Author

commented Oct 27, 2016

@ged-odoo @JKE-be This is partly holding up a rather large enterprise agreement. Customer is on v7 without updates since this regression. It is literally the most important thing. If I get the agreement done, would it get fixed under opw? Or is there any way to work around it?

@gdgellatly

This comment has been minimized.

Copy link
Contributor Author

commented Nov 14, 2016

@ged-odoo after some more research, it is most definitely that commit that introduces it, see below from mailing list

All I did as a test in frustration is revert line 473 of list_view.js such that it read (essentially reverting the commit identified in issue 3964

_(_.keys(values)).each(function(key){
    record.set(key, values[key], {silent: true});
// _.each(values, function (value, key) {
//     if (fields[key] && fields[key].type === 'many2many')
//         record.set(key + '__display', false, {silent: true});
//     record.set(key, value, {silent: true});

and everything worked perfectly as expected. So I guess my questions are now:

What exactly is this piece of code doing, i.e. how do they differ?
The commented code was introduced, so that m2m tags did not display upon deletion, is there another way of approaching this, such that the original issue is fixed without regression?

To me it looks like a simple oversight, the code looks roughly equivelant if we reduce it (assuming the if statement fails), but in the case of new records it is not displaying a value.

_(_.keys(values)).each(function(key){
    record.set(key, values[key], {silent: true});

.each(values, function (value, key) {

    record.set(key, value, {silent: true});
@hbrunn

This comment has been minimized.

Copy link
Contributor

commented Nov 15, 2016

any possibility this is fixed by this commit? OCA@25616b9

@gdgellatly

This comment has been minimized.

Copy link
Contributor Author

commented Nov 16, 2016

@hbrunn thanks for your work here, I confirm this fixes the issue

@gdgellatly gdgellatly closed this Nov 16, 2016

@pedrobaeza

This comment has been minimized.

Copy link
Collaborator

commented Nov 16, 2016

Should this be backported in OCB for any of the prior versions?

@hbrunn

This comment has been minimized.

Copy link
Contributor

commented Nov 16, 2016

sure. But on the other hand, we might better fix the root cause and patch underscore. But on the other hand, the problem is known and documented: http://underscorejs.org/#each, so fixing this might not be as simple.

@hbrunn

This comment has been minimized.

Copy link
Contributor

commented Nov 16, 2016

how about a warning (maybe even error) in mtq that fields called length can have unintended side effects (think of all the addons in the web repo that might produce garbage here), and that the field should better be called extent, size, duration or whatever else fits?

@pedrobaeza

This comment has been minimized.

Copy link
Collaborator

commented Nov 16, 2016

The problem is in the JS linter, that I'm not sure which are the possibilities with it. Maybe @moylop260 can answer us. About the patches, can you please make them?

@hbrunn

This comment has been minimized.

Copy link
Contributor

commented Nov 16, 2016

I leave the honors to @gdgellatly

@moylop260

This comment has been minimized.

Copy link
Contributor

commented Nov 16, 2016

@hbrunn could you create a issue in mqt of what is the new lint required, please?

moylop260 added a commit to vauxoo-dev/pylint-odoo that referenced this issue Nov 21, 2016

moylop260 added a commit to vauxoo-dev/pylint-odoo that referenced this issue Nov 21, 2016

pedrobaeza added a commit to OCA/pylint-odoo that referenced this issue Nov 26, 2016

[REF] attribute-deprecated: Deprecate length class attribute (#86)
* [REF] attribute-deprecated: Deprecate length class attribute
Related issue: OCA/maintainer-quality-tools#385 and odoo/odoo#13780

* [REF] .travis.yml: Disable F999

Disable the error: "dictionary key 'name' repeated with  different value"
This error is duplicated from pylint.

moylop260 added a commit to Vauxoo/pylint-odoo that referenced this issue Dec 9, 2016

[MERGE] From OCA/master (#87)
* [REF] global refactoring: better message output and use real file and line number in non-py files (#62)

[REF] global refactoring: better message output and use real file and line number in non-py files

Change from `pylint_odoo/test_repo/broken_module/__init__:1: [W7910(wrong-tabs-instead-of-spaces), ]  model_view_odoo.xml:23 Use wrong tabs indentation instead of four spaces`
to `pylint_odoo/test_repo/broken_module/model_view_odoo.xml:23: [W7910(wrong-tabs-instead-of-spaces), ]  Use wrong tabs indentation instead of four spaces`

* [FIX] rst-syntax-error: Skip unknown directives

* [FIX] rst-syntax-error: Skip unknown roles

* [ADD] missing-import-error, missing-manifest-dependency

* [REF] README: Update messages list

Signed-off-by: Moisés López <moylop260@vauxoo.com>

* Adding isort dependency (#70)

* [REF] requirements: Update developer version of pylint and astroid

* [ADD] Support for 10.0 manifest name

* [REF] missing-import-error: Skip test file
since these files are loaded only when running tests and in such a case your module and their external dependencies are installed.

* [IMP] manifest-version-format: Add valid_odoo_versions parameter to force a valid version of odoo in the manifest version

* [FIX] missing-import-error: Updating libraries used from requirements.txt but not imported or nested imported from odoo

* [FIX] manifest-version-format: Support -e manifest-version-format only

* [FIX] manifest-version-format: Fix regex to use explicit dot instead of any char

* [FIX] Whitelist `anybox.testing.openerp`
* Add `anybox.testing.openerp` - Fixes #81

* [ADD] missing-return
If you use call a `super` method then you will need return the original
value.
If you want overwrite a original method then you need add documentation
of why and add a `pylint: disable=missing-return`

* [REF] attribute-deprecated: Deprecate length class attribute (#86)

* [REF] attribute-deprecated: Deprecate length class attribute
Related issue: OCA/maintainer-quality-tools#385 and odoo/odoo#13780

* [REF] .travis.yml: Disable F999

Disable the error: "dictionary key 'name' repeated with  different value"
This error is duplicated from pylint.

* [FIX] method-NAME: Fix case compute=None
Fix OCA#88
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.