Skip to content

fix: use the constructor function every time a periodic job is scheduled, instead of when its registered - #420

Merged
brandur merged 2 commits into
riverqueue:masterfrom
elee1766:patch-2
Jul 4, 2024
Merged

fix: use the constructor function every time a periodic job is scheduled, instead of when its registered#420
brandur merged 2 commits into
riverqueue:masterfrom
elee1766:patch-2

Conversation

@elee1766

@elee1766 elee1766 commented Jul 4, 2024

Copy link
Copy Markdown
Contributor

right now, the constructor gets called when converting to an internal job, which causes the constructor to use the value at add time.

however, the comments say

// The constructor function is invoked each time a periodic job's schedule
// elapses, returning job arguments to insert along with optional insertion
// options.

so im assuming this is whats intended?

@elee1766 elee1766 changed the title Properly use the constructor function. use the constructor function every time a periodic job is scheduled, instead of when its registered Jul 4, 2024
@elee1766 elee1766 changed the title use the constructor function every time a periodic job is scheduled, instead of when its registered fix: use the constructor function every time a periodic job is scheduled, instead of when its registered Jul 4, 2024
@brandur

brandur commented Jul 4, 2024

Copy link
Copy Markdown
Contributor

Oops, yes, thanks! I'll try to add a test case to verify this.

@brandur
brandur merged commit d5b5960 into riverqueue:master Jul 4, 2024
brandur added a commit that referenced this pull request Jul 4, 2024
We haven't cut a new release in a while, and the bug in #420 is kind of
bad, so now that there's a fix it's not a bad time to cut a new release.

I'm also including a test case that catches the bug from #420.
Apparently none of our other test cases can catch the problem, so it's
good to have a regression guard.
brandur added a commit that referenced this pull request Jul 4, 2024
We haven't cut a new release in a while, and the bug in #420 is kind of
bad, so now that there's a fix it's not a bad time to cut a new release.

I'm also including a test case that catches the bug from #420.
Apparently none of our other test cases can catch the problem, so it's
good to have a regression guard.
brandur added a commit that referenced this pull request Jul 4, 2024
We haven't cut a new release in a while, and the bug in #420 is kind of
bad, so now that there's a fix it's not a bad time to cut a new release.

I'm also including a test case that catches the bug from #420.
Apparently none of our other test cases can catch the problem, so it's
good to have a regression guard.
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