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

using ordered_members puts parent-child relationship in both directions #361

Open
elrayle opened this issue Mar 28, 2019 · 4 comments
Open

Comments

@elrayle
Copy link
Contributor

elrayle commented Mar 28, 2019

Descriptive summary

Adding a child work to a parent work using ordered members causes the child to respond with parent works using both in_work_ids and member_of_work_ids, resulting in the parent work being listed twice.

Rationale

In the list of parent_works, the parents should not repeat.

Expected behavior

        parent_work1.ordered_members = [child_work]
        parent_work2.ordered_members = [child_work]
        child_work.save
        parent_work1.save
        parent_work2.save

        expect(child_work.parent_work_ids).to match_array [parent_work1.id, parent_work2.id]

Actual behavior

       expected collection contained:  ["wk1", "wk2"]
       actual collection contained:    ["wk1", "wk1", "wk2", "wk2"]

See PR #360 which includes a test that fails because of this bug.

Steps to reproduce the behavior

Use test provided in PR #360 to debug.

  1. Setup the relationships
        parent_work1.ordered_members = [child_work]
        parent_work2.ordered_members = [child_work]
        child_work.save
        parent_work1.save
        parent_work2.save
  1. Get list of parents
        child_work.parent_works

Expect parent_works to return parent_work1 and parent_work2, but you get the two parent works in the list twice.

Related Work

PR samvera/hyrax#3697 - Orders members when converting to AF objects

@elrayle
Copy link
Contributor Author

elrayle commented Mar 28, 2019

# Setup objects
parent_work1 = GenericWork.new(id: 'wk1')
parent_work2 = GenericWork.new(id: 'wk2')
child_work = GenericWork.new(id: 'wk3')

# Setup relationships
parent_work1.ordered_members = [child_work]
parent_work2.ordered_members = [child_work]
child_work.save
parent_work1.save
parent_work2.save

# test relationships
parent_work1.member_ids         # expected: [wk3] actual: [wk3]
parent_work2.member_ids         # expected: [wk3] actual: [wk3]
parent_work1.ordered_member_ids # expected: [wk3] actual: [wk3]
parent_work2.ordered_member_ids # expected: [wk3] actual: [wk3]
child_work.in_work_ids          # expected: [] actual: [wk1, wk2]
child_work.member_of_work_ids   # expected: [wk1, wk2] actual: [wk1, wk2]
child_work.parent_work_ids      # expected: [wk1, wk2] actual: [wk1, wk2, wk1, wk2]

@elrayle
Copy link
Contributor Author

elrayle commented Mar 28, 2019

Implementation path of in_work_ids...

  • in_work_ids calls in_works
  • which calls AF ordered_by
  • which executes a solr query: "{!join from=proxy_in_ssi to=id}ordered_targets_ssim:#{id}"

As written, in_work_ids seems to be using solr to find ordered members. I expected this method to use the members relationship to find all parents and then limit the results to only work parents.

Implementation path of member_of_work_ids...

  • member_of_work_ids calls member_of_works
  • which calls PCDM in_objects
  • which calls member_of defined in PCDM by members ordered_aggregation relationship

I do expect this method to use the member_of relationship to find all parents for a work. I'm just not sure why it is defined. I wouldn't expect ordered_aggregation relationship to define this reverse relationship method.

@elrayle
Copy link
Contributor Author

elrayle commented Mar 28, 2019

Ran same relationship checks when relationship was setup using members instead of ordered_members.

parent_work1.member_ids         # expected: [wk3] actual: [wk3]
parent_work2.member_ids         # expected: [wk3] actual: [wk3]
parent_work1.ordered_member_ids # expected: [] actual: []
parent_work2.ordered_member_ids # expected: [] actual: []
child_work.in_work_ids          # expected: [] actual: []
child_work.member_of_work_ids   # expected: [wk1, wk2] actual: [wk1, wk2]
child_work.parent_work_ids      # expected: [wk1, wk2] actual: [wk1, wk2]

@elrayle
Copy link
Contributor Author

elrayle commented Mar 28, 2019

Relationship Expectations

  • child collections/works know about their parent collection through the member_of_collection relationship
  • parent works know about their child works/filesets through the members relationship

Expected method behaviors:

  • in_works would return parents defined by the members relationship which tracks membership by the parent knowing about the child
  • member_of_works would return parents defined by a member_of relationship which tracks membership by the child knowing about its parents
  • I would expect a members relationship to be defined by using AF ordered_aggregation
  • I would expect a member_of relationship to be defined by using AF indirectly_contains

Summary of Actual method behaviors:

  • parent_works = in_works + member_of_works
  • in_works returns the list of ordered members filtered to just works
  • member_of_works uses reflection method member_of defined through ordered_aggregation relationship members and filters to just works (Not confident this is exactly right because I would not expect ordered_aggregation to defined member_of)

As written, the methods are behaving as expected. But I don't believe this is what was originally intended.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Backlog
Status: Backlog
Development

No branches or pull requests

1 participant