-
Notifications
You must be signed in to change notification settings - Fork 365
Add support for browser.retain_editor_history setting #1419
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
Changes from 5 commits
7f8d9ea
e3e07a1
6be0961
13d485f
1b92b18
1f768e5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,6 +20,7 @@ | |
|
|
||
| import neo4j from 'neo4j-driver' | ||
| import Rx from 'rxjs/Rx' | ||
| import semver from 'semver' | ||
| import bolt from 'services/bolt/bolt' | ||
| import { isConfigValFalsy } from 'services/bolt/boltHelpers' | ||
| import { APP_START } from 'shared/modules/app/appDuck' | ||
|
|
@@ -59,6 +60,7 @@ import { | |
| isACausalCluster, | ||
| setClientConfig | ||
| } from '../features/featuresDuck' | ||
| import { clearHistory } from 'shared/modules/history/historyDuck' | ||
|
|
||
| export const NAME = 'meta' | ||
| export const UPDATE = 'meta/UPDATE' | ||
|
|
@@ -106,20 +108,26 @@ export const getStoreId = (state: any) => | |
|
|
||
| export const getAvailableSettings = (state: any) => | ||
| (state[NAME] || initialState).settings | ||
| export const allowOutgoingConnections = (state: any) => | ||
| export const getAllowOutgoingConnections = (state: any) => | ||
| getAvailableSettings(state)['browser.allow_outgoing_connections'] | ||
| export const credentialsTimeout = (state: any) => | ||
| getAvailableSettings(state)['browser.credential_timeout'] || 0 | ||
| export const getRemoteContentHostnameAllowlist = (state: any) => | ||
| getAvailableSettings(state)['browser.remote_content_hostname_allowlist'] | ||
| export const getDefaultRemoteContentHostnameAllowlist = (_state: any) => | ||
| initialState.settings['browser.remote_content_hostname_allowlist'] | ||
| export const shouldRetainConnectionCredentials = (state: any) => { | ||
| export const getRetainConnectionCredentials = (state: any) => { | ||
| const settings = getAvailableSettings(state) | ||
| const conf = settings['browser.retain_connection_credentials'] | ||
| if (conf === null || typeof conf === 'undefined') return false | ||
| return !isConfigValFalsy(conf) | ||
| } | ||
| export const getRetainEditorHistory = (state: any) => { | ||
| const settings = getAvailableSettings(state) | ||
| const conf = settings['browser.retain_editor_history'] | ||
| if (conf === null || typeof conf === 'undefined') return false | ||
| return !isConfigValFalsy(conf) | ||
| } | ||
| export const getDatabases = (state: any) => | ||
| (state[NAME] || initialState).databases | ||
| export const getActiveDbName = (state: any) => | ||
|
|
@@ -128,6 +136,23 @@ export const getActiveDbName = (state: any) => | |
| * Helpers | ||
| */ | ||
|
|
||
| export const VERSION_FOR_EDITOR_HISTORY_SETTING = '4.3.0' | ||
|
|
||
| export const versionHasEditorHistorySetting = (version: string) => | ||
| semver.gte(version, VERSION_FOR_EDITOR_HISTORY_SETTING) | ||
|
|
||
| export const supportsEditorHistorySetting = (state: any) => | ||
| isEnterprise(state) && versionHasEditorHistorySetting(getVersion(state)) | ||
|
|
||
| export const shouldAllowOutgoingConnections = (state: any) => | ||
| !isEnterprise(state) || getAllowOutgoingConnections(state) | ||
|
||
|
|
||
| export const shouldRetainConnectionCredentials = (state: any) => | ||
| !isEnterprise(state) || getRetainConnectionCredentials(state) | ||
OskarDamkjaer marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| export const shouldRetainEditorHistory = (state: any) => | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it's a nit but could we move this to by be the other history getters? (so to line 146). Feel free to disregard this comment, just find it slightly more readable |
||
| !supportsEditorHistorySetting(state) || getRetainEditorHistory(state) | ||
|
|
||
| function updateMetaForContext(state: any, meta: any, context: any) { | ||
| if (!meta || !meta.records || !meta.records.length) { | ||
| return { | ||
|
|
@@ -216,7 +241,8 @@ export const initialState = { | |
| settings: { | ||
| 'browser.allow_outgoing_connections': false, | ||
| 'browser.remote_content_hostname_allowlist': 'guides.neo4j.com, localhost', | ||
| 'browser.retain_connection_credentials': false | ||
| 'browser.retain_connection_credentials': false, | ||
| 'browser.retain_editor_history': false | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it not make sense to default this to true?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As that's the current behaviour?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Worth noting that the !isEnterprise stuff defaults to true since edition default/before connect is null
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps it's better to explicitly look for "community" rather than not enterprise?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The server does default this setting to true, but for safety it was decided to default to false on the client side. The settings are only ever loaded after initial discovery is done, so by that point we know the version/edition. Also, only enterprise supports the setting, so that's why I decided to code it as "not enterprise"
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Canny and Sentry both check allow shouldAllowOutgoing connections before connecting |
||
| } | ||
| } | ||
|
|
||
|
|
@@ -605,6 +631,13 @@ export const serverConfigEpic = (some$: any, store: any) => | |
| } | ||
| store.dispatch(setRetainCredentials(retainCredentials)) | ||
| value = retainCredentials | ||
| } else if (name === 'browser.retain_editor_history') { | ||
| let retainHistory = true | ||
OskarDamkjaer marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // Check if we should wipe user history from localstorage | ||
| if (typeof value !== 'undefined' && isConfigValFalsy(value)) { | ||
| retainHistory = false | ||
| } | ||
| value = retainHistory | ||
| } else if (name === 'browser.allow_outgoing_connections') { | ||
| // Use isConfigValFalsy to cast undefined to true | ||
| value = !isConfigValFalsy(value) | ||
|
|
@@ -623,6 +656,12 @@ export const serverConfigEpic = (some$: any, store: any) => | |
| updateUserCapability(USER_CAPABILITIES.serverConfigReadable, true) | ||
| ) | ||
| store.dispatch(updateSettings(settings)) | ||
| if ( | ||
| supportsEditorHistorySetting(store.getState()) && | ||
| !settings['browser.retain_editor_history'] | ||
| ) { | ||
| store.dispatch(clearHistory()) | ||
OskarDamkjaer marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| return Rx.Observable.of(null) | ||
| }) | ||
| }) | ||
|
|
||
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.
does this need to be exported