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

Rework of some of the reservation validation logic #609

Merged
merged 7 commits into from
Mar 7, 2023

Conversation

ddabble
Copy link
Member

@ddabble ddabble commented Mar 7, 2023

Proposed changes

Improvements

  • Refactored the can_change[...] methods of Reservation (2a6a92f)
  • Reworked some of the reservation validation logic (3a271bf)
  • Disable end_time in reservation form if the user can't change it (f1680cc)
  • Also made the user dropdowns on both the quota admin page and the quota form page, function equally (a5fdd91)

Fixes

  • Fixed users being unable to mark reservations as finished when the machine's status is set to OUT_OF_ORDER or MAINTENANCE (57ed822)

Other changes

Areas to review closely

That the reservation logic works as intended.

Checklist

(If any of the points are not relevant, mark them as checked)

  • Created tests that fail without the changes, if relevant/possible
  • Made sure that your code conforms to the code style guides and that any common code smells have been addressed
    • (It's not intended that you read through this whole document, but that you get yourself an overview over its contents, and that you keep it in mind while taking a second look at your code before opening a pull request)
  • Added sufficient documentation - e.g. as docstrings or in the README, if suitable
  • Added your changes to the "Unreleased" section of the changelog - mainly the changes that are of particular interest to users and/or developers, if any
  • Added a "Deployment notes" section above, if anything out of the ordinary should be done when deploying these changes to the server

Now the user dropdowns on both the quota admin page and the quota form page,
sort (alphabetically, after concatenating the first and last names), search (fuzzy `fullTextSearch`) and format (using
an `–` between the full name and username) their user strings equally.
The `can_change()` method doesn't (seemingly arbitrarily) validate the start time anymore - the latter of which has been moved to
`can_change_start_time()` - and has been more fittingly renamed to `can_be_changed_by(user)`.
The `can_change_end_time()` method also doesn't validate the user anymore, as that logic already exists in `can_be_changed_by()`.

With this, the code functionality has changed and the tests fail, as I had slightly misunderstood the previous logic.
This will be fixed in the next commit.

Also renamed the `can_delete()` method.
The tests now work again, and the code should be easier to understand.
This doesn't change anything in practice, as the user is not allowed to view the page if the end time can't be changed, but this ensures that the form
would behave as expected if that were to change.
...when the machine is set to "OUT_OF_ORDER" or "MAINTENANCE".

Also made it possible to change the start time of those reservations, as long as it's set forward in time
- so that the reservation is shortened - as that will at the very least be less disruptive to the people doing maintenance on the machines
- which is the same reason as to why the end time can only be set earlier in time in the same scenario.

This resolves #580.
@ddabble
Copy link
Member Author

ddabble commented Mar 7, 2023

Merging without explicit approval from another member, as the Dev committee agreed to merge these changes and assume they're relatively bug-free, simply to get things done quicker.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

1 participant