fix(size/L): Preserve folder order from seq attribute#8213
Open
sachin-bruno wants to merge 2 commits into
Open
fix(size/L): Preserve folder order from seq attribute#8213sachin-bruno wants to merge 2 commits into
sachin-bruno wants to merge 2 commits into
Conversation
sachin-bruno
requested review from
bijin-bruno,
helloanoop,
lohit-bruno,
naman-bruno and
sid-bruno
as code owners
Contributor
WalkthroughThis PR adds deterministic item ordering to the "Generate Documentation" export by introducing ChangesDocumentation generation item ordering and E2E verification
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/bruno-app/src/utils/common/sort-items-by-sidebar-order.spec.js (1)
1-2: ⚡ Quick winTest file location doesn't match function location.
The test file is at
packages/bruno-app/src/utils/common/sort-items-by-sidebar-order.spec.jsbut the function under test is inpackages/bruno-app/src/utils/collections/index.js.Consider moving this test file to
packages/bruno-app/src/utils/collections/sort-items-by-sidebar-order.spec.jsto co-locate it with the function being tested, improving discoverability and maintainability.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-app/src/utils/common/sort-items-by-sidebar-order.spec.js` around lines 1 - 2, Move the spec to live next to the implementation: relocate the test file named sort-items-by-sidebar-order.spec.js into the same folder containing the collections index module so it directly tests sortItemsBySidebarOrder; update its require/import to reference ../index (or the local module) if necessary and run tests to ensure paths resolve correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/bruno-app/src/utils/collections/index.js`:
- Around line 891-905: The JSDoc claims code/data files are preserved but
sortItemsBySidebarOrder currently only keeps folders and requests (using
sortByNameThenSequence, isItemAFolder, isItemARequest) and drops other item
types; update the function to collect "other" items (those not matching
isItemAFolder and not matching isItemARequest), preserve their original order,
and include them after requestItems when building the returned array (still
recursively sorting folder.items via sortItemsBySidebarOrder), or alternatively
update the JSDoc to reflect that only folders and requests are preserved — pick
the behavior you want and make the code or JSDoc consistent with
sortItemsBySidebarOrder.
In `@tests/collection/generate-docs/generate-docs.spec.ts`:
- Around line 55-66: The test fallback uses new Function(...) when
JSON.parse(literal) fails because jsesc emitted JS-only escapes; instead
configure jsesc in GenerateDocumentation/index.js to produce JSON-compatible
output (e.g., enable the json option / disable JS-only escapes) so the literal
is always valid JSON, then remove the catch fallback in generate-docs.spec.ts
(the literal / yamlContent flow) and rely solely on JSON.parse; if you cannot
guarantee JSON-compatible output, add a brief comment in generate-docs.spec.ts
explaining why using new Function is safe in this controlled test context and
keep the fallback.
---
Nitpick comments:
In `@packages/bruno-app/src/utils/common/sort-items-by-sidebar-order.spec.js`:
- Around line 1-2: Move the spec to live next to the implementation: relocate
the test file named sort-items-by-sidebar-order.spec.js into the same folder
containing the collections index module so it directly tests
sortItemsBySidebarOrder; update its require/import to reference ../index (or the
local module) if necessary and run tests to ensure paths resolve correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 79b826b2-6162-46f1-9871-398476dc0dd6
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (18)
package.jsonpackages/bruno-app/src/components/Sidebar/Collections/Collection/GenerateDocumentation/index.jspackages/bruno-app/src/utils/collections/index.jspackages/bruno-app/src/utils/common/sort-items-by-sidebar-order.spec.jstests/collection/generate-docs/fixtures/collection/Aviary/Parrot.brutests/collection/generate-docs/fixtures/collection/Aviary/folder.brutests/collection/generate-docs/fixtures/collection/ReqAlpha.brutests/collection/generate-docs/fixtures/collection/ReqBeta.brutests/collection/generate-docs/fixtures/collection/Zoo/Bear.brutests/collection/generate-docs/fixtures/collection/Zoo/Lion.brutests/collection/generate-docs/fixtures/collection/Zoo/Reptiles/Gecko.brutests/collection/generate-docs/fixtures/collection/Zoo/Reptiles/Snake.brutests/collection/generate-docs/fixtures/collection/Zoo/Reptiles/folder.brutests/collection/generate-docs/fixtures/collection/Zoo/folder.brutests/collection/generate-docs/fixtures/collection/bruno.jsontests/collection/generate-docs/generate-docs.spec.tstests/collection/generate-docs/init-user-data/preferences.jsontests/utils/page/actions.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
A user organizes their Bruno collection with folders ordered using the seq attribute. When they run "Generate Docs", they expect the HTML output to reflect that order. Today the order is non-deterministic — each regeneration produces a different folder order in the rendered output and in the JSON blob embedded in the HTML.
Ref: https://usebruno.atlassian.net/browse/BRU-3403
Contribution Checklist:
Note: Keeping the PR small and focused helps make it easier to review and merge. If you have multiple changes you want to make, please consider submitting them as separate pull requests.
Publishing to New Package Managers
Please see here for more information.
Summary by CodeRabbit
Improvements
New Features
Tests
Chores