Skip to content
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

NEW add relation type on element_element #29329

Conversation

rycks
Copy link
Contributor

@rycks rycks commented Apr 12, 2024

datamodel evolution

That PR is the first step for a basic need : to be able to make links between elements like thirdparts - thirdparts with an information to complete the relation "what is the role provided".

thirdparts - thirdparts relations

I have a lot of thirdparts in my dolibarr:

  • A: a technical companie A for photocopier
  • B: a technical companie B for photocopier
  • C: a consultant C for training
  • D: a consultant D for training
  • .../...
  • Z: my direct customer

i would make some links between customer and A and an other link between customer and D because wen Z call me i need to know who i have to call to make the intervention if the problem is with photocopier (A) or D if they need a new training ?

for the moment that is in textual notes (private) on thirdpart but we cannot make requests againts that ...

projects are an other solution, we could set up a project and make relations and roles on projects

but in a lot of situations that's just a link between thirdparts then if we could store "relation type" in element_element table we could use that table to make it

is that a good idea ? i hope, the good solution ? i hope too :)

a little schema

image

Note: that's not only for thirdpart - thirdpart relations like in my example

@eldy
Copy link
Member

eldy commented Apr 13, 2024

Looks good to me.
However, you must also add the instruction into migration file for v20

@eldy eldy added the PR to fix - See feedback in comments PR needs to be fixed to be integrated (some comments should describes the fix to do) label Apr 13, 2024
@rycks
Copy link
Contributor Author

rycks commented Apr 16, 2024

@eldy ok thanks i will do it but i just have a question about the new field : it could be better with a foreign key and a dictionary no ? Then people could add relations type via the dictionary and element_element will be lighter (store only a key, not a varchar) ?

@eldy
Copy link
Member

eldy commented Apr 16, 2024

@eldy ok thanks i will do it but i just have a question about the new field : it could be better with a foreign key and a dictionary no ? Then people could add relations type via the dictionary and element_element will be lighter (store only a key, not a varchar) ?

It depends on how you plan to use the new field. Will a different value need dedicated code (in such a case a hard coded "if value" will be added) so an open key is enough. If all cases will be managed the same way and nature of link will be selected the same way in a combo, then having a dictionary is better.
Because both (with dictionary tables and with an open key) are acceptable, i used to validate the first one that is suggested...

@defrance
Copy link
Contributor

Hello
in the "hidden" functions by activating a miscellaneous variable from dolibarr, it is possible to make a link between a third party and a contact of another third party by precisely specifying a role.
Don't you think that this functional need will be met by highlighting this hidden function more than by redeveloping one more thing?

@rycks
Copy link
Contributor Author

rycks commented Apr 24, 2024

it is possible to make a link between a third party and a contact of another third party by precisely specifying a role.

Hello @defrance ,
i'm sorry but i'm working on something more generic than only third party -> contact : with element_element link i will purpose to make links between any dolibarr objects not only third party and contact

@defrance
Copy link
Contributor

defrance commented Apr 24, 2024

I was just specifying that it is ALREADY possible to make links between thirdparties elements, going through the contacts allows you to qualify the type of link.

Your solution provides a qualification of the links between elements but apart from the links between third parties (and I specify that there is also the notion of parent company to link thirdparties together...) I do not know if there is functionally other uses

Another thing, it seems to me that it would be more relevant in your solution to use a dictionary to qualify the link types as is the case for contacts. This would avoid typos.

@lmag lmag self-assigned this Apr 25, 2024
@eldy
Copy link
Member

eldy commented Apr 27, 2024

@rycks Can you complete PR to add the instruction into migration (in same PR that PR that add colum on new installation) ?

@eldy eldy added the PR to fix - OK to merge if suggested fix are done PR was analyzed by PR merger and seems ok to be merged as soon as a fix has been published label Apr 27, 2024
@eldy eldy changed the title add relation type on element_element NEW add relation type on element_element Apr 27, 2024
@rycks
Copy link
Contributor Author

rycks commented Apr 29, 2024

@eldy done, alter table is ok

may we add an index on that column ? do you think we will have situations where we need to make search on that field ?

"search all relation of accounters ..." yes of course we need to be able to do that sort of requests

-> add index ?

@eldy
Copy link
Member

eldy commented Apr 29, 2024

Don't think we need an index on the new field: Because we will never make a seach on the type field "only". We will always have another criteria, like source or target id (at least one). And selectivity with such criteria will be better than the selectivity of the type (where we will have only 1 to 20 different values). So index of other fields will take precedence and the one on type will never be used.
But i don't see all the use cases you plan to do with this new field, so may be we will need one in a future. Currently i don't think we need it.

@eldy eldy merged commit ea9a6c0 into Dolibarr:develop Apr 29, 2024
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR to fix - OK to merge if suggested fix are done PR was analyzed by PR merger and seems ok to be merged as soon as a fix has been published PR to fix - See feedback in comments PR needs to be fixed to be integrated (some comments should describes the fix to do)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants