Skip to content

[FIX] stock: forbid to modify product on done move line#42608

Closed
amoyaux wants to merge 1 commit intoodoo:11.0from
odoo-dev:11.0-import_done_move_line-arm
Closed

[FIX] stock: forbid to modify product on done move line#42608
amoyaux wants to merge 1 commit intoodoo:11.0from
odoo-dev:11.0-import_done_move_line-arm

Conversation

@amoyaux
Copy link
Copy Markdown
Contributor

@amoyaux amoyaux commented Jan 2, 2020

It happens that people modify the product on done stock.move.line
(it's not possible without customisation, at least allow to import or
to modify product and lot_id in the same view).

During the write on stock.move.line only the lot,locations,package and
owner are update on the quant. Not the product since it's not suppose to
be modify. It leads to a stock.move.line with a correct information but
a total mess on the quants with a lot updated and the previous product.
Since the product is not modified, the product on the quant and the
product on the lot linked to the same quant are different.

Task: 2119471

Description of the issue/feature this PR addresses:

Current behavior before PR:

Desired behavior after PR is merged:

--
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

@C3POdoo C3POdoo added the RD research & development, internal work label Jan 2, 2020
@amoyaux amoyaux force-pushed the 11.0-import_done_move_line-arm branch 2 times, most recently from 3b32cd3 to 784faa7 Compare January 7, 2020 13:31
@robodoo robodoo added the CI 🤖 Robodoo has seen passing statuses label Jan 7, 2020
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.

also problematic is assigned and partially available.. and maybe the source of "the cannot unreserve more than.." tickets
if a sml is reserved and the product is changed trough an import, the original quant stays reserved and some of the newly changed producs are unreserved

cc @nvn-odoo @jev-odoo

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@sle-odoo TY for the info ;)

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.

i would change the message to
"Changing the product of this record is forbidden at this point."

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If you suggest changing the message, then the 'at this point' seems not very precise for the customer.

It could be "Changing the product of this record is forbidden on state 'Done'." or something like that. The more precise info for the customer, the less Support ticket ;)

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.

@sle-odoo indeed, thank you for the notification :-)

@amoyaux amoyaux force-pushed the 11.0-import_done_move_line-arm branch from 784faa7 to 37807f5 Compare January 9, 2020 13:59
@robodoo robodoo added CI 🤖 Robodoo has seen passing statuses and removed CI 🤖 Robodoo has seen passing statuses labels Jan 9, 2020
@amoyaux amoyaux force-pushed the 11.0-import_done_move_line-arm branch from 37807f5 to ef3b7a1 Compare January 9, 2020 14:56
@robodoo robodoo added CI 🤖 Robodoo has seen passing statuses and removed CI 🤖 Robodoo has seen passing statuses labels Jan 9, 2020
@amoyaux
Copy link
Copy Markdown
Contributor Author

amoyaux commented Jan 10, 2020

@nim-odoo Could you check?

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.

Missing the _() translation function?

By the way, the wording of the error message is not clear and WILL trigger support tickets of customers asking what means this record and at this point
Can we use something like "Editing the product set on this stock move line is not allowed as some stock changes are already taken into account"?

Thanks :)

Copy link
Copy Markdown
Contributor

@sle-odoo sle-odoo Jan 10, 2020

Choose a reason for hiding this comment

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

please note that you cannot get this error issue without editing the views and removing the readonly or exporting the ml/changing the product_id/reimporting them, so you won't get more tickets before since the concerned users already fucked their inventory at this point and thus already opened tickets with "cannot unreserve more than"

imo it's not important to have a super clear and specific error messages here, blocking the flow is important

anyway let nim decide

@amoyaux amoyaux force-pushed the 11.0-import_done_move_line-arm branch from ef3b7a1 to cfc8bce Compare January 10, 2020 09:46
@robodoo robodoo added CI 🤖 Robodoo has seen passing statuses and removed CI 🤖 Robodoo has seen passing statuses labels Jan 10, 2020
Copy link
Copy Markdown
Contributor

@nim-odoo nim-odoo left a comment

Choose a reason for hiding this comment

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

I'm not against the change, but since the issue can only appear if the user is doing something nasty I'd go more for a fix in v13. About the wording, maybe something like:

Changing the product is only allowed in 'Draft' state.

The average Joe doesn't know what is a 'record'. Moreover, they know when they can do it. With the current wording, they only know when they cannot do it.

@amoyaux amoyaux force-pushed the 11.0-import_done_move_line-arm branch from cfc8bce to bf424b1 Compare January 10, 2020 10:47
@robodoo robodoo removed the CI 🤖 Robodoo has seen passing statuses label Jan 10, 2020
It happens that people modify the product on done stock.move.line
(it's not possible without customisation, at least allow to import or
to modify product and lot_id in the same view).

During the write on stock.move.line only the lot,locations,package and
owner are update on the quant. Not the product since it's not suppose to
be modify. It leads to a stock.move.line with a correct information but
a total mess on the quants with a lot updated and the previous product.
Since the product is not modified, the product on the quant and the
product on the lot linked to the same quant are different.

Task: 2119471
@amoyaux amoyaux force-pushed the 11.0-import_done_move_line-arm branch from bf424b1 to 3491a5d Compare January 10, 2020 10:48
Copy link
Copy Markdown
Contributor

@nim-odoo nim-odoo left a comment

Choose a reason for hiding this comment

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

robodoo r+

robodoo pushed a commit that referenced this pull request Jan 10, 2020
It happens that people modify the product on done stock.move.line
(it's not possible without customisation, at least allow to import or
to modify product and lot_id in the same view).

During the write on stock.move.line only the lot,locations,package and
owner are update on the quant. Not the product since it's not suppose to
be modify. It leads to a stock.move.line with a correct information but
a total mess on the quants with a lot updated and the previous product.
Since the product is not modified, the product on the quant and the
product on the lot linked to the same quant are different.

closes #42608

Task: 2119471
Signed-off-by: Nicolas Martinelli (nim) <nim@odoo.com>
@robodoo robodoo added CI 🤖 Robodoo has seen passing statuses merging 👷 and removed CI 🤖 Robodoo has seen passing statuses merging 👷 labels Jan 10, 2020
@robodoo
Copy link
Copy Markdown
Contributor

robodoo commented Jan 10, 2020

Staging failed: timed out (>120 minutes)

@sle-odoo
Copy link
Copy Markdown
Contributor

robodoo retry

@robodoo robodoo added CI 🤖 Robodoo has seen passing statuses r+ 👌 and removed error 🙅 CI 🤖 Robodoo has seen passing statuses labels Jan 10, 2020
@robodoo
Copy link
Copy Markdown
Contributor

robodoo commented Jan 10, 2020

Staging failed: timed out (>120 minutes)

@sle-odoo
Copy link
Copy Markdown
Contributor

robodoo retry

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

Labels

CI 🤖 Robodoo has seen passing statuses RD research & development, internal work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants