core icon indicating copy to clipboard operation
core copied to clipboard

Keep relation direction when updating content.

Open doenietzomoeilijk opened this issue 3 years ago • 1 comments

This scratches my personal itch, where I want to nudge my Bolt-powered blog slightly more towards a knowledge-base thingy, where I can keep track of posts linking to other posts. To do this properly, I want to distinguish between incoming and outgoing links for a post.

Currently, a Content can have Relations To and From itself. In the edit interface, they're all thrown onto one pile, and when saving such an entry, all the relations will be, from that point on, From that content (even if they were in fact To it, before). This change fixes that, by more carefully storing and removing Relations.

  • Only remove existing Relations if they are actually no longer used
  • Only add new Relations if they weren't already present
  • Keep and fetch Relations in the correct order, default to sorting by position

In addition:

  • Don't return anything (and change the method signature) from updateRelation(), since the output was never actually used.

Tested with content that had one or more relations to and from other contents of the same and of different contenttypes. In my testing, it all seems to work. I can't think of any BC breaks in this. The position being used to actually sort on is new, but not breaking (might be worth a warning, though, not sure if people actually used the default sorting since IMO it was kinda useless, but you never know).

doenietzomoeilijk avatar Sep 20 '22 19:09 doenietzomoeilijk

The thing PHPStan complains about is fixed in #3319

Cypress has been broken for a short while, so we'll need to fix those, soon. :-/

bobdenotter avatar Sep 21 '22 14:09 bobdenotter

Looks good to me! 👍

What isn't apparent (at least to my coffee-deprived brain) is how a user (either a "developer" or "template builder") can leverage this distinction. Is that a matter of updating it in docs, or should we do a follow-up PR for that?

bobdenotter avatar Sep 27 '22 06:09 bobdenotter

What isn't apparent (at least to my coffee-deprived brain) is how a user (either a "developer" or "template builder") can leverage this distinction. Is that a matter of updating it in docs, or should we do a follow-up PR for that?

Well, that's where the $bidirectional switch comes in, which I touched on on Slack. It currently does exactly nothing; setting it to false would, as per your suggestion back then, limit relations to only those to the current content. Personally, I'd go a step further and change the $bidirectional for $direction, and accept "to", "from" and "both" (latter would be the default), with, for BC sake, true (equiv to "both") and false (equiv to "to") thrown in.

Either that, or add two dedicated functions/filters for that. But solving it inside the existing filter, repurposing an existing parameter without BC breaks, would be my preference.

You're the one to decide, though. ;)

doenietzomoeilijk avatar Sep 27 '22 07:09 doenietzomoeilijk