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

check that the underlying entity has the parent reference field befor… #979

Merged
merged 1 commit into from
Sep 20, 2023

Conversation

wgilling
Copy link

…e trying to get() it

GitHub Issue: Islandora/documentation#2259

  • Other Relevant Links (Google Groups discussion, related pull requests,
    Release pull requests, etc.)

What does this Pull Request do?

It prevents a fatal error when trying to edit content which has an Entity Reference field and the site config uses any context which has a "Node parent" check. The code checks that the linked entities would have the parent reference field (field_member_of) before trying to get() that value. See issue 2259.

A brief description of what the intended result of the PR will be and/or what
problem it solves.
No fatal error when editing content (which is set up according to the Issue 2259).

What's new?

A in-depth description of the changes made by this PR. Technical details and
possible side effects.
I cannot imagine any side-effects. The code previously worked perfectly when an entity reference field was used on Repository Item content, but this merely will prevent this hook from causing a fatal error.

  • Changes x feature to such that y
  • Added x
  • Removed y
  • Does this change add any new dependencies?
  • Does this change require any other modifications to be made to the repository
    (i.e. Regeneration activity, etc.)?
  • Could this change impact execution of existing code?

How should this be tested?

(taken from issue 2259):

log in and create a new content type
add a field called "nodes" which is Entity Reference type & save new content type
make sure there is a context rule which has a "Node has parent" condition. If there isn't one, just add a basic rule (I believe that Islandora comes with such a context rule already configured)
now, create a node of this type -- add at least one entity to the "nodes" field and save. The error occurs when this new node is now edited.

When does this issue occur?
When Islandora code runs through possible context rules and any rule has the "Node has parent" condition.

Which page does it occur on?
The error occurs on the node/{nid}/edit route for the new content type.

What happens?
Because the new content type "nodes" does not have the field which is checked inside this logic (only "islandora_object" has this), there is a fatal error when trying to edit these.

To whom does it occur (anonymous visitor, editor, administrator)?
User who attempts to edit the new "nodes" node.

What did you expect to happen?
There should be no error.

Documentation Status

This is a bug which happens only when adding a content type which has Entity Reference list field and an enabled Context which uses the "Node parent" condition. This does not need any documentation.

  • Does this change existing behaviour that's currently documented?
  • Does this change require new pages or sections of documentation?
  • Who does this need to be documented for?
  • Associated documentation pull request(s): ___ or documentation issue ___

Additional Notes:

Any additional information that you think would be helpful when reviewing this
PR.

Interested parties

Tag (@ mention) interested parties or, if unsure, @Islandora/committers

@rosiel
Copy link
Member

rosiel commented Sep 20, 2023

Can you rebase / cherry-pick your PR on top of the latest code? I'm unable to test it as it is because this branch is from before we updated our dependencies to work with Drupal 10.

Screenshot 2023-09-20 at 1 03 36 PM

But I can replicate and your fix looks good.

@rosiel
Copy link
Member

rosiel commented Sep 20, 2023

Screw it - I put your fix on my own branch and tested, this works great.

@rosiel rosiel merged commit 988a1d9 into master Sep 20, 2023
@rosiel rosiel deleted the issue-2259 branch September 20, 2023 16:35
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.

2 participants