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

circulation: Made loan request start date to be configurable and validated at the backend #1191

Conversation

sakshamarora1
Copy link
Contributor

❤️ Thank you for your contribution!

Description

Closes: CERNDocumentServer/cds-ils#835

@sakshamarora1 sakshamarora1 requested a review from kpsherva January 8, 2024 13:12
@sakshamarora1 sakshamarora1 self-assigned this Jan 8, 2024
@@ -104,4 +105,12 @@ def postload_checks(self, data, **kwargs):
"request_expire_date": [message],
}
)
elif end - start < loan_request_offset:
Copy link
Contributor

Choose a reason for hiding this comment

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

Was there any unit/functional test on this? Would it be possible to add tests?

In particular, I would test:

  • if I put a negative value: it looks like you can do that, we should block it
  • if I put 0
  • if I put a few days
  • what happens if I set the config to 2 days and I make the request now at 11am: will the start date be in 2 days, at 11am (exactly 48 hours), or the check will only compare days and I need to select the third day?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will look into the unit test addition for this.

Regarding the 4th point: It will disable today and tomorrow only. The check only compares on the days.

@sakshamarora1 sakshamarora1 force-pushed the configure_loan_request_start_date branch from 5c451fa to 33eec11 Compare January 25, 2024 14:39
@kpsherva kpsherva merged commit 70c314a into inveniosoftware:master Feb 19, 2024
20 checks passed
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.

Require before a certain date
4 participants