bug: have a type for combined ids, fix #653#664
Conversation
a192000 to
f7a60f3
Compare
f7a60f3 to
08175bb
Compare
|
FYI @smoyer64 To deal properly with that PR, there is a bunch of questions that need a good answer (which is why I guess I've been stuck for so long):
I guess? #843 could benefit from this to be able to link to any entity (as long as it does implement the required interface). But it might also be a case of too much generalization and ProjectBoard should just carry that identifier as part of the operation linking to the entity. It's probable that linking to "any entity" won't be such a thing really, so why put that in CombinedId? Or maybe having that information as part of the identifier would help in the long run for sharding or whatever else data routing purpose? If we introduce it now, we would avoid having three different king of identifier (id, id+entity_id, id+entity_id+entity_type). I'm quite unsure here. If we do want that, I suppose encoding would be either the first byte, or some kind of varint like multihash does.
When we construct a CombinedId, we could keep around the original primary/secondary Ids and have them available while we are still working on the entity. When we resolve an entity with a CombinedId, we should have both ids available during the process (?). So, should CombinedId be the resulting interleaved string that encode only two prefixes, or should we keep around the original Ids? I'm kinda leaning on not keeping them around (as a way to avoid having the requirement of always having the original IDs when building one), but I don't have strong argument.
Kinda goes hand to hand with the previous question. I suppose this one is easier to answer as I can try to only have a CombinedId and see where that breaks, but that sort of influence the previous choice anyway.
gqlgen is capable to bind I'm leaning on typing properly, but that's also a breaking change. |
388876f to
45b0435
Compare
|
I think I got on top of it with the following choices:
No. It kinda looked like premature thinking. ProjectBoard should most likely do its own thing and would not really benefit from the identifier carrying an entity type.
Yes. It makes things simpler without edge cases.
No. It's not needed. The only counter example is Comment where also knowing the operation Id is important to edit the comment, so having that as part of the Comment avoid resolving/searching. For this case I added it as
I went for typed ids. Looks like it works well. |
|
Missing here is to adjust a bit the webUI graphql queries. |
|
We've talked about these |
To me |
|
A year+ later, I think this is finally ready! |
fix #653