-
Notifications
You must be signed in to change notification settings - Fork 4.5k
chore: Remove CS URL in client #37665
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
Changes from all commits
926da1b
06cc4b7
8b21083
519d95b
44cb01c
5c75824
f5b2201
0a1937e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,2 @@ | ||
| import { getAppsmithConfigs } from "ee/configs"; | ||
|
|
||
| const { cloudServicesBaseUrl: BASE_URL } = getAppsmithConfigs(); | ||
|
|
||
| export const authorizeDatasourceWithAppsmithToken = (appsmithToken: string) => | ||
| `${BASE_URL}/api/v1/integrations/oauth/authorize?appsmithToken=${appsmithToken}`; | ||
| `/api/v1/saas/authorize?appsmithToken=${appsmithToken}`; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| package com.appsmith.server.controllers; | ||
|
|
||
| import com.appsmith.server.configurations.CloudServicesConfig; | ||
| import com.appsmith.server.constants.Url; | ||
| import com.appsmith.server.controllers.ce.SaasControllerCE; | ||
| import com.appsmith.server.solutions.AuthenticationService; | ||
|
|
@@ -12,7 +13,7 @@ | |
| @RequestMapping(Url.SAAS_URL) | ||
| public class SaasController extends SaasControllerCE { | ||
|
|
||
| public SaasController(AuthenticationService authenticationService) { | ||
| super(authenticationService); | ||
| public SaasController(AuthenticationService authenticationService, CloudServicesConfig cloudServicesConfig) { | ||
| super(authenticationService, cloudServicesConfig); | ||
|
Comment on lines
+16
to
+17
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Constructor injection needs to be updated in SaasControllerCE The parent class SaasControllerCE is using
🔗 Analysis chainLGTM: Constructor properly handles new dependency The constructor correctly injects and passes the CloudServicesConfig to the parent class, aligning with the goal of making the server the source of truth for CS URLs. Let's verify the parent class is ready for this change: 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Verify SaasControllerCE has matching constructor signature
ast-grep --pattern 'class SaasControllerCE {
$$$
SaasControllerCE($_, CloudServicesConfig $_) {
$$$
}
$$$
}'
Length of output: 122 Script: #!/bin/bash
# Let's try to find the parent class first
rg -l "SaasControllerCE" --type java
# Then examine its content
rg "class SaasControllerCE" -A 10 --type java
Length of output: 1762 Script: #!/bin/bash
# Let's also check for any CloudServicesConfig usage in the parent class
rg "CloudServicesConfig" --type java -A 2
Length of output: 21203 Script: #!/bin/bash
# Let's find the actual file path for SaasControllerCE
fd "SaasControllerCE.java"
Length of output: 124 Script: #!/bin/bash
# Let's check the constructor in SaasControllerCE
cat app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/SaasControllerCE.java
Length of output: 3719 |
||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.