Validate LogEntry metadata; increase flash z-index#257
Validate LogEntry metadata; increase flash z-index#257rashusharda wants to merge 4 commits intomainfrom
Conversation
Add a custom validation to LogEntry to ensure at least one metadata field is present before saving. Also bump the flash partial's z-index class from z-50 to z-9999 so flash notifications render above other UI elements.
There was a problem hiding this comment.
Pull request overview
This PR prevents creating LogEntry records with effectively empty metadata (addressing issue #249) and attempts to ensure flash notifications render above modal overlays.
Changes:
- Added a model-level validation on
LogEntryto require at least one non-emptymetadatavalue before saving. - Increased the flash container’s z-index to avoid being covered by the modal overlay.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| app/views/shared/_flash.html.erb | Raises flash z-index to display above modal overlay. |
| app/models/log_entry.rb | Adds custom validation to block “empty” metadata submissions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -1,4 +1,4 @@ | |||
| <div class="fixed bottom-4 right-4 space-y-1 z-50 w-100 animate-fade-in-out"> | |||
| <div class="fixed bottom-4 right-4 space-y-1 z-9999 w-100 animate-fade-in-out"> | |||
There was a problem hiding this comment.
z-9999 is not a default Tailwind z-index utility, so this class likely won’t generate any CSS and may leave the flash at z-index: auto (still behind the modal overlay). Consider using Tailwind arbitrary values (e.g., z-[9999]) or another project-standard z-index that’s known to compile.
| <div class="fixed bottom-4 right-4 space-y-1 z-9999 w-100 animate-fade-in-out"> | |
| <div class="fixed bottom-4 right-4 space-y-1 z-[9999] w-100 animate-fade-in-out"> |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| when "numerical" | ||
| Float(raw) | ||
| when "boolean" | ||
| return true if raw == "on" |
There was a problem hiding this comment.
Removing handling for raw == "off" breaks boolean metadata coming from the checkbox input: LogEntries::MetadataInputComponent always submits a hidden "off" value when unchecked, so booleans will now be saved as nil (and existing false values will be overwritten to nil on update). This changes reporting/aggregation semantics (false vs missing) and prevents explicitly storing false. Restore an explicit "off" => false conversion (or otherwise map unchecked values to false consistently) so boolean fields can round-trip correctly.
| return true if raw == "on" | |
| return true if raw == "on" | |
| return false if raw == "off" |
| belongs_to :subproject | ||
| belongs_to :user | ||
|
|
||
| validate :at_least_one_metadata_field_present |
There was a problem hiding this comment.
NIT: This name is very verbose... Why not name it not_empty
| def at_least_one_metadata_field_present | ||
| return if metadata.is_a?(Hash) && metadata.values.any?(&:present?) | ||
|
|
||
| errors.add(:base, "At least one field must be filled in before saving.") |
There was a problem hiding this comment.
This must also be internationalized.
TL;DR
Fixes a bug where a
LogEntrycould be saved with no content. Adds a model-level validation to block empty submissions, and fixes flash notifications rendering behind the modal overlay.What is this PR trying to achieve?
Addresses #249 — a log entry could be created without any fields filled in. Clicking "Create Log Entry" on a blank form would succeed, resulting in empty records in the database.
How did you achieve it?
Added a custom validate method at_least_one_metadata_field_present that checks whether the metadata JSON column contains at least one non-blank value. If all fields are empty, an error is added to :base and the save is blocked.
Checklist