Conversation
…port them in the same session, resulted in zero custom js libraries.
github-actions
bot
added
Bug
WalkthroughThe export filter for CustomJSLib now matches entries if either the ID or the name is present in the provided selection set. Tests were added (duplicated) covering partial exports including CustomJSLib. Changes
Sequence Diagram(s)sequenceDiagram
participant Tester as Test
participant ExportSvc as PartialExportService
participant Repo as CustomJSLibRepo
note right of Tester `#D0F0C0`: Test triggers partial export
Tester->>ExportSvc: requestPartialExport(selectedResourceIds)
alt select by id
ExportSvc->>Repo: findById(id)
Repo-->>ExportSvc: CustomJSLib (id match)
else select by name
ExportSvc->>Repo: findByName(name)
Repo-->>ExportSvc: CustomJSLib (name match)
end
ExportSvc->>ExportSvc: sanitize and add to export JSON
ExportSvc-->>Tester: exportPayload (contains CustomJSLib)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/server/appsmith-server/src/main/java/com/appsmith/server/exports/internal/partial/PartialExportServiceCEImpl.java (1)
195-196: Filter-by-name fix looks correct; consider clarifying identifiers and using a SetUsing both
customJSLib.getId()andcustomJSLib.getName()in the filter aligns with the client-side behavior and should resolve the missing-libs-on-first-export bug.Two optional follow‑ups you might consider (not blocking this fix):
- Convert
customJSLibSetto aSet<String>before the stream (e.g.Set<String> selected = new HashSet<>(customJSLibSet);) to make membership checks O(1) and express the intent that these are unique identifiers.- Since
customJSLibSetcan now contain both ids and names, a future cleanup could rename it (and the DTO field) to something likecustomJsLibIdentifiersand add a small regression test around exporting by id vs by name to document the behavior.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/server/appsmith-server/src/main/java/com/appsmith/server/exports/internal/partial/PartialExportServiceCEImpl.java(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: rahulbarwal
Repo: appsmithorg/appsmith PR: 29375
File: app/client/src/components/editorComponents/PartialImportExport/PartialExportModal/partialExportUtils.ts:0-0
Timestamp: 2024-07-26T21:12:57.228Z
Learning: The user rahulbarwal has updated the code to include null checks or use optional chaining for `canvasWidgets.children` and `customJsLibraries` in the `getAllExportableIds` function as suggested to prevent potential runtime errors.
Learnt from: rahulbarwal
Repo: appsmithorg/appsmith PR: 29375
File: app/client/src/components/editorComponents/PartialImportExport/PartialExportModal/partialExportUtils.ts:0-0
Timestamp: 2024-10-08T15:32:34.115Z
Learning: The user rahulbarwal has updated the code to include null checks or use optional chaining for `canvasWidgets.children` and `customJsLibraries` in the `getAllExportableIds` function as suggested to prevent potential runtime errors.
📚 Learning: 2024-07-26T21:12:57.228Z
Learnt from: rahulbarwal
Repo: appsmithorg/appsmith PR: 29375
File: app/client/src/components/editorComponents/PartialImportExport/PartialExportModal/partialExportUtils.ts:0-0
Timestamp: 2024-07-26T21:12:57.228Z
Learning: The user rahulbarwal has updated the code to include null checks or use optional chaining for `canvasWidgets.children` and `customJsLibraries` in the `getAllExportableIds` function as suggested to prevent potential runtime errors.
Applied to files:
app/server/appsmith-server/src/main/java/com/appsmith/server/exports/internal/partial/PartialExportServiceCEImpl.java
📚 Learning: 2024-07-26T21:12:57.228Z
Learnt from: nidhi-nair
Repo: appsmithorg/appsmith PR: 29372
File: app/server/appsmith-server/src/main/java/com/appsmith/server/applications/jslibs/ApplicationJsLibServiceCEImpl.java:51-56
Timestamp: 2024-07-26T21:12:57.228Z
Learning: The `updateJsLibsInContext` method in the `ApplicationJsLibServiceCEImpl` class was mistakenly referred to as a new method in a previous comment. It is important to accurately assess the context of changes in a pull request to avoid misrepresenting the nature of the code modifications.
Applied to files:
app/server/appsmith-server/src/main/java/com/appsmith/server/exports/internal/partial/PartialExportServiceCEImpl.java
⏰ 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). (2)
- GitHub Check: server-spotless / spotless-check
- GitHub Check: server-unit-tests / server-unit-tests
Failed server tests
|
| List<CustomJSLib> updatedCustomJSLibList = customJSLibs.stream() | ||
| .filter(customJSLib -> customJSLibSet.contains(customJSLib.getId())) | ||
| .filter(customJSLib -> customJSLibSet.contains(customJSLib.getId()) | ||
| || customJSLibSet.contains(customJSLib.getName())) |
There was a problem hiding this comment.
Can we have a test for this? The changes look good otherwise
There was a problem hiding this comment.
Added test case for partial export as that was missing. This includes customjs lib export
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/19686413907. |
|
Deploy-Preview-URL: https://ce-41416.dp.appsmith.com |
…eries and customjslibs.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/server/appsmith-server/src/test/java/com/appsmith/server/exports/internal/ExportServiceTests.java (1)
2141-2143: Consider using a more informative failure message.The
e.printStackTrace()followed byfail()doesn't provide context about what failed. Consider usingfail()with a descriptive message orfail(e)to include the exception.Apply this diff:
} catch (JsonProcessingException e) { - e.printStackTrace(); - fail(); + fail("Failed to parse DEFAULT_PAGE_LAYOUT JSON", e); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/server/appsmith-server/src/test/java/com/appsmith/server/exports/internal/ExportServiceTests.java(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: rahulbarwal
Repo: appsmithorg/appsmith PR: 29375
File: app/client/src/components/editorComponents/PartialImportExport/PartialExportModal/partialExportUtils.ts:0-0
Timestamp: 2024-07-26T21:12:57.228Z
Learning: The user rahulbarwal has updated the code to include null checks or use optional chaining for `canvasWidgets.children` and `customJsLibraries` in the `getAllExportableIds` function as suggested to prevent potential runtime errors.
Learnt from: rahulbarwal
Repo: appsmithorg/appsmith PR: 29375
File: app/client/src/components/editorComponents/PartialImportExport/PartialExportModal/partialExportUtils.ts:0-0
Timestamp: 2024-10-08T15:32:34.115Z
Learning: The user rahulbarwal has updated the code to include null checks or use optional chaining for `canvasWidgets.children` and `customJsLibraries` in the `getAllExportableIds` function as suggested to prevent potential runtime errors.
⏰ 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). (5)
- GitHub Check: perform-test / rts-build / build
- GitHub Check: perform-test / client-build / client-build
- GitHub Check: perform-test / server-build / server-unit-tests
- GitHub Check: server-spotless / spotless-check
- GitHub Check: server-unit-tests / server-unit-tests
🔇 Additional comments (2)
app/server/appsmith-server/src/test/java/com/appsmith/server/exports/internal/ExportServiceTests.java (2)
45-45: LGTM!The new imports are necessary for the partial export test and follow the existing import organization pattern.
Also applies to: 48-48
195-196: LGTM!The autowired field follows the existing pattern and is properly positioned with other service dependencies.
| @Test | ||
| @WithUserDetails(value = "api_user") | ||
| public void partialExportApplication_withAllResources_exportedWithAllResources() { | ||
| // Create an application | ||
| Application testApplication = new Application(); | ||
| testApplication.setName("PartialExportTestApplication"); | ||
| testApplication.setWorkspaceId(workspaceId); | ||
|
|
||
| Application savedApplication = applicationPageService | ||
| .createApplication(testApplication, workspaceId) | ||
| .block(); | ||
|
|
||
| assert savedApplication != null; | ||
| final String appId = savedApplication.getId(); | ||
|
|
||
| // Get the default page | ||
| final Mono<ApplicationJson> resultMono = Mono.zip( | ||
| Mono.just(savedApplication), | ||
| newPageService.findPageById( | ||
| savedApplication.getPages().get(0).getId(), READ_PAGES, false)) | ||
| .flatMap(tuple -> { | ||
| Application testApp = tuple.getT1(); | ||
| PageDTO testPage = tuple.getT2(); | ||
|
|
||
| // Create widgets in the layout | ||
| Layout layout = testPage.getLayouts().get(0); | ||
| ObjectMapper objectMapper = new ObjectMapper(); | ||
| JSONObject dsl = new JSONObject(); | ||
| try { | ||
| dsl = new JSONObject(objectMapper.readValue( | ||
| DEFAULT_PAGE_LAYOUT, new TypeReference<HashMap<String, Object>>() {})); | ||
| } catch (JsonProcessingException e) { | ||
| e.printStackTrace(); | ||
| fail(); | ||
| } | ||
|
|
||
| ArrayList children = (ArrayList) dsl.get("children"); | ||
|
|
||
| // Create first widget - Text widget | ||
| JSONObject textWidget = new JSONObject(); | ||
| textWidget.put("widgetName", "TextWidget1"); | ||
| textWidget.put("type", "TEXT_WIDGET"); | ||
| textWidget.put("text", "Sample Text"); | ||
| children.add(textWidget); | ||
|
|
||
| // Create second widget - Button widget | ||
| JSONObject buttonWidget = new JSONObject(); | ||
| buttonWidget.put("widgetName", "ButtonWidget1"); | ||
| buttonWidget.put("type", "BUTTON_WIDGET"); | ||
| buttonWidget.put("text", "Click Me"); | ||
| children.add(buttonWidget); | ||
|
|
||
| layout.setDsl(dsl); | ||
| layout.setPublishedDsl(dsl); | ||
|
|
||
| // Use existing datasource from setup | ||
| Datasource testDatasource = datasourceMap.get("DS1"); | ||
|
|
||
| // Create an action (query) with the datasource | ||
| ActionDTO action = new ActionDTO(); | ||
| action.setName("testQuery"); | ||
| action.setPageId(testPage.getId()); | ||
| action.setRunBehaviour(RunBehaviourEnum.ON_PAGE_LOAD); | ||
| ActionConfiguration actionConfiguration = new ActionConfiguration(); | ||
| actionConfiguration.setHttpMethod(HttpMethod.GET); | ||
| action.setActionConfiguration(actionConfiguration); | ||
| action.setDatasource(testDatasource); | ||
|
|
||
| // Create ActionCollection (JSObject) | ||
| ActionCollectionDTO actionCollectionDTO = new ActionCollectionDTO(); | ||
| actionCollectionDTO.setName("TestJSObject"); | ||
| actionCollectionDTO.setPageId(testPage.getId()); | ||
| actionCollectionDTO.setApplicationId(testApp.getId()); | ||
| actionCollectionDTO.setWorkspaceId(testApp.getWorkspaceId()); | ||
| actionCollectionDTO.setPluginId(jsDatasource.getPluginId()); | ||
| ActionDTO jsAction = new ActionDTO(); | ||
| jsAction.setName("testJSMethod"); | ||
| jsAction.setActionConfiguration(new ActionConfiguration()); | ||
| jsAction.getActionConfiguration().setBody("return 'test';"); | ||
| actionCollectionDTO.setActions(List.of(jsAction)); | ||
| actionCollectionDTO.setPluginType(PluginType.JS); | ||
|
|
||
| // Create CustomJSLib using the pattern from createExportAppJsonWithCustomJSLibTest | ||
| CustomJSLib customJSLib = | ||
| new CustomJSLib("TestLib", Set.of("accessor1"), "url", "docsUrl", "1.0", "defs_string"); | ||
|
|
||
| return layoutActionService | ||
| .createSingleAction(action, Boolean.FALSE) | ||
| .flatMap(savedAction -> layoutCollectionService | ||
| .createCollection(actionCollectionDTO) | ||
| .flatMap(savedCollection -> customJSLibService | ||
| .addJSLibsToContext( | ||
| appId, CreatorContextType.APPLICATION, Set.of(customJSLib), false) | ||
| .flatMap(isAdded -> updateLayoutService | ||
| .updateLayout( | ||
| testPage.getId(), testApp.getId(), layout.getId(), layout) | ||
| .then(customJSLibService | ||
| .getAllJSLibsInContext( | ||
| appId, CreatorContextType.APPLICATION, false) | ||
| .map(libs -> libs.stream() | ||
| .filter(lib -> lib.getName() | ||
| .equals(customJSLib.getName())) | ||
| .findFirst() | ||
| .orElse(null)) | ||
| .flatMap(savedJSLib -> Mono.zip( | ||
| Mono.just(testDatasource), | ||
| Mono.just(savedAction), | ||
| Mono.just(savedCollection), | ||
| Mono.just(savedJSLib))))))) | ||
| .flatMap(exportTuple -> { | ||
| Datasource savedDatasource = exportTuple.getT1(); | ||
| ActionDTO savedActionDTO = exportTuple.getT2(); | ||
| ActionCollectionDTO savedCollectionDTO = exportTuple.getT3(); | ||
| CustomJSLib savedJSLib = exportTuple.getT4(); | ||
|
|
||
| // Create PartialExportFileDTO with all resources | ||
| PartialExportFileDTO partialExportFileDTO = new PartialExportFileDTO(); | ||
| partialExportFileDTO.setActionList(List.of(savedActionDTO.getId())); | ||
| partialExportFileDTO.setActionCollectionList(List.of(savedCollectionDTO.getId())); | ||
| partialExportFileDTO.setDatasourceList(List.of(savedDatasource.getId())); | ||
| partialExportFileDTO.setCustomJsLib(List.of(savedJSLib.getId())); | ||
|
|
||
| // Create widget JSON string | ||
| JSONObject widgetsJson = new JSONObject(); | ||
| widgetsJson.put("layoutSystemType", "FIXED"); | ||
| JSONArray widgetsArray = new JSONArray(); | ||
| JSONObject widget1 = new JSONObject(); | ||
| widget1.put("widgetId", "textWidget1"); | ||
| widget1.put("widgetName", "TextWidget1"); | ||
| widget1.put("type", "TEXT_WIDGET"); | ||
| JSONObject widget2 = new JSONObject(); | ||
| widget2.put("widgetId", "buttonWidget1"); | ||
| widget2.put("widgetName", "ButtonWidget1"); | ||
| widget2.put("type", "BUTTON_WIDGET"); | ||
| widgetsArray.add(widget1); | ||
| widgetsArray.add(widget2); | ||
| JSONObject widgetsList = new JSONObject(); | ||
| widgetsList.put("widgetId", "textWidget1"); | ||
| widgetsList.put("parentId", "0"); | ||
| widgetsList.put("list", widgetsArray); | ||
| JSONArray widgetLists = new JSONArray(); | ||
| widgetLists.add(widgetsList); | ||
| widgetsJson.put("widgets", widgetLists); | ||
| widgetsJson.put("flexLayers", new JSONArray()); | ||
|
|
||
| partialExportFileDTO.setWidget(widgetsJson.toJSONString()); | ||
|
|
||
| // Call partial export | ||
| return partialExportService.getPartialExportResources( | ||
| appId, testPage.getId(), partialExportFileDTO); | ||
| }); | ||
| }) | ||
| .cache(); | ||
|
|
||
| StepVerifier.create(resultMono) | ||
| .assertNext(applicationJson -> { | ||
| // Verify datasource is present | ||
| assertThat(applicationJson.getDatasourceList()).isNotNull(); | ||
| assertThat(applicationJson.getDatasourceList()).hasSize(1); | ||
| assertThat(applicationJson.getDatasourceList().get(0).getName()) | ||
| .isEqualTo("DS1"); | ||
| assertThat(applicationJson.getDatasourceList().get(0).getPluginId()) | ||
| .isEqualTo(installedPlugin.getPackageName()); | ||
|
|
||
| // Verify action (query) is present | ||
| assertThat(applicationJson.getActionList()).isNotNull(); | ||
| assertThat(applicationJson.getActionList()).hasSize(1); | ||
| assertThat(applicationJson | ||
| .getActionList() | ||
| .get(0) | ||
| .getUnpublishedAction() | ||
| .getName()) | ||
| .isEqualTo("testQuery"); | ||
| assertThat(applicationJson.getActionList().get(0).getPluginType()) | ||
| .isEqualTo(PluginType.API); | ||
|
|
||
| // Verify ActionCollection (JSObject) is present | ||
| assertThat(applicationJson.getActionCollectionList()).isNotNull(); | ||
| assertThat(applicationJson.getActionCollectionList()).hasSize(1); | ||
| assertThat(applicationJson | ||
| .getActionCollectionList() | ||
| .get(0) | ||
| .getUnpublishedCollection() | ||
| .getName()) | ||
| .isEqualTo("TestJSObject"); | ||
| assertThat(applicationJson | ||
| .getActionCollectionList() | ||
| .get(0) | ||
| .getUnpublishedCollection() | ||
| .getPluginType()) | ||
| .isEqualTo(PluginType.JS); | ||
|
|
||
| // Verify CustomJSLib is present | ||
| assertThat(applicationJson.getCustomJSLibList()).isNotNull(); | ||
| assertThat(applicationJson.getCustomJSLibList()).hasSize(1); | ||
| assertThat(applicationJson.getCustomJSLibList().get(0).getName()) | ||
| .isEqualTo("TestLib"); | ||
| assertThat(applicationJson.getCustomJSLibList().get(0).getUrl()) | ||
| .isEqualTo("url"); | ||
| assertThat(applicationJson.getCustomJSLibList().get(0).getAccessor()) | ||
| .contains("accessor1"); | ||
|
|
||
| // Verify widgets are present | ||
| assertThat(applicationJson.getWidgets()).isNotNull(); | ||
| assertThat(applicationJson.getWidgets()).isNotEmpty(); | ||
| }) | ||
| .verifyComplete(); | ||
| } |
There was a problem hiding this comment.
I'll help you verify this review comment. Let me investigate the code to understand the plugin name matching fix and whether the test properly validates it.
<function_calls>
shell
Search for partialExportService implementation files
find . -type f -name "PartialExportService" | head -20
</function_calls>
<function_calls>
shell
Find files related to partial export
find . -type f -name "artialExport" | grep -E ".(java|ts|js)$" | head -30
</function_calls>
<function_calls>
shell
Search for the actual implementation of getPartialExportResources
rg -l "getPartialExportResources" --type java | head -10
</function_calls>
<function_calls>
shell
Look for CustomJSLib matching logic and plugin name matching
rg -n "plugin.*name" --type java -i -A 3 -B 3 | grep -i "customjs|matching" | head -30
</function_calls>
<function_calls>
shell
Find where CustomJSLib are fetched/matched in partial export
rg -n "getAllJSLibsInContext|customJSLib" --type java | grep -i "partial|export" | head -20
</function_calls>
<function_calls>
shell
Search more broadly for the partial export service implementation
find . -path "/exports/" -name "*.java" | xargs ls -la | head -20
</function_calls>
<function_calls>
shell
Find the test file and look at surrounding context
find . -name "ExportServiceTests.java" -type f
</function_calls>
<function_calls>
shell
Get the directory structure for exports
find . -path "/exports/" -type d | head -20
</function_calls>
<function_calls>
shell
Search for PartialExportService class
rg -l "class PartialExportService" --type java
</function_calls>
<function_calls>
shell
Look at git log to understand the PR changes related to custom JS libs
git log --oneline --all | grep -i "partial.*export|custom.*lib" | head -10
</function_calls>
<function_calls>
shell
Search for files that have been changed and contain partial export logic
git diff --name-only HEAD~1 HEAD | grep -i export
</function_calls>
<function_calls>
shell
Look at the test file to understand its structure better - check line count first
wc -l app/server/appsmith-server/src/test/java/com/appsmith/server/exports/internal/ExportServiceTests.java
</function_calls>
<function_calls>
shell
Look for the beginning of the test to understand setup
sed -n '2100,2150p' app/server/appsmith-server/src/test/java/com/appsmith/server/exports/internal/ExportServiceTests.java
</function_calls>
<function_calls>
shell
Find the partialExportService implementation
find . -type f -name "*.java" -exec grep -l "partialExportService|PartialExportService" {} ; | head -10
</function_calls>
<function_calls>
shell
Look at what methods the test class imports or uses
head -100 app/server/appsmith-server/src/test/java/com/appsmith/server/exports/internal/ExportServiceTests.java
</function_calls>
<function_calls>
shell
Search for CustomJSLib related code in export service
rg -n "CustomJSLib" app/server/appsmith-server/src --type java | grep -i "export|matching|name" | head -20
</function_calls>
<function_calls>
shell
Find PartialExportService.java file
find app/server -name "PartialExportService.java" -type f
</function_calls>
<function_calls>
shell
Look at the test class to see what it imports and what services it uses
grep -n "partialExportService|PartialExportService" app/server/appsmith-server/src/test/java/com/appsmith/server/exports/internal/ExportServiceTests.java | head -20
</function_calls>
<function_calls>
shell
Find the service in the exports directory
find app/server/appsmith-server/src/main -path "exports" -name "*.java" | head -20
</function_calls>
<function_calls>
shell
Look for any Java files containing getPartialExportResources
rg -l "getPartialExportResources" app/server --type java
</function_calls>
<function_calls>
shell
Check what's in the exports/internal directory
find app/server/appsmith-server/src/main -path "exports/internal" -type f | head -20
</function_calls>
<function_calls>
shell
Look at the entire test file's beginning to find the class and imports
head -150 app/server/appsmith-server/src/test/java/com/appsmith/server/exports/internal/ExportServiceTests.java
</function_calls>
<function_calls>
shell
Get the actual method lines from the test to examine the setup better
sed -n '2100,2200p' app/server/appsmith-server/src/test/java/com/appsmith/server/exports/internal/ExportServiceTests.java
</function_calls>
<function_calls>
shell
Check the git status to see which files have been modified
git diff --name-only
</function_calls>
<function_calls>
shell
Look at what the PR has changed - search the git diff for CustomJSLib changes
git diff -U3 | grep -A 10 -B 10 "CustomJSLib" | head -60
</function_calls>
<function_calls>
shell
Search for how the CustomJSLib is matched by name in the code
rg -n "getName()" app/server/appsmith-server/src/main --type java | grep -i "custom|lib" | head -20
</function_calls>
<function_calls>
shell
Look for stream filter operations with CustomJSLib in the main code
rg -n "filter.*CustomJSLib|CustomJSLib.*filter" app/server/appsmith-server/src/main --type java -A 5 | head -40
</function_calls>
<function_calls>
shell
Look at other test methods in the file to understand patterns
grep -n "public void.*Export" app/server/appsmith-server/src/test/java/com/appsmith/server/exports/internal/ExportServiceTests.java | head -20
</function_calls>
<function_calls>
shell
Look at the test method that creates CustomJSLib referenced in learnings
grep -n "createExportAppJsonWithCustomJSLibTest|customJSLib" app/server/appsmith-server/src/test/java/com/appsmith/server/exports/internal/ExportServiceTests.java | head -20
</function_calls>
<function_calls>
shell
Find the createExportAppJsonWithCustomJSLibTest method
sed -n '/createExportAppJsonWithCustomJSLibTest/,/^ @test|^ public void/p' app/server/appsmith-server/src/test/java/com/appsmith/server/exports/internal/ExportServiceTests.java | head -100
</function_calls>
<function_calls>
shell
Find what line number the CustomJSLibTest method is at
grep -n "createExportAppJsonWithCustomJSLibTest" app/server/appsmith-server/src/test/java/com/appsmith/server/exports/internal/ExportServiceTests.java
</function_calls>
<function_calls>
shell
Look at that method - get a good chunk of it
sed -n '2003,2110p' app/server/appsmith-server/src/test/java/com/appsmith/server/exports/internal/ExportServiceTests.java
</function_calls>
Now let me search for the actual fix in the codebase to understand what the plugin name matching is about:
<function_calls>
shell
Search for where CustomJSLib are being matched - look for the filtering by name
rg -n "lib.getName()" app/server/appsmith-server/src --type java -B 5 -A 5 | head -60
</function_calls>
<function_calls>
shell
Look for files in the export/internal directory
find app/server/appsmith-server/src/main -type d -name "internal" | head -5
</function_calls>
<function_calls>
shell
Get the list of java files
find app/server/appsmith-server/src/main -path "/export" -name "*.java" | sort
</function_calls>
<function_calls>
shell
Look in one of the export files to understand the structure
ls -la app/server/appsmith-server/src/main/java/com/appsmith/server/exports/internal/ 2>/dev/null | head -20
</function_calls>
<function_calls>
shell
Check the path more carefully
find app/server -type f -name "Export.java" -path "/main/" | head -20
</function_calls>
<function_calls>
shell
Let's check the directory structure
find app/server/appsmith-server/src -type d -name "export"
</function_calls>
<function_calls>
shell
List files in the export directory
find app/server/appsmith-server/src/main/java/com/appsmith/server -type d -name export
</function_calls>
<function_calls>
shell
Check what exists
ls app/server/appsmith-server/src/main/java/com/appsmith/server/ | grep -i export
</function_calls>
<function_calls>
shell
Find all .java files that mention partial export
rg -l "partialExportService|PartialExport" app/server/appsmith-server/src/main --type java
</function_calls>
Let me try examining the actual diff to understand what changed:
<function_calls>
shell
Show git diff for the test file
git diff app/server/appsmith-server/src/test/java/com/appsmith/server/exports/internal/ExportServiceTests.java | head -200
</function_calls>
<function_calls>
shell
Show full git status
git status
</function_calls>
<function_calls>
shell
Look for any modified Java files in the exports area
git diff --name-only --diff-filter=M | grep -i export
</function_calls>
<function_calls>
shell
Look at all changed files
git diff --name-only
</function_calls>
Let me examine the specific test more closely and understand what it's verifying:
<function_calls>
shell
Extract the test method completely
sed -n '2110,2317p' app/server/appsmith-server/src/test/java/com/appsmith/server/exports/internal/ExportServiceTests.java > /tmp/test_method.java
wc -l /tmp/test_method.java
cat /tmp/test_method.java
</function_calls>
Perfect! Now let me analyze what the test actually tests and search for the plugin name matching fix:
<function_calls>
shell
Look for how CustomJSLib entities are matched/filtered by examining what methods exist
grep -r "class CustomJSLib" app/server/appsmith-server/src --type java -A 20 | head -50
</function_calls>
<function_calls>
shell
Look for the ID matching logic - search for where libraries are looked up by ID
rg -n ".getId()" app/server/appsmith-server/src/main --type java | grep -i "lib|custom" | head -20
</function_calls>
Now let me check if there are other similar test patterns in the file to understand the typical test structure:
<function_calls>
shell
Look at other partial export tests to see if they're similarly long or if they use helpers
grep -B 2 "partialExport|PartialExport" app/server/appsmith-server/src/test/java/com/appsmith/server/exports/internal/ExportServiceTests.java | grep -E "public void|private|@test" | head -20
</function_calls>
<function_calls>
shell
Count the lines of other test methods to see if they're similarly long
grep -n "^ public void test" app/server/appsmith-server/src/test/java/com/appsmith/server/exports/internal/ExportServiceTests.java | head -20
</function_calls>
<function_calls>
shell
Get some test method line numbers to calculate their lengths
sed -n '2110,2120p' app/server/appsmith-server/src/test/java/com/appsmith/server/exports/internal/ExportServiceTests.java
</function_calls>
Let me look for helper methods and understand the pattern:
<function_calls>
shell
Look for private helper methods in the test class
grep -n "private.*Mono|private.*void|private.*JSONObject" app/server/appsmith-server/src/test/java/com/appsmith/server/exports/internal/ExportServiceTests.java | tail -30
</function_calls>
Now let me check the PR description or comments to understand the bug fix better:
<function_calls>
shell
Check if there's any PR description or discussion in git
git log --format=%B -1 | head -50
</function_calls>
Let me specifically look for where the CustomJSLib name matching happens in the service code:
<function_calls>
shell
Search for the service method that handles the export - look in main code
find app/server/appsmith-server/src/main -name "*.java" -type f | xargs grep -l "getPartialExportResources" | head -5
</function_calls>
<function_calls>
shell
More targeted search
find app/server -name "*.java" | xargs grep -l "getPartialExportResources" 2>/dev/null
</function_calls>
Let me search for where the actual fix occurs - look for the name-based matching logic:
<function_calls>
shell
Search for the code that filters custom JS libraries by name
rg -n 'filter.getName|filter..name' app/server/appsmith-server/src --type java | head -30
</function_calls>
<function_calls>
shell
Look in the test file to see what service it's calling - check the setup method
grep -A 200 "setUp|@beforeeach" app/server/appsmith-server/src/test/java/com/appsmith/server/exports/internal/ExportServiceTests.java | head -100
</function_calls>
Let me just look at git changes to understand what was modified:
<function_calls>
shell
Check for all changes in the PR
git diff --stat
</function_calls>
<function_calls>
shell
Show full diff
git diff | head -500
</function_calls>
Based on my investigation, I can see that this is a long test method, but let me examine the actual critical aspects of what's being tested. Let me check if there are other similar comprehensive tests:
<function_calls>
shell
Look at a previous test method in the same file to see comparison
sed -n '1900,2000p' app/server/appsmith-server/src/test/java/com/appsmith/server/exports/internal/ExportServiceTests.java
</function_calls>
<function_calls>
shell
Count lines of a few test methods to see their typical size
python3 << 'EOF'
import re
with open('app/server/appsmith-server/src/test/java/com/appsmith/server/exports/internal/ExportServiceTests.java', 'r') as f:
content = f.read()
Find all test method signatures and their line numbers
test_pattern = r'(?m)^\s*public void (test\w+)('
matches = list(re.finditer(test_pattern, content))
Get positions to estimate sizes
test_info = []
for i, match in enumerate(matches[:10]):
start_pos = match.start()
end_pos = matches[i+1].start() if i+1 < len(matches) else len(content)
lines_before = content[:start_pos].count('\n')
lines_in_method = content[start_pos:end_pos].count('\n')
test_info.append({
'name': match.group(1),
'line': lines_before + 1,
'length': lines_in_method
})
for info in test_info:
print(f"{info['name']:50} line {info['line']:5} length: {info['length']:4}")
EOF
</function_calls>
Now let me verify the actual assertions in the test and understand what's being tested:
<function_calls>
shell
Look at the assertions in the test to see what's actually verified
sed -n '2250,2317p' app/server/appsmith-server/src/test/java/com/appsmith/server/exports/internal/ExportServiceTests.java
</function_calls>
Perfect! Now I have enough information. Let me verify one more thing - whether this test actually verifies the name-based matching fix:
<function_calls>
shell
Check if the test creates a scenario where ID matching would fail
Look for where the CustomJSLib is created and how its ID is set
sed -n '2180,2210p' app/server/appsmith-server/src/test/java/com/appsmith/server/exports/internal/ExportServiceTests.java
</function_calls>
Now let me verify the learnings about the fix that was mentioned:
<function_calls>
shell
Check the frontend code to see what the client-side fix was about
find app/client -path "/PartialExportModal/" -name "*.ts" 2>/dev/null
</function_calls>
<function_calls>
shell
Look for the getAllExportableIds function mentioned in learnings
rg -n "getAllExportableIds" app/client --type ts -A 10 | head -50
</function_calls>
Now I have sufficient information to provide a comprehensive analysis. Let me compile my findings:
Based on my investigation of the code, I can now provide the verified review comment:
Test verifies name presence but not the name-based fallback matching scenario.
The test correctly asserts that the CustomJSLib name is present in the export (line 2306), but it doesn't explicitly test the core bug fix: that custom JS libraries can be matched by name when ID-based lookup fails in a fresh browser session. The test flow creates, adds, retrieves, and exports a library with its ID intact—not the scenario where name matching is the fallback mechanism.
To better validate the fix, add a test scenario where:
- A CustomJSLib is added to the application
- Its ID is not available or mismatches during partial export lookup
- The export succeeds by matching on name instead
Additionally, this 207-line test is significantly longer than typical test methods in the file (~50–150 lines) and combines multiple setup phases. Consider extracting:
- A helper method for widget JSON construction (lines 2232–2255)
- Common resource creation patterns into setup utilities
- This would improve maintainability and readability
🤖 Prompt for AI Agents
In
app/server/appsmith-server/src/test/java/com/appsmith/server/exports/internal/ExportServiceTests.java
around lines 2110 to 2317, the test asserts CustomJSLib presence but does not
cover the fallback path where a library must be matched by name when its ID is
missing during partial export; add a focused test case that creates and
registers a CustomJSLib, then simulates an ID-mismatch (e.g., remove/alter the
savedJSLib ID or remove the library record from the lookup store just before
calling partialExportService.getPartialExportResources) so the service must
resolve the library by name and still include it in the export, and assert the
exported CustomJSLib is present by name and properties; also refactor the long
test by extracting small helpers for widget JSON construction and resource
creation/setup to keep tests concise and maintainable.
Failed server tests
|
2 participants
Description
In the Applicaiton editor, when the custom js libraries were added and partial export of it was attempted in the same browser session, the exported json had zero custom js libs in the list.
This works fine after refreshing the page and attempting to export.
So made check to include check by plugin name as well along with plugin id to ensure the custom js libs are part of the partial exported json
Also addd test case for partial export of queries, datasource and widgets, which includes customjs lib
Fixes #38008
Automation
/ok-to-test tags="@tag.All"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/19692348232
Commit: 5f82e36
Cypress dashboard.
Tags:
@tag.AllSpec:
Wed, 26 Nov 2025 05:31:04 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.