Skip to content

Feature/enhance person group functions #45

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

Open
wants to merge 21 commits into
base: main
Choose a base branch
from

Conversation

mdwRepository
Copy link

The pull request addresses "Enhance EPerson and Group management" #44

It includes functions based on the API documentation: https://github.com/DSpace/RestContract/blob/main/epersons.md and https://github.com/DSpace/RestContract/blob/main/epersongroups.md.

A sample example_eperson_group.py is included showing the usage of these functions.

@kshepherd
Copy link
Collaborator

hi @mdwRepository , sorry your pull requests have been sitting neglected for so long - I am now trying to get more time for reviewing and discussing!

All of these methods look really great to have, my only initial concern is whether we need to think about some more systematic / structured way of organising and naming the methods... right now if I wanted to know how to do something, I would need to search the code for the method name based on keywords.. there is no intuitive way to guess or derive "how to do X".

As a (totally made up) brainstorm, something like a more generic search function wrapper that accepts an object type enum and some other params, so you can do something like search(type=EPERSON, by=email, value='[email protected]')? So we can use the same kind of thing for GROUP, COMMUNITY, or so on as well?
Or am I perhaps overthinking how much we can 'flatten' or abstract for these?

In any case I wouldn't want to block this necessarily - we can always merge it first and then talk about restructing for a later major release, but I hope we can avoid eventually ending up with 500 functions that are all named in different ways with no easy way for a script developer to learn usage beyond just scrolling through the pydoc or manaully searching...

@kshepherd kshepherd self-requested a review March 27, 2025 17:47
Copy link
Collaborator

@kshepherd kshepherd left a comment

Choose a reason for hiding this comment

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

@mdwRepository do you mind if I try refactor this a bit to consolidate the search / get operations down and set the "by" values as parameters instead of separate functions? I could either push here, or open as a separate PR for us to compare / discuss.

Would that be a difficult change to merge into your client implementations?

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.

3 participants