-
Notifications
You must be signed in to change notification settings - Fork 0
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
allow SpotlightDialog person search on non-matrix attributes #247
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
/* | ||
Copyright 2023 The Matrix.org Foundation C.I.C. | ||
|
||
Licensed under the Apache License, Version 2.0 (the "License"); | ||
you may not use this file except in compliance with the License. | ||
You may obtain a copy of the License at | ||
|
||
http://www.apache.org/licenses/LICENSE-2.0 | ||
|
||
Unless required by applicable law or agreed to in writing, software | ||
distributed under the License is distributed on an "AS IS" BASIS, | ||
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
See the License for the specific language governing permissions and | ||
limitations under the License. | ||
*/ | ||
|
||
import { useEffect } from "react"; | ||
import { PosthogAnalytics } from "../../../src/PosthogAnalytics"; | ||
|
||
export const useWebSearchMetrics = (numResults: number, queryLength: number, viaSpotlight: boolean): void => { | ||
useEffect(() => { | ||
if (!queryLength) return; | ||
|
||
// send metrics after a 1s debounce | ||
const timeoutId = window.setTimeout(() => { | ||
PosthogAnalytics.instance.trackEvent<WebSearchEvent>({ | ||
eventName: "WebSearch", | ||
viaSpotlight, | ||
numResults, | ||
queryLength, | ||
}); | ||
}, 1000); | ||
|
||
return () => { | ||
clearTimeout(timeoutId); | ||
}; | ||
}, [numResults, queryLength, viaSpotlight]); | ||
}; |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -449,4 +449,59 @@ describe("Spotlight Dialog", () => { | |||||||||||||||||||||||
expect(screen.getByText(potatoRoom.name!)).toBeInTheDocument(); | ||||||||||||||||||||||||
}); | ||||||||||||||||||||||||
}); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
describe("user directory non-matrix attributes", () => { | ||||||||||||||||||||||||
interface IUserExternalAttributesChunkMember extends IUserChunkMember { | ||||||||||||||||||||||||
location?: string; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
const testPersonExternalAttributtes: IUserExternalAttributesChunkMember = { | ||||||||||||||||||||||||
user_id: "@earthling:matrix.org", | ||||||||||||||||||||||||
display_name: "Earth Person", | ||||||||||||||||||||||||
avatar_url: undefined, | ||||||||||||||||||||||||
location: "Earth", // this attribute could come from a matrix homeserver connected to LDAP | ||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this test case is not clear: with this, searching by location would require searching for (a substring of)
with a query of |
||||||||||||||||||||||||
}; | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
let mockedClient: MatrixClient; | ||||||||||||||||||||||||
beforeEach(() => { | ||||||||||||||||||||||||
const users: IUserExternalAttributesChunkMember[] = [testPersonExternalAttributtes] | ||||||||||||||||||||||||
mockedClient = mockClient({ rooms: [], users }); | ||||||||||||||||||||||||
/* overwrite the function searchUserDirectory, to include non-matrix search attributes, used by the homeserver */ | ||||||||||||||||||||||||
mockedClient.searchUserDirectory = jest.fn(({ term, limit }) => { | ||||||||||||||||||||||||
const searchTerm = term?.toLowerCase(); | ||||||||||||||||||||||||
const results = users.filter( | ||||||||||||||||||||||||
(it) => { | ||||||||||||||||||||||||
return ( | ||||||||||||||||||||||||
!searchTerm || | ||||||||||||||||||||||||
it.user_id.toLowerCase().includes(searchTerm) || | ||||||||||||||||||||||||
it.display_name?.toLowerCase().includes(searchTerm) || | ||||||||||||||||||||||||
it.location?.toLowerCase().includes(searchTerm) | ||||||||||||||||||||||||
) | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
); | ||||||||||||||||||||||||
Comment on lines
+472
to
+481
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it seems superfluous to not just do
Suggested change
while this illustrates how the server works, i feel it overcomplicates things without being useful in this place. unless i'm overlooking something? |
||||||||||||||||||||||||
return Promise.resolve({ | ||||||||||||||||||||||||
results: results.slice(0, limit ?? +Infinity), | ||||||||||||||||||||||||
limited: !!limit && limit < results.length, | ||||||||||||||||||||||||
}); | ||||||||||||||||||||||||
}); | ||||||||||||||||||||||||
}); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
it("should display in results, persons with non-matrix attributes matching the search query", async () => { | ||||||||||||||||||||||||
render( | ||||||||||||||||||||||||
<SpotlightDialog | ||||||||||||||||||||||||
initialFilter={Filter.People} | ||||||||||||||||||||||||
initialText={testPersonExternalAttributtes.location} | ||||||||||||||||||||||||
onFinished={() => null} | ||||||||||||||||||||||||
/> | ||||||||||||||||||||||||
); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
// search is debounced | ||||||||||||||||||||||||
jest.advanceTimersByTime(200); | ||||||||||||||||||||||||
await flushPromisesWithFakeTimers(); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
const options = document.querySelectorAll("div.mx_SpotlightDialog_option"); | ||||||||||||||||||||||||
expect(options.length).toBeGreaterThanOrEqual(1); | ||||||||||||||||||||||||
expect(options[0]!.innerHTML).toContain(testPersonExternalAttributtes.display_name); | ||||||||||||||||||||||||
}); | ||||||||||||||||||||||||
}); | ||||||||||||||||||||||||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should probably keep this: