Skip to content

Commit

Permalink
Merge branch 'master' into ADO-814-trial-banner
Browse files Browse the repository at this point in the history
* master:
  fix(editor): Make Source control branch select required (#6619)
  refactor(core): Load `cookieParser` middleware only once (no-changelog) (#6614)
  fix(editor): Prevent keyboard shortcuts to edit workflows in readonly mode (#6613)
  fix(editor): Show appropriate empty workflow list content when instance environment is readonly (#6610)
  ci: Fix linting issues (no-changelog)
  • Loading branch information
MiloradFilipovic committed Jul 7, 2023
2 parents b8e00c0 + 20737b5 commit 44bf3bc
Show file tree
Hide file tree
Showing 14 changed files with 100 additions and 74 deletions.
2 changes: 1 addition & 1 deletion packages/cli/src/commands/Interfaces.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ interface IExecutionResult {
finished: boolean;
executionStatus: ExecutionStatus;
error?: string;
changes?: string;
changes?: object;
coveredNodes: {
[nodeType: string]: number;
};
Expand Down
10 changes: 5 additions & 5 deletions packages/cli/src/commands/executeBatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
import fs from 'fs';
import os from 'os';
import { flags } from '@oclif/command';
import type { ITaskData } from 'n8n-workflow';
import { sleep } from 'n8n-workflow';
import type { IRun, ITaskData } from 'n8n-workflow';
import { jsonParse, sleep } from 'n8n-workflow';
import { sep } from 'path';
import { diff } from 'json-diff';
import pick from 'lodash/pick';
Expand Down Expand Up @@ -778,9 +778,9 @@ export class ExecuteBatch extends BaseCommand {
}${workflowData.id}-snapshot.json`;
if (fs.existsSync(fileName)) {
const contents = fs.readFileSync(fileName, { encoding: 'utf-8' });
const expected = JSON.parse(contents);
const recieved = JSON.parse(serializedData);
const changes = diff(expected, recieved, { keysOnly: true });
const expected = jsonParse<IRun>(contents);
const received = jsonParse<IRun>(serializedData);
const changes = diff(expected, received, { keysOnly: true }) as object;

if (changes !== undefined) {
// If we had only additions with no removals
Expand Down
3 changes: 0 additions & 3 deletions packages/cli/src/middlewares/auth.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import type { Application, NextFunction, Request, RequestHandler, Response } from 'express';
import jwt from 'jsonwebtoken';
import cookieParser from 'cookie-parser';
import passport from 'passport';
import { Strategy } from 'passport-jwt';
import { sync as globSync } from 'fast-glob';
Expand Down Expand Up @@ -78,8 +77,6 @@ export const setupAuthMiddlewares = (
ignoredEndpoints: Readonly<string[]>,
restEndpoint: string,
) => {
// needed for testing; not adding overhead since it directly returns if req.cookies exists
app.use(cookieParser());
app.use(userManagementJwtAuth());

app.use(async (req: Request, res: Response, next: NextFunction) => {
Expand Down
2 changes: 2 additions & 0 deletions packages/cli/test/integration/shared/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { Container } from 'typedi';
import { randomBytes } from 'crypto';
import { existsSync } from 'fs';

import cookieParser from 'cookie-parser';
import bodyParser from 'body-parser';
import { CronJob } from 'cron';
import express from 'express';
Expand Down Expand Up @@ -121,6 +122,7 @@ export async function initTestServer({

testServer.app.use(bodyParser.json());
testServer.app.use(bodyParser.urlencoded({ extended: true }));
testServer.app.use(cookieParser());

config.set('userManagement.jwtSecret', 'My JWT secret');
config.set('userManagement.isInstanceOwnerSetUp', false);
Expand Down
5 changes: 1 addition & 4 deletions packages/editor-ui/src/App.vue
Original file line number Diff line number Diff line change
Expand Up @@ -251,10 +251,7 @@ export default defineComponent({
await this.initBanners();
});
if (
this.sourceControlStore.isEnterpriseSourceControlEnabled &&
this.usersStore.isInstanceOwner
) {
if (this.sourceControlStore.isEnterpriseSourceControlEnabled) {
await this.sourceControlStore.getPreferences();
}
Expand Down
5 changes: 0 additions & 5 deletions packages/editor-ui/src/components/WorkflowSettings.vue
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,6 @@ import {
useRootStore,
useWorkflowsEEStore,
useUsersStore,
useSourceControlStore,
} from '@/stores';
import { createEventBus } from 'n8n-design-system';
Expand Down Expand Up @@ -469,7 +468,6 @@ export default defineComponent({
useSettingsStore,
useWorkflowsStore,
useWorkflowsEEStore,
useSourceControlStore,
),
workflowName(): string {
return this.workflowsStore.workflowName;
Expand All @@ -493,9 +491,6 @@ export default defineComponent({
return this.workflowsEEStore.getWorkflowOwnerName(`${this.workflowId}`, fallback);
},
readOnlyEnv(): boolean {
return this.sourceControlStore.preferences.branchReadOnly;
},
},
async mounted() {
this.executionTimeout = this.rootStore.executionTimeout;
Expand Down
22 changes: 6 additions & 16 deletions packages/editor-ui/src/mixins/genericHelpers.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { defineComponent } from 'vue';
import { mapStores } from 'pinia';
import dateformat from 'dateformat';

import { VIEWS } from '@/constants';
import { useToast } from '@/composables';
import { useSourceControlStore } from '@/stores';

export const genericHelpers = defineComponent({
setup() {
Expand All @@ -17,11 +18,15 @@ export const genericHelpers = defineComponent({
};
},
computed: {
...mapStores(useSourceControlStore),
isReadOnlyRoute(): boolean {
return ![VIEWS.WORKFLOW, VIEWS.NEW_WORKFLOW, VIEWS.LOG_STREAMING_SETTINGS].includes(
this.$route.name as VIEWS,
);
},
readOnlyEnv(): boolean {
return this.sourceControlStore.preferences.branchReadOnly;
},
},
methods: {
displayTimer(msPassed: number, showMs = false): string {
Expand Down Expand Up @@ -49,21 +54,6 @@ export const genericHelpers = defineComponent({
const [date, time] = formattedDate.split('#');
return { date, time };
},
editAllowedCheck(): boolean {
if (this.isReadOnlyRoute) {
this.showMessage({
// title: 'Workflow can not be changed!',
title: this.$locale.baseText('genericHelpers.showMessage.title'),
message: this.$locale.baseText('genericHelpers.showMessage.message'),
type: 'info',
duration: 0,
dangerouslyUseHTMLString: true,
});

return false;
}
return true;
},

/**
* @note Loading helpers extracted as composable in useLoadingService
Expand Down
1 change: 1 addition & 0 deletions packages/editor-ui/src/plugins/i18n/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -1684,6 +1684,7 @@
"workflows.empty.heading": "👋 Welcome {name}!",
"workflows.empty.heading.userNotSetup": "👋 Welcome!",
"workflows.empty.description": "Create your first workflow",
"workflows.empty.description.readOnlyEnv": "No workflows here yet",
"workflows.empty.startFromScratch": "Start from scratch",
"workflows.shareModal.title": "Share '{name}'",
"workflows.shareModal.select.placeholder": "Add users...",
Expand Down
47 changes: 33 additions & 14 deletions packages/editor-ui/src/views/NodeView.vue
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@
@addNode="onAddNode"
/>
<canvas-controls />
<div class="workflow-execute-wrapper" v-if="!isReadOnlyRoute">
<div class="workflow-execute-wrapper" v-if="!isReadOnlyRoute && !readOnlyEnv">
<span
@mouseenter="showTriggerMissingToltip(true)"
@mouseleave="showTriggerMissingToltip(false)"
Expand Down Expand Up @@ -149,7 +149,13 @@
/>

<n8n-icon-button
v-if="!isReadOnlyRoute && workflowExecution && !workflowRunning && !allTriggersDisabled"
v-if="
!isReadOnlyRoute &&
!readOnlyEnv &&
workflowExecution &&
!workflowRunning &&
!allTriggersDisabled
"
:title="$locale.baseText('nodeView.deletesTheCurrentExecutionData')"
icon="trash"
size="large"
Expand Down Expand Up @@ -281,7 +287,6 @@ import {
useSettingsStore,
useUIStore,
useHistoryStore,
useSourceControlStore,
} from '@/stores';
import * as NodeViewUtils from '@/utils/nodeViewUtils';
import { getAccountAge, getConnectionInfo, getNodeViewTab } from '@/utils';
Expand Down Expand Up @@ -483,11 +488,7 @@ export default defineComponent({
useEnvironmentsStore,
useWorkflowsEEStore,
useHistoryStore,
useSourceControlStore,
),
readOnlyEnv(): boolean {
return this.sourceControlStore.preferences.branchReadOnly;
},
nativelyNumberSuffixedDefaults(): string[] {
return this.rootStore.nativelyNumberSuffixedDefaults;
},
Expand Down Expand Up @@ -635,6 +636,21 @@ export default defineComponent({
this.unregisterCustomAction('showNodeCreator');
},
methods: {
editAllowedCheck(): boolean {
if (this.isReadOnlyRoute || this.readOnlyEnv) {
this.showMessage({
// title: 'Workflow can not be changed!',
title: this.$locale.baseText('genericHelpers.showMessage.title'),
message: this.$locale.baseText('genericHelpers.showMessage.message'),
type: 'info',
duration: 0,
dangerouslyUseHTMLString: true,
});
return false;
}
return true;
},
showTriggerMissingToltip(isVisible: boolean) {
this.showTriggerMissingTooltip = isVisible;
},
Expand Down Expand Up @@ -961,7 +977,7 @@ export default defineComponent({
e.stopPropagation();
e.preventDefault();
if (this.isReadOnlyRoute) {
if (this.isReadOnlyRoute || this.readOnlyEnv) {
return;
}
Expand Down Expand Up @@ -1015,13 +1031,13 @@ export default defineComponent({
} else if (e.key === 'Tab') {
this.onToggleNodeCreator({
source: NODE_CREATOR_OPEN_SOURCES.TAB,
createNodeActive: !this.createNodeActive && !this.isReadOnlyRoute,
createNodeActive: !this.createNodeActive && !this.isReadOnlyRoute && !this.readOnlyEnv,
});
} else if (e.key === this.controlKeyCode) {
this.ctrlKeyPressed = true;
} else if (e.key === ' ') {
this.moveCanvasKeyPressed = true;
} else if (e.key === 'F2' && !this.isReadOnlyRoute) {
} else if (e.key === 'F2' && !this.isReadOnlyRoute && !this.readOnlyEnv) {
const lastSelectedNode = this.lastSelectedNode;
if (lastSelectedNode !== null && lastSelectedNode.type !== STICKY_NODE_TYPE) {
void this.callDebounced(
Expand Down Expand Up @@ -1067,7 +1083,10 @@ export default defineComponent({
const lastSelectedNode = this.lastSelectedNode;
if (lastSelectedNode !== null) {
if (lastSelectedNode.type === STICKY_NODE_TYPE && this.isReadOnlyRoute) {
if (
lastSelectedNode.type === STICKY_NODE_TYPE &&
(this.isReadOnlyRoute || this.readOnlyEnv)
) {
return;
}
this.ndvStore.activeNodeName = lastSelectedNode.name;
Expand Down Expand Up @@ -1307,7 +1326,7 @@ export default defineComponent({
},
cutSelectedNodes() {
const deleteCopiedNodes = !this.isReadOnlyRoute;
const deleteCopiedNodes = !this.isReadOnlyRoute && !this.readOnlyEnv;
this.copySelectedNodes(deleteCopiedNodes);
if (deleteCopiedNodes) {
this.deleteSelectedNodes();
Expand Down Expand Up @@ -2162,7 +2181,7 @@ export default defineComponent({
if (!this.suspendRecordingDetachedConnections) {
this.historyStore.pushCommandToUndo(new AddConnectionCommand(connectionData));
}
if (!this.isReadOnlyRoute) {
if (!this.isReadOnlyRoute && !this.readOnlyEnv) {
NodeViewUtils.addConnectionActionsOverlay(
info.connection,
() => {
Expand Down Expand Up @@ -2609,7 +2628,7 @@ export default defineComponent({
// Create connections in DOM
this.instance?.connect({
uuids: uuid,
detachable: !this.isReadOnlyRoute,
detachable: !this.isReadOnlyRoute && !this.readOnlyEnv,
});
setTimeout(() => {
Expand Down
49 changes: 35 additions & 14 deletions packages/editor-ui/src/views/SettingsSourceControl.vue
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ const sourceControlDocsSetupUrl = computed(() =>
locale.baseText('settings.sourceControl.docs.setup.url'),
);
const isConnected = ref(false);
const branchNameOptions = computed(() =>
sourceControlStore.preferences.branches.map((branch) => ({
value: branch,
label: branch,
})),
);
const onConnect = async () => {
loadingService.startLoading();
Expand Down Expand Up @@ -106,6 +112,7 @@ const formValidationStatus = reactive<Record<string, boolean>>({
repoUrl: false,
authorName: false,
authorEmail: false,
branchName: false,
});
function onValidate(key: string, value: boolean) {
Expand Down Expand Up @@ -136,6 +143,8 @@ const authorEmailValidationRules: Array<Rule | RuleGroup> = [
},
];
const branchNameValidationRules: Array<Rule | RuleGroup> = [{ name: 'REQUIRED' }];
const validForConnection = computed(
() =>
formValidationStatus.repoUrl &&
Expand Down Expand Up @@ -216,7 +225,7 @@ const refreshBranches = async () => {
@validate="(value) => onValidate('repoUrl', value)"
/>
<n8n-button
class="ml-2xs"
:class="$style.disconnectButton"
type="tertiary"
v-if="isConnected"
@click="onDisconnect"
Expand Down Expand Up @@ -303,21 +312,20 @@ const refreshBranches = async () => {
}}</n8n-heading>
<label>{{ locale.baseText('settings.sourceControl.branches') }}</label>
<div :class="$style.branchSelection">
<n8n-select
:value="sourceControlStore.preferences.branchName"
<n8n-form-input
label
type="select"
id="branchName"
name="branchName"
class="mb-s"
size="medium"
filterable
@input="onSelect"
data-test-id="source-control-branch-select"
>
<n8n-option
v-for="b in sourceControlStore.preferences.branches"
:key="b"
:value="b"
:label="b"
/>
</n8n-select>
validateOnBlur
:validationRules="branchNameValidationRules"
:options="branchNameOptions"
:value="sourceControlStore.preferences.branchName"
@validate="(value) => onValidate('branchName', value)"
@input="onSelect"
/>
<n8n-tooltip placement="top">
<template #content>
<span>
Expand Down Expand Up @@ -441,6 +449,11 @@ const refreshBranches = async () => {
margin: calc(var(--spacing-2xs) * -1) 0 var(--spacing-2xs);
}
.disconnectButton {
margin: 0 0 0 var(--spacing-2xs);
height: 40px;
}
.actionBox {
margin: var(--spacing-2xl) 0 0;
}
Expand All @@ -462,6 +475,14 @@ const refreshBranches = async () => {
.branchSelection {
display: flex;
> div:first-child {
flex: 1;
input {
height: 36px;
}
}
button.refreshBranches {
height: 36px;
width: 36px;
Expand Down
Loading

0 comments on commit 44bf3bc

Please sign in to comment.