Validate and normalize auto-postpone period to days#2672
Conversation
robzolkos
force-pushed
the
entropy-validation-and-days
branch
from
beaa509 to
ff83fbd
Compare
There was a problem hiding this comment.
Pull request overview
This PR tightens validation around entropy auto-postpone periods and updates the UI/controller plumbing so the setting is expressed in days (not raw seconds), preventing crafted requests from persisting arbitrary/negative periods.
Changes:
- Add
Entropy::AUTO_POSTPONE_PERIODSand validateauto_postpone_periodagainst the allowed set. - Introduce/use
auto_postpone_period_in_daysfor forms and entropy update endpoints. - Remove the
entropy_auto_close_optionshelper and update tests to use allowed values.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/models/entropy_test.rb | Updates entropy-touch tests to use an allowed period value. |
| test/models/card/entropic_test.rb | Updates entropic behavior tests to use allowed periods. |
| test/controllers/boards_controller_test.rb | Updates board update test to use an allowed entropy period value. |
| test/controllers/boards/entropies_controller_test.rb | Switches board entropy update requests to auto_postpone_period_in_days. |
| test/controllers/accounts/entropies_controller_test.rb | Switches account entropy update requests to auto_postpone_period_in_days and adds an invalid-value rejection test. |
| app/views/entropy/_knob.html.erb | Adjusts knob rendering to work with day values rather than durations. |
| app/views/entropy/_auto_close.html.erb | Uses Entropy::AUTO_POSTPONE_PERIODS and posts auto_postpone_period_in_days. |
| app/models/entropy.rb | Adds the allowed-periods constant, inclusion validation, and virtual day-based attribute accessors. |
| app/models/board/entropic.rb | Delegates/accepts day-based entropy period updates on boards. |
| app/helpers/entropy_helper.rb | Removes entropy_auto_close_options in favor of the model constant. |
| app/controllers/boards/entropies_controller.rb | Permits auto_postpone_period_in_days for board entropy updates. |
| app/controllers/account/entropies_controller.rb | Permits auto_postpone_period_in_days for account entropy updates. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
robzolkos
force-pushed
the
entropy-validation-and-days
branch
from
ff83fbd to
b910b45
Compare
robzolkos
force-pushed
the
entropy-validation-and-days
branch
from
69ec014 to
27c1100
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
robzolkos
force-pushed
the
entropy-validation-and-days
branch
from
27c1100 to
e42dc5e
Compare
robzolkos
force-pushed
the
entropy-validation-and-days
branch
from
e42dc5e to
1ca81bf
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
robzolkos
force-pushed
the
entropy-validation-and-days
branch
from
1ca81bf to
a2a4847
Compare
robzolkos
force-pushed
the
entropy-validation-and-days
branch
from
a2a4847 to
bed335e
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
robzolkos
force-pushed
the
entropy-validation-and-days
branch
from
bed335e to
fcf9d52
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
flavorjones
left a comment
flavorjones
left a comment
There was a problem hiding this comment.
This looks like a big improvement. Thank you!
I'm a little worried that users may have already set arbitrary entropy periods via the API, though. Do you think we should consider writing a migration script in script/migrations/ to normalize the values?
robzolkos
force-pushed
the
entropy-validation-and-days
branch
from
fcf9d52 to
6f03b70
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 18 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Add Entropy::AUTO_POSTPONE_PERIODS and validate auto_postpone_period against the allowed set - Introduce auto_postpone_period_in_days for forms and entropy update endpoints - Fall back to index 0 in knob partial when current value isn't in options - Remove entropy_auto_close_options helper in favor of model constant - Update tests to use allowed period values
The JSON API was accepting auto_postpone_period (seconds) which bypassed the days normalization and validation. Switch to auto_postpone_period_in_days consistently and add explicit wrap_parameters so flat JSON params are wrapped correctly for the virtual attribute.
- Fall back to first knob option (index 0) when persisted value isn't in the allowed set, preventing nil index errors for legacy values - Use integer division (1.day.to_i) for consistent day calculations
Rescue ActiveRecord::RecordInvalid in entropy controllers so invalid auto_postpone_period_in_days values return 422 Unprocessable Entity instead of raising a 500.
robzolkos
force-pushed
the
entropy-validation-and-days
branch
from
75325fd to
0cbd4b2
Compare
Use container.account.entropy.auto_postpone_period_in_days instead of container.account.auto_postpone_period_in_days since Account doesn't delegate that method.
|
@flavorjones No migration script needed — there was no API for setting arbitrary entropy periods, so existing values should all be valid. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Entropy::AUTO_POSTPONE_PERIODS_IN_DAYSas the single source of truth for allowed entropy values and validateauto_postpone_periodagainst that allowlist (stored in seconds).auto_postpone_period_in_daysfor:BoardsController).Board#auto_postpone_period=seconds writer, but make it strict withupdate!; addBoard#auto_postpone_period_in_days=plus delegation forauto_postpone_period_in_days.entropy_auto_close_optionshelper.11as the last option.Tests
422 Unprocessable Entity, value unchanged).Entropy::AUTO_POSTPONE_PERIODS_IN_DAYSorder and that11is last.