Skip to content

Commit

Permalink
Fix filter URL not updated with the text search input
Browse files Browse the repository at this point in the history
* Fix the URL not updated when the filter form text inputs are changed

* Update the form filter jest tests accordingly to the changes

* Add system specs to test the URL is updated after text search

* Fix additional history state pushes on form submit during pops

* Fix the  value in the UID

* Fix the Jest tests after adding the event argument to form submit

* Add Jest test checking that the state is not re-pushed on popstate
  • Loading branch information
ahukkanen authored and alecslupu committed Jan 26, 2023
1 parent 730e5e6 commit 8fd5bbb
Show file tree
Hide file tree
Showing 8 changed files with 288 additions and 9 deletions.
17 changes: 17 additions & 0 deletions decidim-budgets/spec/system/explore_projects_spec.rb
Expand Up @@ -40,6 +40,23 @@
end
end

it "updates the current URL with the text filter" do
create(:project, budget:, title: { en: "Foobar project" })
create(:project, budget:, title: { en: "Another project" })
visit_budget

within "form.new_filter" do
fill_in("filter[search_text_cont]", with: "foobar")
click_button "Search"
end

expect(page).not_to have_content("Another project")
expect(page).to have_content("Foobar project")

filter_params = CGI.parse(URI.parse(page.current_url).query)
expect(filter_params["filter[search_text_cont]"]).to eq(["foobar"])
end

it "allows filtering by scope" do
scope = create(:scope, organization: organization)
project.scope = scope
Expand Down
153 changes: 148 additions & 5 deletions decidim-core/app/packs/src/decidim/form_filter.component.test.js
@@ -1,4 +1,4 @@
/* global spyOn */
/* global spyOn, jest */
/* eslint-disable id-length */
window.$ = $;

Expand All @@ -7,6 +7,20 @@ import DataPicker from "./data_picker"

const FormFilterComponent = require("./form_filter.component_for_testing.js");

const expectedPushState = (state, filters) => {
const queryString = Object.keys(filters).map((key) => {
const name = `filter[${key}]`;
const val = filters[key];
if (Array.isArray(val)) {
return val.map((v) => `${encodeURIComponent(`${name}[]`)}=${encodeURIComponent(v)}`).join("&");
}

return `${encodeURIComponent(name)}=${encodeURIComponent(val)}`;
}).join("&");

return [state, null, `/filters?${queryString}`];
}

