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

Add remove services for collections, objects, and related objects. #104

Merged
merged 1 commit into from
May 19, 2015

Conversation

elrayle
Copy link
Contributor

@elrayle elrayle commented May 18, 2015

New Services:

  • RemoveCollectionFromCollection
  • RemoveObjectFromCollection
  • RemoveRelatedObjectFromCollection
  • RemoveObjectFromObject
  • RemoveRelatedObjectFromObject

Updated Services:

  • GetRelatedObjectsFromCollection - return array
  • GetRelatedObjectsFromObject - return array

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.04%) to 95.98% when pulling 8a6662d on new_service_for_removing into 1da7004 on master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-1.04%) to 95.98% when pulling 8a6662d on new_service_for_removing into 1da7004 on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.06%) to 97.07% when pulling ae111d5 on new_service_for_removing into 1da7004 on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.06%) to 97.07% when pulling ae111d5 on new_service_for_removing into 1da7004 on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.06%) to 97.08% when pulling 7b4486c on new_service_for_removing into 1da7004 on master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+0.06%) to 97.08% when pulling 7b4486c on new_service_for_removing into 1da7004 on master.

@@ -10,8 +10,7 @@ class GetRelatedObjectsFromCollection

def self.call( parent_collection )
raise ArgumentError, "parent_collection must be a pcdm collection" unless Hydra::PCDM.collection? parent_collection

parent_collection.related_objects
parent_collection.related_objects.to_a
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to call to_a here? Why? to_a can be less efficient if they only want the first element of the association.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Two reasons for the related_object.to_a

  1. related_objects.include? raises an error in some cases (This should be fixed when issue related_objects.delete should return deleted object (not array) or nil samvera-deprecated/activefedora-aggregation#36 is fixed)
  2. col1.collections and col1.objects in collection_behaviors each returns an Array and is surfaced through services as GetCollectionsFromCollection and GetObjectsFromCollection. For consistency in processing, col1.related_objects surfaced through GetRelatedObjectsFromCollection should return an Array.

NOTE: Same applies for related objects in pcdm objects.

New Services:
* RemoveCollectionFromCollection
* RemoveObjectFromCollection
* RemoveRelatedObjectFromCollection
* RemoveObjectFromObject
* RemoveRelatedObjectFromObject

Updated Services:
* GetRelatedObjectsFromCollection - return array
* GetRelatedObjectsFromObject - return array
@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) to 97.06% when pulling f5e842f on new_service_for_removing into 1da7004 on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.07%) to 97.06% when pulling fd27ef1 on new_service_for_removing into 0d1da2e on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.07%) to 97.06% when pulling fd27ef1 on new_service_for_removing into 0d1da2e on master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+0.07%) to 97.06% when pulling fd27ef1 on new_service_for_removing into 0d1da2e on master.

jcoyne added a commit that referenced this pull request May 19, 2015
Add remove services for collections, objects, and related objects.
@jcoyne jcoyne merged commit d9c5b7f into master May 19, 2015
@jcoyne jcoyne deleted the new_service_for_removing branch May 19, 2015 13:17
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.

None yet

3 participants