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

feat(editor): Add new /templates/search endpoint #8227

Merged
merged 27 commits into from
Jan 15, 2024

Conversation

RicardoE105
Copy link
Contributor

@RicardoE105 RicardoE105 commented Jan 4, 2024

Updating n8n front-end to use the new search endpoint powered by TypeSense.

Endpoint is deployed on staging API so, in order to test it, use this env var:
export N8N_TEMPLATES_HOST=https://api-staging.n8n.io/api

NOTE: This PR should not be merged until backend changes are merged and released.

Related tickets and issues

https://linear.app/n8n/issue/ADO-1555/update-in-app-search-to-work-with-the-new-back-end

Review / Merge checklist

  • PR title and summary are descriptive. Remember, the title automatically goes into the changelog. Use (no-changelog) otherwise. (conventions)
  • 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.

@RicardoE105 RicardoE105 marked this pull request as draft January 4, 2024 13:20
@n8n-assistant n8n-assistant bot added n8n team Authored by the n8n team ui Enhancement in /editor-ui or /design-system labels Jan 4, 2024
@RicardoE105 RicardoE105 changed the title Ado 1555 use new search enpoint feat: Add new /template/search endpoint Jan 8, 2024
@RicardoE105 RicardoE105 changed the title feat: Add new /template/search endpoint feat(editor): Add new /template/search endpoint Jan 8, 2024
@RicardoE105 RicardoE105 marked this pull request as ready for review January 8, 2024 21:17
@@ -6,6 +6,7 @@ import type {
ITemplatesCollectionResponse,
ITemplatesWorkflowResponse,
IWorkflowTemplate,
TemplateSearchFacet,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we drop the endpoint to retrieve the categories since we are not using it anymore? @MiloradFilipovic

Copy link
Contributor

@MiloradFilipovic MiloradFilipovic Jan 9, 2024

Choose a reason for hiding this comment

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

I think we need it for backward compatibility

RicardoE105 and others added 3 commits January 8, 2024 16:51
* master:
  ci: Validate docs urls for langchain nodes as well (no-changelog) (#8271)
  fix: Small ui fixes to workflow cred setup modal (no-changelog) (#8280)
  fix: Fix issue with API key being required for the Qdrant Node (#8237)
  feat(Coda Node): Add User-Agent for requests to Coda (no-changelog) (#7771)
  fix(FTP Node): FTP connection failed due to missing password credential in node (#8131)
  ci: Fix lint setup in `chat` package (no-changelog) (#8275)
  ci: Fix codecov reporting, and include all packages (no-changelog) (#8276)
  fix(core): Fix test webhook deregistration (#8247)
  docs: Update links in deprecated langchain nodes to avoid linting errors (no-changelog) (#8273)
  fix: Fix user reinvites on FE and BE (#8261)
  docs: Update primaryDocumentation urls for nodes updated in #7651 (no-changelog) (#8270)
  feat: Add Chat Trigger node (#7409)
  fix(Github Trigger Node): Enforce SSL validation by default (#8265)
  fix(editor): Tweaking button sizes in execution preview (#8206)
  fix(core): Avoid wrapping `ExecutionBaseError` to prevent misreporting to Sentry (no-changelog) (#8262)
@MiloradFilipovic MiloradFilipovic self-assigned this Jan 10, 2024
@MiloradFilipovic MiloradFilipovic removed their request for review January 10, 2024 10:37
packages/editor-ui/src/components/TemplateList.vue Outdated Show resolved Hide resolved
Comment on lines 56 to 57
skip: query.skip,
rows: query.limit,
Copy link
Contributor

Choose a reason for hiding this comment

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

how does this map to the page parameter typesense expects? https://typesense.org/docs/0.24.0/api/search.html#pagination-parameters

Copy link
Contributor

Choose a reason for hiding this comment

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

This is mapped on the backend:
per_page --> rows
page --> skip

packages/editor-ui/src/api/templates.ts Outdated Show resolved Hide resolved
packages/editor-ui/src/stores/templates.store.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@mutdmour mutdmour left a comment

Choose a reason for hiding this comment

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

Thanks for addressing my feedback. Looks good overall. Would be great to add some E2E tests please to this area that never had any coverage. 🙏🏽

packages/editor-ui/src/stores/templates.store.ts Outdated Show resolved Hide resolved
packages/editor-ui/src/stores/templates.store.ts Outdated Show resolved Hide resolved
@MiloradFilipovic MiloradFilipovic changed the title feat(editor): Add new /template/search endpoint feat(editor): Add new /templates/search endpoint Jan 11, 2024
* master:
  fix: Adjust cred setup telemetry (no-changelog) (#8294)
  fix(HTTP Request Node): Delete `response.request` only when it's a valid circular references (#8293)
  fix: Properly output saml validation errors (#8284)
  🚀 Release 1.24.0 (#8290)
  feat(editor): Migrate deviceSupportHelpers mixin to useDeviceSupport composable (no-changelog) (#8289)
  fix: Store workflow settings when saving an execution (#8288)
  fix(MySQL Node): Only escape table names when needed (#8246)
  feat(Google Sheets Node): Add "By Name" option to selector for Sheets (#8241)
  feat(MongoDB Node): Add support for TLS (#8266)
  feat: Update @n8n/chat exports and add example images (no-changelog) (#8274)
  ci: Add support for optionally including ARM64 support in custom images (no-changelog) (#8286)
  ci: Update Cypress & increase amount of e2e containers (#8282)
  fix(Monday.com Node): Migrate to api 2023-10 (#8254)
mutdmour
mutdmour previously approved these changes Jan 11, 2024
Copy link
Contributor

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

Copy link

cypress bot commented Jan 11, 2024

1 flaky test on run #3762 ↗︎

0 333 5 0 Flakiness 1

Details:

🌳 🖥️ browsers:node18.12.0-chrome107 🤖 RicardoE105 🗃️ e2e/*
Project: n8n Commit: 2b90208e91
Status: Passed Duration: 38:42 💡
Started: Jan 15, 2024 2:18 PM Ended: Jan 15, 2024 2:56 PM
Flakiness  cypress/e2e/17-sharing.cy.ts • 1 flaky test

View Output Video

Test Artifacts
Sharing > should work for admin role on credentials created by others (also can share it with themselves) Test Replay Screenshots Video

Review all test suite changes for PR #8227 ↗︎

@RicardoE105
Copy link
Contributor Author

@mutdmour can we get this one approved?

Copy link
Contributor

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

Copy link
Contributor

✅ All Cypress E2E specs passed

@RicardoE105 RicardoE105 merged commit 4277e92 into master Jan 15, 2024
51 checks passed
@RicardoE105 RicardoE105 deleted the ADO-1555-use-new-search-enpoint branch January 15, 2024 21:19
@github-actions github-actions bot mentioned this pull request Jan 17, 2024
@janober
Copy link
Member

janober commented Jan 17, 2024

Got released with [email protected]

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

Successfully merging this pull request may close these issues.

4 participants