Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
58f26ff
Add address search component and relay results to USPS search
allthesignals Nov 22, 2022
14924cc
Revert "Remove unused PasswordToggle React component (#7278)"
allthesignals Nov 10, 2022
5f5d5b4
Shape the address search results into expected shape for querying USPS
allthesignals Nov 22, 2022
7eb507d
changelog: Improvements, In-Person Proofing, AddressSearch component
allthesignals Nov 22, 2022
5caebcd
Extract request into separate package; set types
allthesignals Nov 23, 2022
09e5d77
Track whether a search has been performed once. Show message on init
allthesignals Nov 23, 2022
a0ae26a
Todo tests added; request is more flexible for plain get requests
allthesignals Nov 23, 2022
0c4f507
Update app/javascript/packages/request/index.ts
allthesignals Nov 23, 2022
93e8871
Update app/javascript/packages/request/README.md
allthesignals Nov 23, 2022
b872679
Infer initial search from presence of foundAddress
allthesignals Nov 23, 2022
43cfed2
Drop unused helper func
allthesignals Nov 28, 2022
f715f35
Rename to something more specific to the effect itself
allthesignals Nov 28, 2022
01a3538
Hide AddressSearch implement behind feature flag
allthesignals Nov 28, 2022
842085b
Add type def
allthesignals Nov 28, 2022
4f27e91
Update app/javascript/packages/document-capture/components/in-person-…
allthesignals Nov 28, 2022
2459a3e
Include search results in address search component for testing
allthesignals Nov 28, 2022
f7e221a
Use window.fetch so it’s stubbable
allthesignals Nov 28, 2022
9a53438
Remove prod-facing markup in favor of AddressSearch component
allthesignals Nov 28, 2022
e450a67
Merge branch 'main' into wmg/7927-po-search-fe-only
allthesignals Nov 28, 2022
fe0ae4f
Correct place to include header
allthesignals Nov 28, 2022
cadd296
Merge branch 'main' into wmg/7927-po-search-fe-only
allthesignals Nov 29, 2022
62e5fff
Remove styling and UX-specific work until ready...
allthesignals Nov 29, 2022
42f31ec
Use snake_case for data
allthesignals Nov 29, 2022
d71c237
Minimal test to ensure happy path
allthesignals Nov 29, 2022
01b0edd
Failing test
allthesignals Nov 29, 2022
a134f67
Add types for onChange signature
allthesignals Nov 30, 2022
f4f5aaa
Add passing test for request lib
allthesignals Nov 30, 2022
0d4684e
Fill out the rest of the tests
allthesignals Nov 30, 2022
279762c
Add polyfill for Response
allthesignals Nov 30, 2022
f2608d7
Refactor csrf to be injected another way
allthesignals Nov 30, 2022
011a2bd
Extract feature flag into new, IPP-specific Context
allthesignals Nov 30, 2022
8efd0f7
Refactor to use index syntax
allthesignals Nov 30, 2022
ea0895a
Move Response polyfil to spec helper
allthesignals Nov 30, 2022
c48bd3f
Add explicit assertion on window.fetch being called
allthesignals Nov 30, 2022
7430c7c
React isn’t used here
allthesignals Nov 30, 2022
1a673cc
No JSX, use TS
allthesignals Nov 30, 2022
6d7107c
Use memoization correctly
allthesignals Nov 30, 2022
eceb0ba
Merge branch 'main' into wmg/7927-po-search-fe-only
allthesignals Nov 30, 2022
3534a2a
Update app/javascript/packages/request/index.ts
allthesignals Nov 30, 2022
0fbf794
Test when CSRF is explicitly false
allthesignals Nov 30, 2022
fca9490
Try moving this up
allthesignals Nov 30, 2022
e619635
Rework approach to import
allthesignals Dec 1, 2022
2f0997b
Reverse render order
allthesignals Dec 1, 2022
f1ca342
Dropping this for now because of conflict with other type definitions
allthesignals Dec 1, 2022
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
10 changes: 3 additions & 7 deletions app/controllers/idv/in_person/address_search_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ class AddressSearchController < ApplicationController
check_or_render_not_found -> { IdentityConfig.store.arcgis_search_enabled }

def index
render json: addresses
render json: addresses(params[:address])
end

protected

