Merge pull request #19841 from ahmedhamidawan/fix_directory_form_element_encoding (68be79f1) · Commits · NDIP / Galaxy

client/src/components/Form/Elements/FormDirectory.test.js

+43 −11

Original line number	Diff line number	Diff line
		@@ -14,6 +14,28 @@ jest.mock("app");

		const { server, http } = useServerMock();

		async function init(wrapper, data) {
		// the file dialog modal should exist
		const filesDialogComponent = wrapper.findComponent(FilesDialog);
		expect(filesDialogComponent.exists()).toBe(true);
		filesDialogComponent.vm.callback({ url: data.url });
		// HACK to avoid https://github.com/facebook/jest/issues/2549 (URL implementation is not the same as global node)
		wrapper.vm.pathChunks = data.pathChunks;
		await flushPromises();
		await validateLatestEmittedPath(wrapper, data.url);
		}

		async function validateLatestEmittedPath(wrapper, expectedPath) {
		const latestEmitIndex = wrapper.emitted()["input"].length - 1;
		const latestPath = wrapper.emitted()["input"][latestEmitIndex][0];
		expect(latestPath).toBe(expectedPath);

		// also manually change prop value to be able to test the value being displayed
		await wrapper.setProps({ value: latestPath });
		const fullPathDisplayed = wrapper.find("[data-description='directory full path']");
		expect(fullPathDisplayed.text()).toBe(`Directory Path: ${expectedPath}`);
		}

		describe("DirectoryPathEditableBreadcrumb", () => {
		let wrapper;
		let spyOnUrlSet;
		@@ -29,6 +51,11 @@ describe("DirectoryPathEditableBreadcrumb", () => {
		const validPath = "validpath";
		const invalidPath = "./];";

		const testingDataWithSpecialChars = {
		url: "gxfiles://directory/sub directory/with%20percent",
		pathChunks: [{ pathChunk: "directory" }, { pathChunk: "sub%20directory" }, { pathChunk: "with%2520percent" }],
		};

		const saveNewChunk = async (path) => {
		// enter a new path chunk
		const input = wrapper.find("#path-input-breadcrumb");
		@@ -38,15 +65,6 @@ describe("DirectoryPathEditableBreadcrumb", () => {
		input.trigger("keyup.enter");
		return input;
		};
		const init = async () => {
		// the file dialog modal should exist
		const filesDialogComponent = wrapper.findComponent(FilesDialog);
		expect(filesDialogComponent.exists()).toBe(true);
		filesDialogComponent.vm.callback({ url: testingData.url });
		// HACK to avoid https://github.com/facebook/jest/issues/2549 (URL implementation is not the same as global node)
		wrapper.vm.pathChunks = testingData.pathChunks;
		await flushPromises();
		};

		beforeEach(async () => {
		spyOnUrlSet = jest.spyOn(FormDirectory.methods, "setUrl");
		@@ -67,13 +85,12 @@ describe("DirectoryPathEditableBreadcrumb", () => {

		wrapper = mount(FormDirectory, {
		propsData: {
		callback: () => {},
		value: null,
		},
		localVue: localVue,
		pinia,
		});
		await flushPromises();
		await init();
		});
		afterEach(async () => {
		if (wrapper) {
		@@ -83,6 +100,7 @@ describe("DirectoryPathEditableBreadcrumb", () => {
		});

		it("should render Breadcrumb", async () => {
		await init(wrapper, testingData);
		// after initial folder is chosen, setUrl() should be called and modal disappear
		expect(spyOnUrlSet).toHaveBeenCalled();
		expect(wrapper.findComponent(FilesDialog).exists()).toBe(false);
		@@ -107,6 +125,7 @@ describe("DirectoryPathEditableBreadcrumb", () => {
		});

		it("should prevent invalid Paths", async () => {
		await init(wrapper, testingData);
		// enter a new path chunk
		const input = await saveNewChunk(invalidPath);
		await flushPromises();
		@@ -117,6 +136,7 @@ describe("DirectoryPathEditableBreadcrumb", () => {
		});

		it("should save and remove new Paths", async () => {
		await init(wrapper, testingData);
		// enter a new path chunk
		const input = await saveNewChunk(validPath);

		@@ -129,14 +149,20 @@ describe("DirectoryPathEditableBreadcrumb", () => {
		expect(wrapper.findAll("li.breadcrumb-item").length).toBe(testingData.expectedNumberOfPaths + 1);
		// find newly added chunk
		const addedChunk = wrapper.findAll("li.breadcrumb-item button").wrappers.find((e) => e.text() === validPath);

		await validateLatestEmittedPath(wrapper, `${testingData.url}/${validPath}`);

		// remove chunk from the path
		await addedChunk.trigger("click");
		await flushPromises();
		// number of elements should be the same again
		expect(wrapper.findAll("li.breadcrumb-item").length).toBe(testingData.expectedNumberOfPaths);

		await validateLatestEmittedPath(wrapper, testingData.url);
		});

		it("should update new path", async () => {
		await init(wrapper, testingData);
		// enter a new path chunk
		expect(spyOnUpdateURL).toHaveBeenCalled();
		await saveNewChunk(validPath);
		@@ -144,4 +170,10 @@ describe("DirectoryPathEditableBreadcrumb", () => {

		expect(spyOnUpdateURL).toHaveBeenCalled();
		});

		it("should retain special characters in path", async () => {
		// the init function itself validates that the emits and path display
		// retain special characters in the url
		await init(wrapper, testingDataWithSpecialChars);
		});
		});

client/src/components/Form/Elements/FormDirectory.vue

+36 −12

Original line number	Diff line number	Diff line
		@@ -11,7 +11,7 @@
		:require-writable="true"
		:is-open="isModalShown" />
		</div>
		<b-breadcrumb v-if="url">
		<b-breadcrumb v-if="url" class="mb-0">
		<b-breadcrumb-item title="Select another folder" class="align-items-center" @click="reset">
		<b-button class="pathname" variant="primary">
		<FontAwesomeIcon icon="folder-open" /> {{ url.protocol }}</b-button
		@@ -22,8 +22,8 @@
		:key="index"
		class="existent-url-path align-items-center">
		<b-button class="regular-path-chunk" :disabled="!editable" variant="dark" @click="removePath(index)">
		{{ pathChunk }}</b-button
		>
		{{ decodeURIComponent(pathChunk) }}
		</b-button>
		</b-breadcrumb-item>
		<b-breadcrumb-item class="directory-input-field align-items-center">
		<b-input
		@@ -38,6 +38,11 @@
		@keydown.8.capture="removeLastPath" />
		</b-breadcrumb-item>
		</b-breadcrumb>

		<div v-if="value" class="px-2" data-description="directory full path">
		<span v-localize>Directory Path:</span>
		<code>{{ value }}</code>
		</div>
		</div>
		</template>

		@@ -46,8 +51,11 @@ import { library } from "@fortawesome/fontawesome-svg-core";
		import { faFolder, faFolderOpen } from "@fortawesome/free-solid-svg-icons";
		import { FontAwesomeIcon } from "@fortawesome/vue-fontawesome";
		import { FilesDialog } from "components/FilesDialog";
		import { Toast } from "composables/toast";
		import _l from "utils/localization";

		import { errorMessageAsString } from "@/utils/simple-error";

		library.add(faFolder, faFolderOpen);

		const getDefaultValues = () => ({
		@@ -62,6 +70,12 @@ export default {
		FontAwesomeIcon,
		FilesDialog,
		},
		props: {
		value: {
		type: String,
		default: null,
		},
		},
		data() {
		return { ...getDefaultValues(), modalKey: 0, selectText: _l("Select") };
		},
		@@ -80,6 +94,7 @@ export default {
		methods: {
		removePath(index) {
		this.pathChunks = this.pathChunks.slice(0, index);
		this.updateURL();
		},
		reset() {
		const data = getDefaultValues();
		@@ -102,7 +117,8 @@ export default {
		}
		},
		setUrl({ url }) {
		this.url = new URL(url);
		try {
		this.url = new URL(encodeURI(url));
		// split path and keep only valid entries
		this.pathChunks = this.url.href
		.split(/[/\\]/)
		@@ -112,6 +128,9 @@ export default {
		if (url) {
		this.updateURL();
		}
		} catch (error) {
		Toast.error(errorMessageAsString(error), "Invalid directory path");
		}
		},
		addPath({ key }) {
		if ((key === "Enter" \|\| key === "/") && this.isValidName) {
		@@ -125,7 +144,12 @@ export default {
		let url = undefined;
		if (!isReset) {
		// create an string of path chunks separated by `/`
		url = encodeURI(`${this.url.protocol}//${this.pathChunks.map(({ pathChunk }) => pathChunk).join("/")}`);
		url = encodeURI(
		`${this.url.protocol}//${this.pathChunks
		.map(({ pathChunk }) => decodeURIComponent(pathChunk))
		.join("/")}`
		);
		url = decodeURI(url);
		}
		this.$emit("input", url);
		},

client/src/components/Form/FormElement.vue

+1 −6

Original line number	Diff line number	Diff line
		@@ -297,12 +297,7 @@ function onAlert(value: string \| undefined) {
		:options="attrs.options"
		:optional="attrs.optional"
		:multiple="attrs.multiple" />
		<FormDataUri
		v-else-if="isUriDataField"
		:id="id"
		v-model="currentValue"
		:value="attrs.value"
		:multiple="attrs.multiple" />
		<FormDataUri v-else-if="isUriDataField" :id="id" v-model="currentValue" :multiple="attrs.multiple" />
		<FormData
		v-else-if="['data', 'data_collection'].includes(props.type)"
		:id="id"