[CDX-373] Explicitly type FmtOptions by esezen · Pull Request #94 · Constructor-io/constructorio-netcore

esezen · 2026-02-05T16:52:28Z

This is a breaking change. Please do not merge until we land on a release strategy.

Copilot

Pull request overview

This PR refactors the FmtOptions parameter from a generic Dictionary<string, string> to a strongly-typed FmtOptions class, improving type safety and API clarity.

Changes:

Introduced a new FmtOptions class with explicitly typed properties for all format options
Updated request models to use the new FmtOptions type instead of Dictionary<string, string>
Modified URL building logic in Helpers.cs to handle the strongly-typed FmtOptions object with separate handling for scalar and array properties

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/constructor.io/models/common/FmtOptions.cs	New strongly-typed class defining all format option properties with documentation
src/constructor.io/utils/Helpers.cs	Updated URL building to serialize `FmtOptions` properties individually with new helper methods
src/constructor.io/models/common/IPlpRequest.cs	Changed `FmtOptions` property type from dictionary to typed class
src/constructor.io/models/Search/SearchRequest.cs	Updated `FmtOptions` property type to use new class
src/constructor.io/models/Browse/BrowseRequest.cs	Updated `FmtOptions` property type to use new class
src/constructor.io/models/Browse/BrowseItemsRequest.cs	Updated `FmtOptions` property type to use new class
src/constructor.io/models/Browse/BrowseFacetsRequest.cs	Refactored to use strongly-typed `FmtOptions` with null-coalescing initialization
src/constructor.io/models/Browse/BrowseFacetOptionsRequest.cs	Refactored to use strongly-typed `FmtOptions` with null-coalescing initialization
src/Constructorio_NET.Tests/utils/HelpersTest.cs	Added comprehensive tests for new `FmtOptions` serialization including array handling

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/constructor.io/utils/Helpers.cs

Alexey-Pavlov

Looks great, thanks so much! I’ve left a few comments—when you get a moment, could you take a look?

src/constructor.io/models/Search/SearchRequest.cs

src/constructor.io/utils/Helpers.cs

src/constructor.io/models/common/FmtOptions.cs

src/constructor.io/models/Browse/BrowseFacetsRequest.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Alexey-Pavlov

Thank you for making changes! LGTM!

Mudaafi

Thanks for working on this refactor @esezen! I had some comments on the implementation, but lmk what you think

src/constructor.io/models/Browse/BrowseFacetsRequest.cs

src/constructor.io/models/Browse/BrowseFacetOptionsRequest.cs

src/constructor.io/utils/Helpers.cs

* Add rex support * Update src/Constructorio_NET.Tests/client/modules/RecommendationsTests.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update tests * Fix flaky test --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-26T15:36:35Z

