Skip to content
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.

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

fix: [IOPID-1714] IOS crashes on EIC malformed URLs #5660

Merged
merged 12 commits into from
Apr 4, 2024

Conversation

shadowsheep1
Copy link
Member

@shadowsheep1 shadowsheep1 commented Apr 2, 2024

Short description

Warning

This PR depends on pagopa/io-cie-sdk#51

Important

The misleading copy on the error page under the NFC Session Manager is not a subject of this PR.

Tip

Translations ✅

This PR update the io-cie-sdk library to handle malformed responses to avoid crashes on iOS devices in those use cases.

demo

Generic error on iOS NFC Session Manager Alert
RPReplay_Final1712070613.MOV

List of changes proposed in this pull request

  • Fix UI regression in LoadingSpinnerOverlay.
  • Add a generic error message in case of malformed URLs.

How to test

Run the app in production with a real iOS device and try to login with EIC several times. No crash'd happen.

@pagopa-github-bot pagopa-github-bot changed the title [IOPID-1714] iOS crashes on EIC malformed URLs fix: [IOPID-1714] IOS crashes on EIC malformed URLs Apr 2, 2024
@pagopa-github-bot
Copy link
Collaborator

pagopa-github-bot commented Apr 2, 2024

Affected stories

  • 🐞 IOPID-1714: [APP] L'app va in crash dopo l'identificazione con CIE

Generated by 🚫 dangerJS against 7dec1a6

Copy link

codecov bot commented Apr 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 48.15%. Comparing base (94271fc) to head (7dec1a6).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5660      +/-   ##
==========================================
- Coverage   48.16%   48.15%   -0.01%     
==========================================
  Files        1461     1461              
  Lines       30971    30966       -5     
  Branches     7515     7512       -3     
==========================================
- Hits        14917    14912       -5     
  Misses      15986    15986              
  Partials       68       68              
Files Coverage Δ
ts/components/LoadingSpinnerOverlay.tsx 100.00% <100.00%> (ø)
...screens/authentication/cie/CieCardReaderScreen.tsx 5.52% <ø> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 94271fc...7dec1a6. Read the comment docs.

@shadowsheep1
Copy link
Member Author

@mariateresaventura copy review needed 💪

@mariateresaventura
Copy link
Contributor

@mariateresaventura copy review needed 💪

Copy edited.

@shadowsheep1 shadowsheep1 marked this pull request as ready for review April 4, 2024 06:42
@shadowsheep1 shadowsheep1 requested review from thisisjp and a team as code owners April 4, 2024 06:42
Copy link

dpulls bot commented Apr 4, 2024

🎉 All dependencies have been resolved !

Copy link
Contributor

@hevelius hevelius left a comment

Choose a reason for hiding this comment

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

LGTM. Tested on iOS physical device. It works correctly.

@LazyAfternoons LazyAfternoons self-requested a review April 4, 2024 09:24
Copy link
Contributor

@LazyAfternoons LazyAfternoons left a comment

Choose a reason for hiding this comment

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

LGTM

@shadowsheep1 shadowsheep1 merged commit 9c640a4 into master Apr 4, 2024
13 checks passed
@shadowsheep1 shadowsheep1 deleted the IOPID-1714-cie-crash-ios branch April 4, 2024 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
IO-A&I IO - Autenticazione e Identità
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants