Skip to content

feat: Add config-manager push connector-mappings command#79

Open
dallinjsevy wants to merge 6 commits into
mainfrom
feature/config-manager-push-connector-mappings
Open

feat: Add config-manager push connector-mappings command#79
dallinjsevy wants to merge 6 commits into
mainfrom
feature/config-manager-push-connector-mappings

Conversation

@dallinjsevy
Copy link
Copy Markdown

No description provided.

@dallinjsevy dallinjsevy force-pushed the feature/config-manager-push-connector-mappings branch 4 times, most recently from cc181ee to a974fab Compare March 25, 2026 22:10
@phalestrivir phalestrivir force-pushed the feature/config-manager-push-connector-mappings branch 2 times, most recently from 801b950 to 19c8063 Compare March 31, 2026 22:03
Copy link
Copy Markdown

@phalestrivir phalestrivir left a comment

Choose a reason for hiding this comment

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

See comments below (there is a bug that needs to be fixed). I also removed some of your test configuration that you included since some of it was client related, plus we only need 2 mappings to test with anyways (I left two in that looked like test mappings that you might've created that would be good for testing with). For the extract_script_test mappings, it looks like you got some scripts already in there, but I would go through the configuration and make sure you have an example of all possible scripts for the mapping to test extraction with since there are several scripts in mappings and it's possible you missed a few (I would probably ask Sean Ku for help creating a good test mapping since he's familiar with what all the possible scripts are that could get extracted). I'm not sure how many scripts config-manager supports extracting, but we'll want to support importing those ones in particular.

Comment thread src/configManagerOps/FrConfigConnectorMappingOps.ts Outdated
@phalestrivir phalestrivir force-pushed the feature/config-manager-push-connector-mappings branch from e40c115 to 7bd8d12 Compare April 3, 2026 18:19
Copy link
Copy Markdown

@phalestrivir phalestrivir left a comment

Choose a reason for hiding this comment

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

I just rebased the branch. I deleted your old testing configuration in favor of some that I created that have all possible mapping scripts in them so you can test to verify your code works for importing them all.

Comment thread src/configManagerOps/FrConfigConnectorMappingOps.ts Outdated
Comment thread src/configManagerOps/FrConfigConnectorMappingOps.ts
Comment thread src/configManagerOps/FrConfigConnectorMappingOps.ts Outdated
Comment thread src/configManagerOps/FrConfigConnectorMappingOps.ts Outdated
Comment thread test/e2e/exports/fr-config-manager/cloud/sync/mappings/test/test.json Outdated
Comment thread test/e2e/mocks/github_584874290/recording.har
Comment thread test/e2e/mocks/npmjs_1455397529/recording.har
Comment thread test/e2e/config-manager-push-connector-mappings.e2e.test.js
@phalestrivir phalestrivir force-pushed the feature/config-manager-push-connector-mappings branch from 4320d96 to a8667bf Compare April 9, 2026 21:44
Copy link
Copy Markdown

@phalestrivir phalestrivir left a comment

Choose a reason for hiding this comment

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

A few more changes needed, but everything else looks good

Comment thread test/e2e/config-manager-push-connector-mappings.e2e.test.js Outdated
Comment thread src/configManagerOps/FrConfigConnectorMappingOps.ts Outdated
Comment thread src/configManagerOps/FrConfigConnectorMappingOps.ts Outdated
Comment thread src/configManagerOps/FrConfigConnectorMappingOps.ts Outdated
Comment thread src/configManagerOps/FrConfigConnectorMappingOps.ts Outdated
@dallinjsevy dallinjsevy force-pushed the feature/config-manager-push-connector-mappings branch from a8667bf to 3267952 Compare April 10, 2026 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants