Skip to content

LG-10518: Delete capture troubleshooting component and update tests#9017

Merged
night-jellyfish merged 11 commits intomainfrom
brittany/lg-10518-delete-capture-troubleshooting-component-update-tests
Aug 18, 2023
Merged

LG-10518: Delete capture troubleshooting component and update tests#9017
night-jellyfish merged 11 commits intomainfrom
brittany/lg-10518-delete-capture-troubleshooting-component-update-tests

Conversation

@night-jellyfish
Copy link
Contributor

@night-jellyfish night-jellyfish commented Aug 16, 2023

🎫 Ticket

LG-10518

🛠 Summary of changes

We don't want to show troubleshooting tips this way any longer. Instead, we're looking to use the help center for showing this sort of advice.

In #8842, in order to avoid showing the tips, we increased the amount of tries before showing the tips to well past the maximum amount of total tries. Upon further discussion, we decided it would be better to remove the CaptureTroubleshooting component entirely, as we have no plans to reuse it in the future.

Once CaptureTroubleshooting was removed, a few more things were now unused and could also be removed - the config setting the amount of tries before tips were shown, the copy used in the tips, the CaptureAdvice component, and any place calling these things.

We also wanted to add a test to make sure we see what we are expecting to see when max attempts are reached.

📜 Testing Plan

(Stolen shamelessly from @eileen-nava's PR linked above, as that PR and this one are testing the same outcome)

  • Enable local mobile development
  • Make a local change to force sdk image capture failure (eg increase glareThreshold in onAcuantImageCaptureSuccess method to 100)
  • Attempt to capture an image with the sdk three times
  • After the third failed attempt, confirm that you are redirected to the native camera and do not see the Document Capture Tips

👀 Screenshots

(Also stolen shamelessly from @eileen-nava's PR linked above, for the same reason)

Here is a screenshot of the page that we DON'T want the user to see

Document Capture Tips:

Screen Shot 2023-07-24 at 2 18 41 PM

@night-jellyfish night-jellyfish requested review from a team, eileen-nava and kellular and removed request for a team August 16, 2023 18:56
Copy link

@kellular kellular left a comment

Choose a reason for hiding this comment

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

LGTM - I'm ok with deleting this copy if we are certain this is not being used anywhere right now

@eileen-nava
Copy link
Contributor

@night-jellyfish I'm not sure if you saw this in the build yet, but if you run the task i18n-tasks unused, it will show you unused keys in the yml translation files. Below is the output.

i18n-tasks unused     
#StandWithUkraine
Unused keys (3) | i18n-tasks v1.0.12
+--------+---------------------------------------------------+-------------------------------------------+
| Locale | Key                                               | Value                                     |
+--------+---------------------------------------------------+-------------------------------------------+
|   en   | idv.troubleshooting.headings.still_having_trouble | Still having trouble?                     |
|   es   | idv.troubleshooting.headings.still_having_trouble | ¿Sigue teniendo dificultades?             |
|   fr   | idv.troubleshooting.headings.still_having_trouble | Vous rencontrez toujours des difficultés? |
+--------+---------------------------------------------------+-------------------------------------------+

@night-jellyfish
Copy link
Contributor Author

Thank you, @eileen-nava ! I had just figured out how to log back into Gitlab and saw that error, but I didn't realize we had a check like that. That's pretty cool! I wonder if it should be in make lint...

Anyways, I pushed up a commit deleting those unused translations.

Copy link
Contributor

@eileen-nava eileen-nava left a comment

Choose a reason for hiding this comment

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

I left a few comments. Also, I agree with @aduth that if the photos from the deleted component aren't being used anywhere else, it would be good to delete them.

@eileen-nava eileen-nava changed the title LG 10518: Delete capture troubleshooting component and update tests LG-10518: Delete capture troubleshooting component and update tests Aug 17, 2023
@dawei-nava
Copy link
Contributor

@night-jellyfish I'm not sure if you saw this in the build yet, but if you run the task i18n-tasks unused, it will show you unused keys in the yml translation files. Below is the output.

i18n-tasks unused     
#StandWithUkraine
Unused keys (3) | i18n-tasks v1.0.12
+--------+---------------------------------------------------+-------------------------------------------+
| Locale | Key                                               | Value                                     |
+--------+---------------------------------------------------+-------------------------------------------+
|   en   | idv.troubleshooting.headings.still_having_trouble | Still having trouble?                     |
|   es   | idv.troubleshooting.headings.still_having_trouble | ¿Sigue teniendo dificultades?             |
|   fr   | idv.troubleshooting.headings.still_having_trouble | Vous rencontrez toujours des difficultés? |
+--------+---------------------------------------------------+-------------------------------------------+

or run bundle exec i18n-tasks health

@night-jellyfish night-jellyfish force-pushed the brittany/lg-10518-delete-capture-troubleshooting-component-update-tests branch from 45f527f to 2754d1f Compare August 17, 2023 20:54
@eileen-nava eileen-nava self-requested a review August 18, 2023 17:25
Copy link
Contributor

@eileen-nava eileen-nava left a comment

Choose a reason for hiding this comment

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

Approved! Thanks for being so diligent about responding to feedback.

Brittany Greaner added 11 commits August 18, 2023 10:28
Originally we were going to test that the tips copy wasn't shown. But
now that we're deleting the copy from the repo entirely, expecting the
tips not to be shown, in a way, would be keeping the tips in the repo.

So instead, we are are adding a test to make sure we see what we expect
on the mobile side.

Implementation note: We have to freeze time; otherwise the `timeout`
will cause the test to flicker. (Example: The test will look for
"try again in 6 hours" but the page will say "try again in 5 hours and
59 minutes".)
Includes removing imports that are now unused within the spec file
Initially I found I needed this when I had the test running in the
desktop context. But when I ran the test in the mobile context, I didn't
seem to need it.

I just saw the test flicker again with a console log error, so I am
adding it back in to guard against future flickering.
@night-jellyfish night-jellyfish force-pushed the brittany/lg-10518-delete-capture-troubleshooting-component-update-tests branch from b274e88 to 3bf8136 Compare August 18, 2023 17:28
@night-jellyfish night-jellyfish merged commit 6edf864 into main Aug 18, 2023
@night-jellyfish night-jellyfish deleted the brittany/lg-10518-delete-capture-troubleshooting-component-update-tests branch August 18, 2023 17:46
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.

5 participants