FIX public project form return an error if SOCIETE_EMAIL_UNIQUE

[iouston]

if SOCIETE_EMAIL_UNIQUE is 1, this form returns an error because the third party already exists.

[eldy]

$GETPOST ? Are you sure ?
Also why adding 'alpha' as parameter of GETPOST instead of default value ?

[iouston]

Oops!
Of course, only GETPOST() is sufficient.
I added 'alpha' out of habit, but in fact, the default parameter is sufficient too.

[rycks]

@lvessiller-opendsi it could be our first PR to review together on 18.0 ? little one :)

@eldy, that PR makes me a new question about our new role : @iouston makes a PR againts "email" ... ok, i understand our role is only to say if that fix is ok or not

but ... thanks to @iouston's PR i make a little review of the file (htdocs/public/project/new.php) and i found something not clear / (a bug i think) in relation with email property (line 350 in develop), do i open a new ticket with that ? is that a point where lionel and me could make some "good job" or not ?

[eldy]

@lvessiller-opendsi it could be our first PR to review together on 18.0 ? little one :)

@eldy, that PR makes me a new question about our new role : @iouston makes a PR againts "email" ... ok, i understand our role is only to say if that fix is ok or not

but ... thanks to @iouston's PR i make a little review of the file (htdocs/public/project/new.php) and i found something not clear / (a bug i think) in relation with email property (line 350 in develop), do i open a new ticket with that ? is that a point where lionel and me could make some "good job" or not ?

Sure, if by analizing one pr, you find something you don't think good, you can open an issue but this must be done out of analyzed pr to avoid noise in comments.
I think you can work this way for dual validation of pr. The first one of you (eric or lionel) that anlyze the PR add a comment "ok for me or ko because..., ping @theother"
The second one confirm comment by adding its comment too pinging the other. If both are ok, the second one also assign the issue for approbation to me.

[lvessiller-opendsi]

@iouston
I think it's ok but what is the relation of the error with "SOCIETE_EMAIL_UNIQUE" const ?

[iouston]

if you use this constant, the old code return an error beacause the thirdparty already exist. Project isn't added.
this error is returned because the fecth method send 1 but, after the SOCIETE_EMAIL_UNIQUE stop the process

[lvessiller-opendsi]

if you use this constant, the old code return an error beacause the thirdparty already exist. Project isn't added. this error is returned because the fecth method send 1 but, after the SOCIETE_EMAIL_UNIQUE stop the process

-You would say the fetch method return 0 (not found) and after the third-party can't be created because it exists with the posted email and the SOCIETE_EMAIL_UNIQUE stop the process.
With your fix the third-party is found with the posted email and the project is added.
Is it correct ?

[rycks]

ok for me, in all case object->email could not exists because object is a "Project" and Project do not have email attribute ...

[iouston]

if you use this constant, the old code return an error beacause the thirdparty already exist. Project isn't added. this error is returned because the fecth method send 1 but, after the SOCIETE_EMAIL_UNIQUE stop the process

-You would say the fetch method return 0 (not found) and after the third-party can't be created because it exists with the posted email and the SOCIETE_EMAIL_UNIQUE stop the process. With your fix the third-party is found with the posted email and the project is added. Is it correct ?

That's quite right

[lvessiller-opendsi]

ok for me @rycks

[lvessiller-opendsi]

@eldy
We are agree with @rycks, could you have a check and could we have your feelings about it ?

[eldy]

PR started with around 25 lines modified and finished with finally just one line modified => good.
I'm ok for what remain. Can you tell me if you have the Merge/Squash button on the PR ? Like on this screenshot ?

[lvessiller-opendsi]

PR started with around 25 lines modified and finished with finally just one line modified => good. I'm ok for what remain. Can you tell me if you have the Merge/Squash button on the PR ? Like on this screenshot ?

No I haven't this "merge" button :

Only those with write access to this repository can merge pull requests.

[eldy]

Hum, so we may find a better worklow.
Do you have permission to set the Reviewers here

If yes, can you set you and eric as Reviewer and set the review status to OK ? (i am still experimenting all the solution the github give us to be able to collaborate...)

[rycks]

review ok for me

[lvessiller-opendsi]

@eldy
Only you can merge :)

[rycks]

@eldy we don't have the "squash and merge" button -> 💪