-
Notifications
You must be signed in to change notification settings - Fork 47.3k
Comparing changes
Open a pull request
base repository: n8n-io/n8n
base: [email protected]
head repository: n8n-io/n8n
compare: [email protected]
- 12 commits
- 31 files changed
- 9 contributors
Commits on Oct 23, 2023
-
Configuration menu - View commit details
-
Copy full SHA for 60ca02e - Browse repository at this point
Copy the full SHA 60ca02eView commit details -
fix(editor): Open only one tab with plans page (#7377)
## Issue In community edition, clicking on "View plans" button on "Settings" -> "Usage and plan" page (e.g. http://127.0.0.1:5678/settings/usage) opens two new tabs with n8n pricing (one of them with UTM tracking, another without). This was introduced in #6317 , when click handler of "View plans" link container [started calling](https://github.com/n8n-io/n8n/pull/6317/files#diff-0bf26afac8a06e03b3d39d0668f22408859355b585a9ab420800c125e33f0691R109) `uiStore.goToUpgrade(...)` which opens n8n pricing in a new tab, while browser opens another tab for the link URL. The simplest fix, implemented in this PR, is to prevent default event handling (so that, after `onViewPlans` is called, browser will not attempt to process the click additionally as clicking on the link), similarly to how it is prevented on some other pages. It only solves the immediate problem of browser opening two new tabs on clicking "View plans". Note that **I didn't implement any tests for the changed behavior**, because it was not covered by tests before, and I couldn't quite figure out how to cover it now within the existing test approach (considering that testing the fact that only one new tab is open will likely require to write entirely new tests relying on puppeteer; as far as I can see, no existing `editor-ui` tests are doing anything like that). I'll gladly implement tests for the new behavior if you tell me how you would like them to look. The existing tests for `editor-ui` still pass; I didn't run tests for other subpackages (see "additional contribution notes" below). ## Additional notes on the issue. I'm not sure that the change in this PR is the correct long-term solution for the issue, because the URLs for these two methods (custom click handler for link container and default link handling) are slightly different: * Custom click handler calls `useTelemetryStore().track('User clicked upgrade CTA', ...)`; then calls `sendUsageTelemetry('view_plans')` (it feels weird that two calls to telemetry are made); then opens new tab for `https://n8n.io/pricing?utm_campaign=open&source=usage_page` (note that prior to #7316 the second call to telemetry was done after the new tab is opened, not before); * Link itself refers to another page, with slightly different tracking parameters: `https://subscription.n8n.io/?instanceid=[REDACTED]&version=1.10.0&callback=http%3A%2F%2F127.0.0.1%3A5678%2Fsettings%2Fusage&source=usage_page`; but this page redirects to `https://n8n.io/pricing/`. It is not clear which one of the two is the right way of doing things. Although `goToUpgrade` is called in 20 places throughout `editor-ui`, while `viewPlansUrl`, as far as I can see, is used for this button only. Additionally, since Settings pages don't work without JS anyway, I can only think of two separate scenarios where any tab would be opened: * Left-clicking the link (or Ctrl-clicking, or pressing Space or Enter when the link is focused, or tapping): previously, both custom click handler was executed and link's `href` was opened; in this PR, only custom click handler is executed (similarly to how it is done in the other places where `goToUpgrade` is called); * Right-clicking (or long tapping, or opening context menu in any other way) and selecting "open link in new tab" (or similar): opens a new tab for URL from the `href` attribute (and does not send any telemetry at all). I'd say that the better permanent solution would probably be to get rid of one of these methods entirely, and only rely on another in all cases (for me, as an outside contributor, the preferred way would be for custom click handler to only send telemetry, while letting my browser handle the actual navigation). However, that would be a large change, much more than one line in this PR. Additionally, other similar places where `goToUpgrade` is currently called (directly or indirectly) would also need to be adapted for this change. ## Additional contribution notes As a first-time contributor, I've encountered several things I didn't expect; I'm not sure if they should be expected or are issues: 1. Tests for the entire monorepo consume a lot of RAM; 20GB free RAM was not enough, so I couldn't run tests for the entire monorepo and had to only run them for `packages/editor-ui`; 2. Linting is very slow; `pnpm lint` in `packages/editor-ui` takes ten minutes to complete; 3. It seems that types are not checked. Code OSS highlights numerous errors in code files: for example, `'debug'` is incompatible with `CloudUpdateLinkSourceType` expected by `goToUpgrade` here: https://github.com/n8n-io/n8n/blob/3e7a4d3b2cc12fcb1b011fccd0773bb807986884/packages/editor-ui/src/composables/useExecutionDebugging.ts#L128 However, I'm not getting any errors during build. There is a `typecheck` script defined in `package.json`, but `pnpm typecheck` fails with: ``` n8n-toy-demo:~/projects/n8n/packages/editor-ui$ pnpm typecheck > [email protected] typecheck /home/inga/projects/n8n/packages/editor-ui > vue-tsc --emitDeclarationOnly error TS5069: Option 'emitDeclarationOnly' cannot be specified without specifying option 'declaration' or option 'composite'. Found 1 error. ELIFECYCLE Command failed with exit code 1. n8n-toy-demo:~/projects/n8n/packages/editor-ui$ ``` Replacing `--emitDeclarationsOnly` with `--noEmit` in `package.json` unblocks typechecking and results in seemingly, at first glance, correct "Found 1924 errors in 306 files" (at least several of the reported errors that I've checked seem to be correct). But maybe I'm missing something and there are not in fact two thousands type errors in `editor-ui`?
Configuration menu - View commit details
-
Copy full SHA for d14e9cb - Browse repository at this point
Copy the full SHA d14e9cbView commit details -
fix(core): Prevent false stalled jobs in queue mode from displaying a…
…s errored (#7435) This is related to an issue with how Bull handles stalled jobs, see OptimalBits/bull#1415 for reference. CPU intensive workflows can in certain cases take a long while to finish up, thereby blocking the thread and causing Bull queue to think the job has stalled, even though it finished successfully. In these cases the error handling could then overwrite the successful execution data with the error message.
Configuration menu - View commit details
-
Copy full SHA for 465a952 - Browse repository at this point
Copy the full SHA 465a952View commit details -
fix(Convert to/from binary data Node): Change Mime to MIME (no-change…
…log) (#7446) Github issue / Community forum post (link here to close automatically):
Configuration menu - View commit details
-
Copy full SHA for 41ee8cc - Browse repository at this point
Copy the full SHA 41ee8ccView commit details -
fix(TheHive 5 Node): Observable encoding in alert > create fix (#7450)
Github issue / Community forum post (link here to close automatically):
Configuration menu - View commit details
-
Copy full SHA for b9547ad - Browse repository at this point
Copy the full SHA b9547adView commit details -
Configuration menu - View commit details
-
Copy full SHA for f43ff71 - Browse repository at this point
Copy the full SHA f43ff71View commit details -
fix(editor): Fix canvas selection breaking after interacting with nod…
Configuration menu - View commit details
-
Copy full SHA for 90ce8de - Browse repository at this point
Copy the full SHA 90ce8deView commit details -
fix(MySQL Node): Resolve expressions in v1 (#7464)
Github issue / Community forum post (link here to close automatically):
1Configuration menu - View commit details
-
Copy full SHA for 2b18909 - Browse repository at this point
Copy the full SHA 2b18909View commit details -
Configuration menu - View commit details
-
Copy full SHA for 33e3df8 - Browse repository at this point
Copy the full SHA 33e3df8View commit details -
fix(core): Reduce logging overhead for levels that do not output (#7479)
all current logging calls execute `callsites()` to figure out what code tried to log. This happens even for logging methods that aren't supposed to create any output. Under moderate load, this can take up quite a lot of resources. This PR changes the logger to make all ignorable logging methods a No-Op. In a small benchmark with a simple webhook, with log-level set to `warn`, and using `ab -c 50 -n 500 http://localhost:5678/webhook/testing`, these were the response times: ### Before  ### After 
Configuration menu - View commit details
-
Copy full SHA for 010aa57 - Browse repository at this point
Copy the full SHA 010aa57View commit details -
fix(editor): Fix connections disappearing after reactivating canvas a…
…nd renaming a node (#7483) Github issue / Community forum post (link here to close automatically): - https://community.n8n.io/t/1-11-1-possible-bug-all-nodes-randomly-losing-their-connectors/31856 - https://community.n8n.io/t/lines-between-nodes-have-disappeared/31846 --------- Signed-off-by: Oleg Ivaniv <[email protected]>
Configuration menu - View commit details
-
Copy full SHA for b0bd0d8 - Browse repository at this point
Copy the full SHA b0bd0d8View commit details -
## [1.11.2](https://github.com/n8n-io/n8n/compare/[email protected]@1.11.2) (2023-10-23) ### Bug Fixes * **core:** Handle gzip and deflate compressed request payloads ([#7461](#7461)) ([f43ff71](f43ff71)) * **core:** Prevent false stalled jobs in queue mode from displaying as errored ([#7435](#7435)) ([465a952](465a952)) * **core:** Reduce logging overhead for levels that do not output ([#7479](#7479)) ([010aa57](010aa57)) * **editor:** Allow importing the same workflow multiple times ([#7458](#7458)) ([33e3df8](33e3df8)), closes [#7457](#7457) * **editor:** Fix canvas selection breaking after interacting with node actions ([#7466](#7466)) ([90ce8de](90ce8de)) * **editor:** Fix connections disappearing after reactivating canvas and renaming a node ([#7483](#7483)) ([b0bd0d8](b0bd0d8)) * **editor:** Open only one tab with plans page ([#7377](#7377)) ([d14e9cb](d14e9cb)) * **Ldap Node:** Fix issue with connections not closing correctly ([#7432](#7432)) ([60ca02e](60ca02e)) * **MySQL Node:** Resolve expressions in v1 ([#7464](#7464)) ([2b18909](2b18909)) * **TheHive 5 Node:** Observable encoding in alert > create fix ([#7450](#7450)) ([b9547ad](b9547ad)) Co-authored-by: netroy <[email protected]>
1Configuration menu - View commit details
-
Copy full SHA for a16bc1d - Browse repository at this point
Copy the full SHA a16bc1dView commit details
This comparison is taking too long to generate.
Unfortunately it looks like we can’t render this comparison for you right now. It might be too big, or there might be something weird with your repository.
You can try running this command locally to see the comparison on your machine:
git diff [email protected]@1.11.2