def addresses
suggestion = geocoder.suggest(permitted_params[:address]).first
def addresses(search_term)
suggestion = geocoder.suggest(search_term).first
return [] unless suggestion
geocoder.find_address_candidates(suggestion.magic_key).slice(0, 1)
rescue Faraday::ConnectionFailed
Expand All @@ -22,10 +22,6 @@ def addresses
def geocoder
@geocoder ||= ArcgisApi::Geocoder.new
end

def permitted_params
params.permit(:address)
end
end
end
end
2 changes: 2 additions & 0 deletions app/javascript/packages/components/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@ export { default as ScrollIntoView } from './scroll-into-view';
export { default as SpinnerDots } from './spinner-dots';
export { default as StatusPage } from './status-page';
export { default as Tag } from './tag';
export { default as TextInput } from './text-input';
export { default as TroubleshootingOptions } from './troubleshooting-options';

export type { ButtonProps } from './button';
export type { FullScreenRefHandle } from './full-screen';
export type { LinkProps } from './link';
export type { TextInputProps } from './text-input';
48 changes: 48 additions & 0 deletions app/javascript/packages/components/text-input.spec.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { createRef } from 'react';
import { render } from '@testing-library/react';
import TextInput from './text-input';

describe('TextInput', () => {
it('renders with an associated label', () => {
const { getByLabelText } = render(<TextInput label="Input" />);

const input = getByLabelText('Input');

expect(input).to.be.an.instanceOf(HTMLInputElement);
expect(input.classList.contains('usa-input')).to.be.true();
});

it('uses an explicitly-provided ID', () => {
const customId = 'custom-id';
const { getByLabelText } = render(<TextInput label="Input" id={customId} />);

const input = getByLabelText('Input');

expect(input.id).to.equal(customId);
});

it('applies additional given classes', () => {
const customClass = 'custom-class';
const { getByLabelText } = render(<TextInput label="Input" className={customClass} />);

const input = getByLabelText('Input');

expect([...input.classList.values()]).to.have.all.members(['usa-input', customClass]);
});

it('applies additional input attributes', () => {
const type = 'password';
const { getByLabelText } = render(<TextInput label="Input" type={type} />);

const input = getByLabelText('Input') as HTMLInputElement;

expect(input.type).to.equal(type);
});

it('forwards ref', () => {
const ref = createRef<HTMLInputElement>();
render(<TextInput label="Input" ref={ref} />);

expect(ref.current).to.be.an.instanceOf(HTMLInputElement);
});
});
40 changes: 40 additions & 0 deletions app/javascript/packages/components/text-input.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { forwardRef } from 'react';
import type { InputHTMLAttributes, ForwardedRef } from 'react';
import { useInstanceId } from '@18f/identity-react-hooks';

export interface TextInputProps extends InputHTMLAttributes<HTMLInputElement> {
/**
* Text of label associated with input.
*/
label: string;

/**
* Optional explicit ID to use in place of default behavior.
*/
id?: string;

/**
* Additional class name to be applied to the input element.
*/
className?: string;
}

function TextInput(
{ label, id, className, ...inputProps }: TextInputProps,
ref: ForwardedRef<HTMLInputElement>,
) {
const instanceId = useInstanceId();
const inputId = id ?? `text-input-${instanceId}`;
const classes = ['usa-input', className].filter(Boolean).join(' ');

return (
<>
<label className="usa-label" htmlFor={inputId}>
{label}
</label>
<input ref={ref} className={classes} id={inputId} {...inputProps} />
</>
);
}

export default forwardRef(TextInput);
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import { TextInput, Button } from '@18f/identity-components';
import { request } from '@18f/identity-request';
import { useState, useCallback, ChangeEvent } from 'react';

interface Location {
street_address: string;
city: string;
state: string;
zip_code: string;
address: string;
}

interface AddressSearchProps {
onAddressFound?: (location: Location) => void;
}

export const ADDRESS_SEARCH_URL = '/api/addresses';

