feat: Add option to allow http in serviceUrl, default false#460
Merged
esezen merged 2 commits intoJun 11, 2026
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds an opt-in capability to allow http:// service URLs (instead of force-upgrading to HTTPS), primarily for local/dev environments, and updates types/tests accordingly.
Changes:
- Extend
addHTTPSToStringto optionally keephttp://URLs whenallowHttpis enabled. - Add
allowHttpServiceUrlto client options typings and wire it throughConstructorIOinitialization. - Update/add unit tests covering default vs
allowHttpbehavior.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/utils/helpers.js | Adds allowHttp flag to preserve http:// URLs instead of upgrading to HTTPS. |
| src/types/index.d.ts | Exposes allowHttpServiceUrl?: boolean in the public constructor options type. |
| src/constructorio.js | Threads allowHttpServiceUrl into service URL normalization via addHTTPSToString. |
| spec/src/utils/helpers.js | Expands tests for addHTTPSToString to cover the new flag. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * @param {object} parameters - Parameters for client instantiation | ||
| * @param {string} parameters.apiKey - Constructor.io API key | ||
| * @param {string} [parameters.serviceUrl='https://ac.cnstrc.com'] - API URL endpoint | ||
| * @param {string} [parameters.allowHttpServiceUrl=false] - Allow HTTP protocol in API URL endpoint |
| expect(addHTTPSToString(testUrl)).to.equal(null); | ||
| }); | ||
| }); | ||
| describe('when allowHttp is not given', () => { |
| if (allowHttp) { | ||
| return url; | ||
| } | ||
| return url.replace('http', 'https'); |
Contributor
Author
There was a problem hiding this comment.
This is not possible, since in this branch the http:// prefix is confirmed, so didn't change the code unnecessarily. I added tests though to verify this remains the case.
xkr47
force-pushed
the
feat/option-to-allow-http-in-serviceUrl
branch
from
ad755d7 to
176bf87
Compare
xkr47
force-pushed
the
feat/option-to-allow-http-in-serviceUrl
branch
from
176bf87 to
c3aa875
Compare
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
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.
Would you approve adding an option to disable the "force https" feature from #309?
I have implemented a "fake Constructor" in our java backend so we can prototype changes to our catalog updates on our local machines without having to actually upload the catalog to a Constructor index. If not, we'll have keep using a fork of this repo, which would mean added maintenance work and delayed updates for the client library. Nevertheless it is net beneficial for us by letting us develop our Constructor experience in an agile manner.