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

renewals: add renew buttons for patrons checked-out items #610

Merged
merged 1 commit into from
Nov 20, 2019
Merged

renewals: add renew buttons for patrons checked-out items #610

merged 1 commit into from
Nov 20, 2019

Conversation

BadrAly
Copy link

@BadrAly BadrAly commented Nov 7, 2019 •
edited
Loading

  • Displays renew button when renewal of checked-out item is possible.
  • Hides renew button when circulation policies do not permit the renewal action.
  • Increases units testing coverage.
  • Corrects indentation of the patron_profile Jinja template.

Co-Authored-by: Aly Badr aly.badr@rero.ch

Why are you opening this PR?

How to test?

  • Login as a patron and visit the patron profile page
  • For the checked-out tab, if checked-out items can be renewed, the renew button will be displayed
  • Click the renew button to change the due date of the current check-out transaction

Code review check list

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

@BadrAly BadrAly added the WIP label Nov 7, 2019
@BadrAly BadrAly force-pushed the baa-renewal-items-by-patron branch 5 times, most recently from a541433 to 632366b Compare November 11, 2019 06:26
@BadrAly BadrAly removed the WIP label Nov 11, 2019
jma
jma requested changes Nov 11, 2019
View reviewed changes
Copy link
Contributor

@jma jma left a comment •
edited by BadrAly
Loading

Choose a reason for hiding this comment

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

If can_extend well cover by the tests?

Tested

rero_ils/modules/items/api.py Outdated Show resolved Hide resolved
rero_ils/modules/patrons/views.py Outdated Show resolved Hide resolved
rero_ils/modules/patrons/views.py Show resolved Hide resolved
rero_ils/modules/patrons/views.py Outdated Show resolved Hide resolved
rero_ils/modules/patrons/views.py Outdated Show resolved Hide resolved
@BadrAly BadrAly force-pushed the baa-renewal-items-by-patron branch from 632366b to 5267e8d Compare November 11, 2019 13:09
@BadrAly BadrAly requested a review from jma November 11, 2019 13:10
@jma jma removed their request for review November 11, 2019 13:58
@BadrAly BadrAly force-pushed the baa-renewal-items-by-patron branch from 5267e8d to 55d80f3 Compare November 11, 2019 14:04
@BadrAly BadrAly requested a review from jma November 11, 2019 14:04
@BadrAly BadrAly force-pushed the baa-renewal-items-by-patron branch from 55d80f3 to 37ee7dc Compare November 12, 2019 07:14
iGormilhit
iGormilhit suggested changes Nov 13, 2019
View reviewed changes
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.

Maybe you can add that you have corrected indentation of Jinja templates in the commit message?

rero_ils/modules/items/api.py Outdated Show resolved Hide resolved
@BadrAly BadrAly force-pushed the baa-renewal-items-by-patron branch from 37ee7dc to cc39288 Compare November 13, 2019 07:53
@BadrAly BadrAly requested a review from iGormilhit November 13, 2019 07:56
@BadrAly BadrAly force-pushed the baa-renewal-items-by-patron branch 2 times, most recently from d330d95 to a6476b5 Compare November 15, 2019 07:27
@BadrAly BadrAly requested a review from a team November 18, 2019 12:12
pronguen
pronguen reviewed Nov 18, 2019
View reviewed changes
Copy link
Contributor

@pronguen pronguen left a comment •
edited by BadrAly
Loading

Choose a reason for hiding this comment

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

  • the new due date is different in the patron interface than in the admin interface (probably related to Due date according to opening hours not working #599)
  • according to me, the "Renew" button does not really look like a button. Is it possible to have a plain colour filling, ideally the same than for the "Request" button?
* according to me, the "Renew" button does not really look like a button. Is it possible to have a plain colour filling, ideally the same than for the "Request" button?

fixed

@BadrAly BadrAly self-assigned this Nov 18, 2019
@BadrAly BadrAly force-pushed the baa-renewal-items-by-patron branch from a6476b5 to 5bdf0ce Compare November 18, 2019 15:30
@BadrAly BadrAly requested a review from pronguen November 18, 2019 15:31
renewals: add renew buttons for patrons checked-out items
5f95c0e 
* Displays renew button when renewal of checked-out item is possible.
* Hides renew button when circulation policies do not permit the renewal action.
* Increases units testing coverage.
* Corrects indentation of the patron_profile Jinja template.

Co-Authored-by: Aly Badr <aly.badr@rero.ch>
@BadrAly BadrAly force-pushed the baa-renewal-items-by-patron branch from 5bdf0ce to 5f95c0e Compare November 20, 2019 09:23
@BadrAly BadrAly requested a review from jma November 20, 2019 09:23
@BadrAly BadrAly merged commit 87be6be into rero:dev Nov 20, 2019
@BadrAly BadrAly deleted the baa-renewal-items-by-patron branch January 27, 2020 10:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rerowep rerowep rerowep approved these changes

@pronguen pronguen pronguen approved these changes

@iGormilhit iGormilhit iGormilhit approved these changes

@jma jma jma approved these changes

@Garfield-fr Garfield-fr Garfield-fr approved these changes

@AoNoOokami AoNoOokami Awaiting requested review from AoNoOokami

Assignees

@BadrAly BadrAly

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@BadrAly @Garfield-fr @jma @iGormilhit @pronguen @rerowep