ThomasRossmeisl commented 2 years ago

Description

The function "CompareErpBom" and "Check-Items" incorrectly marks assemblys and parts as "new" in multiple situations

When an error other than 500 is thrown it is not collected by Edit-ResponseWithErrorMessage
-> Fixed by change to GetPowerGateError
When an error is collected by Edit-ResponseWithErrorMessage it is not caught in Check-Items and Check-Boms as the condition for error and "New" is the same
-> Fixed by #210
If an exception is thrown in Check-Boms it causes entities to stay in wrong state. This happens, because CompareErpBom creates multiple objects for the same entity and the most recently added will also be the last to modify the UI. But if an error happens e.g. after setting a bom head to "new" and before setting it to "error" it will stay on "new".
-> Won't be addressed as this is an edge case and the change would require tremendous time effort.

This is a major issue as this can cause duplicate data to be created in the ERP system which might end up costing the customer tons of money

Steps to reproduce

It might be necessary to remake the compare functions. If major changes are needed it would be good to not use an if cascade and instead make use of return, continue and break statements. This allows a programmer to immediately see whether a change has effects on the following script or not. It also allows to define parts of the script with clear responsibilities like checking bom head for errors comparing vault bom row with erp bomrow, comparing erp bomrow with vault bomrow, and so on.
Also New-Object allocates memory on the heap, which is performance heavy. If the function needs to be remade it would be good to only create as many objects as needed to improve performance