describe("FormFilterComponent", () => {
const selector = "form#new_filter";
let subject = null;
Expand All @@ -15,6 +29,10 @@ describe("FormFilterComponent", () => {
beforeEach(() => {
let form = `
<form id="new_filter" action="/filters" method="get">
<fieldset>
<input id="filter_search_text_cont" placeholder="Search" data-disable-dynamic-change="true" type="search" name="filter[search_text_cont]">
</fieldset>
<fieldset>
<div id="filter_somerandomid_scope_id" class="data-picker picker-multiple" data-picker-name="filter[scope_id]">
<div class="picker-values">
Expand Down Expand Up @@ -67,11 +85,25 @@ describe("FormFilterComponent", () => {
`;
$("body").append(form);

const $form = $(document).find("form");

window.Decidim = window.Decidim || {};

window.theDataPicker = new DataPicker($(".data-picker"));
window.theCheckBoxesTree = new CheckBoxesTree();
subject = new FormFilterComponent($(document).find("form"));
window.Rails = {
fire: (htmlElement, event) => {
// Hack to call trigger on the correct instance of the form, as fetching
// with the selector does not work.
if (htmlElement === $form[0]) {
$form.trigger(event);
}
}
};

subject = new FormFilterComponent($form);

jest.useFakeTimers();
});

it("exists", () => {
Expand All @@ -88,7 +120,19 @@ describe("FormFilterComponent", () => {

describe("when mounted", () => {
beforeEach(() => {
spyOn(subject.$form, "on");
// Jest doesn't implement listening on the form submit event so we need
// to hack it.
const originalOn = subject.$form.on.bind(subject.$form);
jest.spyOn(subject.$form, "on").mockImplementation((...args) => {
if (args[0] === "submit") {
subject.$form.submitHandler = args[1];
} else if (args[0] === "change" && typeof args[1] === "string") {
subject.$form.changeHandler = args[2];
} else {
originalOn(...args);
}
});

subject.mountComponent();
});

Expand All @@ -100,8 +144,98 @@ describe("FormFilterComponent", () => {
expect(subject.mounted).toBeTruthy();
});

it("binds the form change event", () => {
it("binds the form change and submit events", () => {
expect(subject.$form.on).toHaveBeenCalledWith("change", "input:not([data-disable-dynamic-change]), select:not([data-disable-dynamic-change])", subject._onFormChange);
expect(subject.$form.on).toHaveBeenCalledWith("submit", subject._onFormSubmit);
});

describe("form changes", () => {
beforeEach(() => {
spyOn(window.history, "pushState");

// This is a hack to be able to trigger the events even somewhat close
// to an actual situation. In real browser environment the change events
// would be triggered by the input/select elements but to simplify the
// test implementation, we trigger them directly on the form.
const originalTrigger = subject.$form.trigger.bind(subject.$form);
jest.spyOn(subject.$form, "trigger").mockImplementation((...args) => {
if (args[0] === "submit") {
subject.$form.submitHandler(
$.event.fix(new CustomEvent("submit", { bubbles: true, cancelable: true }))
);
} else if (args[0] === "change") {
subject.$form.changeHandler();
} else {
originalTrigger(...args);
}

jest.runAllTimers();
});
});

it("does not save the state in case there were no changes to previous state", () => {
subject.$form.trigger("change");

expect(window.history.pushState).not.toHaveBeenCalled();
});

it("saves the state after dynamic form changes", () => {
$("#filter_somerandomid_category_id").val(2);

subject.$form.trigger("change");

const state = {
"filter_somerandomid_scope_id": [
{
"text": "Scope 1",
"url": "picker_url_1",
"value": "3"
},
{
"text": "Scope 2",
"url": "picker_url_2",
"value": "4"
}
]
};
const filters = {
"search_text_cont": "",
"scope_id": [3, 4],
"category_id": 2,
"state": [""]
};
expect(window.history.pushState).toHaveBeenCalledWith(...expectedPushState(state, filters));
});

it("saves the state after form submission through input element", () => {
const textInput = document.getElementById("filter_search_text_cont");
textInput.value = "search";

subject.$form.trigger("submit");

const state = {
"filter_somerandomid_scope_id": [
{
"text": "Scope 1",
"url": "picker_url_1",
"value": "3"
},
{
"text": "Scope 2",
"url": "picker_url_2",
"value": "4"
}
]
}
const filters = {
"search_text_cont": "search",
"scope_id": [3, 4],
"category_id": 1,
"state": [""]
}

expect(window.history.pushState).toHaveBeenCalledWith(...expectedPushState(state, filters));
});
});

describe("onpopstate event", () => {
Expand Down Expand Up @@ -131,6 +265,14 @@ describe("FormFilterComponent", () => {
expect(checked.map((input) => input.value)).toEqual(["", "accepted", "evaluating"]);
expect(checked.filter((input) => input.indeterminate).map((input) => input.value)).toEqual([""]);
});

it("does not save the state", () => {
spyOn(window.history, "pushState");

window.onpopstate({ isTrusted: true, state: scopesPickerState});

expect(window.history.pushState).not.toHaveBeenCalled();
});
});
});

Expand All @@ -145,8 +287,9 @@ describe("FormFilterComponent", () => {
expect(subject.mounted).toBeFalsy();
});

it("unbinds the form change event", () => {
it("unbinds the form change and submit events", () => {
expect(subject.$form.off).toHaveBeenCalledWith("change", "input, select", subject._onFormChange);
expect(subject.$form.off).toHaveBeenCalledWith("submit", subject._onFormSubmit);
});
});

Expand Down
30 changes: 26 additions & 4 deletions decidim-core/app/packs/src/decidim/form_filter.js
Expand Up @@ -23,6 +23,7 @@ export default class FormFilterComponent {

this._updateInitialState();
this._onFormChange = delayed(this, this._onFormChange.bind(this));
this._onFormSubmit = delayed(this, this._onFormSubmit.bind(this));
this._onPopState = this._onPopState.bind(this);

if (window.Decidim.PopStateHandler) {
Expand All @@ -42,6 +43,7 @@ export default class FormFilterComponent {
if (this.mounted) {
this.mounted = false;
this.$form.off("change", "input, select", this._onFormChange);
this.$form.off("submit", this._onFormSubmit);

unregisterCallback(`filters-${this.id}`)
}
Expand All @@ -62,6 +64,7 @@ export default class FormFilterComponent {
contentContainer = this.$form.data("remoteFill");
}
this.$form.on("change", "input:not([data-disable-dynamic-change]), select:not([data-disable-dynamic-change])", this._onFormChange);
this.$form.on("submit", this._onFormSubmit);

this.currentFormRequest = null;
this.$form.on("ajax:beforeSend", (e) => {
Expand Down Expand Up @@ -254,14 +257,16 @@ export default class FormFilterComponent {

// Only one instance should submit the form on browser history navigation
if (this.popStateSubmiter) {
Rails.fire(this.$form[0], "submit");
Rails.fire(this.$form[0], "submit", { from: "pop" });
}

this.changeEvents = true;
}

/**
* Handles the logic to update the current location after a form change event.
* Handles the logic to decide whether the form should be submitted or not
* after a form change event. The form is only submitted when changes have
* occurred.
* @private
* @returns {Void} - Returns nothing.
*/
Expand All @@ -270,14 +275,31 @@ export default class FormFilterComponent {
return;
}

const [newPath, newState] = this._currentStateAndPath();
const [newPath] = this._currentStateAndPath();
const path = this._getLocation(false);

if (newPath === path) {
return;
}

Rails.fire(this.$form[0], "submit");
}

/**
* Saves the current state of the search on form submit to update the search
* parameters to the URL and store the picker states.
* @private
* @param {jQuery.Event} ev The event that caused the form to submit.
* @returns {Void} - Returns nothing.
*/
_onFormSubmit(ev) {
const eventDetail = ev.originalEvent.detail;
if (eventDetail && eventDetail.from === "pop") {
return;
}

const [newPath, newState] = this._currentStateAndPath();

pushState(newPath, newState);
this._saveFilters(newPath);
}
Expand Down Expand Up @@ -314,7 +336,7 @@ export default class FormFilterComponent {
* @returns {String} - Returns a unique identifier
*/
_getUID() {
return `filter-form-${new Date().setUTCMilliseconds()}-${Math.floor(Math.random() * 10000000)}`;
return `filter-form-${new Date().getUTCMilliseconds()}-${Math.floor(Math.random() * 10000000)}`;
}

/**
Expand Down
19 changes: 19 additions & 0 deletions decidim-debates/spec/system/explore_debates_spec.rb
Expand Up @@ -108,6 +108,25 @@
end

context "when filtering" do
context "when filtering by text" do
it "updates the current URL" do
create(:debate, component:, title: { en: "Foobar debate" })
create(:debate, component:, title: { en: "Another debate" })
visit_component

within "form.new_filter" do
fill_in("filter[search_text_cont]", with: "foobar")
click_button "Search"
end

expect(page).not_to have_content("Another debate")
expect(page).to have_content("Foobar debate")

filter_params = CGI.parse(URI.parse(page.current_url).query)
expect(filter_params["filter[search_text_cont]"]).to eq(["foobar"])
end
end

context "when filtering by origin" do
context "with 'official' origin" do
let!(:debates) { create_list(:debate, 2, component: component, skip_injection: true) }
Expand Down
19 changes: 19 additions & 0 deletions decidim-meetings/spec/system/explore_meeting_directory_spec.rb
Expand Up @@ -49,6 +49,25 @@
end
end

describe "text filter" do
it "updates the current URL" do
create(:meeting, :published, component: components[0], title: { en: "Foobar meeting" })
create(:meeting, :published, component: components[1], title: { en: "Another meeting" })
visit directory

within "form.new_filter" do
fill_in("filter[title_or_description_cont]", with: "foobar")
click_button "Search"
end

expect(page).not_to have_content("Another meeting")
expect(page).to have_content("Foobar meeting")

filter_params = CGI.parse(URI.parse(page.current_url).query)
expect(filter_params["filter[title_or_description_cont]"]).to eq(["foobar"])
end
end

describe "category filter" do
context "with a category" do
let!(:category1) { create(:category, participatory_space: participatory_process, name: { en: "Category1" }) }
Expand Down
19 changes: 19 additions & 0 deletions decidim-meetings/spec/system/explore_meetings_spec.rb
Expand Up @@ -125,6 +125,25 @@
end

context "when filtering" do
context "when filtering by text" do
it "updates the current URL" do
create(:meeting, :published, component:, title: { en: "Foobar meeting" })
create(:meeting, :published, component:, title: { en: "Another meeting" })
visit_component

within "form.new_filter" do
fill_in("filter[search_text_cont]", with: "foobar")
click_button "Search"
end

expect(page).not_to have_content("Another meeting")
expect(page).to have_content("Foobar meeting")

filter_params = CGI.parse(URI.parse(page.current_url).query)
expect(filter_params["filter[search_text_cont]"]).to eq(["foobar"])
end
end

context "when filtering by origin" do
let!(:component) do
create(:meeting_component,
Expand Down

0 comments on commit 8fd5bbb

Please sign in to comment.