-
-
Notifications
You must be signed in to change notification settings - Fork 3.2k
FIX Link member to third party - no check, no feedback 26864 #36570
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 18.0
Are you sure you want to change the base?
FIX Link member to third party - no check, no feedback 26864 #36570
Conversation
|
I've run into some questions that needs to be determined. First questionI'm adding a check for users access to the thirdparties included, and that is always granted. I think this is due to this check missing in v18, that I only added in v22. https://github.com/Dolibarr/dolibarr/pull/36408/files Do we want to backport that fix to v18, do we want to ignore it? Or do we want to fix is from some other version between v18 and v22? Second questionAt first If I try to break the thirdparty link with a PUT with a socid=0, then nothing happens, most likely this right here: Do we want to support the API breaking a link to a thirdparty? Either using the PUT or do we really want to make a special endpoint just for that? So far I "fixed" it in PUT with using $member->setThirdPartyId(0) but we could probably also remove that check in Line 823 sub decisionI decided to ban direct from thirdpartyA to thirdpartyB updates, forcing a 2 step process by first resetting and removing the link with socid=0, and then the 2. step would be to update again with the new thirdparty value. test casesmemberA and thirdpartyA not linked both with emailA + thirdpartyB with different email worksfrom any socid, update memberA with socid=0 failsfrom socid=thirdpartyA, update memberA with thirdpartyB fails, stating use socid=0 work arroundfirst set it to socid=0 and then to new socid |
rycks
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hello @JonBendtsen
i'm sorry but i'm quite sure that such PR could not be integrated into 18.0 because as you says it introduce a break ... i share your point of view "that's a bug" but as a merger
we can't apply it i think ... but maybe @eldy will have an other point of view ?
Hey @rycks. I appreciate that you are concerned about breaking changes, I am too, which is why I mentioned that this was a breaking change. I'll wait for @eldy's input before making any changes. |
FIX Link member to third party - no check, no feedback 26864 - BREAKING CHANGE
This PR is a partial fix of bug 26864, it only handles the API. The bug was that one could silently overwrite an existing link between memberA and a thirdpartyA by specifying a new link to thirdpartyB. This PR introduces bans direct thirdparty links when there is an existing link and instead reports back that the user has to use a 2 step process by first setting the link to 0, aka remove the link, and then introduce the new link.
Breaking Change
You can no longer just update socid using the API, you have to use a 2 step process by first setting it to 0, and then setting it to the new value