BUG: Mo::deleteLine removes the "main" MoLine if consumed line is delete + GETPOST in class

[Humml87]

Bug

At MoLine with Product type "Services":
Delete consumed line...

The result... the parent MoLine is deleted:

In the database is the main MoLine deleted, but the linked MoLine is existing!

Environment Version

19

Environment OS

No response

Environment Web server

No response

Environment PHP

No response

Environment Database

No response

Environment URL(s)

No response

Expected and actual behavior

No response

Steps to reproduce the behavior

No response

Attached files

No response

[eldy]

Seems fixed with: b870a84
If not can you describe a process to reproduce the bug. I am not able to reproduce after the fix b870a84

[Humml87]

Seems fixed with: b870a84 If not can you describe a process to reproduce the bug. I am not able to reproduce after the fix b870a84

I saw this fix (the main bug was fixed), but i think it's not a real clean fix, i found something wich is not good in my opinion:

Mo.class.php
Line 902 => $fk_movement = GETPOST('fk_movement', 'int');
-> I think a GETPOST is not good in this class, this make problems if the function will be called from elsewhere (e.g. WebService).

Line 936 / 959 / 967 => setEventMessages(....);
-> It's a part of view, i think also its not ok in the class.

Others

• It's able to delete a line if the Mo is in a invalid status. I think the test must near on the delete function. To make it safe to use the delete of a MoLine from elsewhere.
• Some logic works only if the call comes from the mo_production.php. If we call as example MoLine::delete from elsewhere, it make some trouble. I think this make some new Bug´s in the future.

After your fix, is it now possible to rename this PR to a QUAL PR?

I work currently on the WebService API and there i become all these problems.

Best regards Christian

[eldy]

You're right when you say that GETPOST should not appear into CRUD files. This is an old bad practice. We are removing them step by step.
For example, the case mentionned about Line 902 => $fk_movement = GETPOST('fk_movement', 'int');
It has been already replaced into develop branch.

Except the setEventMessage, not yet removed, I still not see why you may have trouble when calling the MoLine->delete() from a non GUI context.

[Humml87]

I still not see why you may have trouble when calling the MoLine->delete() from a non GUI context.

Hi @eldy ,
as example:

Now, if i call from non GUI MoLine->delete() for id 430:

What is not good currently:

• It's possible to make a invalid db state.
• Now if we have a invalid db state, the call GET MoLines give some confuse Lines.
• It's possible to delete a MoLine, without verify the Mo state

... now its possible to make a new (copy) of the logic to the WebService function, but this is not the target i think.

... i think i make a good code with reduce some problems, but i see you don't like this. What is your target, what can i make better?

Best regards
Christian