Skip to content

Commit

Permalink
fix(editor): Fix local storage flags defaulting to undefined string (#…
Browse files Browse the repository at this point in the history
…7603)

useStorage takes the default value `undefined` and sets it in local
storage.. also returns "undefined" as string which breaks onboarding
flows

Github issue / Community forum post (link here to close automatically):
  • Loading branch information
mutdmour committed Nov 7, 2023
1 parent 78b84af commit 151e60f
Show file tree
Hide file tree
Showing 12 changed files with 97 additions and 42 deletions.
3 changes: 2 additions & 1 deletion packages/editor-ui/src/components/ActivationModal.vue
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import { getActivatableTriggerNodes, getTriggerNodeServiceName } from '@/utils';
import { useUIStore } from '@/stores/ui.store';
import { useWorkflowsStore } from '@/stores/workflows.store';
import { useNodeTypesStore } from '@/stores/nodeTypes.store';
import { useStorage } from '@/composables/useStorage';
export default defineComponent({
name: 'ActivationModal',
Expand Down Expand Up @@ -88,7 +89,7 @@ export default defineComponent({
},
handleCheckboxChange(checkboxValue: boolean) {
this.checked = checkboxValue;
window.localStorage.setItem(LOCAL_STORAGE_ACTIVATION_FLAG, checkboxValue.toString());
useStorage(LOCAL_STORAGE_ACTIVATION_FLAG).value = checkboxValue.toString();
},
},
computed: {
Expand Down
9 changes: 3 additions & 6 deletions packages/editor-ui/src/components/NDVDraggablePanels.vue
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ import { defineComponent } from 'vue';
import type { PropType } from 'vue';
import { mapStores } from 'pinia';
import { get } from 'lodash-es';
import { useStorage } from '@vueuse/core';
import { useStorage } from '@/composables/useStorage';
import type { INodeTypeDescription } from 'n8n-workflow';
import PanelDragButton from './PanelDragButton.vue';
Expand Down Expand Up @@ -348,7 +348,6 @@ export default defineComponent({
restorePositionData() {
const storedPanelWidthData = useStorage(
`${LOCAL_STORAGE_MAIN_PANEL_RELATIVE_WIDTH}_${this.currentNodePaneType}`,
undefined,
).value;
if (storedPanelWidthData) {
Expand All @@ -362,10 +361,8 @@ export default defineComponent({
return false;
},
storePositionData() {
window.localStorage.setItem(
`${LOCAL_STORAGE_MAIN_PANEL_RELATIVE_WIDTH}_${this.currentNodePaneType}`,
this.mainPanelDimensions.relativeWidth.toString(),
);
useStorage(`${LOCAL_STORAGE_MAIN_PANEL_RELATIVE_WIDTH}_${this.currentNodePaneType}`).value =
this.mainPanelDimensions.relativeWidth.toString();
},
onDragStart() {
this.isDragging = true;
Expand Down
9 changes: 3 additions & 6 deletions packages/editor-ui/src/components/Node.vue
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@
<script lang="ts">
import { defineComponent } from 'vue';
import { mapStores } from 'pinia';
import { useStorage } from '@vueuse/core';
import { useStorage } from '@/composables/useStorage';
import {
CUSTOM_API_CALL_KEY,
LOCAL_STORAGE_PIN_DATA_DISCOVERY_CANVAS_FLAG,
Expand Down Expand Up @@ -582,10 +582,7 @@ export default defineComponent({
},
},
created() {
const hasSeenPinDataTooltip = useStorage(
LOCAL_STORAGE_PIN_DATA_DISCOVERY_CANVAS_FLAG,
undefined,
).value;
const hasSeenPinDataTooltip = useStorage(LOCAL_STORAGE_PIN_DATA_DISCOVERY_CANVAS_FLAG).value;
if (!hasSeenPinDataTooltip) {
this.unwatchWorkflowDataItems = this.$watch('workflowDataItems', (dataItemsCount: number) => {
this.showPinDataDiscoveryTooltip(dataItemsCount);
Expand Down Expand Up @@ -626,7 +623,7 @@ export default defineComponent({
)
return;
localStorage.setItem(LOCAL_STORAGE_PIN_DATA_DISCOVERY_CANVAS_FLAG, 'true');
useStorage(LOCAL_STORAGE_PIN_DATA_DISCOVERY_CANVAS_FLAG).value = 'true';
this.pinDataDiscoveryTooltipVisible = true;
this.unwatchWorkflowDataItems();
Expand Down
11 changes: 4 additions & 7 deletions packages/editor-ui/src/components/RunData.vue
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,7 @@
import { defineAsyncComponent, defineComponent } from 'vue';
import type { PropType } from 'vue';
import { mapStores } from 'pinia';
import { useStorage } from '@vueuse/core';
import { useStorage } from '@/composables/useStorage';
import { saveAs } from 'file-saver';
import type {
ConnectionTypes,
Expand Down Expand Up @@ -940,10 +940,7 @@ export default defineComponent({
return;
}
const pinDataDiscoveryFlag = useStorage(
LOCAL_STORAGE_PIN_DATA_DISCOVERY_NDV_FLAG,
undefined,
).value;
const pinDataDiscoveryFlag = useStorage(LOCAL_STORAGE_PIN_DATA_DISCOVERY_NDV_FLAG).value;
if (value && value.length > 0 && !this.isReadOnlyRoute && !pinDataDiscoveryFlag) {
this.pinDataDiscoveryComplete();
Expand All @@ -963,8 +960,8 @@ export default defineComponent({
}
},
pinDataDiscoveryComplete() {
localStorage.setItem(LOCAL_STORAGE_PIN_DATA_DISCOVERY_NDV_FLAG, 'true');
localStorage.setItem(LOCAL_STORAGE_PIN_DATA_DISCOVERY_CANVAS_FLAG, 'true');
useStorage(LOCAL_STORAGE_PIN_DATA_DISCOVERY_NDV_FLAG).value = 'true';
useStorage(LOCAL_STORAGE_PIN_DATA_DISCOVERY_CANVAS_FLAG).value = 'true';
},
enterEditMode({ origin }: EnterEditModeArgs) {
const inputData = this.pinData
Expand Down
48 changes: 48 additions & 0 deletions packages/editor-ui/src/composables/useStorage.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { nextTick } from 'vue';
import { useStorage } from './useStorage';

describe('useStorage', () => {
beforeEach(() => {
localStorage.clear();
});

it('should initialize with null if no value is stored in localStorage', () => {
const key = 'test-key';
const data = useStorage(key);

expect(data.value).toBeNull();
});

it('should initialize with the stored value if it exists in localStorage', () => {
const key = 'test-key';
const value = 'test-value';
localStorage.setItem(key, value);

const data = useStorage(key);
expect(data.value).toBe(value);
});

it('should update localStorage when the data ref is updated', async () => {
const key = 'test-key';
const value = 'test-value';
const data = useStorage(key);

data.value = value;
await nextTick();

expect(localStorage.getItem(key)).toBe(value);
});

it('should remove the key from localStorage when the data ref is set to null', async () => {
const key = 'test-key';
const value = 'test-value';
localStorage.setItem(key, value);

const data = useStorage(key);

data.value = null;
await nextTick();

expect(localStorage.getItem(key)).toBeNull();
});
});
13 changes: 13 additions & 0 deletions packages/editor-ui/src/composables/useStorage.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { useStorage as useStorageComposable } from '@vueuse/core';
import type { Ref } from 'vue';

export function useStorage(key: string): Ref<string | null> {
const data = useStorageComposable(key, null, undefined, { writeDefaults: false });

// bug in 1.15.1
if (data.value === 'undefined') {
data.value = null;
}

return data;
}
7 changes: 2 additions & 5 deletions packages/editor-ui/src/mixins/workflowActivate.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { defineComponent } from 'vue';
import { mapStores } from 'pinia';
import { useStorage } from '@vueuse/core';
import { useStorage } from '@/composables/useStorage';

import { externalHooks } from '@/mixins/externalHooks';
import { workflowHelpers } from '@/mixins/workflowHelpers';
Expand Down Expand Up @@ -120,10 +120,7 @@ export const workflowActivate = defineComponent({
this.updatingWorkflowActivation = false;

if (isCurrentWorkflow) {
if (
newActiveState &&
useStorage(LOCAL_STORAGE_ACTIVATION_FLAG, undefined).value !== 'true'
) {
if (newActiveState && useStorage(LOCAL_STORAGE_ACTIVATION_FLAG).value !== 'true') {
this.uiStore.openModal(WORKFLOW_ACTIVE_MODAL_KEY);
} else {
await this.settingsStore.fetchPromptsData();
Expand Down
5 changes: 3 additions & 2 deletions packages/editor-ui/src/router.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { useStorage } from '@vueuse/core';
import { useStorage } from '@/composables/useStorage';

import type { RouteLocation, RouteRecordRaw } from 'vue-router';
import { createRouter, createWebHistory } from 'vue-router';
import type { IPermissions } from './Interface';
Expand Down Expand Up @@ -802,7 +803,7 @@ export const routes = [
role: [ROLE.Owner],
},
deny: {
shouldDeny: () => !useStorage('audit-logs', undefined).value,
shouldDeny: () => !useStorage('audit-logs').value,
},
},
},
Expand Down
13 changes: 7 additions & 6 deletions packages/editor-ui/src/stores/__tests__/posthog.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import { useSettingsStore } from '@/stores/settings.store';
import { useRootStore } from '@/stores/n8nRoot.store';
import { useTelemetryStore } from '@/stores/telemetry.store';
import type { IN8nUISettings } from 'n8n-workflow';
import { LOCAL_STORAGE_EXPERIMENT_OVERRIDES } from '@/constants';
import { nextTick } from 'vue';

const DEFAULT_POSTHOG_SETTINGS: IN8nUISettings['posthog'] = {
enabled: true,
Expand Down Expand Up @@ -55,7 +57,6 @@ function setup() {

vi.spyOn(window.posthog, 'init');
vi.spyOn(window.posthog, 'identify');
vi.spyOn(window.Storage.prototype, 'setItem');
vi.spyOn(telemetryStore, 'track');
}

Expand Down Expand Up @@ -117,7 +118,7 @@ describe('Posthog store', () => {
});
});

it('sets override feature flags', () => {
it('sets override feature flags', async () => {
const TEST = 'test';
const flags = {
[TEST]: 'variant',
Expand All @@ -126,17 +127,17 @@ describe('Posthog store', () => {
posthog.init(flags);

window.featureFlags?.override(TEST, 'override');
await nextTick();

expect(posthog.getVariant('test')).toEqual('override');
expect(window.posthog?.init).toHaveBeenCalled();
expect(window.localStorage.setItem).toHaveBeenCalledWith(
'N8N_EXPERIMENT_OVERRIDES',
expect(window.localStorage.getItem(LOCAL_STORAGE_EXPERIMENT_OVERRIDES)).toEqual(
JSON.stringify({ test: 'override' }),
);

window.featureFlags?.override('other_test', 'override');
expect(window.localStorage.setItem).toHaveBeenCalledWith(
'N8N_EXPERIMENT_OVERRIDES',
await nextTick();
expect(window.localStorage.getItem(LOCAL_STORAGE_EXPERIMENT_OVERRIDES)).toEqual(
JSON.stringify({ test: 'override', other_test: 'override' }),
);
});
Expand Down
6 changes: 3 additions & 3 deletions packages/editor-ui/src/stores/ndv.store.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useStorage } from '@vueuse/core';
import { useStorage } from '@/composables/useStorage';
import { LOCAL_STORAGE_MAPPING_IS_ONBOARDED, STORES } from '@/constants';
import type {
INodeUi,
Expand Down Expand Up @@ -49,7 +49,7 @@ export const useNDVStore = defineStore(STORES.NDV, {
canDrop: false,
stickyPosition: null,
},
isMappingOnboarded: useStorage(LOCAL_STORAGE_MAPPING_IS_ONBOARDED, undefined).value === 'true',
isMappingOnboarded: useStorage(LOCAL_STORAGE_MAPPING_IS_ONBOARDED).value === 'true',
}),
getters: {
activeNode(): INodeUi | null {
Expand Down Expand Up @@ -228,7 +228,7 @@ export const useNDVStore = defineStore(STORES.NDV, {
disableMappingHint(store = true) {
this.isMappingOnboarded = true;
if (store) {
window.localStorage.setItem(LOCAL_STORAGE_MAPPING_IS_ONBOARDED, 'true');
useStorage(LOCAL_STORAGE_MAPPING_IS_ONBOARDED).value = 'true';
}
},
updateNodeParameterIssues(issues: INodeIssues): void {
Expand Down
6 changes: 3 additions & 3 deletions packages/editor-ui/src/stores/posthog.store.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { Ref } from 'vue';
import { ref } from 'vue';
import { defineStore } from 'pinia';
import { useStorage } from '@vueuse/core';
import { useStorage } from '@/composables/useStorage';
import { useUsersStore } from '@/stores/users.store';
import { useRootStore } from '@/stores/n8nRoot.store';
import { useSettingsStore } from '@/stores/settings.store';
Expand Down Expand Up @@ -41,7 +41,7 @@ export const usePostHog = defineStore('posthog', () => {

if (!window.featureFlags) {
// for testing
const cachedOverrides = useStorage(LOCAL_STORAGE_EXPERIMENT_OVERRIDES, undefined).value;
const cachedOverrides = useStorage(LOCAL_STORAGE_EXPERIMENT_OVERRIDES).value;
if (cachedOverrides) {
try {
console.log('Overriding feature flags', cachedOverrides);
Expand All @@ -60,7 +60,7 @@ export const usePostHog = defineStore('posthog', () => {
[name]: value,
};
try {
localStorage.setItem(LOCAL_STORAGE_EXPERIMENT_OVERRIDES, JSON.stringify(overrides.value));
useStorage(LOCAL_STORAGE_EXPERIMENT_OVERRIDES).value = JSON.stringify(overrides.value);
} catch (e) {}
},

Expand Down
9 changes: 6 additions & 3 deletions packages/editor-ui/src/stores/ui.utils.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import type { AppliedThemeOption, ThemeOption } from '@/Interface';
import { useStorage } from '@/composables/useStorage';
import { LOCAL_STORAGE_THEME } from '@/constants';

const themeRef = useStorage(LOCAL_STORAGE_THEME);

export function addThemeToBody(theme: AppliedThemeOption) {
window.document.body.setAttribute('data-theme', theme);
}
Expand All @@ -11,7 +14,7 @@ export function isValidTheme(theme: string | null): theme is AppliedThemeOption

// query param allows overriding theme for demo view in preview iframe without flickering
export function getThemeOverride() {
return getQueryParam('theme') || localStorage.getItem(LOCAL_STORAGE_THEME);
return getQueryParam('theme') || themeRef.value;
}

function getQueryParam(paramName: string): string | null {
Expand All @@ -21,10 +24,10 @@ function getQueryParam(paramName: string): string | null {
export function updateTheme(theme: ThemeOption) {
if (theme === 'system') {
window.document.body.removeAttribute('data-theme');
localStorage.removeItem(LOCAL_STORAGE_THEME);
themeRef.value = null;
} else {
addThemeToBody(theme);
localStorage.setItem(LOCAL_STORAGE_THEME, theme);
themeRef.value = theme;
}
}

Expand Down

0 comments on commit 151e60f

Please sign in to comment.