Skip to content

DRAFT Shannon/lg 8410 po search log#7668

Closed
svalexander wants to merge 13 commits intomainfrom
shannon/lg-8410-po-search-log
Closed

DRAFT Shannon/lg 8410 po search log#7668
svalexander wants to merge 13 commits intomainfrom
shannon/lg-8410-po-search-log

Conversation

@svalexander
Copy link
Contributor

No description provided.

def addresses(search_term)
suggestion = geocoder.suggest(search_term).first
# this happens multiple times
if !suggestion
Copy link
Contributor Author

Choose a reason for hiding this comment

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

if we input an address like PO Box 168, Yellowstone National Park, WY 82190-0168 arcgis will not find an appropriate suggestion. We want to log this in analytics. That happens successfully but multiple times rather than a single time.

Comment on lines +637 to +641
track_event('IdV: in person proofing location search submitted',
success: success,
errors: errors,
**extra,
)
Copy link
Contributor

Choose a reason for hiding this comment

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

the indentation is off here, I'd also suggest bumping the string to the next line

Suggested change
track_event('IdV: in person proofing location search submitted',
success: success,
errors: errors,
**extra,
)
track_event(
'IdV: in person proofing location search submitted',
success: success,
errors: errors,
**extra,
)

end
rescue => err
Rails.logger.warn(err)
analytics.idv_in_person_location_searched(success: false, errors: err)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

will update err here to reflect more robust err when pr 7675 is merged

// server.close();
// });

it('logs an event when clicking the call to action button', async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

analytics test i'd like eyes on

Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you suspect this isn't really testing something? Is there a way you can get it to fail first? For example, remove the source code that logs the event and run the test. If it passes, it isn't really working.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it seemed to me it wasn't truly working bc i changed the value of the error to something random and it still worked. Can confirm with your suggestion the test still passes.

@svalexander svalexander marked this pull request as draft January 26, 2023 17:02
@svalexander svalexander marked this pull request as ready for review January 27, 2023 17:00
@svalexander svalexander closed this Feb 3, 2023
@svalexander svalexander deleted the shannon/lg-8410-po-search-log branch February 15, 2023 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants