Repository navigation
Feature/admin custom short notice warning per venue - #165
Merged
Merged
Conversation
Contributor
|
LGTM |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The short-notice booking warning added in #163 was hardcoded: every venue warned at 7 days with the same fixed text, and changing either meant a code change and redeploy. Venues have different lead times (e.g. MPSH needs ~2 weeks to arrange equipment and staff, while a seminar room needs less), and some venues don't need the warning at all.
This PR lets admins configure the warning per venue from the existing Venue Update page.
What changed
Admin (Admin → Venues → Edit)
Users (booking creation)
Backend
Venuegets 4 fields:short_notice_enabled,short_notice_days,short_notice_title,short_notice_message(migration0018).venue_to_json(full_details=True); accepted byPOST /venues/andPUT /venues/:idthroughVenueSerializer.Approach and alternatives
Venuerather than a separate settings table. We considered an org-levelBookingSettingsmodel with per-venue overrides, but each venue only ever needs one set of settings, and per-venue columns reuse the existing venue API, form and fetches (the booking page already loads full venue details), with no new model or endpoint. Org-wide defaults can be added later on top of this without breaking anything.{days}placeholder. We prototyped a templated message, then dropped it for simplicity. The default message deliberately contains no number, so it can't drift out of sync with the days setting.Where to look
frontend/src/components/venue-details-form/venue-details-form.tsx. It holds the new section, the validation, andwithValidShortNoticeSettings.venues/views.py(get_short_notice_settings),venues/logic.py,venues/models.py.booking-creation-time-slot-selector.tsx: missing settings fall back toDEFAULT_SHORT_NOTICE_WARNING(on, 7 days). This keeps the warning on for booking drafts saved in localStorage before this deploys.Testing
venues/tests.py, the backend's first tests (4 cases): defaults on create, partial update keeps the other settings, the serializer rejects 0 and 366 days, and omitted fields stay out ofvalidated_data. All pass.tsc --noEmit, ESLint and Prettier are clean.Screenshots
Risks and rollout
venues.0018adds 4 columns with defaults to the existing venue table. There's no new table and no data backfill beyond the defaults. On deploy, every venue is on at 7 days with the default message, which matches today's behaviour except for the wording.venues/views.pyextend_schemablocks are not updated and don't show the new fields. They also already disagreed with the real responses before this PR (categoryandorganizationare shown as objects but are actually strings). This could be a follow-up.