Skip to content

Conversation

@RicardoE105
Copy link
Contributor

@RicardoE105 RicardoE105 commented May 7, 2025

Summary

This PR enhances the folder retrieval endpoint to return the path from a folder to the root, allowing clients to display breadcrumb navigation or full folder paths in the UI.

Technical Details

  1. The implementation uses a Common Table Expression (CTE) with a recursive query to efficiently calculate the path from any folder to the root
  2. The path is returned as an array of folder names, starting with the root folder and ending with the requested folder
  3. The method only performs the additional path calculation when the path field is explicitly requested in the select parameter, maintaining backward compatibility

Related Linear tickets, Github issues, and Community forum posts

https://linear.app/n8n/issue/ADO-3546/add-path-to-folders-returned-from-endpoint

Review / Merge checklist

  • PR title and summary are descriptive. (conventions)
  • Docs updated or follow-up ticket created.
  • Tests included.
  • PR Labeled with release/backport (if the PR is an urgent fix that needs to be backported)

@RicardoE105 RicardoE105 changed the title Update endpoint to retrieve folder to allow returning the path to root feat(core): Update endpoint to retrieve folder to allow returning the path to root (no-changelog) May 7, 2025
@RicardoE105 RicardoE105 requested a review from Cadiac May 7, 2025 01:47
@codecov
Copy link

codecov bot commented May 7, 2025

Codecov Report

Attention: Patch coverage is 32.25806% with 21 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...ages/@n8n/db/src/repositories/folder.repository.ts 0.00% 21 Missing ⚠️

📢 Thoughts on this report? Let us know!

@n8n-assistant n8n-assistant bot added core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team labels May 7, 2025
@RicardoE105 RicardoE105 marked this pull request as ready for review May 8, 2025 00:43
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

mrge found 1 issue across 6 files. Review it in mrge.io

Copy link
Contributor

@Cadiac Cadiac left a comment

Choose a reason for hiding this comment

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

I pushed a suggestion of how we could do this instead to branch https://github.com/n8n-io/n8n/tree/ado-3546-add-path-to-folders-returned-from-endpoint-simplified, lets talk about this today when you're online?

async getManyAndCount(options: ListQuery.Options = {}) {
const query = this.getManyQuery(options);
return await query.getManyAndCount();
return (await query.getManyAndCount()) as unknown as [
Copy link
Contributor

Choose a reason for hiding this comment

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

These casts seem fishy and unnecessary 🤔

export type FolderWithWorkflowAndSubFolderCount = Folder & {
workflowCount: boolean;
subFolderCount: number;
workflowCount?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we avoid changing these to optional? They should still be present always if folder is of this type

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm suggesting we simplify this, instead of introducing the new type can we change this type to

export type FolderWithWorkflowAndSubFolderCount = Folder & {
	workflowCount: boolean;
	subFolderCount: number;
	path?: string[];
};

and then enrich the path only if the select option is used, otherwise just leaving it out? This allows us to keep the FolderRepository typed as

export class FolderRepository extends Repository<FolderWithWorkflowAndSubFolderCount> {

and removes the need for casts

Copy link
Contributor Author

@RicardoE105 RicardoE105 May 8, 2025

Choose a reason for hiding this comment

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

The reason I did not want the repository to take FolderWithWorkflowAndSubFolderCount it's because does not have those values in the entity. So if you in this repo do: this.create, it looks like you can pass the fields in FolderWithWorkflowAndSubFolderCount and that is not correct. The same with update. Those fields are only included if the getMany method and the select parameter explicitly include them. Thoughts?

CleanShot 2025-05-08 at 18 50 17

@RicardoE105 RicardoE105 requested a review from Cadiac May 13, 2025 13:50
Copy link
Contributor

@Cadiac Cadiac left a comment

Choose a reason for hiding this comment

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

Show must go on, lets refactor the repository types later 👍

@github-actions
Copy link
Contributor

✅ All Cypress E2E specs passed

@RicardoE105 RicardoE105 merged commit 4201614 into master May 13, 2025
13 checks passed
@RicardoE105 RicardoE105 deleted the ado-3546-add-path-to-folders-returned-from-endpoint branch May 13, 2025 14:09
@janober
Copy link
Member

janober commented May 19, 2025

Got released with n8n@1.94.0

ozcangungor pushed a commit to ozcangungor/n8n that referenced this pull request May 25, 2025
etobella pushed a commit to etobella/n8n that referenced this pull request May 25, 2025
Alexandero89 pushed a commit to Alexandero89/n8n that referenced this pull request May 28, 2025
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants