-
Notifications
You must be signed in to change notification settings - Fork 37
feat: return workato webhook url in the response of /uiconfig #558
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: return workato webhook url in the response of /uiconfig #558
Conversation
WalkthroughThe changes introduce support for exposing a Workato webhook URL configuration property. A dependency version is updated, a new accessor method is added to the configuration wrapper, and the webhook URL is exposed in the UI config response with corresponding tests. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
pkg/controller/uiconfig_test.go (1)
68-70: Consider testing with a non-default value.The test currently validates the default empty string behavior. For more comprehensive coverage, consider setting an explicit
WorkatoWebHookURLvalue in the test configuration and asserting that value is correctly propagated to the response.Apply this diff to enhance the test:
s.OverrideApplicationDefault(testconfig.RegistrationService(). RegistrationServiceURL("https://signup.domain.com"), + WorkatoWebHookURL("https://hooks.workato.com/webhooks/test"), )Then verify the test asserts the expected value instead of just the default.
pkg/controller/uiconfig.go (1)
15-15: Add a documentation comment for the new field.The
UICanaryDeploymentWeightfield has a helpful comment explaining its purpose. Consider adding a similar comment forWorkatoWebHookURLto document the field's purpose in the API response.Apply this diff to add documentation:
UICanaryDeploymentWeight int `json:"uiCanaryDeploymentWeight"` + // WorkatoWebHookURL holds the URL for the Workato webhook endpoint used by the UI to push events. WorkatoWebHookURL string `json:"workatoWebHookURL"`
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (4)
go.mod(1 hunks)pkg/configuration/configuration.go(1 hunks)pkg/controller/uiconfig.go(2 hunks)pkg/controller/uiconfig_test.go(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: GolangCI Lint
- GitHub Check: Build & push operator bundles for e2e tests
- GitHub Check: Test with Coverage
🔇 Additional comments (4)
go.mod (1)
9-9: LGTM! Dependency update aligns with the related API PR.The version update for
codeready-toolchain/apiis necessary to pull in the newWorkatoWebHookURLfield definition from the related PR #492.pkg/controller/uiconfig_test.go (1)
37-38: LGTM! Minor formatting improvement.The trailing comma after
RegistrationServiceURLis a good practice for cleaner diffs when adding additional configuration parameters in the future.pkg/configuration/configuration.go (1)
150-152: LGTM! Accessor method follows established patterns.The
WorkatoWebHookURL()method implementation is consistent with other configuration accessors in the codebase, usingcommonconfig.GetStringwith an appropriate default empty string.pkg/controller/uiconfig.go (1)
33-33: LGTM! Field correctly populated from configuration.The
WorkatoWebHookURLfield is properly populated using the configuration accessor method, consistent with the existing pattern forUICanaryDeploymentWeight.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alexeykazakov, mfrancisc The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest fixed e2e test |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #558 +/- ##
==========================================
- Coverage 81.60% 81.54% -0.06%
==========================================
Files 46 46
Lines 2799 2802 +3
==========================================
+ Hits 2284 2285 +1
- Misses 429 431 +2
Partials 86 86
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|



Add
workatoWebHookURLfield in the/uiconfigapi response.It will be used by the UI to push events to workato.
Related PRs:
E2E tests:
Jira: https://issues.redhat.com/browse/SANDBOX-1484
Summary by CodeRabbit
New Features
Chores
Tests