Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport 'Fix filter URL not updated with the text search input' to v0.27 #10264

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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: budget, title: { en: "Foobar project" })
create(:project, budget: 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: component, title: { en: "Foobar debate" })
create(:debate, component: 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: component, title: { en: "Foobar meeting" })
create(:meeting, :published, component: 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