Skip to content
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.

Sign up for GitHub

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

chore: Remove CS URL in client #37665

Merged
merged 8 commits into from
Nov 27, 2024
Merged

chore: Remove CS URL in client #37665

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
926da1b
Remove CS URL in client
sharat87 Nov 24, 2024
06cc4b7
fmt
sharat87 Nov 24, 2024
8b21083
Looks like it isn't used at all 🤔
sharat87 Nov 24, 2024
519d95b
Discard changes to app/server/appsmith-server/src/main/java/com/appsm…
sharat87 Nov 24, 2024
44cb01c
Revert "Discard changes to app/server/appsmith-server/src/main/java/c…
sharat87 Nov 25, 2024
5c75824
Revert "Looks like it isn't used at all 🤔"
sharat87 Nov 25, 2024
f5b2201
Update app/server/appsmith-server/src/main/java/com/appsmith/server/c…
sharat87 Nov 26, 2024
0a1937e
fmt
sharat87 Nov 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 5 additions & 7 deletions .github/workflows/ci-test-custom-script.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,10 +166,8 @@ jobs:
-e APPSMITH_JWT_SECRET=appsmith-cloud-services-jwt-secret-dummy-key \
-e APPSMITH_ENCRYPTION_SALT=encryption-salt \
-e APPSMITH_ENCRYPTION_PASSWORD=encryption-password \
-e APPSMITH_CLOUD_SERVICES_URL=https://cs-dev.appsmith.com \
-e APPSMITH_CUSTOMER_PORTAL_URL=https://dev.appsmith.com \
-e APPSMITH_CLOUD_SERVICES_BASE_URL=https://cs-dev.appsmith.com \
-e APPSMITH_CLOUD_SERVER_BASE_URL=https://release.app.appsmith.com \
-e AUTH0_ISSUER_URL=https://login.release-customer.appsmith.com/ \
-e AUTH0_CLIENT_ID=dummy-client-id \
-e AUTH0_CLIENT_SECRET=dummy-secret-id \
Expand Down Expand Up @@ -198,7 +196,7 @@ jobs:
uses: actions/setup-node@v4
with:
node-version-file: app/client/package.json

- name: Check DB URL
if: steps.run_result.outputs.run_result != 'success'
run: |
Expand Down Expand Up @@ -457,20 +455,20 @@ jobs:
name: server-logs-${{ matrix.job }}
path: app/server/server-logs.log
overwrite: true

- name: Collect docker log as file
if: always()
run: |
docker logs appsmith >& app/server/docker-logs.log
- name: Upload server docker logs bundle on failure

- name: Upload server docker logs bundle on failure
uses: actions/upload-artifact@v4
if: always()
with:
name: docker-logs-${{ matrix.job }}
path: app/server/docker-logs.log
overwrite: true

sharat87 marked this conversation as resolved.
Show resolved Hide resolved
# Set status = success
- name: Save the status of the run
run: |
Expand Down
12 changes: 5 additions & 7 deletions .github/workflows/ci-test-limited-with-count.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ on:
run_count:
description: 'Number of times to repeat the test run'
required: false
type: number
type: number
default: 1
workflow_call:
inputs:
Expand Down Expand Up @@ -176,10 +176,8 @@ jobs:
-e APPSMITH_JWT_SECRET=appsmith-cloud-services-jwt-secret-dummy-key \
-e APPSMITH_ENCRYPTION_SALT=encryption-salt \
-e APPSMITH_ENCRYPTION_PASSWORD=encryption-password \
-e APPSMITH_CLOUD_SERVICES_URL=https://cs-dev.appsmith.com \
-e APPSMITH_CUSTOMER_PORTAL_URL=https://dev.appsmith.com \
-e APPSMITH_CLOUD_SERVICES_BASE_URL=https://cs-dev.appsmith.com \
-e APPSMITH_CLOUD_SERVER_BASE_URL=https://release.app.appsmith.com \
-e AUTH0_ISSUER_URL=https://login.release-customer.appsmith.com/ \
-e AUTH0_CLIENT_ID=dummy-client-id \
-e AUTH0_CLIENT_SECRET=dummy-secret-id \
Expand Down Expand Up @@ -351,18 +349,18 @@ jobs:
npx cypress-repeat-pro run -n ${{ inputs.run_count }} --force \
--spec ${{ env.specs_to_run }} \
--config-file "cypress_ci_custom.config.ts"
cat cy-repeat-summary.txt
cat cy-repeat-summary.txt
# Define the path for the failure flag file
FAILURE_FLAG_FILE="ci_test_status.txt"

