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

feat: make redirects via the input component case-insensitive #875

Merged
merged 3 commits into from
Feb 8, 2023
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
6 changes: 6 additions & 0 deletions .changeset/strong-tomatoes-sneeze.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@sajari/react-search-ui': minor
'sajari-sdk-docs': patch
---

feat: make redirects case-insensitive for search input component
3 changes: 2 additions & 1 deletion .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@
"react/react-in-jsx-scope": "off",

// TODO: We should re-enable this if we think it's worthwhile
"@typescript-eslint/explicit-module-boundary-types": "off"
"@typescript-eslint/explicit-module-boundary-types": "off",
"no-plusplus": "off"
},
"settings": {
"react": {
Expand Down
6 changes: 6 additions & 0 deletions docs/pages/search-ui/input.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,12 @@ function Example() {
}
```
### Note
The `Input` component treats redirects as _case-insensitive_, meaning the user-inputted value `Computer` returns the redirect for the query `computer` and vice versa.
The component does not support the handling of multiple redirects for queries of the same word with varied letter casing. For example, if you have created redirects for each of the queries `computer`, `Computer`, and `COMPUTER`, only one of them is used (dependent upon your browser).
## Results
```jsx
Expand Down
44 changes: 40 additions & 4 deletions packages/search-ui/src/Input/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,12 @@ const redirectTarget = {
token:
'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJwdXJwb3NlIjoic2VhcmNoIiwiZGVzdGluYXRpb24iOiJodHRwOi8vdGFyZ2V0LmNvbS5hdS9zaGVldHMiLCJ2YWxzIjp7ImNvbGxlY3Rpb24iOlsiYmVzdGJ1eSJdLCJpZGVudGlmaWVyIjpbInJlZGlyZWN0Il0sInByb2plY3QiOlsiMTU5NDE1MzcxMTkwMTcyNDIyMCJdLCJxIjpbInNoZWV0cyJdLCJxLmlkIjpbIjc2ZGJiOGU2LWE3MDctNDU2NC1iYTYxLWY0NjNiYTRhZDdlYSJdLCJxLnVpZCI6WyI3NmRiYjhlNi1hNzA3LTQ1NjQtYmE2MS1mNDYzYmE0YWQ3ZWEwIl0sInJlZGlyZWN0LkNvbmRpdGlvbiI6WyJxIH4gJ3NoZWV0cyciXSwicmVkaXJlY3QuSUQiOlsiMjJ3MFZGdkdWYVlhQ1Jzc0NBUm11YkQ2bGdUIl0sInJlZGlyZWN0LlRhcmdldCI6WyJodHRwOi8vdGFyZ2V0LmNvbS5hdS9zaGVldHMiXX19.BhcAVPB4z9LjlIoV42CUaEW-H0qCJ2JKngs6OGAXTf8',
};
const redirectTarget2 = {
id: '12w0VFvGVaYaCRssCARmubD6lgA',
target: 'http://target.com.au/sheets2',
token:
'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJwdXJwb3NlIjoic2VhcmNoIiwiZGVzdGluYXRpb24iOiJodHRwOi8vdGFyZ2V0LmNvbS5hdS9zaGVldHMiLCJ2YWxzIjp7ImNvbGxlY3Rpb24iOlsiYmVzdGJ1eSJdLCJpZGVudGlmaWVyIjpbInJlZGlyZWN0Il0sInByb2plY3QiOlsiMTU5NDE1MzcxMTkwMTcyNDIyMCJdLCJxIjpbInNoZWV0cyJdLCJxLmlkIjpbIjc2ZGJiOGU2LWE3MDctNDU2NC1iYTYxLWY0NjNiYTRhZDdlYSJdLCJxLnVpZCI6WyI3NmRiYjhlNi1hNzA3LTQ1NjQtYmE2MS1mNDYzYmE0YWQ3ZWEwIl0sInJlZGlyZWN0LkNvbmRpdGlvbiI6WyJxIH4gJ3NoZWV0cyciXSwicmVkaXJlY3QuSUQiOlsiMjJ3MFZGdkdWYVlhQ1Jzc0NBUm11YkQ2bGdUIl0sInJlZGlyZWN0LlRhcmdldCI6WyJodHRwOi8vdGFyZ2V0LmNvbS5hdS9zaGVldHMiXX19.BhcAVPB4z9LjlIoV42CUaEW-H0qCJ2JKngs6OGAXTf9',
};
const iPhoneResult = {
values: {
_id: {
Expand Down Expand Up @@ -49,10 +55,11 @@ const server = setupServer(
values: {
q: 'sheets',
'q.original': 'sheets',
'q.suggestions': 'sheets',
'q.suggestions': 'suggestion',
},
redirects: {
sheets: redirectTarget,
jkaho marked this conversation as resolved.
Show resolved Hide resolved
Sheets: redirectTarget2,
},
searchResponse: {
reads: '141',
Expand All @@ -77,6 +84,7 @@ describe('Input', () => {
});
afterEach(() => {
jest.clearAllMocks();
jest.resetAllMocks();
server.resetHandlers();
});
afterAll(() => {
Expand All @@ -92,12 +100,12 @@ describe('Input', () => {
const input = screen.getByTestId('mysearch');
input.focus(); // need this else we get TypeError: element.ownerDocument.getSelection is not a function
await user.keyboard('sheets');
await waitFor(() => expect(screen.getByText('sheets')).toBeInTheDocument());
await waitFor(() => expect(screen.getByRole('option')).toHaveTextContent('suggestion'));
await user.keyboard('{enter}');

expect(onRedirectSpy).toHaveBeenCalledWith({
...redirectTarget,
token: `https://re.sajari.com/token/${redirectTarget.token}`, // sdk-js prepends the clickTokenURL
...redirectTarget2,
token: `https://re.sajari.com/token/${redirectTarget2.token}`, // sdk-js appends the clickTokenURL
});
});

Expand Down Expand Up @@ -156,3 +164,31 @@ describe('Input', () => {
await waitFor(() => expect(input.attributes.getNamedItem('readonly')).toBeNull());
});
});

describe('redirects', () => {
beforeAll(() => {
server.listen({ onUnhandledRequest: 'warn' });
});
afterAll(() => {
jest.restoreAllMocks();
server.close();
});

it('ignores letter casing of input query on redirect lookup', async () => {
const onRedirectSpy = jest.spyOn(eventTrackingPipeline.getTracking(), 'onRedirect');
customRender(<Input data-testid="mysearch" mode="suggestions" />, {
search: { pipeline: eventTrackingPipeline },
});
const input = screen.getByTestId('mysearch');
input.focus(); // need this else we get TypeError: element.ownerDocument.getSelection is not a function
await userEvent.type(input, 'ShEeTs');
await waitFor(() => expect(screen.getByRole('option')).toHaveTextContent('suggestion'));
await userEvent.keyboard('{enter}');
await waitFor(() => {
expect(onRedirectSpy).toHaveBeenCalledWith({
...redirectTarget2,
token: `https://re.sajari.com/token/${redirectTarget2.token}`, // sdk-js appends the clickTokenURL
});
});
});
});
10 changes: 6 additions & 4 deletions packages/search-ui/src/Input/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { useSearchUIContext } from '../ContextProvider';
import { ResultValues } from '../Results/types';
import mapResultFields from '../utils/mapResultFields';
import { InputProps } from './types';
import { lowercaseObjectKeys } from './utils';

const Input = React.forwardRef((props: InputProps<any>, ref: React.ForwardedRef<HTMLInputElement>) => {
const { t } = useTranslation('input');
Expand All @@ -36,8 +37,9 @@ const Input = React.forwardRef((props: InputProps<any>, ref: React.ForwardedRef<
redirects,
searching: autocompleteSearching,
} = useAutocomplete();
const redirectsRef = useRef(redirects);
redirectsRef.current = redirects;
const lowercasedRedirects = lowercaseObjectKeys(redirects);
const redirectsRef = useRef(lowercasedRedirects);
redirectsRef.current = lowercasedRedirects;
const { customClassNames, disableDefaultStyles = false, tracking } = useSearchUIContext();
const { query } = useQuery();
const [internalSuggestions, setInternalSuggestions] = useState<Array<any>>([]);
Expand Down Expand Up @@ -174,7 +176,7 @@ const Input = React.forwardRef((props: InputProps<any>, ref: React.ForwardedRef<
if (!retainFilters) {
resetFilters();
}
const redirectValue = redirectsRef.current[value];
const redirectValue = redirectsRef.current[value.toLowerCase()];
jkaho marked this conversation as resolved.
Show resolved Hide resolved
if (!disableRedirects && redirectValue) {
tracking.onRedirect(redirectValue);
window.location.assign(redirectValue.token || redirectValue.target);
Expand All @@ -183,7 +185,7 @@ const Input = React.forwardRef((props: InputProps<any>, ref: React.ForwardedRef<
// If we're performing an autocomplete search, wait a tick to recheck redirects before unloading
e.preventDefault();
setTimeout(() => {
const redirectTarget = redirectsRef.current[value];
const redirectTarget = redirectsRef.current[value.toLowerCase()];
if (redirectTarget) {
tracking.onRedirect(redirectTarget);
window.location.assign(redirectTarget.token || redirectTarget.target);
Expand Down
20 changes: 20 additions & 0 deletions packages/search-ui/src/Input/utils/index.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { lowercaseObjectKeys } from '.';

test('lowercaseRedirects', () => {
const redirects = {
FoO: { id: '1', target: 'a' },
BAR: { id: '2', target: 'b' },
baz: { id: '3', target: 'c' },
test: { id: '4', target: 'd' },
Test: { id: '5', target: 'e' },
};
const want = {
foo: { id: '1', target: 'a' },
bar: { id: '2', target: 'b' },
baz: { id: '3', target: 'c' },
test: { id: '5', target: 'e' },
};

const got = lowercaseObjectKeys(redirects);
expect(got).toStrictEqual(want);
});
9 changes: 9 additions & 0 deletions packages/search-ui/src/Input/utils/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
export function lowercaseObjectKeys(obj: object) {
const newObj = {};
const keys = Object.keys(obj);
for (let i = 0; i < keys.length; i++) {
const key = keys[i];
newObj[key.toLowerCase()] = obj[key];
}
return newObj;
}