QUAL: API contract: RestException update, check if thirdparty exists before creating/updating contract

[JonBendtsen]

QUAL API contract: RestException update, check if thirdparty exists before use in POST/PUT

This PR adds some RestExceptions to the contract api. It also checks if thirdparty exists before creating/updating a contract. As well as introduce some hurl testing of api contract

[JonBendtsen]

New result if thirtparty id does not exist

Old result

{
  "error": {
    "0": "UnknownError: Cannot add or update a child row: a foreign key constraint fails (`dolidb`.`llx_contrat`, CONSTRAINT `fk_contrat_fk_soc` FOREIGN KEY (`fk_soc`) REFERENCES `llx_societe` (`rowid`))",
    "code": 500,
    "message": "Internal Server Error: Error creating contract"
  }
}

[JonBendtsen]

PUT error before changes

{
  "error": {
    "code": 500,
    "message": "Internal Server Error: Error Cannot add or update a child row: a foreign key constraint fails (`dolidb`.`llx_contrat`, CONSTRAINT `fk_contrat_fk_soc` FOREIGN KEY (`fk_soc`) REFERENCES `llx_societe` (`rowid`))"
  }
}

New put error message :-)

[JonBendtsen]

The improvements that this PR addresses was detected during bug #35655

[JonBendtsen]

Hurl tests good - one of these days autocorrect is gonna write hurl tastes good ;-)

JonSweet16:hurl jonbendtsen$ ./run.sh 
----- Run hurl test on APIs ---
First we run tests that do not require authentication
Success gui/00_HOME.hurl (1 request(s) in 293 ms)
Success api/00_foobar.hurl (1 request(s) in 326 ms)
Success api/status/00_status.hurl (1 request(s) in 343 ms)
Success api/00_explorer.hurl (1 request(s) in 349 ms)
Success public/payment/00_payment_newpayment.hurl (1 request(s) in 381 ms)
Success public/onlinesign/00_onlinesign_newonlinesign.hurl (1 request(s) in 400 ms)
Success api/login/00_login_POST.hurl (1 request(s) in 1347 ms)
Success api/login/00_login_GET.hurl (1 request(s) in 1347 ms)
--------------------------------------------------------------------------------
Executed files:    8
Executed requests: 8 (5.9/s)
Succeeded files:   8 (100.0%)
Failed files:      0 (0.0%)
Duration:          1350 ms

Now we are ready to run API tests that do require authentication
Success api/status/10_status.hurl (1 request(s) in 137 ms)
Success api/setup/10_setup_conf.hurl (1 request(s) in 514 ms)
Success api/setup/10_setup_company.hurl (1 request(s) in 535 ms)
Success api/mailings/10_mailings.hurl (34 request(s) in 3238 ms)
Success api/emailtemplates/10_emailtemplates.hurl (19 request(s) in 3553 ms)
Success api/users/10_users.hurl (17 request(s) in 3615 ms)
Success api/contracts/10_contracts.hurl (21 request(s) in 3813 ms)
Success api/warehouses/10_warehouses.hurl (34 request(s) in 5433 ms)
Success api/setup/10_setup_modules.hurl (14 request(s) in 5657 ms)
--------------------------------------------------------------------------------
Executed files:    10
Executed requests: 155 (27.4/s)
Succeeded files:   9 (90.0%)
Failed files:      1 (10.0%)
Duration:          5659 ms

[JonBendtsen]

Maybe #36024 has to be merged before, because they might conflict?

[hregis]

@JonBendtsen

for acivateLine function the right is : DolibarrApiAccess::$user->hasRight('contrat', 'activer')
for unactivateLine function the right is : DolibarrApiAccess::$user->hasRight('contrat', 'desactiver')
in post function $this->contract->entity don't exists during the creation !

[hregis]

@JonBendtsen @eldy

this is not good !! getEntity() can return "1,2,3,x" !

if ($field == 'entity' && $value != getEntity('contrat')) {

[hregis]

@JonBendtsen getEntity('contrat') return contract sharings with the current entity, not the authorized sharing of a user

[eldy]

@JonBendtsen getEntity('contrat') return contract sharings with the current entity, not the authorized sharing of a user

We could make the test on $conf->entity because conf->entity is already value we are logged on with DOLAPIENTITY
Or we can just replace with a inarray

We can also add a check in phpunit in CodingPhpTest to return a CI error if we find "= getEntity" to avoid error in future.

[eldy]

@JonBendtsen getEntity('contrat') return contract sharings with the current entity, not the authorized sharing of a user

@hregis I am not sure I understand where and why there is an issue.

getEntity() is used in public function index which I did not change, and in public function getLineswhich I also did not change?

Do you want me to use getEntity() more places? or remove the usage?

Pb is just that you do a != on string with several int instead of an int.
Try
!in_array((string) $entity, explode(',', getEntity('contract')))

[JonBendtsen]

I had some editor update issues such that it did not show the right file contents even though I had switched the branch - probably because I changed the branch out under it. I will submit a new PR

[hregis]

@JonBendtsen @eldy The creation and updating of contracts (or other objects) should be limited to "$conf->entity" because contracts can be shared, but if we are in the current entity with value 1 and the "$entity" field at the time of creation is set to 2... we should also check if the user has the right to create contracts on entity 2!