Skip to content
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

refactor(core): Decouple server started event from internal hooks (no-changelog) #10221

Merged
merged 1 commit into from
Jul 29, 2024

Conversation

ivov
Copy link
Contributor

@ivov ivov commented Jul 29, 2024

Follow-up to #10162

Comment on lines +486 to +490
const firstWorkflow = await this.workflowRepository.findOne({
select: ['createdAt'],
order: { createdAt: 'ASC' },
where: {},
});
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initially tried to pass this as an event arg from Server but importing the workflow repository there leads to ServiceNotFoundError: Service with "MaybeConstructable<DataSource>" identifier was not found in the container. Register it before usage via explicitly calling the "Container.set" function or using the "@Service()" decorator.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leaving this in Server meant that this query would have run even if telemetry was disabled. This file is the correct place for this query.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking to bring this back later, else let me know if we should rewrite it now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's fine to do this later.

Comment on lines +486 to +490
const firstWorkflow = await this.workflowRepository.findOne({
select: ['createdAt'],
order: { createdAt: 'ASC' },
where: {},
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leaving this in Server meant that this query would have run even if telemetry was disabled. This file is the correct place for this query.

@n8n-assistant n8n-assistant bot added core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team labels Jul 29, 2024
Copy link
Contributor

⚠️ Some Cypress E2E specs are failing, please fix them before merging

Copy link

cypress bot commented Jul 29, 2024

1 flaky test on run #6175 ↗︎

0 389 0 0 Flakiness 1

Details:

🌳 🖥️ browsers:node18.12.0-chrome107 🤖 ivov 🗃️ e2e/*
Project: n8n Commit: d0805e5dc9
Status: Passed Duration: 04:05 💡
Started: Jul 29, 2024 9:17 AM Ended: Jul 29, 2024 9:21 AM
Flakiness  e2e/13-pinning.cy.ts • 1 flaky test

View Output Video

Test Artifacts
Data pinning > should use pin data in manual executions that are started by a webhook Test Replay Screenshots Video

Review all test suite changes for PR #10221 ↗︎

Copy link
Contributor

✅ All Cypress E2E specs passed

@ivov ivov merged commit 24ffca7 into master Jul 29, 2024
39 checks passed
@ivov ivov deleted the decouple-server-started-event-from-internal-hooks branch July 29, 2024 09:41
ivov added a commit that referenced this pull request Jul 29, 2024
MiloradFilipovic added a commit that referenced this pull request Jul 30, 2024
* master:
  docs: Update login url for OpenAI node (no-changelog) (#10222)
  fix(LinkedIn Node): Fix issue with some characters cutting off posts early (#10185)
  fix(Google BigQuery Node): Send timeoutMs in query, pagination support (#10205)
  feat(core): Show Public API key value only once (no-changelog) (#10126)
  refactor(core): Display stack trace in error reporter (no-changelog) (#10225)
  fix(core): Fix missing successful items on continueErrorOutput with multiple outputs (#10218)
  fix(n8n Form Trigger Node): Remove custom attribution option (no-changelog) (#10229)
  feat(n8n Form Trigger Node): Improvements (#10092)
  refactor(core): Port path, host, port, listen_address and protocol config (no-changelog) (#10223)
  docs: Update add options text (no-changelog) (#10049)
  fix(Postgres Node): Option to treat query parameters enclosed in single quotas as text (#10214)
  refactor(core): Decouple server started event from internal hooks (no-changelog) (#10221)
  feat(Shopify Node): Update Shopify API version (#10155)
  fix(editor): Defer `User saved credentials` telemetry event for OAuth credentials (#10215)
  fix(editor): Fix parameter input glitch when there was an error loading remote options (#10209)
ivov added a commit that referenced this pull request Jul 31, 2024
@janober
Copy link
Member

janober commented Jul 31, 2024

Got released with n8n@1.53.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team Released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants