-
Notifications
You must be signed in to change notification settings - Fork 11
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
Fix manual session control enablement #1408
base: main
Are you sure you want to change the base?
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Manifest Files |
@@ -111,7 +111,7 @@ internal class SessionOrchestratorImpl( | |||
payloadFactory.startSessionWithManual(timestamp) | |||
}, | |||
earlyTerminationCondition = { | |||
return@transitionState shouldEndManualSession( | |||
return@transitionState !shouldEndManualSession( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
earlyTerminationCondition
collects functions that prevent the execution.
*/ | ||
fun isSessionControlEnabled(): Boolean |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can keep this as enabled at this level (app usage) so it matches the other kill switches. It's at the Config level that we should make this be identify itself as disabled
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good other than flipping the method name in the Behavior back to isSessionControlEnabled()
to match the other kill switches.
@fractalwrench should look at the logic flip in OrchestratorImpl
. Maybe a method rename to blockManualSessionEnd()
while keep the true/false returns might read better?
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1408 +/- ##
==========================================
- Coverage 82.54% 82.45% -0.10%
==========================================
Files 476 477 +1
Lines 10942 10964 +22
Branches 1668 1668
==========================================
+ Hits 9032 9040 +8
- Misses 1184 1195 +11
- Partials 726 729 +3
|
This PR is stale because it has been open 20 days with no activity. Remove stale label or comment or this PR will be closed in 10 days. |
Goal
Have a kill switch to disable ending sessions manually. There was a confused implementation with the previous key.
Testing
Manual and Unit test.