chore: 16884 preview pdfLayoutName

[walldenfilippa]

Description

Fix preview of pdfLayoutName. The pdfLayoutName is now shown as a page in the UI‑editor instead of displaying the first page in the app.

Issue: #16884

Verification

• Related issues are connected (if applicable)
• Your code builds clean without any errors or warnings
• Manual testing done (required)
• Relevant automated test added (if you find this hard, leave it and we'll help out)

Summary by CodeRabbit

• Bug Fixes
  • PDF layout names are now correctly included in page ordering when retrieving layout settings, ensuring page order reflects selected PDF layouts for both standard and V4 app retrievals.

[coderabbitai]

Caution

Review failed

The head commit changed during the review from 651ebb8 to e864d4d.

📝 Walkthrough

Walkthrough

Added a private helper to PreviewController that appends a non-empty pdfLayoutName into pages.order when present. Both LayoutSettings and LayoutSettingsForV4Apps call this helper immediately after layout retrieval/creation and before the response is serialized.

Changes

Cohort / File(s)	Summary
PreviewController update src/Designer/backend/src/Designer/Controllers/PreviewController.cs	Added AddPdfLayoutNameToPageOrder(JsonNode) helper which reads pages.pdfLayoutName and appends it to pages.order if non-empty and order is a JsonArray. Invoked from both LayoutSettings and LayoutSettingsForV4Apps after fetching/creating layout settings and before serializing the response.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 I found a PDF name hiding in the trees,
I nudged it gently into the pages' breeze.
Two endpoints now sing the same tidy tune,
No more lost names beneath the moon. ✨📄

🚥 Pre-merge checks | ✅ 1 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title is vague and does not clearly convey the main change; it uses abbreviation '16884' instead of describing what the fix accomplishes.	Consider revising the title to be more descriptive, such as 'Fix pdfLayoutName display in app preview' to better communicate the actual change to teammates reviewing the history.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description covers the main change, includes a reference to the related issue (#16884), and completes most of the verification checklist items appropriately.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (2)

src/Designer/backend/src/Designer/Controllers/PreviewController.cs (2)

901-922: Consider centralising this layout-settings normalisation in the service layer.

This helper currently protects preview endpoints only, while AppDevelopmentService.GetLayoutSettings (src/Designer/backend/src/Designer/Services/Implementation/AppDevelopmentService.cs Line 150-175) still returns raw layout settings. Moving the transformation closer to retrieval would reduce endpoint divergence and regression risk.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/Designer/backend/src/Designer/Controllers/PreviewController.cs` around
lines 901 - 922, The helper AddPdfLayoutNameToPageOrder in PreviewController
currently normalises layoutSettings only for preview endpoints; move this
normalization into the service layer by centralising the logic (e.g. extract
AddPdfLayoutNameToPageOrder logic into a shared private/internal method on
AppDevelopmentService or a new LayoutSettingsNormalizer used by
AppDevelopmentService.GetLayoutSettings) so GetLayoutSettings returns
already-normalised settings; update PreviewController to consume the
service-returned settings (remove duplicate normalisation) and ensure the new
method is called from AppDevelopmentService.GetLayoutSettings to avoid endpoint
divergence and regressions.

---

903-913: Extract JSON key literals into constants.

The new helper introduces hard-coded keys ("pages", "pdfLayoutName", "order"). Please lift these to private constants to reduce typo risk and improve maintainability.

♻️ Proposed refactor

+        private const string PagesPropertyName = "pages";
+        private const string PdfLayoutNamePropertyName = "pdfLayoutName";
+        private const string OrderPropertyName = "order";
+
         private static void AddPdfLayoutNameToPageOrder(JsonNode layoutSettings)
         {
-            if (layoutSettings?["pages"] is not JsonObject pagesObject)
+            if (layoutSettings?[PagesPropertyName] is not JsonObject pagesObject)
             {
                 return;
             }
-            string pdfLayoutName = pagesObject["pdfLayoutName"]?.GetValue<string>();
+            string pdfLayoutName = pagesObject[PdfLayoutNamePropertyName]?.GetValue<string>();
             if (string.IsNullOrEmpty(pdfLayoutName))
             {
                 return;
             }
-            if (pagesObject["order"] is not JsonArray orderArray)
+            if (pagesObject[OrderPropertyName] is not JsonArray orderArray)
             {
                 return;
             }

As per coding guidelines, "Avoid hard-coded numbers and strings".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/Designer/backend/src/Designer/Controllers/PreviewController.cs` around
lines 903 - 913, The code in PreviewController uses hard-coded JSON key literals
("pages", "pdfLayoutName", "order") when reading layoutSettings; define private
const string fields (e.g., PagesKey, PdfLayoutNameKey, OrderKey) at the top of
the class and replace occurrences in the method that references layoutSettings,
pagesObject, pdfLayoutName, and orderArray to use these constants instead of
literal strings to reduce typo risk and improve maintainability.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/Designer/backend/src/Designer/Controllers/PreviewController.cs`:
- Around line 901-922: The helper AddPdfLayoutNameToPageOrder in
PreviewController currently normalises layoutSettings only for preview
endpoints; move this normalization into the service layer by centralising the
logic (e.g. extract AddPdfLayoutNameToPageOrder logic into a shared
private/internal method on AppDevelopmentService or a new
LayoutSettingsNormalizer used by AppDevelopmentService.GetLayoutSettings) so
GetLayoutSettings returns already-normalised settings; update PreviewController
to consume the service-returned settings (remove duplicate normalisation) and
ensure the new method is called from AppDevelopmentService.GetLayoutSettings to
avoid endpoint divergence and regressions.
- Around line 903-913: The code in PreviewController uses hard-coded JSON key
literals ("pages", "pdfLayoutName", "order") when reading layoutSettings; define
private const string fields (e.g., PagesKey, PdfLayoutNameKey, OrderKey) at the
top of the class and replace occurrences in the method that references
layoutSettings, pagesObject, pdfLayoutName, and orderArray to use these
constants instead of literal strings to reduce typo risk and improve
maintainability.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 34287d80-48eb-41e5-b34a-032b33c370d3

📥 Commits

Reviewing files that changed from the base of the PR and between c9f8440 and b7d2f95.

📒 Files selected for processing (1)

• src/Designer/backend/src/Designer/Controllers/PreviewController.cs

[Jondyr]

Nice! Works well, but I noticed it has issues with layouts using page groups:

[codecov]

Codecov Report

❌ Patch coverage is 80.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.05%. Comparing base (ff7a05e) to head (a4661d0).
⚠️ Report is 52 commits behind head on main.

Files with missing lines	Patch %	Lines
...-editor/src/hooks/mutations/useAddGroupMutation.ts	80.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #18372      +/-   ##
==========================================
+ Coverage   94.98%   95.05%   +0.06%     
==========================================
  Files        2535     2536       +1     
  Lines       32328    32358      +30     
  Branches     3966     3979      +13     
==========================================
+ Hits        30708    30758      +50     
+ Misses       1291     1276      -15     
+ Partials      329      324       -5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Jondyr]

Nice!