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

fix: correct circulation policy #633

Merged
merged 1 commit into from
Nov 27, 2019

Conversation

AoNoOokami
Copy link
Contributor

@AoNoOokami AoNoOokami commented Nov 19, 2019

Co-Authored-by: Alicia Zangger alicia.zangger@rero.ch

Why are you opening this PR?

Some circulation policies have item types and patron types not related to their organisation.

How to test?

Circulation policies create (issue #213):

Run ./scripts/setup -> should proceed as usual with no error
Then change a patron type or an item type of a circulation policy for one that doesn't belong to the correct organisation in : data/circulation_policies.json
Run ./scripts/setup : the script will raise an error and stop.
Run ./scripts/setup -p : the script will raise an error and continue.

Issue #625 :

On rero-ils: open ci-po editor and confirm that patron type and item type settings are loaded.
See also the issue details for more checks.

Issue #626 :

Login as Irma Pince: reroilstest+irma@gmail.com
Open ci-po editor to add or edit a ci-po. Saving new ci-po or modifications doesn't work on rero-ils-ui with proxy, but on rero-ils it's ok.

Code review check list

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

@AoNoOokami AoNoOokami self-assigned this Nov 19, 2019
@AoNoOokami AoNoOokami added the WIP label Nov 19, 2019
@jma jma removed their request for review November 20, 2019 06:04
@AoNoOokami AoNoOokami force-pushed the zaa-fix-ci-po-item-types branch 2 times, most recently from 7ed70d0 to d2c15fc Compare November 20, 2019 14:30
@AoNoOokami AoNoOokami removed the WIP label Nov 20, 2019
rero_ils/modules/circ_policies/api.py Outdated Show resolved Hide resolved
rero_ils/modules/circ_policies/api.py Show resolved Hide resolved
scripts/setup Outdated Show resolved Hide resolved
Copy link

@iGormilhit iGormilhit left a comment

Choose a reason for hiding this comment

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

This is very cool, cause it solves many issues, IMHO #625 also, as I have checked. Could you verify this?

If this is the case, it could be nice to be merged into master, or into a specific branch to be deployed on ils.test for the next workshop.

@AoNoOokami
Copy link
Contributor Author

AoNoOokami commented Nov 22, 2019

This is very cool, cause it solves many issues, IMHO #625 also, as I have checked. Could you verify this?

If this is the case, it could be nice to be merged into master, or into a specific branch to be deployed on ils.test for the next workshop.

I checked that. Unfortunately, it doesn't fix this issue, which is related to the editor form and not to the brief or detailed view. Actually, I wasn't able to save a new circulation policy. I also saw that cancelling the add or edit generates errors in the console, even if the ui does what is expected. So this form really needs to be reviewed.

EDIT: Actually, I tested it on rero-ils-ui. There are some issues there, but I was able to create a new ci-po on rero-ils with expected saving of settings. Those settings were not available anymore for another ci-po creation, as expected. I will add Closes #625 to the commit message.

@AoNoOokami AoNoOokami force-pushed the zaa-fix-ci-po-item-types branch 3 times, most recently from 3b23791 to 6bf99d5 Compare November 22, 2019 10:05
* Corrects item types and patron_types of circulation policy with pid = 10 and 11.
* Adds test to check data consistency when ci-po are created.
* Adds new patron type.
* Corrects import sorting.
* Closes rero#625.
* Closes rero#626.
* Closes rero#213.

Co-Authored-by: Alicia Zangger <alicia.zangger@rero.ch>
@iGormilhit iGormilhit self-requested a review November 25, 2019 09:28
@jma jma changed the base branch from dev to master November 25, 2019 11:05
@jma jma changed the base branch from master to dev November 25, 2019 11:05
@AoNoOokami AoNoOokami merged commit 0e71baa into rero:dev Nov 27, 2019
@AoNoOokami AoNoOokami deleted the zaa-fix-ci-po-item-types branch December 6, 2019 09:53
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.

6 participants