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

feature: add OAI-PMH API #891

Merged
merged 8 commits into from
Mar 9, 2022

Conversation

rekt-hard
Copy link
Contributor

@rekt-hard rekt-hard commented Feb 7, 2022

depends on inveniosoftware/invenio-oaiserver#206

Adds API for OAI-PMH sets

@rekt-hard rekt-hard force-pushed the oaiserver-service branch 2 times, most recently from 05d8389 to b643d00 Compare February 16, 2022 07:42
@rekt-hard rekt-hard marked this pull request as ready for review February 16, 2022 10:56
@lnielsen lnielsen self-requested a review March 7, 2022 08:20
feature: implement search

fix: pass correct action to require_permission
fix marshmallow deprecation warnings
@rekt-hard rekt-hard force-pushed the oaiserver-service branch from b643d00 to e9fa14b Compare March 7, 2022 10:29
Copy link
Member

@lnielsen lnielsen left a comment

Choose a reason for hiding this comment

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

Overall looks very good!!! The review is on some few minor details, and once fixed it's good to merge.

invenio_rdm_records/oaiserver/services/config.py Outdated Show resolved Hide resolved
invenio_rdm_records/oaiserver/services/config.py Outdated Show resolved Hide resolved

"""Result items for OAI-PMH services."""

from invenio_records_resources.pagination import Pagination
Copy link
Member

Choose a reason for hiding this comment

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

Comment: Not something to fix now for this PR, but I think we need to move some of this code to Records-Resources, and see how we can make it more reusable. I've seen it slightly duplicated many places with minor modifications. Again, this is more a comment for myself :-)

invenio_rdm_records/oaiserver/services/services.py Outdated Show resolved Hide resolved
invenio_rdm_records/oaiserver/services/permissions.py Outdated Show resolved Hide resolved
@@ -0,0 +1,232 @@
# -*- coding: utf-8 -*-
Copy link
Member

Choose a reason for hiding this comment

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

Minor: Extra tests I'd like to see:

  • It should not be possible to edit a managed set.
  • Try with bogus search patterns, very long strings etc. (try to break it a bit more 💣 😀 )

Copy link
Contributor Author

@rekt-hard rekt-hard Mar 8, 2022

Choose a reason for hiding this comment

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

@lnielsen lnielsen merged commit d91a733 into inveniosoftware:master Mar 9, 2022
@rekt-hard rekt-hard deleted the oaiserver-service branch September 30, 2023 12:55
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.

2 participants