LG-8507 Search Results Pluralization

[JackRyan1989]

🎫 Ticket

🛠 Summary of changes

• Added one and other fields to in_person_proofing.body.location.po_search.results_description
• Updated copy in each appropriate field for each available translation: en, es, and fr.
• Added two test cases: 1. checking correct pluralization for single results 2. checking correct pluralization for multiple (in this case 2) results.

📜 Testing Plan

Provide a checklist of steps to confirm the changes.

• Confirm front end changes for each translation: english, spanish and french
• Confirm that i18n tests pass.
• Confirm that new test spec passes.

[allthesignals]

I just had a concern about false positives, let me know if I'm missing something

[allthesignals]

I can't recall exactly how i18n works in the test context but it surprises me that this would work... are you sure this isn't a false positive? Like, maybe it isn't interpolating those values and you're just asserting for in_person_proofing.body.location.po_search.results_description (I'd expect it t to ignore the params).

That said, I'm not really sure!

[aduth]

findByText returns a promise, which will be truthy, but I don't think that's what you're actually wanting to test.

!!new Promise(() => {})
// true

I expect you'd want to await the findByText, which will throw if it never finds the text.

https://testing-library.com/docs/queries/about/#types-of-queries

[JackRyan1989]

Hey Matt, I'm trying to replicate as closely as possible what's happening in in-person-locations.tsx:

<h3 role="status">
        {!isPilot &&
          t('in_person_proofing.body.location.po_search.results_description', {
            address,
            count: locations?.length,
          })}
 </h3>

So I think t does take in a second parameter that is an object.

[JackRyan1989]

findByText returns a promise, which will be truthy, but I don't think that's what you're actually wanting to test.

!!new Promise(() => {})
// true

I expect you'd want to await the findByText, which will throw if it never finds the text.

Great catch! I'm incorporating that change.

[allthesignals]

@JackRyan1989 yup, it does take that as an argument, but I'm not sure this works the same way in the test environment since it displays the translation paths.

[JackRyan1989]

Ok gotcha. Yeah you're right. It's not actually translating the content:

[allthesignals]

Yeah, I was thinking about this a little bit more... I'm not sure what one should do in this case.

It'd be nice if our i18n library would append dynamic arguments to the dictionary path when under test. Then we could assert for evidence of the behavior. Or it could even append which one it rendered: results_description.one or results_description.other.

@aduth any thoughts on this?

[zachmargolis]

Or it could even append which one it rendered: results_description.one or results_description.other.

yeah that matches my expectations, we'd render the one or the other "string" which in this case happens to be the key

[allthesignals]

@JackRyan1989 @zachmargolis made a great suggestion, and I gave it a shot in a branch here: https://github.com/18F/identity-idp/tree/wmg/lg-8507-test-setup

I did run into some snags with the (confusing) way I set up the test here...

1. Note that you'll want your MULTI_LOCATION_RESPONSE as the response body of LOCATIONS_URL, not ADDRESS_SEARCH_URL.
2. I think the before hook you were using wasn't getting applied to the relevant test. I went ahead and moved it inside the test in the branch linked above... open to feedback on a better way to organize test-specific server responses because it's still a pain point to use.

[aduth]

By default, there is no locale data loaded in the test environment, and the default behavior of the i18n library absent locale strings is to render the given string. This is usually okay for testing, but unfortunately it doesn't account for pluralization, so you either have to be content in testing the parent key, or set up the locale data in the tests with pluralization data (plain JS approach, React context approach). (Edit: I see this is what @zachmargolis recommended in the linked Slack thread 🤦 )

[allthesignals]

Great!