Skip to content

Commit

Permalink
Merge branch 'master' into speed-up-opening-workflows
Browse files Browse the repository at this point in the history
* master:
  perf(editor): Improve node rendering performance when opening large workflows (#7904)
  docs: Create PR template (no-changelog) (#7902)
  fix(editor): Tune styles of template credential setup (no-changelog) (#7898)

# Conflicts:
#	packages/editor-ui/src/views/NodeView.vue
  • Loading branch information
MiloradFilipovic committed Dec 1, 2023
2 parents 2b62b75 + a8049a0 commit 43d17ac
Show file tree
Hide file tree
Showing 8 changed files with 165 additions and 20 deletions.
24 changes: 23 additions & 1 deletion .github/pull_request_template.md
Original file line number Diff line number Diff line change
@@ -1 +1,23 @@
Github issue / Community forum post (link here to close automatically):
## Summary
Provide details about your pull request and what it adds, fixes, or changes. Photos and videos are recommended.

...

#### How to test the change:
1. ...


## Issues fixed
Include links to Github issue or Community forum post or **Linear ticket**:
> Important in order to close automatically and provide context to reviewers
...


## Review / Merge checklist
- [ ] PR title and summary are descriptive. **Remember, the title automatically goes into the changelog. Use `(no-changelog)` otherwise.** ([conventions](https://github.com/n8n-io/n8n/blob/master/.github/pull_request_title_conventions.md))
- [ ] [Docs updated](https://github.com/n8n-io/n8n-docs) or follow-up ticket created.
- [ ] Tests included.
> A bug is not considered fixed, unless a test is added to prevent it from happening again. A feature is not complete without tests.
>
> *(internal)* You can use Slack commands to trigger [e2e tests](https://www.notion.so/n8n/How-to-use-Test-Instances-d65f49dfc51f441ea44367fb6f67eb0a?pvs=4#a39f9e5ba64a48b58a71d81c837e8227) or [deploy test instance](https://www.notion.so/n8n/How-to-use-Test-Instances-d65f49dfc51f441ea44367fb6f67eb0a?pvs=4#f6a177d32bde4b57ae2da0b8e454bfce) or [deploy early access version on Cloud](https://www.notion.so/n8n/Cloudbot-3dbe779836004972b7057bc989526998?pvs=4#fef2d36ab02247e1a0f65a74f6fb534e).
112 changes: 112 additions & 0 deletions .github/pull_request_title_conventions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
# PR Title Convention

We have very precise rules over how Pull Requests (to the `master` branch) must be formatted. This format basically follows the [Angular Commit Message Convention](https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit). It leads to easier to read commit history and allows for automated generation of release notes:

A PR title consists of these elements:

```
<type>(<scope>): <summary>
│ │ │
│ │ └─⫸ Summary: In imperative present tense.
| | Capitalized
| | No period at the end.
│ │
│ └─⫸ Scope: API|core|editor|* Node
└─⫸ Type: build|ci|docs|feat|fix|perf|refactor|test
```

- PR title
- type
- scope (*optional*)
- summary
- PR description
- body (optional)
- blank line
- footer (optional)

The structure looks like this:

### **Type**

Must be one of the following:

- `feat` - A new feature
- `fix` - A bug fix
- `perf` - A code change that improves performance
- `test` - Adding missing tests or correcting existing tests
- `docs` - Documentation only changes
- `refactor` - A code change that neither fixes a bug nor adds a feature
- `build` - Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm)
- `ci` - Changes to our CI configuration files and scripts (e.g. Github actions)

If the prefix is `feat``fix` or `perf`, it will appear in the changelog. However if there is any BREAKING CHANGE (see Footer section below), the commit will always appear in the changelog.

### **Scope (optional)**

The scope should specify the place of the commit change as long as the commit clearly addresses one of the following supported scopes. (Otherwise, omit the scope!)

- `API` - changes to the *public* API
- `core` - changes to the core / private API / backend of n8n
- `editor` - changes to the Editor UI
- `* Node` - changes to a specific node or trigger node (”`*`” to be replaced with the node name, not its display name), e.g.
- mattermost → Mattermost Node
- microsoftToDo → Microsoft To Do Node
- n8n → n8n Node

### **Summary**

The summary contains succinct description of the change:

- use the imperative, present tense: "change" not "changed" nor "changes"
- capitalize the first letter
- *no* dot (.) at the end
- do *not* include Linear ticket IDs etc. (e.g. N8N-1234)
- suffix with “(no-changelog)” for commits / PRs that should not get mentioned in the changelog.

### **Body (optional)**

Just as in the **summary**, use the imperative, present tense: "change" not "changed" nor "changes". The body should include the motivation for the change and contrast this with previous behavior.

### **Footer (optional)**

The footer can contain information about breaking changes and deprecations and is also the place to [reference GitHub issues](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword), Linear tickets, and other PRs that this commit closes or is related to. For example:

```
BREAKING CHANGE: <breaking change summary>
<BLANK LINE>
<breaking change description + migration instructions>
<BLANK LINE>
<BLANK LINE>
Fixes #<issue number>
```

or

```
DEPRECATED: <what is deprecated>
<BLANK LINE>
<deprecation description + recommended update path>
<BLANK LINE>
<BLANK LINE>
Closes #<pr number>
```

A Breaking Change section should start with the phrase "`BREAKING CHANGE:` " followed by a summary of the breaking change, a blank line, and a detailed description of the breaking change that also includes migration instructions.

> 💡 A breaking change can additionally also be marked by adding a “`!`” to the header, right before the “`:`”, e.g. `feat(editor)!: Remove support for dark mode`
>
> This makes locating breaking changes easier when just skimming through commit messages.
> 💡 The breaking changes must also be added to the [packages/cli/BREAKING-CHANGES.md](https://github.com/n8n-io/n8n/blob/master/packages/cli/BREAKING-CHANGES.md) file located in the n8n repository.
Similarly, a Deprecation section should start with "`DEPRECATED:` " followed by a short description of what is deprecated, a blank line, and a detailed description of the deprecation that also mentions the recommended update path.

### **Revert commits**

If the commit reverts a previous commit, it should begin with `revert:` , followed by the header of the reverted commit.

The content of the commit message body should contain:

- information about the SHA of the commit being reverted in the following format: `This reverts commit <SHA>`,
- a clear description of the reason for reverting the commit message.
16 changes: 5 additions & 11 deletions packages/editor-ui/src/components/Node.vue
Original file line number Diff line number Diff line change
Expand Up @@ -286,15 +286,11 @@ export default defineComponent({
return this.data.type === MANUAL_TRIGGER_NODE_TYPE;
},
isConfigNode(): boolean {
return this.nodeTypesStore.isConfigNode(
this.getCurrentWorkflow(),
this.data,
this.data?.type ?? '',
);
return this.nodeTypesStore.isConfigNode(this.workflow, this.data, this.data?.type ?? '');
},
isConfigurableNode(): boolean {
return this.nodeTypesStore.isConfigurableNode(
this.getCurrentWorkflow(),
this.workflow,
this.data,
this.data?.type ?? '',
);
Expand Down Expand Up @@ -349,9 +345,8 @@ export default defineComponent({
};
if (this.node && this.nodeType) {
const workflow = this.workflowsStore.getCurrentWorkflow();
const inputs =
NodeHelpers.getNodeInputs(workflow, this.node, this.nodeType) ||
NodeHelpers.getNodeInputs(this.workflow, this.node, this.nodeType) ||
([] as Array<ConnectionTypes | INodeInputConfiguration>);
const inputTypes = NodeHelpers.getConnectionTypes(inputs);
Expand All @@ -372,7 +367,7 @@ export default defineComponent({
}
const outputs =
NodeHelpers.getNodeOutputs(workflow, this.node, this.nodeType) ||
NodeHelpers.getNodeOutputs(this.workflow, this.node, this.nodeType) ||
([] as Array<ConnectionTypes | INodeOutputConfiguration>);
const outputTypes = NodeHelpers.getConnectionTypes(outputs);
Expand Down Expand Up @@ -634,8 +629,7 @@ export default defineComponent({
// and ends up bogging down the UI with big workflows, for example when pasting a workflow or even opening a node...
// so we only update it when necessary (when node is mounted and when it's opened and closed (isActive))
try {
const nodeSubtitle =
this.getNodeSubtitle(this.data, this.nodeType, this.getCurrentWorkflow()) || '';
const nodeSubtitle = this.getNodeSubtitle(this.data, this.nodeType, this.workflow) || '';
this.nodeSubtitle = nodeSubtitle.includes(CUSTOM_API_CALL_KEY) ? '' : nodeSubtitle;
} catch (e) {
Expand Down
11 changes: 7 additions & 4 deletions packages/editor-ui/src/mixins/nodeBase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import type {
INodeInputConfiguration,
INodeTypeDescription,
INodeOutputConfiguration,
Workflow,
} from 'n8n-workflow';
import { useUIStore } from '@/stores/ui.store';
import { useWorkflowsStore } from '@/stores/workflows.store';
Expand Down Expand Up @@ -104,6 +105,10 @@ export const nodeBase = defineComponent({
showCustomTooltip: {
type: Boolean,
},
workflow: {
type: Object as () => Workflow,
required: true,
},
},
methods: {
__addEndpointTestingData(endpoint: Endpoint, type: string, inputIndex: number) {
Expand All @@ -123,9 +128,8 @@ export const nodeBase = defineComponent({
[key: string]: number;
} = {};

const workflow = this.workflowsStore.getCurrentWorkflow();
const inputs: Array<ConnectionTypes | INodeInputConfiguration> =
NodeHelpers.getNodeInputs(workflow, this.data!, nodeTypeData) || [];
NodeHelpers.getNodeInputs(this.workflow, this.data!, nodeTypeData) || [];
this.inputs = inputs;

const sortedInputs = [...inputs];
Expand Down Expand Up @@ -338,8 +342,7 @@ export const nodeBase = defineComponent({
[key: string]: number;
} = {};

const workflow = this.workflowsStore.getCurrentWorkflow();
this.outputs = NodeHelpers.getNodeOutputs(workflow, this.data, nodeTypeData) || [];
this.outputs = NodeHelpers.getNodeOutputs(this.workflow, this.data, nodeTypeData) || [];

// TODO: There are still a lot of references of "main" in NodesView and
// other locations. So assume there will be more problems
Expand Down
4 changes: 4 additions & 0 deletions packages/editor-ui/src/views/NodeView.vue
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
:isActive="!!activeNode && activeNode.name === nodeData.name"
:hideActions="pullConnActive"
:isProductionExecutionPreview="isProductionExecutionPreview"
:workflow="currentWorkflowObject"
>
<template #custom-tooltip>
<span
Expand Down Expand Up @@ -694,6 +695,9 @@ export default defineComponent({
isLoading(): boolean {
return this.loadingService !== null;
},
currentWorkflowObject(): Workflow {
return this.workflowsStore.getCurrentWorkflow();
},
},
data() {
return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,15 @@ const appNodeCounts = computed(() => {
</script>

<template>
<n8n-notice theme="info">
<n8n-notice :class="$style.notice" theme="info">
<i18n-t tag="span" keypath="templateSetup.instructions" scope="global">
<span v-html="appNodeCounts" />
</i18n-t>
</n8n-notice>
</template>

<style lang="scss" module>
.notice {
margin-top: 0;
}
</style>
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ const onCredentialDeselected = () => {
.heading {
display: flex;
align-items: center;
margin-bottom: var(--spacing-xs);
margin-bottom: var(--spacing-2xs);
}
.headingOrder {
Expand All @@ -132,6 +132,8 @@ const onCredentialDeselected = () => {
.description {
margin-bottom: var(--spacing-l);
font-size: var(--font-size-s);
color: var(--color-text-base);
}
.credentials {
Expand All @@ -146,6 +148,7 @@ const onCredentialDeselected = () => {
.credentialOk {
margin-left: var(--spacing-2xs);
font-size: 24px;
}
.invisible {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ onMounted(async () => {
:disabled="setupTemplateStore.numCredentialsLeft === 0"
>
<n8n-button
size="large"
:label="$locale.baseText('templateSetup.continue.button')"
:disabled="setupTemplateStore.numCredentialsLeft > 0 || setupTemplateStore.isSaving"
@click="setupTemplateStore.createWorkflow($router)"
Expand All @@ -163,7 +164,7 @@ onMounted(async () => {
.grid {
display: grid;
grid-template-columns: repeat(12, 1fr);
padding: var(--spacing-l) var(--spacing-l) 0;
padding: 0 var(--spacing-l);
justify-content: center;
}
Expand Down Expand Up @@ -191,7 +192,7 @@ onMounted(async () => {
.appCredential:not(:last-of-type) {
padding-bottom: var(--spacing-2xl);
border-bottom: 1px solid var(--prim-gray-540);
border-bottom: 1px solid var(--color-foreground-light);
}
.actions {
Expand Down

0 comments on commit 43d17ac

Please sign in to comment.