Skip to content

4473 post pr bugfix #5352

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.

Sign up for GitHub

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

Merged
awwaiid merged 2 commits into from
Sep 14, 2025
Merged

4473 post pr bugfix #5352

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1495b2b
Fixed deadline_date_controller incorrectly using RRule library, causi…
Benjamin-Couey Sep 1, 2025
15b7005
Added disclaimer clarifying that tests could change behavior based on…
Benjamin-Couey Sep 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions app/javascript/controllers/deadline_day_controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export default class extends Controller {

if (this.byDayOfMonthTarget.checked && this.dayOfMonthTarget.value) {
const rule = new RRule({
dtstart: today,
dtstart: new Date(Date.UTC(today.getFullYear(), today.getMonth(), today.getDate(), 12)),
freq: RRule.MONTHLY,
interval: monthlyInterval,
bymonthday: parseInt(this.dayOfMonthTarget.value),
Expand All @@ -57,7 +57,7 @@ export default class extends Controller {
}
if (this.byDayOfWeekTarget.checked && this.everyNthDayTarget.value && (this.dayOfWeekTarget.value)) {
const rule = new RRule({
dtstart: today,
dtstart: new Date(Date.UTC(today.getFullYear(), today.getMonth(), today.getDate(), 12)),
freq: RRule.MONTHLY,
interval: monthlyInterval,
byweekday: WEEKDAY_NUM_TO_OBJ[ parseInt(this.dayOfWeekTarget.value) ].nth( parseInt(this.everyNthDayTarget.value) ),
Expand Down
5 changes: 5 additions & 0 deletions spec/support/deadline_day_fields_shared_example.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,11 @@ def safe_subtract_days(date, num)
expect(page).to have_content("Deadline day must be between 1 and 28")
end

# These tests make assertions about the dates calculated by the javascript in deadline_day_controller.js
# Because we currently don't have a great way of spoofing the date the headless browser sees during tests, they
# cannot effectively set the date for the tests (like with travel_to) and have to make due with the actual datetime
# of whenever the test is run. To the best of my knowledge, the tests are sound but be aware that they could
# pass or fail based on when they are run!
Copy link
Copy Markdown
Collaborator

@dorner dorner Sep 2, 2025

Choose a reason for hiding this comment

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

Have you taken a look at this https://stackoverflow.com/a/59387506/5199431

or this? https://geoffharcourt.github.io/2021/04/11/mocking-time-in-capybara-spec.html

Copy link
Copy Markdown
Contributor Author

@Benjamin-Couey Benjamin-Couey Sep 3, 2025

Choose a reason for hiding this comment

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

I have not. I'll give them a look and see if I can apply them to the deadline_day_fields_shared_example.rb tests.

describe "reported reminder and deadline dates" do
context "when the reminder is a day of the month" do
before do
Expand Down