NEW Allow a single has_one to manage multiple reciprocal has_many

[GuySartorelli]

This PR provides a new has_one syntax that allows for a single has_one relation to be pointed at by multiple has_many relations - and each will correctly retain only their own items, without bleeding between them.

Without this PR, if you wanted multiple has_many relations on class A all pointing at class B, you would need to have a separate has_one relation on class B for each of the has_many relations.

Issue

• https://github.com/silverstripe/silverstripe-linkfield/issues/5

[michalkleiner]

I looked at the linked ticket for linkfield and looked at the PR here but it's still not clear to me what high value issue we are solving here and whether it even was an issue to have to have a multiple has_ones for the same class. It made it clear and separated.. now it feels a bit more like magic with all the required extra config. I guess both approaches will now work.

[GuySartorelli]

I looked at the linked ticket for linkfield and looked at the PR here but it's still not clear to me what high value issue we are solving here and whether it even was an issue to have to have a multiple has_ones for the same class. It made it clear and separated.. now it feels a bit more like magic with all the required extra config. I guess both approaches will now work.

The specific scenario we're trying to solve here is that we want the Link model to know who its "owner" record is, so it can ask that record what permissions the current user has (e.g. the link can only be edited if its owner can be edited).

This also makes it easier to say (for example) "I want 3 different lists of links on my SiteConfig" - you just have to define the has_many relations in your SiteConfig extension. You don't also have to add 3 new has_one relations to the link class via yaml.

It made it clear and separated.

Having to define multiple has_one relations to handle corresponding has_many relations has never been clear to me. Might just be a me thing, but that's caught me off guard time and again, and has looked untidy to me having to declare:

Some\Vendor\Model:
  has_one:
    SiteConfig1: SilverStripe\SiteConfig\SiteConfig
    SiteConfig2: SilverStripe\SiteConfig\SiteConfig
    SiteConfig3: SilverStripe\SiteConfig\SiteConfig

[GuySartorelli]

When looking up the has_many relation e.g. Link->Owner() should raise something that gets logged but not hard exception if {relationname}Relation does not exist on the model because the relation was either deleted or renamed. Can't use hard exception because we're dealing with data which can easily be different in prod than local when testing

I was hoping we could do this, but looking again at this, I don't think there's a sensible place where we can throw this sort of warning. The only times we really look at the relation name are:

• Saving a record to the relation, in which case we're setting the value not reading it
• Getting the list of records in a PolymorphicHasManyList, in which case we're filtering by the value rather than interrogating the has_one on a specific record

Renaming relations is pretty uncommon anyway, I'd hope - and for most relations if you change the name you'll need to do some sort of data migration already. I think we can accept that this is another case where a manual data migration will be necessary and until such a migration is done there'll be orphaned records.

Just an idea, is it possible to simplify this so that you don't need to 'opt-in' this this behaviour ... then do the new clever stuff if the relationship is polymorphic

I can't think of a way to make that work without data loss, for essentially the same reason we can't throw a warning for has_ones.

The way it works with the new config relies on passing the has_many relation to the PolymorphicHasManyList only if the relation is pointing at a multi-reciprocal has_one relation. Then, it always stores the relation name if it's given one, and it always filters against the relation name if it's given one.

If we were to apply multi-reciprocal behaviour to all polymorphic has_many relations, we'd have to filter all of them by both the relation name and relation name = null. But because we're allowing relation name to be null, this makes no distinction between "that item belongs in this list" and "we're really not sure where that item belongs".

Why this breaks down

People could decide "I want to add a second has_many to point at that existing polymorphic has_one". If they do so, they'll end up with all the records from their existing has_many relation showing up in their new has_many list, which is exactly what this PR is intended to avoid.

[michalkleiner]

I looked at the linked ticket for linkfield and looked at the PR here but it's still not clear to me what high value issue we are solving here and whether it even was an issue to have to have a multiple has_ones for the same class. It made it clear and separated.. now it feels a bit more like magic with all the required extra config. I guess both approaches will now work.

The specific scenario we're trying to solve here is that we want the Link model to know who its "owner" record is, so it can ask that record what permissions the current user has (e.g. the link can only be edited if its owner can be edited).

This also makes it easier to say (for example) "I want 3 different lists of links on my SiteConfig" - you just have to define the has_many relations in your SiteConfig extension. You don't also have to add 3 new has_one relations to the link class via yaml.

I never had a need where many_many wouldn't be sufficient for that. Of course there might be others with different requirements.

It made it clear and separated.

Having to define multiple has_one relations to handle corresponding has_many relations has never been clear to me. Might just be a me thing, but that's caught me off guard time and again, and has looked untidy to me having to declare:

Some\Vendor\Model:
  has_one:
    SiteConfig1: SilverStripe\SiteConfig\SiteConfig
    SiteConfig2: SilverStripe\SiteConfig\SiteConfig
    SiteConfig3: SilverStripe\SiteConfig\SiteConfig

Fair enough. I would name them based on what they were representing, even if all would link to the same model for some reason, but more often we'd use an extension to the vendor model where you'd put all the config and code, like you'd do with project models.

Even though the use case here might be narrow (and doesn't talk about the advantages of using has_one/has_many vs many_many), I'm not against it and could be solving a legitimate problem. It's just yet another thing that most folks outside of the CMS squad don't have visibility of (with a lack of public roadmap and a list of high value/prioritised issues somewhere), which might be contributing to my lack of understanding and buy-in here.