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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

Fix issue #5331. #5332

Merged
merged 14 commits into from
May 3, 2021
Merged

Fix issue #5331. #5332

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
fc54b7a
Fix issue #5331.
mwu2018 Mar 17, 2021
8cd31d3
Update CHANGE.md.
mwu2018 Mar 17, 2021
b5ebc9e
prettier
mwu2018 Mar 17, 2021
4d9b45a
Update filterQuery type based on review.
mwu2018 Mar 18, 2021
a0f91d0
Revert the last commit.
mwu2018 Mar 18, 2021
94385a7
Remove unwanted import.
mwu2018 Mar 18, 2021
908c80f
Revise filterQuery type and improve dynamic type checking.
mwu2018 Mar 19, 2021
96ecaee
Improve robustness and add unit tests.
mwu2018 Mar 21, 2021
33d4b68
Add missing import.
mwu2018 Mar 21, 2021
2ceb403
Minor refactor.
mwu2018 Mar 21, 2021
d2a5e1e
Minor refactor.
mwu2018 Mar 21, 2021
df4c8c8
Merge remote-tracking branch 'origin/next' into fix-ckan-group-bug
nf-s May 3, 2021
3e8fb21
update changes
nf-s May 3, 2021
5c55f52
org imports
nf-s May 3, 2021
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
1 change: 1 addition & 0 deletions CHANGES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ Change Log
* Fix region mapping feature highlighting.
* Update clipboard to fix clipboard typing error.
* Add support for time on `ArcGisMapServerCatalogItem`
* Fix CkanCatalogGroup filterQuery issue. [#5332](https://github.com/TerriaJS/terriajs/pull/5332)
* [The next improvement]

#### 8.0.0-alpha.67
Expand Down
11 changes: 8 additions & 3 deletions lib/Models/CkanCatalogGroup.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import i18next from "i18next";
import { action, computed, observable, runInAction } from "mobx";
import URI from "urijs";
import isDefined from "../Core/isDefined";
import { JsonObject } from "../Core/Json";
import loadJson from "../Core/loadJson";
import TerriaError from "../Core/TerriaError";
import CatalogMemberMixin from "../ModelMixins/CatalogMemberMixin";
Expand Down Expand Up @@ -84,9 +85,13 @@ export class CkanServerStratum extends LoadableStratum(CkanCatalogGroupTraits) {
.segment("api/3/action/package_search")
.addQuery({ start: 0, rows: 1000, sort: "metadata_created asc" });

Object.keys(filterQuery).forEach((key: string) =>
uri.addQuery(key, filterQuery[key])
);
if (typeof filterQuery === "string") {
uri.query(uri.query() + "&" + filterQuery.toString());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good @mwu2018
Sorry to nitpick, but is it possible to use uri.addQuery() instead of uri.query(uri.query() + ...?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nf-s You did not nitpick at all. Instead, you have opened a can of worms 😃

  1. The filterQuery trait is changed similar to what you suggested initially. Your later suggestion does not allow mixed elements in the array.
  2. We can not use uri.addQuery(filterQuery) because the URI encoder will escape =, which is not what we want. I have added some comments in the code.
  3. The code is slightly refactored in order to do simple unit tests.

Can you please take another look?

} else {
Object.keys(filterQuery).forEach((key: string) =>
uri.addQuery(key, (filterQuery as JsonObject)[key])
);
}

const result = await paginateThroughResults(uri, catalogGroup);
if (ckanServerResponse === undefined) {
Expand Down
2 changes: 1 addition & 1 deletion lib/Traits/CkanCatalogGroupTraits.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export default class CkanCatalogGroupTraits extends mixTraits(
* To get all datasets with no filter, you can use ['']
`
})
filterQuery?: JsonObject[] = [
filterQuery?: JsonObject[] | string[] = [
{
fq:
'(res_format:(czml OR CZML OR geojson OR GeoJSON OR WMS OR wms OR kml OR KML OR kmz OR KMZ OR WFS OR wfs OR CSV-GEO-AU OR csv-geo-au OR "Esri REST"))'
Expand Down