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.

Sign up for GitHub

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

Jump to bottom

circulation: update ILL request JSON schema #1441

Merged
merged 1 commit into from
Dec 7, 2020
Merged

circulation: update ILL request JSON schema #1441

merged 1 commit into from
Dec 7, 2020

Conversation

zannkukai
Copy link
Contributor

@zannkukai zannkukai commented Nov 16, 2020 •
edited
Loading

  • Allows to manage manually the loan status.
  • Adds notes (public/staff) to an ILL request.
  • Configures request resource to allow faceting.
  • Fixes search serializer configuration to include created and updated
    date into search results.
  • Fixes object serializer with resolve argument. "created" and
    "updated" keys were null, date are now correct.

Closes #1320

Co-Authored-by: Renaud Michotte [email protected]

Code review check list

  • Commit message template compliance.
  • Commit message without typos.
  • File names.
  • Functions names.
  • Functions docstrings.
  • Unnecessary commited files?

@zannkukai zannkukai self-assigned this Nov 16, 2020
@zannkukai zannkukai force-pushed the zan-#1852-update-ill-request branch from e4221bb to 89b8de2 Compare November 16, 2020 10:10
@iGormilhit iGormilhit added f: circulation Concerns the circulation interface or backend f: data About data model, importation, transformation, exportation of data, specific for bibliographic data labels Nov 18, 2020
@iGormilhit iGormilhit added this to the v0.15.0 milestone Nov 18, 2020
@zannkukai zannkukai force-pushed the zan-#1852-update-ill-request branch 7 times, most recently from b88e710 to 3f3562e Compare November 24, 2020 12:14
@zannkukai zannkukai marked this pull request as ready for review November 24, 2020 12:14
@zannkukai zannkukai force-pushed the zan-#1852-update-ill-request branch from 3f3562e to ce5a47d Compare November 24, 2020 12:58
@ManaDeweerdt
Copy link
Contributor

Cannot see the "add" button in the ILL requests result if I"m logged in as a librarian. (ok for the system librarian)

@ManaDeweerdt
Copy link
Contributor

When I add a ILL request for a patron, if I don"t fill the patron field, it spins forever.

@ManaDeweerdt
Copy link
Contributor

ManaDeweerdt commented Nov 24, 2020 •
edited
Loading

I can choose patron who has only "librarian" as role. Ex : Elena Rodriguez. --> The requester field is empty.
It should only find patron with the role patron ??
image

@pronguen
Copy link
Contributor

pronguen commented Nov 24, 2020 •
edited by zannkukai
Loading

  • The field label "Source" on the request detailed view should be named "Published in"
  • The field "Trouvé dans" should be placed in the first section of the detailed view (this is actually also the case in the public interface)
  • The field "Trouvé dans" should be placed in the first section of the editor (see next comment)
  • Is is possible in the editor to have subsections, so that the librarian see quickly where to find the loan status and request statuts? I imagine the same sections than in the detailed view: Information about the document, Information about the request.
  • A librarian (not system librarian) should be able to edit only the request of her/his library (confirmed by @iGormilhit !)
  • When a librarian adds a request, it spins forever if a required field is not entered (patron, pick-up library).
  • To ease the translations, I would put the same string "no data" everywhere, and would not use "no source". I am wondering if it is really relevant to display this lack of information and would prefer a shorter detailed view.
    (@iGormilhit : 👍, wondering if it wouldn"t be better to display only fileds with value.)
    image
  • Cannot see the "add" button in the ILL requests result if I"m logged in as a librarian (Mana"s comment)

@iGormilhit iGormilhit changed the title request: update JSON schema for ill_request resource. circulation: update ILL request JSON schema Nov 25, 2020
iGormilhit
iGormilhit suggested changes Nov 25, 2020
View reviewed changes
Copy link

@iGormilhit iGormilhit left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Commit message proposition:

circulation: update ILL request JSON schema

* Allows to manage manually the loan status.
* Adds notes (public/staff) to an ILL request.
* Configures request resource to allow faceting.
* Fixes search serializer configuration to include created and updated
  date into search results.
* Fixes object serializer with `resolve` argument. "created" and
  "updated" keys were null, date are now correct.

Is there actually a link between an ILL request and a loan? Or just a way to update the loan_status field manually (as I expected)?

rero_ils/modules/ill_requests/jsonschemas/ill_requests/ill_request-v0.0.1.json Show resolved Hide resolved
rero_ils/modules/ill_requests/jsonschemas/ill_requests/ill_request-v0.0.1.json Show resolved Hide resolved
@zannkukai zannkukai force-pushed the zan-#1852-update-ill-request branch 3 times, most recently from 014d384 to 6a6d4ce Compare November 25, 2020 11:19
@BadrAly
Copy link

BadrAly commented Nov 25, 2020

I do not think Allows to link an ill_request to a loan is valid. as I see it is the librarian to manually enter the loan status

@BadrAly BadrAly self-requested a review November 25, 2020 12:08
BadrAly
BadrAly suggested changes Nov 25, 2020
View reviewed changes
Copy link

@BadrAly BadrAly left a comment

Choose a reason for hiding this comment

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

Test is failing

@zannkukai zannkukai force-pushed the zan-#1852-update-ill-request branch 3 times, most recently from 4e7694a to 0dbc8c9 Compare December 2, 2020 09:58
@zannkukai zannkukai force-pushed the zan-#1852-update-ill-request branch 3 times, most recently from 53de044 to 58bc719 Compare December 3, 2020 14:42
@zannkukai
circulation: update ILL request JSON schema
b7f1558 
* Allows to manage manually the loan status.
* Adds notes (public/staff) to an ILL request.
* Configures request resource to allow faceting.
* Fixes search serializer configuration to include created and updated
  date into search results.
* Fixes object serializer with `resolve` argument. "created" and
  "updated" keys were null, date are now correct.
* Fixes translation problems on the user ILL request form.

Closes rero#1320

Co-Authored-by: Renaud Michotte <[email protected]>
@zannkukai zannkukai force-pushed the zan-#1852-update-ill-request branch from 58bc719 to b7f1558 Compare December 7, 2020 10:40
@zannkukai zannkukai merged commit a333847 into rero:dev Dec 7, 2020
@zannkukai zannkukai deleted the zan-#1852-update-ill-request branch December 7, 2020 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lauren-d lauren-d lauren-d approved these changes

@BadrAly BadrAly BadrAly approved these changes

@iGormilhit iGormilhit iGormilhit approved these changes

Assignees

@zannkukai zannkukai

Labels
f: circulation Concerns the circulation interface or backend f: data About data model, importation, transformation, exportation of data, specific for bibliographic data
Projects
None yet
Milestone
v0.15.0
Development

Successfully merging this pull request may close these issues.

ILL request form is not translated
6 participants
@zannkukai @ManaDeweerdt @pronguen @BadrAly @iGormilhit @lauren-d