MBS-14075: Adjust edits_pending for entities with open relationship edits

[mwiencek]

Problem

When a relationship edit is left open, we don't adjust edits_pending for any of the endpoints, only the relationship itself. Which allows any of the linked entities to be auto-removed by the RemoveEmpty script before the edit closes.

I can't think of any reason we shouldn't do that.

Testing

Added new automated tests.

[reosarevok]

This seems good in general, and could probably be merged as is - I see three very similar if not entirely the same additions to the files though and it kinda feels like we should try and share the code as much as possible? But if it's harder than it looks, go ahead with this.

[mwiencek]

@reosarevok I realized all of the entity gathering was already implemented before in directly_related_entities. So now I'm just using that, which simplifies the implementation a lot (while still combining the database and Redis calls, which is nice).

There's a bigger issue here. This PR makes the problem from the original ticket worse, because remove_unused_url now deletes the URL immediately, as soon as the relationship edit is opened. The reason is that remove_unused_url triggers on any change to the url table, so the simple act of updating url.edits_pending will delete the URL. (And since the edit is still open, no relationship references it yet to prevent the deletion.)

I believe the reason this issue doesn't occur in the Perl tests is that they run inside a transaction, and remove_unused_url is a deferred trigger.

Fixing this is a schema change. And I'm mostly leaning towards removing these triggers now (a missed opportunity from last schema change...).

[reosarevok]

Was this really the only purpose of Edit::Relationship here? 🫠 I guess edit_category is required for stats and stuff... wonder if we have more cases where we are importing a bunch of useless stuff just for a category?

[reosarevok]

Not quite the only purpose :) Undefined subroutine &MusicBrainz::Server::Edit::Relationship::EditLinkType::l. Will amend.

[reosarevok]

Fixing this is a schema change. And I'm mostly leaning towards removing these triggers now (a missed opportunity from last schema change...).

So the suggestion would be to just let URLs be autoremoved more slowly via the usual entity process? It will slow down spam removal a bit but probably something we can live with.

Is there a need to wait until the next schema change with this? These triggers are not in mirrors, right? So couldn't we update our own triggers now, and release to standalones in October?

[mwiencek]

So the suggestion would be to just let URLs be autoremoved more slowly via the usual entity process? It will slow down spam removal a bit but probably something we can live with.

We could probably just move RemoveEmpty from daily.sh to hourly.sh if it's an issue. Or at least reduce the last_updated < now() - '2 day'::interval check to 1 day for URLs only, if that's short enough.

Is there a need to wait until the next schema change with this? These triggers are not in mirrors, right? So couldn't we update our own triggers now, and release to standalones in October?

Right, none of these triggers are on mirrors. No problem at all with dropping them in production before October.

[reosarevok]

Right, none of these triggers are on mirrors. No problem at all with dropping them in production before October

In that case I'd suggest adding that to this PR, because it probably cannot be merged anyway until we do so since it would make things worse :) (or, if you feel it's cleaner, I guess it could be better to have a second PR and ticket for that and mark this as blocked by it)

[reosarevok]

Changed to draft since it needs the schema change to be mergeable without making things worse.