sadpandajoe commented on code in PR #35828:
URL: https://github.com/apache/superset/pull/35828#discussion_r2462081205
##########
cypress/e2e/dashboards/export-special-chars.cy.js:
##########
@@ -0,0 +1,175 @@
+// cypress/e2e/dashboards/export-special-chars.cy.js
+// Superset Issue #31158 - E2E Test for Dashboard Export with Special
Characters
+
+describe("Dashboard Export API - Issue #31158", () => {
+ beforeEach(() => {
+ // We test the API directly without UI since we're in headless environment
+ // In a real Superset instance, these would interact with the UI
+ cy.session("admin", () => {
+ cy.visit("http://localhost:8088/login/";);
+ cy.get('input[id="username"]').type("admin");
+ cy.get('input[id="password"]').type("admin");
+ cy.get("button[type='submit']").click();
+ cy.url().should("include", "/dashboard/list");
+ });
+ });
+
+ it("should properly encode dashboard export filename with special
characters", () => {
+ // Test the export API endpoint directly
+ // This verifies the fix for issue #31158
+
+ // Test Case 1: Normal dashboard name (baseline)
+ cy.request({
+ method: "GET",
+ url: "http://localhost:8088/api/v1/dashboard/export";,
+ headers: {
+ Authorization: "Bearer admin",
+ },
+ }).then((response) => {
+ expect(response.status).to.eq(200);
+ expect(response.headers["content-type"]).to.include("application/zip");
+
+ // Verify Content-Disposition header is present
+ const contentDisposition = response.headers["content-disposition"];
+ expect(contentDisposition).to.exist;
+ expect(contentDisposition).to.include("attachment");
+ expect(contentDisposition).to.include("filename=");
+
+ cy.log("✅ Normal export works");
+ });
+ });
+
+ it("should handle dashboard names with brackets [2024]", () => {
+ // Test that quote() properly encodes brackets
+ cy.request({
+ method: "GET",
+ url: "http://localhost:8088/api/v1/dashboard/export";,
+ query: { q: "[1]" }, // Export dashboard with ID 1
+ headers: {
+ Authorization: "Bearer admin",
+ },
+ }).then((response) => {
+ expect(response.status).to.eq(200);
+
+ const contentDisposition = response.headers["content-disposition"];
+
+ // Verify brackets are either not present or properly encoded
+ if (contentDisposition.includes("[")) {
Review Comment:
not really good to have if statements in your test. Example is what if this
block never runs, then this test will still pass if the expectations outside
pass. If this is what the test case is testing, remove the if statement as part
of the test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
