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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PB-877: add swisssearch legacy url limit attribute #1098

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 18 additions & 7 deletions src/api/search.api.js
Original file line number Diff line number Diff line change
Expand Up @@ -181,13 +181,14 @@ function parseLocationResult(result, outputProjection) {
}
}

async function searchLayers(queryString, lang, cancelToken) {
async function searchLayers(queryString, lang, cancelToken, limit = 0) {
try {
const layerResponse = await generateAxiosSearchRequest(
queryString,
lang,
'layers',
cancelToken.token
cancelToken.token,
{ limit }
)
// checking that there is something of interest to parse
const resultWithAttrs = layerResponse?.data.results?.filter((result) => result.attrs)
Expand All @@ -206,15 +207,17 @@ async function searchLayers(queryString, lang, cancelToken) {
* @param queryString
* @param lang
* @param cancelToken
* @param limit
* @returns {Promise<LocationSearchResult[]>}
*/
async function searchLocation(outputProjection, queryString, lang, cancelToken) {
async function searchLocation(outputProjection, queryString, lang, cancelToken, limit = 0) {
try {
const locationResponse = await generateAxiosSearchRequest(
queryString,
lang,
'locations',
cancelToken.token
cancelToken.token,
{ limit }
)
// checking that there is something of interest to parse
const resultWithAttrs = locationResponse?.data.results?.filter((result) => result.attrs)
Expand Down Expand Up @@ -280,10 +283,17 @@ let cancelToken = null
* @param {String} config.lang The lang ISO code in which the search must be conducted
* @param {GeoAdminLayer[]} [config.layersToSearch=[]] List of searchable layers for which to fire
* search requests. Default is `[]`
* @param {number} config.limit The maximum number of results to return
* @returns {Promise<SearchResult[]>}
*/
export default async function search(config) {
const { outputProjection = null, queryString = null, lang = null, layersToSearch = [] } = config
const {
outputProjection = null,
queryString = null,
lang = null,
layersToSearch = [],
limit = 0,
} = config
if (!(outputProjection instanceof CoordinateSystem)) {
const errorMessage = `A valid output projection is required to start a search request`
log.error(errorMessage)
Expand All @@ -307,10 +317,11 @@ export default async function search(config) {

/** @type {Promise<SearchResult[]>[]} */
const allRequests = [
searchLayers(queryString, lang, cancelToken),
searchLocation(outputProjection, queryString, lang, cancelToken),
searchLayers(queryString, lang, cancelToken, limit),
searchLocation(outputProjection, queryString, lang, cancelToken, limit),
]

// TODO limit also in the local kml and gpx files ?
if (layersToSearch.some((layer) => layer.searchable)) {
allRequests.push(
...layersToSearch
Expand Down
26 changes: 15 additions & 11 deletions src/modules/menu/components/search/SearchBar.vue
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const dispatcher = { dispatcher: 'SearchBar' }

const store = useStore()

const isPristine = ref(true)
const isPristine = ref(true) // if search bar is not yet modified by the user
const showResults = ref(false)
const searchInput = ref(null)
const searchValue = ref('')
Expand All @@ -20,16 +20,20 @@ const searchQuery = computed(() => store.state.search.query)
const hasResults = computed(() => store.state.search.results.length > 0)
const isPhoneMode = computed(() => store.getters.isPhoneMode)

watch(hasResults, (newValue) => {
// if an entry has been selected from the list, do not show the list again
// because the list has been hidden by onEntrySelected. Also if the search bar is pristine (not
// yet modified by the user) we don't want to show the result (e.g. at startup with a swisssearch
// query param)
if (!selectedEntry.value && !isPristine.value) {
log.debug(`Search has result changed to ${newValue}, change the show result to ${newValue}`)
showResults.value = newValue
}
})
watch(
hasResults,
(newValue) => {
// if an entry has been selected from the list, do not show the list again
// because the list has been hidden by onEntrySelected.
if (!selectedEntry.value) {
log.debug(
`Search has result changed to ${newValue}, change the show result to ${newValue}`
)
showResults.value = newValue
}
},
{ immediate: true }
)

watch(showResults, (newValue) => {
if (newValue && isPhoneMode.value && store.state.ui.showMenu) {
Expand Down
32 changes: 29 additions & 3 deletions src/router/storeSync/SearchParamConfig.class.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import AbstractParamConfig, {
STORE_DISPATCHER_ROUTER_PLUGIN,
} from '@/router/storeSync/abstractParamConfig.class'

const URL_PARAM_NAME = 'swisssearch'
/**
* The goal is to stop centering on the search when sharing a position. When we share a position,
* both the center and the crosshair are sets.
Expand All @@ -15,16 +16,41 @@ function dispatchSearchFromUrl(to, store, urlParamValue) {
query: urlParamValue,
shouldCenter: !(to.query.crosshair && to.query.center),
dispatcher: STORE_DISPATCHER_ROUTER_PLUGIN,
originUrlParam: true,
})
}

/**
* This will remove the query param from the URL It is necessary to do this in vanilla JS because
* the router does not provide a way to remove a query without reloading the page which then removes
* the value from the store.
*
* @param {Object} key The key to remove from the URL
*/
function removeQueryParamFromHref(key) {
const [baseUrl, queryString] = window.location.href.split('?')
if (!queryString) {
return
}

const params = new URLSearchParams(queryString)
if (!params.has(key)) {
return
}
params.delete(key)

const newQueryString = params.toString()
const newUrl = newQueryString ? `${baseUrl}?${newQueryString}` : baseUrl
window.history.replaceState({}, document.title, newUrl)
}

export default class SearchParamConfig extends AbstractParamConfig {
constructor() {
super({
urlParamName: 'swisssearch',
mutationsToWatch: ['setSearchQuery'],
urlParamName: URL_PARAM_NAME,
mutationsToWatch: [],
setValuesInStore: dispatchSearchFromUrl,
extractValueFromStore: (store) => store.state.search.query,
afterSetValuesInStore: () => removeQueryParamFromHref(URL_PARAM_NAME),
keepInUrlWhenDefault: false,
valueType: String,
defaultValue: '',
Expand Down
27 changes: 26 additions & 1 deletion src/router/storeSync/abstractParamConfig.class.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,19 @@ export default class AbstractParamConfig {
* added to the URL even though its value is set to the default value of the param.
* @param {NumberConstructor | StringConstructor | BooleanConstructor, ObjectConstructor} valueType
* @param {Boolean | Number | String | null} defaultValue
* @param {Function} afterSetValuesInStore A function that will be called after the store values
* have been set
*/
constructor({
urlParamName,
mutationsToWatch,
setValuesInStore,
extractValueFromStore,
extractValueFromStore = (_) => '',
keepInUrlWhenDefault = true,
valueType = String,
defaultValue = null,
validateUrlInput = null,
afterSetValuesInStore = null,
} = {}) {
this.urlParamName = urlParamName
this.mutationsToWatch = mutationsToWatch
Expand All @@ -51,6 +54,7 @@ export default class AbstractParamConfig {
this.defaultValue = false
}
this.validateUrlInput = validateUrlInput
this.afterSetValuesInStore = afterSetValuesInStore
}

/**
Expand Down Expand Up @@ -201,4 +205,25 @@ export default class AbstractParamConfig {
}
})
}

/**
* Triggers an action after the store has been populated with the query value. Returns a promise
*
* @returns {Promise<any>}
*/
afterPopulateStore() {
return new Promise((resolve) => {
if (!this.afterSetValuesInStore) {
resolve()
}
const promiseAfterSetValuesInStore = this.afterSetValuesInStore()
if (promiseAfterSetValuesInStore) {
promiseAfterSetValuesInStore.then(() => {
resolve()
})
} else {
resolve()
}
})
}
}
1 change: 1 addition & 0 deletions src/router/storeSync/storeSync.routerPlugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ function urlQueryWatcher(store, to, from) {

const setValueInStore = async (paramConfig, store, value) => {
await paramConfig.populateStoreWithQueryValue(to, store, value)
await paramConfig.afterPopulateStore()
}

if (
Expand Down
25 changes: 24 additions & 1 deletion src/store/modules/search.store.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,19 @@ const state = {

const getters = {}

function extractLimitNumber(query) {
const regex = / limit: \d+/
const match = query.match(regex)

if (match) {
return {
limit: parseInt(match[0].split(':')[1].trim()),
extractedQuery: query.replace(match[0], ''),
}
}
return { limit: 0, extractedQuery: query }
}

const actions = {
/**
* @param {vuex} vuex
Expand All @@ -35,10 +48,12 @@ const actions = {
*/
setSearchQuery: async (
{ commit, rootState, dispatch, getters },
{ query = '', shouldCenter = true, dispatcher }
{ query = '', originUrlParam = false, shouldCenter = true, dispatcher }
) => {
let results = []
commit('setSearchQuery', { query, dispatcher })
const { limit, extractedQuery } = extractLimitNumber(query)
query = extractedQuery
// only firing search if query is longer than or equal to 2 chars
if (query.length >= 2) {
const currentProjection = rootState.position.projection
Expand Down Expand Up @@ -129,7 +144,14 @@ const actions = {
queryString: query,
lang: rootState.i18n.lang,
layersToSearch: getters.visibleLayers,
limit,
})
if (originUrlParam && results.length === 1) {
dispatch('selectResultEntry', {
dispatcher: `${dispatcher}/setSearchQuery`,
entry: results[0],
})
}
} catch (error) {
log.error(`Search failed`, error)
}
Expand All @@ -147,6 +169,7 @@ const actions = {
* @param {SearchResult} entry
*/
selectResultEntry: ({ dispatch, getters, rootState }, { entry, dispatcher }) => {
console.log('selectResultEntry', entry)
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't use console.log but log.debug from the log library

const dispatcherSelectResultEntry = `${dispatcher}/search.store/selectResultEntry`
switch (entry.resultType) {
case SearchResultTypes.LAYER:
Expand Down
1 change: 1 addition & 0 deletions src/store/plugins/redo-search-when-needed.plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const redoSearchWhenNeeded = (store) => {
query: store.state.search.query,
// we don't center on the search query when redoing a search if there is a crosshair
shouldCenter: store.state.position.crossHair === null,
originUrlParam: true, // necessary to select the first result if there is only one else it will not be because this redo search is done every time the page loaded
})
}
}
Expand Down
60 changes: 57 additions & 3 deletions tests/cypress/tests-e2e/legacyParamImport.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -249,13 +249,18 @@ describe('Test on legacy param import', () => {
cy.intercept('**/rest/services/ech/SearchServer*?type=layers*', {
body: { results: [] },
}).as('search-layers')
const coordinates = [2598633.75, 1200386.75]
cy.intercept('**/rest/services/ech/SearchServer*?type=locations*', {
body: {
results: [
{
attrs: {
detail: '1530 payerne 5822 payerne ch vd',
label: ' <b>1530 Payerne</b>',
lat: 46.954559326171875,
lon: 7.420684814453125,
y: coordinates[0],
x: coordinates[1],
},
},
],
Expand All @@ -267,10 +272,59 @@ describe('Test on legacy param import', () => {
},
false
)
cy.readStoreValue('state.search.query').should('eq', '1530 Payerne')
cy.url().should('include', 'swisssearch=1530+Payerne')
cy.readStoreValue('state.search.query').should('eq', ' 1530 Payerne')
cy.url().should('not.contain', 'swisssearch')
cy.get('[data-cy="searchbar"]').click()
cy.get('[data-cy="search-results-locations"]').should('be.visible')
const acceptableDelta = 0.25

// selects the result if it is only one
cy.readStoreValue('state.map.pinnedLocation').should((feature) => {
expect(feature).to.not.be.null
expect(feature).to.be.a('array').that.is.not.empty
expect(feature[0]).to.be.approximately(coordinates[0], acceptableDelta)
expect(feature[1]).to.be.approximately(coordinates[1], acceptableDelta)
})
cy.get('[data-cy="search-results-locations"]').should('not.be.visible')
})
it('limits the swisssearch with legacy parameter limit', () => {
cy.intercept('**/rest/services/ech/SearchServer*?type=layers*', {
body: { results: [] },
}).as('search-layers')
const coordinates = [2598633.75, 1200386.75]
cy.intercept('**/rest/services/ech/SearchServer*?type=locations*', {
body: {
results: [
{
attrs: {
detail: '1530 payerne 5822 payerne ch vd',
label: ' <b>1530 Payerne</b>',
lat: 46.954559326171875,
lon: 7.420684814453125,
y: coordinates[0],
x: coordinates[1],
},
},
{
attrs: {
detail: '1530 payerne 5822 payerne ch vd 2',
label: ' <b>1530 Payerne</b> 2',
lat: 46.954559326171875,
lon: 7.420684814453125,
y: coordinates[0],
x: coordinates[1],
},
},
],
},
}).as('search-locations')
cy.goToMapView(
{
swisssearch: '1530 Payerne limit: 2',
},
false
)
cy.readStoreValue('state.search.query').should('eq', '1530 Payerne limit: 2')
cy.url().should('not.contain', 'swisssearch')
})
it('External WMS layer', () => {
const layerName = 'OpenData-AV'
Expand Down
Loading
Loading