src/constructor.io/utils/Helpers.cs

                // Add quiz answers to query string
                if (queryParams.Contains(Constants.ANSWERS))
                {


MakeUrl no longer has a dedicated branch for Constants.FMT_OPTIONS / Constants.HIDDEN_FIELDS / Constants.HIDDEN_FACETS. If any caller still passes the legacy shapes (e.g., FMT_OPTIONS as Dictionary<string,string>), those entries will now be silently ignored (no query string output) because the "remaining query params" loop only handles string, int, List<string>, and bool. Consider either keeping backward-compatible serialization for these keys or explicitly throwing a clear exception when legacy keys are present so the failure mode isn’t silent.

src/constructor.io/models/common/FmtOptions.cs

Mudaafi

LGTM, just one comment. @Alexey-Pavlov mind taking a second look?

Mudaafi · 2026-02-26T22:03:46Z

src/Constructorio_NET.Tests/client/modules/RecommendationsTests.cs

+            if (res.Response.Results.Count > 0)
+            {
+                var metadata = res.Response.Results[0].Data?.Metadata;
+                Assert.IsNotNull(metadata, "Result metadata exists");
+                Assert.IsTrue(
+                    metadata.ContainsKey(requestedHiddenField),
+                    "Requested hidden field is present in result metadata"
+                );
+                Assert.IsNotNull(
+                    metadata[requestedHiddenField],
+                    "Requested hidden field has a non-null value in result metadata"
+                );
+            }


Should we throw an error if results.Count = 0? If it's ever zero, and this functionality breaks, we wouldn't be aware.

Good catch, updated

Alexey-Pavlov

LGTM! Thank you! Left some minor comments

src/constructor.io/models/Browse/BrowseFacetsRequest.cs

src/Constructorio_NET.Tests/models/common/FmtOptionsTest.cs

src/constructor.io/models/Browse/BrowseItemsRequest.cs

Properly type FmtOptions

1935dd9

esezen requested a review from Mudaafi as a code owner February 5, 2026 16:52

Copilot AI review requested due to automatic review settings February 5, 2026 16:52

esezen added the do not merge label Feb 5, 2026

Copilot AI reviewed Feb 5, 2026

View reviewed changes

src/constructor.io/utils/Helpers.cs Outdated Show resolved Hide resolved

src/constructor.io/utils/Helpers.cs Outdated Show resolved Hide resolved

esezen mentioned this pull request Feb 5, 2026

fmt_options.fields expecting an array #91

Closed

Alexey-Pavlov requested changes Feb 11, 2026

View reviewed changes

esezen and others added 3 commits February 12, 2026 15:54

Update src/constructor.io/utils/Helpers.cs

e82ee57

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update FmtOptions

aa49bd5

Add tests

b9db2b2

esezen requested a review from Alexey-Pavlov February 12, 2026 12:20

Alexey-Pavlov previously approved these changes Feb 12, 2026

View reviewed changes

Merge branch 'main' into cdx-373-netcore-update-fmtoptions

11cbe38

Mudaafi reviewed Feb 19, 2026

View reviewed changes

esezen added 4 commits February 20, 2026 17:09

Address comments

353790a

Address comments

99f0266

Refactor FmtOptions completley

80d2b1c

Remove legacy fields

aea99f0

esezen dismissed Alexey-Pavlov’s stale review via aea99f0 February 20, 2026 15:37

esezen requested review from Alexey-Pavlov, Mudaafi and Copilot February 26, 2026 15:31

Copilot started reviewing on behalf of esezen February 26, 2026 15:31 View session

Copilot AI reviewed Feb 26, 2026

View reviewed changes

Mudaafi previously approved these changes Feb 26, 2026

View reviewed changes

Alexey-Pavlov previously approved these changes Feb 27, 2026

View reviewed changes

src/constructor.io/models/Browse/BrowseFacetsRequest.cs Outdated Show resolved Hide resolved

src/Constructorio_NET.Tests/models/common/FmtOptionsTest.cs Show resolved Hide resolved

src/constructor.io/models/Browse/BrowseItemsRequest.cs Show resolved Hide resolved

esezen added 3 commits February 27, 2026 20:01

Address comments

5be4d3d

Remove unused constructor

4a3ea27

Address commenst

f35c439

Update test to use a different pod

b6eb23f

esezen dismissed stale reviews from Alexey-Pavlov and Mudaafi via b6eb23f February 27, 2026 19:28

esezen requested review from Alexey-Pavlov and Mudaafi February 27, 2026 19:30

Alexey-Pavlov approved these changes Feb 27, 2026

View reviewed changes

esezen merged commit 0963f79 into main Mar 2, 2026
1 check failed

esezen deleted the cdx-373-netcore-update-fmtoptions branch March 2, 2026 12:27

Conversation

esezen commented Feb 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Alexey-Pavlov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Alexey-Pavlov left a comment

Choose a reason for hiding this comment

Uh oh!

Mudaafi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Mudaafi left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mudaafi Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

esezen Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Alexey-Pavlov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

esezen commented Feb 5, 2026 •

edited

Loading

Mudaafi left a comment •

edited

Loading