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

item: multiple inheritance for the item class #968

Merged
merged 1 commit into from
May 6, 2020

Conversation

BadrAly
Copy link

@BadrAly BadrAly commented Apr 30, 2020

Since the item class is used now to manage two things:

  1. the item record
  2. the item circulation transactions

Two base classes are created for each section above in
api.record:ItemRecord, api.circulation:ItemCirculation
The main Item class inherites from these two classes.

Co-Authored-by: Johnny Mariéthoz [email protected]
Co-Authored-by: Aly Badr[email protected]

Why are you opening this PR?

re-organising the class Item

How to test?

Must have rero_ils_ui.dev
./run_tests.sh

Code review check list

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

@BadrAly BadrAly self-assigned this Apr 30, 2020
@BadrAly BadrAly force-pushed the baa-item-class-separation branch from e822b42 to d989051 Compare April 30, 2020 12:26
@BadrAly BadrAly marked this pull request as ready for review April 30, 2020 13:30
@BadrAly BadrAly requested a review from benerken April 30, 2020 13:30
Copy link
Contributor

@benerken benerken left a comment

Choose a reason for hiding this comment

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

Run-test => OK
Installed locally and tested CKO/CKI/Renew/Request. => OK

@benerken Thanks a lot for the test.

Please verify the creation of a new item.
when you type the barcode, the console shows an error "Backend returned code 400, body was: [object Object]"

The save button is disabled => Impossible to create an item.
I use the Admin UI "included" in rero-ils (via port 5000), so not a specific version on port 4200

I was able to duplicate the problem locally, however if I connect to rero_ils_ui.dev it works nicely. I am adding rero_ils_ui.dev as dependency

Copy link
Contributor

@rerowep rerowep left a comment

Choose a reason for hiding this comment

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

I'am not sure it is the right way to put classes into the __init__.pyfile ?
https://docs.python.org/3/tutorial/modules.html#packages

@BadrAly BadrAly force-pushed the baa-item-class-separation branch from d989051 to 93f797f Compare May 5, 2020 12:32
@BadrAly BadrAly requested review from rerowep and lauren-d May 5, 2020 12:33
@BadrAly BadrAly force-pushed the baa-item-class-separation branch 2 times, most recently from 61be45b to c7f5cca Compare May 5, 2020 12:36
@BadrAly BadrAly changed the title item class: multiple inheritance for the item class item: multiple inheritance for the item class May 5, 2020
Since the item class is used now to manage two things:
1. the item record
2. the item circulation transactions

Two base classes are created for each section above in
api.record:ItemRecord, api.circulation:ItemCirculation
The main Item class inherites from these two classes.

Co-Authored-by: Johnny Mariéthoz <[email protected]>
Co-Authored-by: Aly Badr<[email protected]>
@BadrAly BadrAly force-pushed the baa-item-class-separation branch from c7f5cca to 58002ef Compare May 5, 2020 13:00
@iGormilhit iGormilhit self-requested a review May 6, 2020 04:49
@BadrAly BadrAly merged commit e47b4a6 into rero:dev May 6, 2020
@BadrAly BadrAly deleted the baa-item-class-separation branch November 5, 2020 13:38
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