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

Reservation error fix #339

Merged
merged 8 commits into from
Mar 4, 2021
Merged

Reservation error fix #339

merged 8 commits into from
Mar 4, 2021

Conversation

zootos
Copy link
Contributor

@zootos zootos commented Mar 3, 2021

A fix for an error that occurs when a reservation is not covered by any rules. Currently, this causes an exception to be raised and sent in an email to the dev team. This fix, simply disallows reservations that are not covered by any rule, as is already the case.

I also included some tests to check these cases specifically, and some missing documentation for the involved methods.

@codecov
Copy link

codecov bot commented Mar 3, 2021

Codecov Report

Merging #339 (c02d3b8) into dev (f9adb84) will decrease coverage by 0.10%.
The diff coverage is 85.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #339      +/-   ##
==========================================
- Coverage   72.49%   72.38%   -0.11%     
==========================================
  Files         113      113              
  Lines        3443     3455      +12     
==========================================
+ Hits         2496     2501       +5     
- Misses        947      954       +7     
Impacted Files Coverage Δ
make_queue/views/reservation/reservation.py 78.10% <77.77%> (-2.50%) ⬇️
make_queue/models/models.py 91.75% <91.66%> (-0.50%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f9adb84...c02d3b8. Read the comment docs.

Copy link
Member

@hermabe hermabe left a comment

Choose a reason for hiding this comment

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

Looks good to me

Copy link
Member

@ddabble ddabble left a comment

Choose a reason for hiding this comment

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

Nice! 😄 Would be great if you could add a test for when the duration of a reservation is 0, but this should be merged regardless 😊

@zootos
Copy link
Contributor Author

zootos commented Mar 3, 2021

I added the given test. Additionally, I fixed a bug in the template for editing reservations, that caused the datepicker for the start and end time of the reservation to be unusable. Finally, I added some custom error messages when creation of a reservation fails due to either a duration of 0 or not being covered by any rules.

@sigridge sigridge merged commit e8178ff into dev Mar 4, 2021
@ddabble ddabble deleted the fix/reservation-error branch March 4, 2021 10:43
@sigridge sigridge mentioned this pull request Mar 8, 2021
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.

None yet

4 participants