Join GitHub today
GitHub is home to over 36 million developers working together to host and review code, manage projects, and build software together.
Sign upAllow orphaned object report to find orphaned compound objects. #710
Conversation
This comment has been minimized.
This comment has been minimized.
DiegoPino
commented
Aug 27, 2018
WOHO! @jonathangreen can review tomorrow. Reads fine. Thanks |
DiegoPino
reviewed
Sep 6, 2018
} | ||
!optionals | ||
!filters | ||
FILTER (!bound(?liveparent)) | ||
} ORDER BY ?object | ||
EOQ; | ||
$parent_relationships = array('<fedora-rels-ext:isMemberOfCollection>', '<fedora-rels-ext:isMemberOf>', '<islandora:isPageOf>'); |
This comment has been minimized.
This comment has been minimized.
DiegoPino
Sep 6, 2018
Hi, just wondering... could we eventually put a hook here? Altering that list seems like a pretty sweet idea. Would you like to give that a try? If not, ok too. Tested and works pretty well but wanted just to try my luck
This comment has been minimized.
This comment has been minimized.
jonathangreen
Sep 6, 2018
Author
Member
I do agree perhaps a hook is a cleaner implementation.
I see 3 options really:
- Define something like
hook_islandora_solution_pack_child_predicate
and let each solution pack respond with its predicates. - Leave these values here and also define an alter. Something like
hook_islandora_orphan_child_predicate_alter
and let people alter as they see fit. - Leave it as it.
What option were you thinking of? I think option 1 is probably the best/cleanest option, it expands the scope of this PR a bit though, making it 4 PR instead. Option 2 and 3 are probably the easiest options.
This comment has been minimized.
This comment has been minimized.
DiegoPino
Sep 6, 2018
I agree with you, 1 is the best, but 2, low hanging fruit could lead to a future implementation (like a call of a call) of 1 if you are in a hurry. Totally depends on your energy/time.
Sep 6, 2018
This was referenced
This comment has been minimized.
This comment has been minimized.
Although I may come to regret it, I decided to give option 1 a try and implement a hook for this. I also made PRs to the other solution packs to implement the hook. Testing for this should be the same as before. |
jonathangreen
added some commits
Sep 7, 2018
jonathangreen
force-pushed the
jonathangreen:ISLANDORA-2294
branch
from
dfc6470
to
163fd9d
Sep 7, 2018
jonathangreen
added some commits
Sep 7, 2018
jonathangreen
referenced this pull request
Sep 7, 2018
Merged
Implement hook_islandora_solution_pack_child_relationships(). #161
whikloj
reviewed
Sep 7, 2018
This comment has been minimized.
This comment has been minimized.
DiegoPino
commented
Sep 25, 2018
Ok this looks good to my old eyes and Islandora/islandora_solution_pack_collection#208 then i will let all this sit 48 hours until our next committers call(this week right?) so we get a good applause before merging. Sounds good @Islandora/7-x-1-x-committers ? |
This comment has been minimized.
This comment has been minimized.
Ready to merge? |
DiegoPino
merged commit f3b3ffc
into
Islandora:7.x
Oct 9, 2018
1 check passed
This comment has been minimized.
This comment has been minimized.
DiegoPino
commented
Oct 9, 2018
@jonathangreen let me know if all is OK with this set to close the ticket, or we can keep it open if any other SP needs same work? Fabulous work. If anything goes wrong its my fault because i pressed merge -=) |
jonathangreen
deleted the
jonathangreen:ISLANDORA-2294
branch
Oct 9, 2018
This comment has been minimized.
This comment has been minimized.
There is one other thing before we close the ticket, I believe this PR is on the same ticket: |
jonathangreen commentedAug 24, 2018
JIRA Ticket
https://jira.duraspace.org/browse/ISLANDORA-2294
Was discussed in the committers meeting yesterday.
What does this Pull Request do?
The current orphan objects report does not look for orphaned compound objects. This adds a check to see if the compound solution pack exists, and if it does then it uses the compound solution pack variable to get the predicate the compound solution pack uses.
What's new?
Move the creation of the query components using parent predicates to its own logic based on an array of predicates. This allows us to optionally add the compound solution pack predicate.
How should this be tested?
Additional Notes:
This addition to the report will be more useful after Islandora/islandora_solution_pack_compound#146 is merged so that compounds no longer create parentless orphans.
Interested parties
@DiegoPino @Islandora/7-x-1-x-committers