function AddressSearch({ onAddressFound = () => {} }: AddressSearchProps) {
const [unvalidatedAddressInput, setUnvalidatedAddressInput] = useState('');
const [addressQuery, setAddressQuery] = useState({} as Location);
const handleAddressSearch = useCallback(async () => {
const addressCandidates = await request(ADDRESS_SEARCH_URL, {
Copy link
Contributor

Choose a reason for hiding this comment

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

For future consideration: I think it'd be valuable if addressCandidates was typed here, since currently it's any.

One way I could see this working is to accept a generic parameter to request which indicates the expected response, e.g.

Suggested change
const addressCandidates = await request(ADDRESS_SEARCH_URL, {
const addressCandidates = await request<AddressCandidate[]>(ADDRESS_SEARCH_URL, {

Copy link
Contributor

Choose a reason for hiding this comment

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

What's the effect if a network disconnect causes a Promise rejection here?

Copy link
Contributor Author

@allthesignals allthesignals Dec 6, 2022

Choose a reason for hiding this comment

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

Probably something bad! I think the work we'll do in parallel on state management (Andrew's suggestion) involving SWR or something else will help standardize our network interactions...

method: 'POST',
headers: { 'Content-Type': 'application/json' },
json: { address: unvalidatedAddressInput },
});

const bestMatchedAddress = addressCandidates[0];
setAddressQuery(bestMatchedAddress);
onAddressFound(bestMatchedAddress);
}, [unvalidatedAddressInput]);

return (
<>
<TextInput
value={unvalidatedAddressInput}
onChange={(event: ChangeEvent) => {
const target = event.target as HTMLInputElement;

setUnvalidatedAddressInput(target.value);
}}
label="Search for an address"
/>
<Button onClick={handleAddressSearch}>Search</Button>
Comment on lines 43 to 45
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd expect these strings should be translated

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@aduth yup! 100% We have a ticket for it. cc @kellular

<>{addressQuery.address}</>
</>
);
}

export default AddressSearch;
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import userEvent from '@testing-library/user-event';
import { useSandbox } from '@18f/identity-test-helpers';
import AnalyticsContext, { AnalyticsContextProvider } from '../context/analytics';
import InPersonLocationStep, { LOCATIONS_URL } from './in-person-location-step';
import { ADDRESS_SEARCH_URL } from './address-search';
import InPersonContext from '../context/in-person';

describe('InPersonLocationStep', () => {
const DEFAULT_PROPS = { toPreviousStep() {}, onChange() {}, value: {} };
Expand All @@ -18,6 +20,20 @@ describe('InPersonLocationStep', () => {
.withArgs(LOCATIONS_URL)
.resolves({
json: () => Promise.resolve([{ name: 'Baltimore' }]),
} as Response)
.withArgs(ADDRESS_SEARCH_URL)
.resolves({
json: () =>
Promise.resolve([
{
address: '100 Main St, South Fulton, Tennessee, 38257',
location: { latitude: 36.501462000000004, longitude: -88.875981 },
street_address: '100 Main St',
city: 'South Fulton',
state: 'TN',
zip_code: '38257',
},
]),
} as Response);
});

Expand All @@ -40,4 +56,23 @@ describe('InPersonLocationStep', () => {

await findByText('{"selected_location":"Baltimore"}');
});

it('allows search by address when enabled', async () => {
const { findByText, findByLabelText } = render(
<InPersonContext.Provider value={{ arcgisSearchEnabled: true }}>
<InPersonLocationStep {...DEFAULT_PROPS} />
</InPersonContext.Provider>,
);

await userEvent.type(await findByLabelText('Search for an address'), '100 main');
await userEvent.click(await findByText('Search'));
await findByText('100 Main St, South Fulton, Tennessee, 38257');
expect(window.fetch).to.have.been.calledWith(
LOCATIONS_URL,
sandbox.match({
body: '{"address":{"street_address":"100 Main St","city":"South Fulton","state":"TN","zip_code":"38257"}}',
method: 'post',
}),
);
});
});
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import { useState, useEffect, useCallback, useRef, useContext } from 'react';
import { useI18n } from '@18f/identity-react-i18n';
import { PageHeading, SpinnerDots } from '@18f/identity-components';
import { request } from '@18f/identity-request';
import BackButton from './back-button';
import LocationCollection from './location-collection';
import LocationCollectionItem from './location-collection-item';
import AnalyticsContext from '../context/analytics';
import AddressSearch from './address-search';
import InPersonContext from '../context/in-person';

interface PostOffice {
Copy link
Contributor

Choose a reason for hiding this comment

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

should this interface match the postoffice struct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm i'm not sure. there was some history here... the naming is pretty confusing. i need to go through and rename a bunch of things.

Copy link
Contributor

Choose a reason for hiding this comment

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

the other struct has some other fields that aren't in this interface. I think we'd need those same fields here in order to display all the information that we display for each location on the location page.

address: string;
Expand All @@ -29,64 +32,20 @@ interface FormattedLocation {
sundayHours: string;
weekdayHours: string;
}

interface RequestOptions {
/**
* Whether to send the request as a JSON request. Defaults to true.
*/
json?: boolean;

/**
* Whether to include CSRF token in the request. Defaults to true.
*/
csrf?: boolean;

/**
* Optional. HTTP verb used. Defaults to GET.
*/
method?: string;
interface LocationQuery {
Copy link
Contributor

Choose a reason for hiding this comment

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

since the locationQuery and location interfaces have the same fields can we re-use one interface by pulling into a separate file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, let me look into it. It's very confusing (the naming is pretty bad).

streetAddress: string;
city: string;
state: string;
zipCode: string;
}

const DEFAULT_FETCH_OPTIONS = { csrf: true, json: true };

const getCSRFToken = () =>
document.querySelector<HTMLMetaElement>('meta[name="csrf-token"]')?.content;

const request = async (
url: string,
body: BodyInit | object,
options: Partial<RequestOptions> = {},
) => {
const headers: HeadersInit = {};
const mergedOptions: Partial<RequestOptions> = {
...DEFAULT_FETCH_OPTIONS,
...options,
};

if (mergedOptions.csrf) {
const csrf = getCSRFToken();
if (csrf) {
headers['X-CSRF-Token'] = csrf;
}
}

if (mergedOptions.json) {
headers['Content-Type'] = 'application/json';
body = JSON.stringify(body);
}

const response = await window.fetch(url, {
method: mergedOptions.method,
headers,
body: body as BodyInit,
});

return mergedOptions.json ? response.json() : response.text();
};

export const LOCATIONS_URL = '/verify/in_person/usps_locations';

const getUspsLocations = () => request(LOCATIONS_URL, {}, { method: 'post' });
const getUspsLocations = (address) =>
request(LOCATIONS_URL, {
method: 'post',
json: { address },
});

const formatLocation = (postOffices: PostOffice[]) => {
const formattedLocations = [] as FormattedLocation[];
Expand Down Expand Up @@ -124,10 +83,12 @@ const prepToSend = (location: object) => {
function InPersonLocationStep({ onChange, toPreviousStep }) {
const { t } = useI18n();
const [locationData, setLocationData] = useState([] as FormattedLocation[]);
const [foundAddress, setFoundAddress] = useState({} as LocationQuery);
const [inProgress, setInProgress] = useState(false);
const [autoSubmit, setAutoSubmit] = useState(false);
const [isLoadingComplete, setIsLoadingComplete] = useState(false);
const { setSubmitEventMetadata } = useContext(AnalyticsContext);
const { arcgisSearchEnabled } = useContext(InPersonContext);

// ref allows us to avoid a memory leak
const mountedRef = useRef(false);
Expand Down Expand Up @@ -156,7 +117,8 @@ function InPersonLocationStep({ onChange, toPreviousStep }) {
}
const selected = prepToSend(selectedLocation);
setInProgress(true);
await request(LOCATIONS_URL, selected, {
await request(LOCATIONS_URL, {
json: selected,
method: 'PUT',
})
.then(() => {
Expand All @@ -181,26 +143,35 @@ function InPersonLocationStep({ onChange, toPreviousStep }) {
[locationData, inProgress],
);

const handleFoundAddress = useCallback((address) => {
setFoundAddress({
streetAddress: address.street_address,
city: address.city,
state: address.state,
zipCode: address.zip_code,
});
}, []);

useEffect(() => {
let mounted = true;
let didCancel = false;
(async () => {
try {
const fetchedLocations = await getUspsLocations();
const fetchedLocations = await getUspsLocations(prepToSend(foundAddress));

if (mounted) {
if (!didCancel) {
const formattedLocations = formatLocation(fetchedLocations);
setLocationData(formattedLocations);
}
} finally {
if (mounted) {
if (!didCancel) {
setIsLoadingComplete(true);
}
}
})();
return () => {
mounted = false;
didCancel = true;
};
}, []);
}, [foundAddress]);

let locationsContent: React.ReactNode;
if (!isLoadingComplete) {
Expand Down Expand Up @@ -230,6 +201,7 @@ function InPersonLocationStep({ onChange, toPreviousStep }) {
return (
<>
<PageHeading>{t('in_person_proofing.headings.location')}</PageHeading>
{arcgisSearchEnabled && <AddressSearch onAddressFound={handleFoundAddress} />}
<p>{t('in_person_proofing.body.location.location_step_about')}</p>
{locationsContent}
<BackButton onClick={toPreviousStep} />
Expand Down
Loading