coolOrangeLabs / powerGateTemplate

powerGate full functional sample ERP integration
MIT License
3 stars 3 forks source link

Refactoring: Handling of ERP errors #210

Closed ThomasRossmeisl closed 2 years ago

ThomasRossmeisl commented 2 years ago

Description

Currently ERP errors are captured by the -OnConnect script and checked by the Edit-ResponseWithErrorMessage function after ERP communication. This approach has the following issues:

Remarks

Related Issues

TollJulian commented 2 years ago

I worked on this issue on the Sotawall project and could resolve the following tasks:

Edit-ResponseWithErrorMessage returns different data type depending on input AND external server result. This makes it impossible to know the result of the function and caused nullpointer exceptions in multiple projects as the developer forgot to handle all possible return values. Also it makes conditions harder to read and write as multiple different results need to be handled.

Now the Edit-ResponseWithErrorMessage returns always a hashtable with this structure: $result = @{Entity = (the ERP object the check was made for); ErrorMessage = (the powerGate error)}

The functions using Edit-ResponseWithErrorMessage do not indicate in their name that they check the erp response. This exacerbates the issue with the mixed return values, but also makes the code harder to understand in general as the names indicate an erp object is returned at all times

The names of all the functions using Edit-ResponseWithErrorMessage end with WithPGError. To evidence their return type, I renamed the variables who call those functions. (e.G. $erpSomethingHashtable = doSomethingWithPGError(...)

There are no separate cases for error and "new" case. This makes the code hard to understand and causes entities to be marked as 'new' incorrectly

The hashtable structure allows to differenciate clearly between Error, New and Existant:

The function calls override the same variables multiple times which makes it hard to tell what value is currently passed to what function and leads to more programming errors

renamed some variables to avoid this

this is already open: The OnConnect code is complicated and hard to understand. A lot of code is required to use it which also might introduce new errors

The solution is pushed into branch issue_#208

TollJulian commented 2 years ago

This has to been tested on the Sotawall Project

ThomasRossmeisl commented 2 years ago

https://github.com/coolOrangeLabs/powerGateTemplate/commit/8c8b9cc22b7116cc67b215be65af0a24eae7db67