fix(session): Update session state transition trigger #2046
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mgaffney
reviewed
Apr 29, 2022
internal/db/schema/migrations/oss/postgres/28/01_prior_session_trigger.up.sql
Outdated
Show resolved
Hide resolved
mgaffney
requested changes
May 2, 2022
Comment on lines
102
to
103
| new.prior_state = query.state from( | ||
| select state from session_state where session_id=new.session_id order by start_time desc limit 1) as query; |
There was a problem hiding this comment.
Can this be incorporated into the update statement using a returning clause?
Comment on lines
105
to
107
| if new.prior_state is null then | ||
| new.prior_state='pending'; | ||
| end if; |
There was a problem hiding this comment.
Can this be incorporated into the if not found then block starting on line 91?
| drop function insert_session_state(); | ||
|
|
||
| create function | ||
| insert_session_state() |
There was a problem hiding this comment.
Do we have unit tests to cover this?
mgaffney
previously approved these changes
May 2, 2022
tmessi
requested changes
May 2, 2022
internal/db/schema/migrations/oss/postgres/28/01_prior_session_trigger.up.sql
Outdated
Show resolved
Hide resolved
internal/db/schema/migrations/oss/postgres/28/01_prior_session_trigger.up.sql
Outdated
Show resolved
Hide resolved
irenarindos
force-pushed
the
irindos-session-transition-trigger
branch
from
fe42798
to
a77ee04
Compare
tmessi
reviewed
May 2, 2022
| @@ -0,0 +1,113 @@ | |||
| begin; | |||
There was a problem hiding this comment.
I think this should still be in 28, but should be 28/02
tmessi
previously approved these changes
May 2, 2022
irenarindos
force-pushed
the
irindos-session-transition-trigger
branch
from
b0b1ade
to
c6764e8
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #2040
Session state trigger is updated to set prior state to the most recent session state.
In the prior version of the trigger, prior_state was set to the last most recent state that represented a valid state transition for the inserted state (Ex: A session could go through pending, active, terminated, and then canceling would be inserted with active as prior)
With this change, canceling will fail on insert as terminated-> canceling is an invalid state transition