search

ucsdlib / damsrepo

DAMS Repository
Other
4 stars 2 forks source link

Allow read access of suppressDiscovery CLR #100

Closed lsitu closed 5 years ago

lsitu commented 5 years ago

References #99

Allow read access of suppressDiscovery CLR.

@mcritchlow @ucsdlib/developers Please review.

mcritchlow commented 5 years ago

This looks great. The one thing that stands out is it seems like something we should have a test for somewhere. In this repo or in damspas. Thoughts as to where it makes most sense?

lsitu commented 5 years ago

@mcritchlow Good question! I think the tests should be set it up in damsrepo. There are no tests added for damsrepo at this time but I can add a unique test for the method String accessGroup( Document doc, boolean discover, String objid ) here for now. What do you think?

mcritchlow commented 5 years ago

@lsitu - I think that sounds great! I hesitated to say a test here because, as you said, damsrepo doesn't have tests. So I was nervous about adding a bunch of work to this ticket. We could also split that off as a separate effort if you think it makes more sense, and get this PR in now.

mcritchlow commented 5 years ago

@lsitu - let's go with merging this now so folks can test. i don't want to block based on the test concern. we can look into that as a separate thing/PR I suppose.

lsitu commented 5 years ago

@mcritchlow I've created a PR https://github.com/ucsdlib/damsrepo/pull/101 for unit test, which will demonstrate the behavior of the group produced in method String accessGroup( Document doc, boolean discover, String objid ) for the suppressDiscovery collection CLR in question.