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

add optional anchor time to runs [#186531175] #625

Merged
merged 3 commits into from
Dec 4, 2023

Conversation

uraniumanchor
Copy link
Collaborator

Contributing to the Donation Tracker

  • I've added tests or modified existing tests for the change.
  • I've humanly end-to-end tested the change by running an instance of the tracker.

Issue from Pivotal Tracker

https://www.pivotaltracker.com/story/show/186531175

Description of the Change

This set of changes allows you to set a fixed start time for a given run. It will adjust the setup_time for the immediately preceding run to make sure that the start time is always where you set, or if it is not possible to do so, model validation will fail and whatever change you're trying to perform will not actually occur.

There's several touchpoints where this validation needs to kick in, for example moving a run required some changes because of new validation requirements. While looking for edge cases I discovered one that I'm not sure can easily be reconciled, so for now, you will get an error if you try and move a run directly before an anchored run (because the flex block is suddenly in a different place and I couldn't come up with a good answer for what to do with the old flex block). If you find yourself needing to do this for some reason, the only solution is to remove the anchor time before performing the move. It's possible I'll make this a more explicit error case in the future, but for now it will just fail because the current logic cannot come up with a solution, so the move is prevented.

As part of this, I had to touch up the current schedule editor to display errors that could never occur before, but it was relatively simple to do so.

Verification Process

Attempting to edit runs by hand in the admin, or via the schedule editor, ensuring that flex blocks would either error or end up with the expected values.

faultyserver
faultyserver previously approved these changes Dec 4, 2023
Copy link
Member

@faultyserver faultyserver left a comment

Choose a reason for hiding this comment

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

lgtm overall. nothing stands out. glad to have the ms fields on runs now, too lol

@uraniumanchor uraniumanchor added this pull request to the merge queue Dec 4, 2023
Merged via the queue into master with commit 9614a60 Dec 4, 2023
15 checks passed
@uraniumanchor uraniumanchor deleted the 186531175-time-anchor branch December 4, 2023 22:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants