Skip to content

LG-14958 #11746

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

Merged
WilliamBirdsall merged 1 commit into from
Jan 16, 2025
Merged

LG-14958 #11746

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 0 additions & 2 deletions app/javascript/packages/document-capture/components/document-capture-review-issues.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ describe('DocumentCaptureReviewIssues', () => {
inPersonURL: '/verify/doc_capture',
locationsURL: '',
addressSearchURL: '',
inPersonFullAddressEntryEnabled: false,
inPersonOutageMessageEnabled: false,
optedInToInPersonProofing: false,
usStatesTerritories: [['Los Angeles', 'NY']],
Expand Down Expand Up @@ -125,7 +124,6 @@ describe('DocumentCaptureReviewIssues', () => {
inPersonURL: '/verify/doc_capture',
locationsURL: '',
addressSearchURL: '',
inPersonFullAddressEntryEnabled: false,
inPersonOutageMessageEnabled: false,
optedInToInPersonProofing: false,
usStatesTerritories: [['Los Angeles', 'NY']],
Expand Down
13 changes: 3 additions & 10 deletions app/javascript/packages/document-capture/components/document-capture.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import { UploadFormEntriesError } from '../services/upload';
import SelfieStep from './selfie-step';
import DocumentsStep from './documents-step';
import InPersonPrepareStep from './in-person-prepare-step';
import InPersonLocationPostOfficeSearchStep from './in-person-location-post-office-search-step';
Copy link
Copy Markdown
Contributor

@aduth aduth Jan 16, 2025

Choose a reason for hiding this comment

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

This is the only place that the component is used, can we remove the file at in-person-location-post-office-search-step.tsx?

Copy link
Copy Markdown
Contributor Author

@WilliamBirdsall WilliamBirdsall Jan 16, 2025

Choose a reason for hiding this comment

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

There is another ticket for actually removing the old code, so that will be deleted next!

import InPersonLocationFullAddressEntryPostOfficeSearchStep from './in-person-location-full-address-entry-post-office-search-step';
import InPersonSwitchBackStep from './in-person-switch-back-step';
import ReviewIssuesStep from './review-issues-step';
Expand Down Expand Up @@ -39,22 +38,16 @@ function DocumentCapture({ onStepChange = () => {} }: DocumentCaptureProps) {
const { flowPath } = useContext(UploadContext);
const { trackSubmitEvent, trackVisitEvent } = useContext(AnalyticsContext);
const { isSelfieCaptureEnabled } = useContext(SelfieCaptureContext);
const {
inPersonFullAddressEntryEnabled,
inPersonURL,
skipDocAuthFromHandoff,
skipDocAuthFromHowToVerify,
} = useContext(InPersonContext);
const { inPersonURL, skipDocAuthFromHandoff, skipDocAuthFromHowToVerify } =
useContext(InPersonContext);
useDidUpdateEffect(onStepChange, [stepName]);
useEffect(() => {
if (stepName) {
trackVisitEvent(stepName);
}
}, [stepName]);
const appName = getConfigValue('appName');
const inPersonLocationPostOfficeSearchForm = inPersonFullAddressEntryEnabled
? InPersonLocationFullAddressEntryPostOfficeSearchStep
: InPersonLocationPostOfficeSearchStep;
const inPersonLocationPostOfficeSearchForm = InPersonLocationFullAddressEntryPostOfficeSearchStep;
Copy link
Copy Markdown
Contributor

@eileen-nava eileen-nava Jan 16, 2025

Choose a reason for hiding this comment

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

I only see the variable inPersonLocationPostOfficeSearchForm used once in this file, on L96 in the creation of locationFormStep. Is there any downside to removing the variable inPersonLocationPostOfficeSearchForm and just using InPersonLocationFullAddressEntryPostOfficeSearchStep on L96?


// Define different states to be used in human readable array declaration
const documentFormStep: FormStep = {
Expand Down
1 change: 0 additions & 1 deletion ...capture/components/in-person-location-full-address-entry-post-office-search-step.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ describe('InPersonLocationFullAddressEntryPostOfficeSearchStep', () => {
addressSearchURL: 'https://localhost:3000',
inPersonOutageMessageEnabled: false,
inPersonOutageExpectedUpdateDate: 'January 1, 2024',
inPersonFullAddressEntryEnabled: true,
optedInToInPersonProofing: false,
usStatesTerritories,
}}
Expand Down
1 change: 0 additions & 1 deletion .../packages/document-capture/components/in-person-location-post-office-search-step.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ describe('InPersonLocationPostOfficeSearchStep', () => {
addressSearchURL,
inPersonOutageMessageEnabled: false,
inPersonOutageExpectedUpdateDate: 'January 1, 2024',
inPersonFullAddressEntryEnabled: true,
optedInToInPersonProofing: false,
usStatesTerritories,
}}
Expand Down
1 change: 0 additions & 1 deletion app/javascript/packages/document-capture/components/in-person-outage-alert.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ describe('InPersonOutageAlert', () => {
addressSearchURL: 'https://localhost:3000/unused',
inPersonOutageExpectedUpdateDate: 'January 1, 2024',
inPersonOutageMessageEnabled: true,
inPersonFullAddressEntryEnabled: false,
optedInToInPersonProofing: false,
usStatesTerritories: [],
}}
Expand Down
2 changes: 0 additions & 2 deletions app/javascript/packages/document-capture/components/in-person-prepare-step.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ describe('InPersonPrepareStep', () => {
addressSearchURL: 'https://localhost:3000/unused',
inPersonOutageMessageEnabled: true,
inPersonOutageExpectedUpdateDate: 'January 1, 2024',
inPersonFullAddressEntryEnabled: false,
optedInToInPersonProofing: false,
usStatesTerritories: [],
}}
Expand All @@ -60,7 +59,6 @@ describe('InPersonPrepareStep', () => {
locationsURL: 'https://localhost:3000/unused',
addressSearchURL: 'https://localhost:3000/unused',
inPersonOutageMessageEnabled: false,
inPersonFullAddressEntryEnabled: false,
optedInToInPersonProofing: false,
usStatesTerritories: [],
}}
Expand Down
6 changes: 0 additions & 6 deletions app/javascript/packages/document-capture/context/in-person.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,6 @@ export interface InPersonContextProps {
*/
inPersonOutageExpectedUpdateDate?: string;

/**
* When true users must enter a full address when searching for a Post Office location
*/
inPersonFullAddressEntryEnabled: boolean;

/**
* When true a user has entered ipp by opting in
*/
Expand Down Expand Up @@ -70,7 +65,6 @@ const InPersonContext = createContext<InPersonContextProps>({
locationsURL: '',
addressSearchURL: '',
inPersonOutageMessageEnabled: false,
inPersonFullAddressEntryEnabled: false,
optedInToInPersonProofing: false,
usStatesTerritories: [],
});
Expand Down
2 changes: 0 additions & 2 deletions app/javascript/packs/document-capture.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@ const {
accountUrl: accountURL,
idvInPersonUrl: inPersonURL,
securityAndPrivacyHowItWorksUrl: securityAndPrivacyHowItWorksURL,
inPersonFullAddressEntryEnabled,
inPersonOutageMessageEnabled,
inPersonOutageExpectedUpdateDate,
optedInToInPersonProofing,
Expand Down Expand Up @@ -133,7 +132,6 @@ render(
addressSearchURL,
inPersonOutageExpectedUpdateDate,
inPersonOutageMessageEnabled: inPersonOutageMessageEnabled === 'true',
inPersonFullAddressEntryEnabled: inPersonFullAddressEntryEnabled === 'true',
optedInToInPersonProofing: optedInToInPersonProofing === 'true',
usStatesTerritories: parsedUsStatesTerritories,
skipDocAuthFromHowToVerify: skipDocAuthFromHowToVerify === 'true',
Expand Down
1 change: 0 additions & 1 deletion app/views/idv/shared/_document_capture.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
opted_in_to_in_person_proofing: opted_in_to_in_person_proofing,
idv_in_person_url: (IdentityConfig.store.in_person_doc_auth_button_enabled && Idv::InPersonConfig.enabled_for_issuer?(decorated_sp_session.sp_issuer)) ? idv_in_person_url : nil,
security_and_privacy_how_it_works_url: MarketingSite.security_and_privacy_how_it_works_url,
in_person_full_address_entry_enabled: IdentityConfig.store.in_person_full_address_entry_enabled,
in_person_outage_message_enabled: IdentityConfig.store.in_person_outage_message_enabled,
in_person_outage_expected_update_date: IdentityConfig.store.in_person_outage_expected_update_date,
us_states_territories: @presenter.usps_states_territories,
Expand Down
1 change: 0 additions & 1 deletion config/application.yml.default
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,6 @@ in_person_enrollments_ready_job_max_number_of_messages: 10
in_person_enrollments_ready_job_queue_url: ''
in_person_enrollments_ready_job_visibility_timeout_seconds: 30
in_person_enrollments_ready_job_wait_time_seconds: 20
in_person_full_address_entry_enabled: true
in_person_opt_in_available_completion_survey_url: 'https://handbook.login.gov'
in_person_outage_emailed_by_date: 'November 1, 2024'
# in_person_outage_expected_update_date and in_person_outage_emailed_by_date below
Expand Down
1 change: 0 additions & 1 deletion lib/identity_config.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,6 @@ def self.store
config.add(:in_person_enrollments_ready_job_queue_url, type: :string)
config.add(:in_person_enrollments_ready_job_visibility_timeout_seconds, type: :integer)
config.add(:in_person_enrollments_ready_job_wait_time_seconds, type: :integer)
config.add(:in_person_full_address_entry_enabled, type: :boolean)
config.add(:in_person_opt_in_available_completion_survey_url, type: :string)
config.add(:in_person_outage_emailed_by_date, type: :string)
config.add(:in_person_outage_expected_update_date, type: :string)
Expand Down
6 changes: 1 addition & 5 deletions spec/features/idv/in_person_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -393,13 +393,9 @@
end
end

context 'when full form address entry is enabled for post office search' do
context 'when full form address post office search' do
Copy link
Copy Markdown
Contributor

@eileen-nava eileen-nava Jan 16, 2025

Choose a reason for hiding this comment

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

Nit: This could probably be condensed into just an it statement.

Copy link
Copy Markdown
Contributor Author

@WilliamBirdsall WilliamBirdsall Jan 16, 2025

Choose a reason for hiding this comment

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

Oh like we can remove the wrapping context? Wasn't sure if it was helpful due to the user creation line directly under it, but I guess there aren't any other it blocks that would use that so removing is probably a good idea.

Ill be in here again once the form is simplified to zip. Should add to that ticket to make sure test structure is as simple as possible? Or just simplify it now?

Copy link
Copy Markdown
Contributor

@eileen-nava eileen-nava Jan 16, 2025

Choose a reason for hiding this comment

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

To me, the context block felt unnecessary because the user will always be using full form address post office search. I would see the value if we had another option enabled, like searching by zip code, but that's not the app's current state.

I also think simplifying this to an it statement felt consistent with the top of the file, which uses it statements for conditions that I'd always expect to be true. For example, in the same way that we expect the app to always allow the user to cancel and start over from the beginning, we expect the app to always allow the user to search by full address. Does that make sense?

The it block I used as an example creates the user variable inside of it, and I generally prefer to create that kind of variable outside of it blocks with a let statement. So, maybe the example I'm using isn't the best, either. 😆 I'm also good with you leaving this code as is! It's up to you. The comment is a non-blocking nit.

Copy link
Copy Markdown
Contributor Author

@WilliamBirdsall WilliamBirdsall Jan 16, 2025

Choose a reason for hiding this comment

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

Definitely makes sense! I think ill keep it in there for this PR but it will probably change across the next couple PRs that deal more directly with zip stuff and code removal. I agree with your thinking for sure!

let(:user) { user_with_2fa }

before do
allow(IdentityConfig.store).to receive(:in_person_full_address_entry_enabled).and_return(true)
end

it 'allows the user to search by full address', allow_browser_log: true do
sign_in_and_2fa_user(user)
begin_in_person_proofing(user)
Expand Down