# Check for test results and store the status in the file
if ! grep -q "Total Failed: 0" cy-repeat-summary.txt; then
echo "ci_test_failed=true" > "$FAILURE_FLAG_FILE"
else
echo "ci_test_failed=false" > "$FAILURE_FLAG_FILE"
fi
cat "$FAILURE_FLAG_FILE"

sharat87 marked this conversation as resolved.
Show resolved Hide resolved
- name: Trim number of cypress log files
if: failure()
run: |
Expand All @@ -375,7 +373,7 @@ jobs:
name: cypress-repeat-logs
path: ${{ github.workspace }}/app/client/cy-repeat-summary.txt
overwrite: true

- name: Upload ci_test_status.txt artifact
if: always()
uses: actions/upload-artifact@v4
Expand Down
2 changes: 0 additions & 2 deletions .github/workflows/ci-test-limited.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,10 +166,8 @@ jobs:
-e APPSMITH_JWT_SECRET=appsmith-cloud-services-jwt-secret-dummy-key \
-e APPSMITH_ENCRYPTION_SALT=encryption-salt \
-e APPSMITH_ENCRYPTION_PASSWORD=encryption-password \
-e APPSMITH_CLOUD_SERVICES_URL=https://cs-dev.appsmith.com \
-e APPSMITH_CUSTOMER_PORTAL_URL=https://dev.appsmith.com \
-e APPSMITH_CLOUD_SERVICES_BASE_URL=https://cs-dev.appsmith.com \
-e APPSMITH_CLOUD_SERVER_BASE_URL=https://release.app.appsmith.com \
-e AUTH0_ISSUER_URL=https://login.release-customer.appsmith.com/ \
-e AUTH0_CLIENT_ID=dummy-client-id \
-e AUTH0_CLIENT_SECRET=dummy-secret-id \
Expand Down
1 change: 0 additions & 1 deletion app/client/docker/templates/nginx-app.conf.template
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ server {
sub_filter __APPSMITH_VERSION_RELEASE_DATE__ '${APPSMITH_VERSION_RELEASE_DATE}';
sub_filter __APPSMITH_INTERCOM_APP_ID__ '${APPSMITH_INTERCOM_APP_ID}';
sub_filter __APPSMITH_MAIL_ENABLED__ '${APPSMITH_MAIL_ENABLED}';
sub_filter __APPSMITH_CLOUD_SERVICES_BASE_URL__ '${APPSMITH_CLOUD_SERVICES_BASE_URL}';
sub_filter __APPSMITH_RECAPTCHA_SITE_KEY__ '${APPSMITH_RECAPTCHA_SITE_KEY}';
sub_filter __APPSMITH_DISABLE_INTERCOM__ '${APPSMITH_DISABLE_INTERCOM}';
sub_filter __APPSMITH_ZIPY_SDK_KEY__ '${APPSMITH_ZIPY_SDK_KEY}';
Expand Down
3 changes: 0 additions & 3 deletions app/client/public/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,9 +263,6 @@
},
intercomAppID: INTERCOM_APP_ID,
mailEnabled: parseConfig('{{env "APPSMITH_MAIL_ENABLED"}}'),
cloudServicesBaseUrl:
parseConfig('{{env "APPSMITH_CLOUD_SERVICES_BASE_URL"}}') ||
"https://cs.appsmith.com",
googleRecaptchaSiteKey: parseConfig('{{env "APPSMITH_RECAPTCHA_SITE_KEY"}}'),
hideWatermark: parseConfig('{{env "APPSMITH_HIDE_WATERMARK"}}'),
disableIframeWidgetSandbox: parseConfig(
Expand Down
6 changes: 1 addition & 5 deletions app/client/src/api/CloudServicesApi.ts
View file
Open in desktop
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}`;
6 changes: 0 additions & 6 deletions app/client/src/ce/configs/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ export interface INJECTED_CONFIGS {
};
intercomAppID: string;
mailEnabled: boolean;
cloudServicesBaseUrl: string;
googleRecaptchaSiteKey: string;
supportEmail: string;
disableIframeWidgetSandbox: boolean;
Expand Down Expand Up @@ -116,7 +115,6 @@ export const getConfigsFromEnvVars = (): INJECTED_CONFIGS => {
mailEnabled: process.env.REACT_APP_MAIL_ENABLED
? process.env.REACT_APP_MAIL_ENABLED.length > 0
: false,
cloudServicesBaseUrl: process.env.REACT_APP_CLOUD_SERVICES_BASE_URL || "",
googleRecaptchaSiteKey:
process.env.REACT_APP_GOOGLE_RECAPTCHA_SITE_KEY || "",
supportEmail: process.env.APPSMITH_SUPPORT_EMAIL || "support@appsmith.com",
Expand Down Expand Up @@ -287,10 +285,6 @@ export const getAppsmithConfigs = (): AppsmithUIConfigs => {
ENV_CONFIG.intercomAppID || APPSMITH_FEATURE_CONFIGS?.intercomAppID || "",
mailEnabled:
ENV_CONFIG.mailEnabled || APPSMITH_FEATURE_CONFIGS?.mailEnabled || false,
cloudServicesBaseUrl:
ENV_CONFIG.cloudServicesBaseUrl ||
APPSMITH_FEATURE_CONFIGS?.cloudServicesBaseUrl ||
"",
appsmithSupportEmail: ENV_CONFIG.supportEmail,
disableIframeWidgetSandbox:
ENV_CONFIG.disableIframeWidgetSandbox ||
Expand Down
2 changes: 0 additions & 2 deletions app/client/src/ce/configs/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,6 @@ export interface AppsmithUIConfigs {
intercomAppID: string;
mailEnabled: boolean;

cloudServicesBaseUrl: string;

googleRecaptchaSiteKey: {
enabled: boolean;
apiKey: string;
Expand Down
5 changes: 3 additions & 2 deletions app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/SaasController.java
View file
Open in desktop
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;
Expand All @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The 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 @RequiredArgsConstructor for field injection, but the child class SaasController is trying to use constructor injection. This mismatch needs to be addressed by either:

  • Adding an explicit constructor in SaasControllerCE to match the child class, or
  • Using @RequiredArgsConstructor in the child class instead
🔗 Analysis chain

LGTM: 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 executed

The 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

}
}
23 changes: 18 additions & 5 deletions ...er/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/SaasControllerCE.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,18 @@

import com.appsmith.external.models.OAuth2ResponseDTO;
import com.appsmith.external.views.Views;
import com.appsmith.server.configurations.CloudServicesConfig;
import com.appsmith.server.constants.FieldName;
import com.appsmith.server.constants.Url;
import com.appsmith.server.dtos.RequestAppsmithTokenDTO;
import com.appsmith.server.dtos.ResponseDTO;
import com.appsmith.server.solutions.AuthenticationService;
import com.fasterxml.jackson.annotation.JsonView;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.HttpStatus;
import org.springframework.http.server.reactive.ServerHttpResponse;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestBody;
Expand All @@ -22,14 +25,12 @@

@Slf4j
@RequestMapping(Url.SAAS_URL)
@RequiredArgsConstructor
public class SaasControllerCE {

private final AuthenticationService authenticationService;

@Autowired
public SaasControllerCE(AuthenticationService authenticationService) {
this.authenticationService = authenticationService;
}
private final CloudServicesConfig cloudServicesConfig;

@JsonView(Views.Public.class)
@PostMapping("/{datasourceId}/oauth")
Expand Down Expand Up @@ -63,4 +64,16 @@ public Mono<ResponseDTO<OAuth2ResponseDTO>> getAccessToken(
.getAccessTokenFromCloud(datasourceId, environmentId, appsmithToken)
.map(datasource -> new ResponseDTO<>(HttpStatus.OK.value(), datasource, null));
}

@GetMapping("authorize")
public Mono<Void> redirectForAuthorize(ServerWebExchange exchange, @RequestParam String appsmithToken) {
final String url = cloudServicesConfig.getBaseUrl() + "/api/v1/integrations/oauth/authorize?appsmithToken="
+ appsmithToken;

ServerHttpResponse response = exchange.getResponse();
response.setStatusCode(HttpStatus.TEMPORARY_REDIRECT);
response.getHeaders().set("Location", url);

return response.writeWith(Mono.just(response.bufferFactory().wrap(new byte[] {})));
}
sharat87 marked this conversation as resolved.
Show resolved Hide resolved
}
Loading