diff --git a/data/ill_requests.json b/data/ill_requests.json index aad2bbafb6..c8992e3fb4 100644 --- a/data/ill_requests.json +++ b/data/ill_requests.json @@ -32,7 +32,12 @@ "url": "http://data.rero.ch/01-R007878671/html?view=GE_V1" }, "pages": "18-24", - "note": "Could you please add the front page of the journal issue? Thanks a lot!" + "notes": [ + { + "type": "public_note", + "content": "Could you please add the front page of the journal issue? Thanks a lot!" + } + ] }, { "status": "pending", @@ -61,10 +66,20 @@ "url": "https://www.amazon.fr/grand-roman-maths-pr%C3%A9histoire-jours/dp/2081378760" }, "pages": "19-44", - "note": "Je voudrais obtenir le chapitre 2 de ce livre." + "notes": [ + { + "type": "public_note", + "content": "Je voudrais obtenir le chapitre 2 de ce livre." + }, + { + "type": "staff_note", + "content": "Ce livre ne comprend pas de chapitre" + } + ] }, { - "status": "pending", + "status": "validated", + "loan_status": "ITEM_AT_DESK", "copy": false, "document": { "title": "Harry Potter and the Philosopher's Stone", @@ -76,6 +91,11 @@ "source": "Amazon", "url": "https://read.amazon.com/kp/embed?asin=B019PIOJYU&preview=newtab&linkCode=kpe&ref_=cm_sw_r_kb_dp_MBjwFb8A8N94P" }, - "note": "In english please." + "notes": [ + { + "type": "public_note", + "content": "In english please." + } + ] } -] \ No newline at end of file +] diff --git a/rero_ils/config.py b/rero_ils/config.py index df6dab6247..8c49b8e497 100644 --- a/rero_ils/config.py +++ b/rero_ils/config.py @@ -535,7 +535,7 @@ def _(x): 'rero_ils.modules.serializers:json_v1_response' ), 'application/rero+json': ( - 'rero_ils.modules.items.serializers:json_item_search' + 'rero_ils.modules.ill_requests.serializers:json_ill_request' ) }, search_serializers_aliases={ @@ -545,6 +545,9 @@ def _(x): search_serializers={ 'application/json': ( 'rero_ils.modules.serializers:json_v1_search' + ), + 'application/rero+json': ( + 'rero_ils.modules.ill_requests.serializers:json_ill_request_search' ) }, record_loaders={ @@ -1777,7 +1780,45 @@ def _(x): _('subject'): and_term_filter('subjects.name'), _('teacher'): and_term_filter('teachers.facet') } - ) + ), + ill_requests=dict( + aggs=dict( + status=dict( + terms=dict( + field='status', + size=RERO_ILS_AGGREGATION_SIZE.get( + 'ill_requests', RERO_ILS_DEFAULT_AGGREGATION_SIZE) + ) + ), + loan_status=dict( + terms=dict( + field='loan_status', + size=RERO_ILS_AGGREGATION_SIZE.get( + 'ill_requests', RERO_ILS_DEFAULT_AGGREGATION_SIZE) + ) + ), + requester=dict( + terms=dict( + field='patron.facet', + size=RERO_ILS_AGGREGATION_SIZE.get( + 'ill_requests', RERO_ILS_DEFAULT_AGGREGATION_SIZE) + ) + ), + library=dict( + terms=dict( + field='library.pid', + size=RERO_ILS_AGGREGATION_SIZE.get( + 'ill_requests', RERO_ILS_DEFAULT_AGGREGATION_SIZE) + ) + ) + ), + filters={ + _('status'): and_term_filter('status'), + _('loan_status'): and_term_filter('loan_status'), + _('requester'): and_term_filter('patron.facet'), + _('library'): and_term_filter('library.pid') + } + ), ) # Elasticsearch fields boosting by index diff --git a/rero_ils/modules/ill_requests/api.py b/rero_ils/modules/ill_requests/api.py index ca83758ba0..f96ea42628 100644 --- a/rero_ils/modules/ill_requests/api.py +++ b/rero_ils/modules/ill_requests/api.py @@ -19,7 +19,10 @@ from functools import partial -from .models import ILLRequestIdentifier +from flask_babelex import gettext as _ + +from .models import ILLRequestIdentifier, ILLRequestMetadata, \ + ILLRequestNoteStatus from ..api import IlsRecord, IlsRecordsIndexer, IlsRecordsSearch from ..fetchers import id_fetcher from ..locations.api import Location @@ -54,15 +57,22 @@ class ILLRequest(IlsRecord): minter = ill_request_id_minter fetcher = ill_request_id_fetcher provider = ILLRequestProvider + model_cls = ILLRequestMetadata def extended_validation(self, **kwargs): """Validate record against schema. - If record is a copy request (copy==true) then `pages` property is - required + * If record is a copy request (copy==true) then `pages` property is + required + * Ensures that only one note of each type is present. """ if self.is_copy and self.get('pages') is None: return 'Required property : `pages`' + + note_types = [note.get('type') for note in self.get('notes', [])] + if len(note_types) != len(set(note_types)): + return _('Can not have multiple notes of same type.') + return True @classmethod @@ -113,11 +123,22 @@ def organisation_pid(self): """Get organisation pid for ill_request.""" return self.get_pickup_location().organisation_pid + @property + def public_note(self): + """Get the public note for ill_requests.""" + notes = [note.get('content') for note in self.get('notes', []) + if note.get('type') == ILLRequestNoteStatus.PUBLIC_NOTE] + return next(iter(notes or []), None) + def get_pickup_location(self): """Get the pickup location.""" location_pid = self.replace_refs()['pickup_location']['pid'] return Location.get_record_by_pid(location_pid) + def get_library(self): + """Get the library linked to the ill_request.""" + return self.get_pickup_location().get_library() + class ILLRequestsIndexer(IlsRecordsIndexer): """Indexing documents in Elasticsearch.""" diff --git a/rero_ils/modules/ill_requests/cli.py b/rero_ils/modules/ill_requests/cli.py index 728e6d4473..c31f07bb37 100644 --- a/rero_ils/modules/ill_requests/cli.py +++ b/rero_ils/modules/ill_requests/cli.py @@ -47,9 +47,11 @@ def create_ill_requests(input_file): if 'pid' in request_data: del request_data['pid'] if organisation_pid not in patron_pids: - patron_pids[organisation_pid] = list( - Patron.get_all_pids_for_organisation(organisation_pid) - ) + patron_pids[organisation_pid] = [ + pid for pid in Patron.get_all_pids_for_organisation( + organisation_pid) + if Patron.get_record_by_pid(pid).is_patron + ] patron_pid = random.choice(patron_pids[organisation_pid]) request_data['patron'] = { '$ref': get_ref_for_pid('patrons', patron_pid) diff --git a/rero_ils/modules/ill_requests/forms.py b/rero_ils/modules/ill_requests/forms.py index 127161d2a6..1a6de42ba7 100644 --- a/rero_ils/modules/ill_requests/forms.py +++ b/rero_ils/modules/ill_requests/forms.py @@ -203,9 +203,13 @@ def get_data(self): 'found_in': { 'source': self.source.origin.data, 'url': self.source.url.data - }, - 'note': self.note.data + } }) + if self.note.data: + data['notes'] = [{ + 'type': 'public_note', + 'content': self.note.data + }] # if we put 'copy' in the dict before the dict cleaning and if 'copy' # is set to 'No', then it will be removed by `remove_empties_from_dict` diff --git a/rero_ils/modules/ill_requests/jsonschemas/ill_requests/ill_request-v0.0.1.json b/rero_ils/modules/ill_requests/jsonschemas/ill_requests/ill_request-v0.0.1.json index f9a584e7c5..a27e10fcc6 100644 --- a/rero_ils/modules/ill_requests/jsonschemas/ill_requests/ill_request-v0.0.1.json +++ b/rero_ils/modules/ill_requests/jsonschemas/ill_requests/ill_request-v0.0.1.json @@ -6,12 +6,14 @@ "additionalProperties": false, "propertiesOrder": [ "document", + "found_in", + "patron", "pickup_location", "status", + "loan_status", "copy", "pages", - "found_in", - "note" + "notes" ], "required": [ "$schema", @@ -24,20 +26,20 @@ ], "properties": { "$schema": { + "type": "string", "title": "Schema", "description": "Schema to validate ILL request records against.", - "type": "string", "minLength": 9, "default": "https://ils.rero.ch/schema/ill-requests/ill-request-v0.0.1.json" }, "pid": { - "title": "ILL request ID", "type": "string", + "title": "ILL request ID", "minLength": 1 }, "status": { - "title": "Status", "type": "string", + "title": "Status", "enum": [ "pending", "validated", @@ -46,7 +48,6 @@ ], "default": "pending", "form": { - "type": "selectWithSort", "options": [ { "value": "pending", @@ -68,35 +69,96 @@ } }, "copy": { + "type": "boolean", "title": "Copy", "description": "Define if the request is for a copy or an actual document.", - "type": "boolean" + "default": false }, "patron": { - "title": "Patron", "type": "object", + "title": "Patron", + "required": [ + "$ref" + ], "properties": { "$ref": { - "title": "Patron URI", "type": "string", - "pattern": "^https://ils.rero.ch/api/patrons/.*?$" + "title": "Patron URI", + "pattern": "^https://ils.rero.ch/api/patrons/.*?$", + "form": { + "remoteTypeahead": { + "type": "patrons" + }, + "validation": { + "messages": { + "patternMessage": "Should be in the following format: https://ils.rero.ch/api/patrons/." + } + }, + "templateOptions": { + "label": "" + } + } } } }, + "loan_status": { + "type": "string", + "title": "Loan state", + "enum": [ + "PENDING", + "ITEM_AT_DESK", + "ITEM_ON_LOAN", + "ITEM_RETURNED" + ], + "form": { + "placeholder": "Choose a status", + "options": [ + { + "value": "PENDING", + "label": "PENDING" + }, + { + "value": "ITEM_AT_DESK", + "label": "ITEM_AT_DESK" + }, + { + "value": "ITEM_ON_LOAN", + "label": "ITEM_ON_LOAN" + }, + { + "value": "ITEM_RETURNED", + "label": "ITEM_RETURNED" + } + ] + } + }, "pickup_location": { - "title": "Pickup location", "type": "object", + "title": "Pickup location", + "required": [ + "$ref" + ], "properties": { "$ref": { - "title": "Pickup location URI", "type": "string", - "pattern": "^https://ils.rero.ch/api/locations/.+?$" + "title": "Pickup location URI", + "pattern": "^https://ils.rero.ch/api/locations/.+?$", + "form": { + "remoteOptions": { + "type": "locations", + "query": "is_pickup:true", + "labelField": "pickup_name" + }, + "templateOptions": { + "label": "" + } + } } } }, "document": { "type": "object", - "title": "Document", + "title": "Document information", "required": [ "title" ], @@ -110,39 +172,62 @@ ], "properties": { "title": { - "title": "Title", "type": "string", - "minLength": 3 + "title": "Title", + "minLength": 3, + "form": { + "wrappers": [ + "form-field-horizontal" + ] + } }, "authors": { - "title": "Authors", "type": "string", - "minLength": 3 + "title": "Authors", + "minLength": 3, + "form": { + "wrappers": [ + "form-field-horizontal" + ] + } }, "publisher": { - "title": "Publisher", "type": "string", - "minLength": 3 + "title": "Publisher", + "minLength": 3, + "form": { + "wrappers": [ + "form-field-horizontal" + ] + } }, "year": { - "title": "Year", "type": "string", + "title": "Year", "pattern": "^\\d{4}$", "form": { "validation": { "messages": { "patternMessage": "Should be in the following format: YYYY (2020)." } - } + }, + "wrappers": [ + "form-field-horizontal" + ] } }, "identifier": { + "type": "string", "title": "Identifier", - "type": "string" + "form": { + "wrappers": [ + "form-field-horizontal" + ] + } }, "source": { - "title": "Source", "type": "object", + "title": "Source", "required": [ "journal_title" ], @@ -153,29 +238,67 @@ ], "properties": { "journal_title": { - "title": "Journal title", "type": "string", - "minLength": 3 + "title": "Journal title", + "minLength": 3, + "form": { + "expressionProperties": { + "templateOptions.required": "false" + }, + "wrappers": [ + "form-field-horizontal" + ] + } }, "volume": { + "type": "string", "title": "Volume", - "type": "string" + "form": { + "wrappers": [ + "form-field-horizontal" + ], + "templateOptions": { + "cssClass": "col-md-3 p-0" + } + } }, "number": { + "type": "string", "title": "Number", - "type": "string" + "form": { + "wrappers": [ + "form-field-horizontal" + ], + "templateOptions": { + "cssClass": "col-md-3 p-0" + } + } } } } + }, + "form": { + "templateOptions": { + "cssClass": "editor-title" + } } }, "pages": { + "type": "string", "title": "Pages", - "type": "string" + "form": { + "hideExpression": "model.copy === false", + "expressionProperties": { + "templateOptions.required": "model.copy === true" + }, + "wrappers": [ + "form-field-horizontal" + ] + } }, "found_in": { - "title": "Found in", "type": "object", + "title": "Found in", "required": [ "source", "url" @@ -186,28 +309,96 @@ ], "properties": { "source": { - "title": "Source", "type": "string", - "minLength": 3 + "title": "Source", + "minLength": 3, + "form": { + "expressionProperties": { + "templateOptions.required": "field.parent.model.url !== undefined && field.parent.model.url !== ''" + } + } }, "url": { - "title": "URL", "type": "string", + "title": "URL", "placeholder": "Example: https://www.rero.ch/", "format": "uri", - "minLength": 7 + "minLength": 7, + "form": { + "expressionProperties": { + "templateOptions.required": "field.parent.model.source !== undefined && field.parent.model.source !== ''" + } + } } } }, - "note": { - "title": "Note", - "description": "Add further information to the librarian.", - "type": "string", - "maxLength": 2000, + "notes": { + "type": "array", + "title": "Notes", + "items": { + "type": "object", + "title": "Note", + "additionalProperties": false, + "propertiesOrder": [ + "type", + "content" + ], + "required": [ + "type", + "content" + ], + "properties": { + "type": { + "type": "string", + "title": "Type", + "enum": [ + "public_note", + "staff_note" + ], + "default": "public_note", + "form": { + "type": "selectWithSort", + "options": [ + { + "label": "public_note", + "value": "public_note" + }, + { + "label": "staff_note", + "value": "staff_note" + } + ] + } + }, + "content": { + "type": "string", + "title": "Content", + "maxLength": 2000, + "minLength": 3, + "form": { + "type": "textarea", + "templateOptions": { + "rows": 3 + } + } + } + } + }, "form": { - "type": "textarea", "templateOptions": { - "rows": 3 + "cssClass": "editor-title" + }, + "validation": { + "validators": { + "uniqueValueKeysInObject": { + "keys": [ + "type" + ] + } + }, + "messages": { + "uniqueValueKeysInObjectMessage": "Only one note per type is allowed" + } } } } diff --git a/rero_ils/modules/ill_requests/listener.py b/rero_ils/modules/ill_requests/listener.py index d3668c9da5..ef1f06161e 100644 --- a/rero_ils/modules/ill_requests/listener.py +++ b/rero_ils/modules/ill_requests/listener.py @@ -18,6 +18,8 @@ """Signals connector for Item.""" from .api import ILLRequest, ILLRequestsSearch +from ..locations.api import Location +from ..utils import extracted_data_from_ref def enrich_ill_request_data(sender, json=None, record=None, index=None, @@ -35,3 +37,14 @@ def enrich_ill_request_data(sender, json=None, record=None, index=None, json['organisation'] = { 'pid': record.organisation_pid } + # add patron name to ES index (for faceting) + patron = extracted_data_from_ref( + record.get('patron').get('$ref'), 'record') + json['patron']['name'] = patron.formatted_name + # add library informations to ES index (for faceting) + loc_pid = json.get('pickup_location', {}).get('pid') + if loc_pid: + parent_lib = Location.get_record_by_pid(loc_pid).get_library() + json['library'] = { + 'pid': parent_lib.pid + } diff --git a/rero_ils/modules/ill_requests/mappings/v7/ill_requests/ill_request-v0.0.1.json b/rero_ils/modules/ill_requests/mappings/v7/ill_requests/ill_request-v0.0.1.json index 3e51e66101..85cc0f0c06 100644 --- a/rero_ils/modules/ill_requests/mappings/v7/ill_requests/ill_request-v0.0.1.json +++ b/rero_ils/modules/ill_requests/mappings/v7/ill_requests/ill_request-v0.0.1.json @@ -21,6 +21,23 @@ "type": "boolean" }, "patron": { + "properties": { + "pid": { + "type": "keyword" + }, + "name": { + "type": "text", + "copy_to": "patron.facet" + }, + "facet": { + "type": "keyword" + } + } + }, + "loan_status": { + "type": "keyword" + }, + "library": { "properties": { "pid": { "type": "keyword" @@ -83,6 +100,16 @@ } } }, + "notes": { + "properties": { + "type": { + "type": "keyword" + }, + "content": { + "type": "text" + } + } + }, "_created": { "type": "date" }, diff --git a/rero_ils/modules/ill_requests/models.py b/rero_ils/modules/ill_requests/models.py index 34afc5bb37..6d9463fdbf 100644 --- a/rero_ils/modules/ill_requests/models.py +++ b/rero_ils/modules/ill_requests/models.py @@ -21,6 +21,7 @@ from invenio_db import db from invenio_pidstore.models import RecordIdentifier +from invenio_records.models import RecordMetadataBase class ILLRequestIdentifier(RecordIdentifier): @@ -35,6 +36,12 @@ class ILLRequestIdentifier(RecordIdentifier): ) +class ILLRequestMetadata(db.Model, RecordMetadataBase): + """ILLRequest record metadata.""" + + __tablename__ = 'ill_request_metadata' + + class ILLRequestStatus: """Available status for an ILL request.""" @@ -42,3 +49,10 @@ class ILLRequestStatus: VALIDATED = 'validated' DENIED = 'denied' CLOSED = 'closed' + + +class ILLRequestNoteStatus: + """Available note status for an ILL request.""" + + PUBLIC_NOTE = 'public_note' + STAFF_NOTE = 'staff_note' diff --git a/rero_ils/modules/ill_requests/permissions.py b/rero_ils/modules/ill_requests/permissions.py index 47b74f8616..baf8b107b5 100644 --- a/rero_ils/modules/ill_requests/permissions.py +++ b/rero_ils/modules/ill_requests/permissions.py @@ -79,8 +79,14 @@ def update(cls, user, record): # record cannot be null if not current_patron or not current_patron.is_librarian or not record: return False - return current_organisation['pid'] \ - == ILLRequest(record).organisation_pid + if current_organisation['pid'] == ILLRequest(record).organisation_pid: + # 'sys_lib' can update all request + if current_patron.is_system_librarian: + return True + # 'lib' can only update request linked to its own library + return current_patron.library_pid and \ + record.get_library().pid == current_patron.library_pid + return False @classmethod def delete(cls, user, record): @@ -90,7 +96,5 @@ def delete(cls, user, record): :param record: Record to check. :return: True if action can be done. """ - if not record: - return False - # same as create - return cls.update(user, record) + # no one can delete an ill_request. (Use closed status instead) + return False diff --git a/rero_ils/modules/ill_requests/serializers.py b/rero_ils/modules/ill_requests/serializers.py new file mode 100644 index 0000000000..4daa21ec10 --- /dev/null +++ b/rero_ils/modules/ill_requests/serializers.py @@ -0,0 +1,42 @@ +# -*- coding: utf-8 -*- +# +# RERO ILS +# Copyright (C) 2019 RERO +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published by +# the Free Software Foundation, version 3 of the License. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . + +"""ILL Request serialization.""" + +from invenio_records_rest.serializers.response import search_responsify + +from ..libraries.api import Library +from ..serializers import JSONSerializer, RecordSchemaJSONV1 + + +class ILLRequestJSONSerializer(JSONSerializer): + """Mixin serializing records as JSON.""" + + def post_process_serialize_search(self, results, pid_fetcher): + """Post process the search results.""" + aggrs = results.get('aggregations', {}) + # replace the library pid by the library name for faceting + for lib_term in aggrs.get('library', {}).get('buckets', []): + pid = lib_term.get('key') + lib_term['name'] = Library.get_record_by_pid(pid).get('name') + + return super().post_process_serialize_search(results, pid_fetcher) + + +json_ill_request = ILLRequestJSONSerializer(RecordSchemaJSONV1) +json_ill_request_search = search_responsify(json_ill_request, + 'application/rero+json') diff --git a/rero_ils/modules/ill_requests/templates/rero_ils/_macro_form.html b/rero_ils/modules/ill_requests/templates/rero_ils/_macro_form.html index d9f0ea8004..ca2d6456c1 100644 --- a/rero_ils/modules/ill_requests/templates/rero_ils/_macro_form.html +++ b/rero_ils/modules/ill_requests/templates/rero_ils/_macro_form.html @@ -55,7 +55,7 @@ {%- for item in field %} {{ item(**kwargs) }} - {{ item.label.text|safe }} + {{ _(item.label.text)|safe }} {%- endfor %} {%- if field.description %} diff --git a/rero_ils/modules/ill_requests/templates/rero_ils/ill_request_form.html b/rero_ils/modules/ill_requests/templates/rero_ils/ill_request_form.html index 52ac49c491..35eee7706f 100644 --- a/rero_ils/modules/ill_requests/templates/rero_ils/ill_request_form.html +++ b/rero_ils/modules/ill_requests/templates/rero_ils/ill_request_form.html @@ -22,9 +22,9 @@ {%- block body %}
diff --git a/rero_ils/modules/patrons/templates/rero_ils/_patron_profile_ill_requests.html b/rero_ils/modules/patrons/templates/rero_ils/_patron_profile_ill_requests.html index bbeef9f17b..95f55746f3 100644 --- a/rero_ils/modules/patrons/templates/rero_ils/_patron_profile_ill_requests.html +++ b/rero_ils/modules/patrons/templates/rero_ils/_patron_profile_ill_requests.html @@ -99,9 +99,9 @@
{{ _('Details') }}
{{ request.found_in.url }} {%- endif %} - {%- if request.note %} + {%- if request.public_note %}
{{ _('Note')}}
-
{{ request.note }}
+
{{ request.public_note }}
{%- endif %} diff --git a/rero_ils/modules/serializers.py b/rero_ils/modules/serializers.py index 7041cff620..4a838f7c4b 100644 --- a/rero_ils/modules/serializers.py +++ b/rero_ils/modules/serializers.py @@ -16,7 +16,6 @@ # along with this program. If not, see . """Record serialization.""" - from flask import json, request, url_for from invenio_records_rest.schemas import \ RecordSchemaJSONV1 as _RecordSchemaJSONV1 @@ -45,27 +44,25 @@ def preprocess_record(self, pid, record, links_factory=None, **kwargs): rec = record if request and request.args.get('resolve') == '1': rec = record.replace_refs() + # because the replace_refs loose the record original model. We need + # to resetting it to have correct 'created'/'updated' output data + rec.model = record.model return super(JSONSerializer, self).preprocess_record( pid=pid, record=rec, links_factory=links_factory, kwargs=kwargs) @staticmethod def preprocess_search_hit(pid, record_hit, links_factory=None, **kwargs): """Prepare a record hit from Elasticsearch for serialization.""" - super(JSONSerializer, JSONSerializer).preprocess_search_hit( + record = _JSONSerializer.preprocess_search_hit( pid=pid, record_hit=record_hit, links_factory=links_factory, kwargs=kwargs ) - search_hit = dict( - pid=pid, - metadata=record_hit['_source'], - links=links_factory(pid, record_hit=record_hit, **kwargs), - revision=record_hit['_version'] - ) - if record_hit.get('_explanation'): - search_hit['explanation'] = record_hit.get('_explanation') - return search_hit + if record.get('_explanation'): + record[1:] = record.get('_explanation') + del record['_explanation'] + return record def post_process_serialize_search(self, results, pid_fetcher): """Post process the search results.""" diff --git a/tests/api/ill_requests/test_ill_requests_permissions.py b/tests/api/ill_requests/test_ill_requests_permissions.py index 15d139e343..999c7890c1 100644 --- a/tests/api/ill_requests/test_ill_requests_permissions.py +++ b/tests/api/ill_requests/test_ill_requests_permissions.py @@ -58,7 +58,7 @@ def test_ill_requests_permissions_api(client, librarian_martigny_no_email, assert data['read']['can'] assert data['create']['can'] assert data['update']['can'] - assert data['delete']['can'] + assert not data['delete']['can'] res = client.get(illr_sion_permissions_url) data = get_json(res) assert not data['read']['can'] @@ -68,6 +68,7 @@ def test_ill_requests_permissions_api(client, librarian_martigny_no_email, def test_ill_requests_permissions(patron_martigny_no_email, librarian_martigny_no_email, + system_librarian_martigny_no_email, ill_request_martigny, ill_request_sion, org_martigny): """Test patron types permissions class.""" @@ -111,10 +112,24 @@ def test_ill_requests_permissions(patron_martigny_no_email, assert ILLRequestPermission.read(None, ill_request_martigny) assert ILLRequestPermission.create(None, ill_request_martigny) assert ILLRequestPermission.update(None, ill_request_martigny) - assert ILLRequestPermission.delete(None, ill_request_martigny) + assert not ILLRequestPermission.delete(None, ill_request_martigny) assert ILLRequestPermission.list(None, ill_request_sion) assert not ILLRequestPermission.read(None, ill_request_sion) assert ILLRequestPermission.create(None, ill_request_sion) assert not ILLRequestPermission.update(None, ill_request_sion) assert not ILLRequestPermission.delete(None, ill_request_sion) + + # As System-librarian + with mock.patch( + 'rero_ils.modules.ill_requests.permissions.current_patron', + system_librarian_martigny_no_email + ), mock.patch( + 'rero_ils.modules.ill_requests.permissions.current_organisation', + org_martigny + ): + assert ILLRequestPermission.list(None, ill_request_martigny) + assert ILLRequestPermission.read(None, ill_request_martigny) + assert ILLRequestPermission.create(None, ill_request_martigny) + assert ILLRequestPermission.update(None, ill_request_martigny) + assert not ILLRequestPermission.delete(None, ill_request_martigny) diff --git a/tests/api/ill_requests/test_ill_requests_rest.py b/tests/api/ill_requests/test_ill_requests_rest.py index e660ccb165..7aa9545d16 100644 --- a/tests/api/ill_requests/test_ill_requests_rest.py +++ b/tests/api/ill_requests/test_ill_requests_rest.py @@ -72,6 +72,8 @@ def test_ill_requests_get(client, ill_request_martigny): metadata = data['hits']['hits'][0]['metadata'] del metadata['organisation'] # organisation is added only for indexation + del metadata['library'] # library is added only for indexation + del metadata['patron']['name'] # patron name is added only for indexation assert metadata == ill_request.replace_refs() @@ -124,12 +126,6 @@ def test_ill_requests_post_put_delete(client, org_martigny, json_header, data = get_json(res)['hits']['hits'][0] assert data['metadata']['document']['title'] == 'Title test' - # Delete record/DELETE - res = client.delete(item_url) - assert res.status_code == 204 - res = client.get(item_url) - assert res.status_code == 410 - def test_ill_requests_can_delete(client, ill_request_martigny): """Test can delete an ill request.""" @@ -147,18 +143,16 @@ def test_filtered_ill_requests_get( res = client.get(list_url) assert res.status_code == 200 data = get_json(res) - assert data['hits']['total']['value'] == 1 - hit = data['hits']['hits'][0]['metadata'] - assert hit['pid'] == ill_request_martigny.pid + pids = [hit['metadata']['pid'] for hit in data['hits']['hits']] + assert ill_request_martigny.pid in pids # Sion login_user_via_session(client, librarian_sion_no_email.user) res = client.get(list_url) assert res.status_code == 200 data = get_json(res) - assert data['hits']['total']['value'] == 1 - hit = data['hits']['hits'][0]['metadata'] - assert hit['pid'] == ill_request_sion.pid + pids = [hit['metadata']['pid'] for hit in data['hits']['hits']] + assert ill_request_sion.pid in pids def test_ill_request_secure_api(client, json_header, ill_request_martigny, @@ -227,4 +221,4 @@ def test_ill_request_secure_api_delete(client, ill_request_martigny, pid_value=ill_request_martigny.pid ) res = client.delete(record_url) - assert res.status_code == 204 + assert res.status_code == 403 diff --git a/tests/api/test_serializers.py b/tests/api/test_serializers.py index c3848f5071..2d02b99381 100644 --- a/tests/api/test_serializers.py +++ b/tests/api/test_serializers.py @@ -35,6 +35,13 @@ def test_patrons_serializers( response = client.get(list_url, headers=json_header) assert response.status_code == 200 + # Get the first result and check if it contains all desired keys. + data = get_json(response) + hit = data['hits']['hits'][0] + for key in ['created', 'updated', 'id', 'links', 'metadata']: + assert key in hit + assert hit[key] + def test_items_serializers( client, @@ -56,21 +63,25 @@ def test_items_serializers( response = client.get(item_url, headers=json_header) assert response.status_code == 200 data = get_json(response) - assert data['metadata'].get('item_type').get('$ref') + assert data['metadata'].get('item_type', {}).get('$ref') item_url = url_for( 'invenio_records_rest.item_item', pid_value=item_lib_martigny.pid) response = client.get(item_url, headers=json_header) assert response.status_code == 200 data = get_json(response) - assert data['metadata'].get('item_type').get('$ref') + assert data['metadata'].get('item_type', {}).get('$ref') item_url = url_for( 'invenio_records_rest.item_item', pid_value=item_lib_fully.pid, resolve=1) response = client.get(item_url, headers=json_header) - data = get_json(response)['metadata'] - assert data.get('item_type').get('pid') + data = get_json(response) + assert data['metadata'].get('item_type', {}).get('pid') + # test if all key exist into response with a value + for key in ['created', 'updated', 'id', 'links', 'metadata']: + assert key in data + assert data[key] list_url = url_for('invenio_records_rest.item_list') response = client.get(list_url, headers=rero_json_header) diff --git a/tests/ui/ill_requests/test_ill_requests_api.py b/tests/ui/ill_requests/test_ill_requests_api.py index 4a6a8db589..c6f321eac6 100644 --- a/tests/ui/ill_requests/test_ill_requests_api.py +++ b/tests/ui/ill_requests/test_ill_requests_api.py @@ -20,10 +20,12 @@ from __future__ import absolute_import, print_function from rero_ils.modules.ill_requests.api import ILLRequest +from rero_ils.modules.ill_requests.models import ILLRequestNoteStatus def test_ill_request_properties(ill_request_martigny, ill_request_sion, - loc_public_martigny_data, org_martigny_data): + loc_public_martigny_data, org_martigny_data, + lib_martigny): """Test ill request properties.""" assert not ill_request_martigny.is_copy assert ill_request_sion.is_copy @@ -32,6 +34,23 @@ def test_ill_request_properties(ill_request_martigny, ill_request_sion, == loc_public_martigny_data['pid'] assert ill_request_martigny.organisation_pid == org_martigny_data['pid'] + # test notes + assert ill_request_martigny.public_note is None + note_content = 'public note test' + ill_request_martigny['notes'] = [{ + 'type': ILLRequestNoteStatus.PUBLIC_NOTE, + 'content': note_content + }] + assert ill_request_martigny.public_note == note_content + ill_request_martigny['notes'] = [{ + 'type': ILLRequestNoteStatus.STAFF_NOTE, + 'content': note_content + }] + assert ill_request_martigny.public_note is None + del ill_request_martigny['notes'] + + assert ill_request_martigny.get_library().pid == lib_martigny.pid + def test_ill_request_get_request(ill_request_martigny, ill_request_sion, patron_martigny_no_email): diff --git a/tests/unit/test_ill_requests_jsonschema.py b/tests/unit/test_ill_requests_jsonschema.py index b4dc850024..b874d93b2b 100644 --- a/tests/unit/test_ill_requests_jsonschema.py +++ b/tests/unit/test_ill_requests_jsonschema.py @@ -19,12 +19,15 @@ from __future__ import absolute_import, print_function +import copy + import pytest from jsonschema import validate from jsonschema.exceptions import ValidationError from rero_ils.modules.errors import RecordValidationError from rero_ils.modules.ill_requests.api import ILLRequest +from rero_ils.modules.ill_requests.models import ILLRequestNoteStatus def test_required(ill_request_schema, ill_request_martigny_data_tmp): @@ -60,11 +63,25 @@ def test_required(ill_request_schema, ill_request_martigny_data_tmp): def test_extended_validation(app, ill_request_martigny_data_tmp): """Test extended validation for ill request.""" - data = ill_request_martigny_data_tmp + data = copy.deepcopy(ill_request_martigny_data_tmp) - # pages are reqiured if request is a request copy + # pages are required if request is a request copy data['copy'] = True if 'pages' in data: del data['pages'] with pytest.raises(RecordValidationError): ILLRequest.validate(ILLRequest(data)) + + # test on 'notes' field :: have 2 note of the same type is disallowed + data = copy.deepcopy(ill_request_martigny_data_tmp) + data['notes'] = [{ + 'type': ILLRequestNoteStatus.PUBLIC_NOTE, + 'content': 'dummy content' + }] + ILLRequest.validate(ILLRequest(data)) + with pytest.raises(RecordValidationError): + data['notes'].append({ + 'type': ILLRequestNoteStatus.PUBLIC_NOTE, + 'content': 'second dummy note' + }) + ILLRequest.validate(ILLRequest(data))