chore: Page rename API client validation (#32788)
1. The page names can accept any character except for `/`, backslash and `:`. This is the validation that the server does now. This PR fixes the regex that checks the page name to align with this. 2. When typing a character that's not allowed, we end up with a space at the beginning or end of the page name, that doesn't really make sense, and is confusing. Then we end up with trailing or leading spaces in the page name, making the page name display look misaligned. This PR will make it so that the disallowed characters just can't be typed, and just don't make a difference in the page name. 3. We're also adding server-side validation for the icon slug. [Slack conversation](https://theappsmith.slack.com/archives/C03RPDB936Z/p1713161688607509). /ok-to-test tags="@tag.Sanity" <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/8746416301> > Commit: 21e8d3a0060b9b4721b49149f307354b073332d7 > Cypress dashboard url: <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=8746416301&attempt=1" target="_blank">Click here!</a> <!-- end of auto-generated comment: Cypress test results --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced page name validation to improve naming consistency across the application. - Updated validation for page icons and custom slugs to allow more flexibility and ensure inputs meet new standards. - **Bug Fixes** - Fixed issues in page naming functions to prevent the use of certain special characters and limit the length, enhancing data integrity and user experience. - **Tests** - Added new tests to verify the handling of invalid and empty custom slugs, ensuring robustness in page management functionalities. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Shrikant Sharat Kandula
GitHub
parent
01510d6f26
commit
f339f9d4f2
|
|
@ -33,7 +33,7 @@ import TextLoaderIcon from "../Components/TextLoaderIcon";
|
|||
import { filterAccentedAndSpecialCharacters, getUrlPreview } from "../Utils";
|
||||
import type { AppState } from "@appsmith/reducers";
|
||||
import { getUsedActionNames } from "selectors/actionSelectors";
|
||||
import { isNameValid, resolveAsSpaceChar } from "utils/helpers";
|
||||
import { isNameValid, toValidPageName } from "utils/helpers";
|
||||
import { useFeatureFlag } from "utils/hooks/useFeatureFlag";
|
||||
import { FEATURE_FLAG } from "@appsmith/entities/FeatureFlag";
|
||||
import { getHasManagePagePermission } from "@appsmith/utils/BusinessFeatures/permissionPageHelpers";
|
||||
|
|
@ -169,7 +169,7 @@ function PageSettings(props: { page: Page }) {
|
|||
}
|
||||
|
||||
setIsPageNameValid(isValid);
|
||||
setPageName(resolveAsSpaceChar(value, 30));
|
||||
setPageName(toValidPageName(value));
|
||||
};
|
||||
|
||||
const onPageSlugChange = (value: string) => {
|
||||
|
|
|
|||
|
|
@ -18,7 +18,7 @@ import {
|
|||
import { getCurrentApplication } from "@appsmith/selectors/applicationSelectors";
|
||||
import type { AppState } from "@appsmith/reducers";
|
||||
import { StyledEntity } from "pages/Editor/Explorer/Common/components";
|
||||
import { resolveAsSpaceChar } from "utils/helpers";
|
||||
import { toValidPageName } from "utils/helpers";
|
||||
import { updatePage } from "actions/pageActions";
|
||||
import { useGetPageFocusUrl } from "pages/Editor/IDE/hooks";
|
||||
import AnalyticsUtil from "utils/AnalyticsUtil";
|
||||
|
|
@ -113,7 +113,7 @@ const PageElement = ({
|
|||
isDefaultExpanded={isCurrentPage}
|
||||
key={page.pageId}
|
||||
name={page.pageName}
|
||||
onNameEdit={resolveAsSpaceChar}
|
||||
onNameEdit={toValidPageName}
|
||||
ref={ref}
|
||||
searchKeyword={""}
|
||||
step={0}
|
||||
|
|
|
|||
|
|
@ -352,18 +352,9 @@ function getWidgetElementToScroll(
|
|||
}
|
||||
}
|
||||
|
||||
export const resolveAsSpaceChar = (value: string, limit?: number) => {
|
||||
// ensures that all special characters are disallowed
|
||||
// while allowing all utf-8 characters
|
||||
const removeSpecialCharsRegex =
|
||||
/`|\~|\!|\@|\#|\$|\%|\^|\&|\*|\(|\)|\+|\=|\[|\{|\]|\}|\||\\|\'|\<|\,|\.|\>|\?|\/|\""|\;|\:|\s/;
|
||||
const duplicateSpaceRegex = /\s+/;
|
||||
return value
|
||||
.split(removeSpecialCharsRegex)
|
||||
.join(" ")
|
||||
.split(duplicateSpaceRegex)
|
||||
.join(" ")
|
||||
.slice(0, limit || 30);
|
||||
export const toValidPageName = (value: string) => {
|
||||
// Ensure that `/`, `\` and `:` are not allowed in page names, aligning with server-side validation.
|
||||
return value.replaceAll(/[/\\:]+/g, "").slice(0, 30);
|
||||
};
|
||||
|
||||
export const PLATFORM_OS = {
|
||||
|
|
|
|||
|
|
@ -5,8 +5,8 @@ import jakarta.validation.constraints.Size;
|
|||
|
||||
public record PageUpdateDTO(
|
||||
@Pattern(regexp = "[^/\\\\:]+", message = "/, \\, : not allowed in page names") @Size(max = 30) String name,
|
||||
String icon,
|
||||
@Pattern(regexp = "[-\\w]+") String customSlug) {
|
||||
@Pattern(regexp = "[-a-z]+") @Size(max = 20) String icon,
|
||||
@Pattern(regexp = "[-\\w]*") String customSlug) {
|
||||
|
||||
public PageDTO toPageDTO() {
|
||||
final PageDTO page = new PageDTO();
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
package com.appsmith.server.controllers;
|
||||
|
||||
import com.appsmith.server.configurations.SecurityTestConfig;
|
||||
import com.appsmith.server.dtos.PageDTO;
|
||||
import com.appsmith.server.newpages.base.NewPageService;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.junit.jupiter.params.ParameterizedTest;
|
||||
|
|
@ -14,10 +15,17 @@ import org.springframework.http.MediaType;
|
|||
import org.springframework.security.test.context.support.WithMockUser;
|
||||
import org.springframework.test.web.reactive.server.WebTestClient;
|
||||
import org.springframework.web.reactive.function.BodyInserters;
|
||||
import reactor.core.publisher.Mono;
|
||||
|
||||
import java.util.Map;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
import static org.mockito.Mockito.any;
|
||||
import static org.mockito.Mockito.anyString;
|
||||
import static org.mockito.Mockito.doReturn;
|
||||
import static org.mockito.Mockito.eq;
|
||||
import static org.mockito.Mockito.times;
|
||||
import static org.mockito.Mockito.verify;
|
||||
import static org.mockito.Mockito.verifyNoInteractions;
|
||||
|
||||
@SpringBootTest
|
||||
|
|
@ -41,7 +49,7 @@ public class PageControllerTest {
|
|||
@ValueSource(
|
||||
strings = {
|
||||
"../mal", "..\\mal", "/mal", "C:\\mal", "mal/", "/mal/", "\\mal", "mal\\", "\\mal\\", ":mal", ":mal/",
|
||||
":mal\\"
|
||||
":mal\\", "",
|
||||
})
|
||||
@WithMockUser
|
||||
void invalidName(String name) {
|
||||
|
|
@ -58,4 +66,55 @@ public class PageControllerTest {
|
|||
|
||||
verifyNoInteractions(newPageService);
|
||||
}
|
||||
|
||||
@ParameterizedTest
|
||||
@ValueSource(
|
||||
strings = {
|
||||
"../mal",
|
||||
"..\\mal",
|
||||
"/mal",
|
||||
"C:\\mal",
|
||||
"mal/",
|
||||
"/mal/",
|
||||
"\\mal",
|
||||
"mal\\",
|
||||
"\\mal\\",
|
||||
":mal",
|
||||
":mal/",
|
||||
":mal\\",
|
||||
"spaced content",
|
||||
"newline\ncontent",
|
||||
" untrimmed ",
|
||||
})
|
||||
@WithMockUser
|
||||
void invalidCustomSlug(String slug) {
|
||||
client.put()
|
||||
.uri("/api/v1/pages/abcdef")
|
||||
.contentType(MediaType.APPLICATION_JSON)
|
||||
.body(BodyInserters.fromValue(Map.of("customSlug", slug)))
|
||||
.exchange()
|
||||
.expectStatus()
|
||||
.isBadRequest()
|
||||
.expectBody()
|
||||
.jsonPath("$.errorDisplay")
|
||||
.value(s -> assertThat((String) s).contains("Validation Failure"));
|
||||
|
||||
verifyNoInteractions(newPageService);
|
||||
}
|
||||
|
||||
@Test
|
||||
@WithMockUser
|
||||
void emptyCustomSlugShouldBeOkay() {
|
||||
doReturn(Mono.just(new PageDTO()))
|
||||
.when(newPageService)
|
||||
.updatePageByDefaultPageIdAndBranch(anyString(), any(), anyString());
|
||||
|
||||
client.put()
|
||||
.uri("/api/v1/pages/abcdef")
|
||||
.contentType(MediaType.APPLICATION_JSON)
|
||||
.body(BodyInserters.fromValue(Map.of("customSlug", "")))
|
||||
.exchange();
|
||||
|
||||
verify(newPageService, times(1)).updatePageByDefaultPageIdAndBranch(eq("abcdef"), any(), eq(null));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user