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

[4.x] Check the reference as part of recursive editing check #10012

Merged
merged 5 commits into from May 8, 2024

Conversation

duncanmcclean
Copy link
Member

@duncanmcclean duncanmcclean commented May 7, 2024

This pull request fixes an issue with the recursive editing check added in #9841, where it'd sometimes prevent editing unintentionally when it shouldn't be.

This issue can be seen in two scenarios:

  • Eloquent Driver
    • Entries & terms are both using incrementing IDs.
    • When you're editing entry 1, then try to edit term 1 inline, it'd error out because the IDs are the same, even though they're different "things".
  • Runway
    • Two resources, with a Has Many or Belongs To fieldtype configured.
    • In this example, we have a Product model and a Manufacturer model.
    • When you're editing Product 1, then try to edit manufacturer 1 inline, it'd error out because the IDs are the same, even though they're different models.

To fix this, we're now passing the item's "reference" to the publish container and checking it against the related item in the RelationshipItem component.

Fixes #10005.

Copy link
Member

@jasonvarga jasonvarga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already have the concept of a "reference" for this purpose.

It's basically an id plus some other identifier. e.g. an entry's reference would be entry::123, an asset would be asset::container::path, a term would be term::topics::news::en, etc.

Please use this instead.

@duncanmcclean duncanmcclean changed the title [4.x] Check the blueprint as part of recursive editing check [4.x] Check the reference as part of recursive editing check May 7, 2024
Copy link
Member

@jasonvarga jasonvarga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need that addition in Item.vue?

@duncanmcclean
Copy link
Member Author

Do we still need that addition in Item.vue?

I've realised we don't need to check the id if we're now checking the reference. Simplified!

@jasonvarga jasonvarga merged commit 7a08a20 into 4.x May 8, 2024
36 checks passed
@jasonvarga jasonvarga deleted the include-blueprint-in-recursive-editing-check branch May 8, 2024 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Invalid error thrown on recursive editing check
2 participants