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

items: Add identifiers object list field #1205

Merged

Conversation

sakshamarora1
Copy link
Contributor

❤️ Thank you for your contribution!

Description

Closes: CERNDocumentServer/cds-ils#834

@sakshamarora1 sakshamarora1 self-assigned this Apr 4, 2024
from marshmallow import EXCLUDE, Schema, fields


class IdentifiersSchema(Schema):
Copy link
Contributor

Choose a reason for hiding this comment

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

this schema already exists under document.py, it would be better to reuse it than create a duplicate

@kpsherva
Copy link
Contributor

kpsherva commented Apr 8, 2024

General: it looks we are missing the identifier schemes json update, to add a new identifier schema type ("call_number", which appears in the production script PR)

Copy link
Contributor

@kpsherva kpsherva left a comment

Choose a reason for hiding this comment

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

LGTM! do we need any upgrade recipe since a new field is added?
at least I think mapping update (check our internal docs how to do it)
we will need to have a plan for the deployment

@kpsherva kpsherva merged commit 6736dcf into inveniosoftware:master May 13, 2024
10 checks passed
@@ -16,6 +16,7 @@
set_changed_by,
)
from invenio_app_ils.records.loaders.schemas.price import PriceSchema
from invenio_app_ils.documents.loaders.jsonschemas.document import IdentifierSchema
Copy link
Contributor

Choose a reason for hiding this comment

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

Identifiers schema to be moved to records.loaders (like above)

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.

Location plan for books
2 participants