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

Prevent invalid dates from throwing an error on Android for expo-contacts #6694

Merged
merged 149 commits into from Feb 13, 2020

Conversation

EvanBacon
Copy link
Contributor

Why

fix #4757

How

Wrap the parse in a try/catch to prevent corrupting the entire query.

Test Plan

  • Add invalid date to android then attempt to parse it

@EvanBacon EvanBacon self-assigned this Jan 7, 2020
Copy link
Contributor

@esamelson esamelson left a comment

Choose a reason for hiding this comment

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

cool, thanks for bashing this bug 💪 my only comment is that i think it might be confusing to completely swallow this exception -- i think we should at least log the exception natively. since the date is originally a string, could we maybe also fall back to returning the original date string under rawBirthday or something like that if we can't parse it?

@EvanBacon
Copy link
Contributor Author

EvanBacon commented Jan 13, 2020

This is currently on hold while @lukmccall investigates bare-expo permissions not working.

update: seems permissions are just broken in bare when you request a permission that is granted. Can't verify that this returns because there doesn't seem to be an API to write incorrect dates.

Can verify that this doesn't break any existing functionality.

sjchmiela and others added 22 commits February 12, 2020 17:54
* [payments-stripe] move index to src/

* update docs
* Fix typo

* unversioned

Co-authored-by: Charlie Cruzan <35579283+cruzach@users.noreply.github.com>
import * as ErrorRecovery from 'expo-error-recovery'
import * as ErrorRecovery from 'expo-error-recovery'
* [linking] dont add extra slash to expoPrefix

* fix host for published projects, fix/add tests

* change test to localhost address

* make host null in client

* remove string interplation
When running `$yarn setup:native`, it may happen that the sdkmanager cannot be executed (`sdkmanager: command not found`) and the Android NDK is not installed. This happens when your bash_profile already contains the Android SDK path (ANDROID_HOME),but not the `${sdk}/tool/bin` path.

Checking whether `../tools/bin` is in the path is tricky and sketchy, instead this PR therefore makes the script more resilient by using the absolute path to reference the `sdkmanager`. Fixes #6617
* Fix library name from expo-three to expo-three-ar

- Change library name to expo-three-ar.
- Fix links.

* Change library name to expo-three-ar for v36 doc

* Change library name to expo-three-ar for v35 doc

* Change library name to expo-three-ar of v34 doc

- change library name.
- fix links

* Change library name to expo-three-ar for v33 doc

- Change library name.
- Fix links.

* Fix examples link for v36 doc

* Fix examples link for v35 doc

* Fix examples link for v34 doc

* Fix examples link for v33 doc
* [docs] errorRecovery supported in bare

* v36
* [docs] clarify FB app id directions

* v36

* typo

* typo
* Update localization.md

* fix typos

* Apply suggestions from code review

Co-Authored-By: Charlie Cruzan <35579283+cruzach@users.noreply.github.com>

* backdate localization docs

Co-authored-by: Charlie Cruzan <35579283+cruzach@users.noreply.github.com>
kopax and others added 18 commits February 12, 2020 17:55
# Why

Another step of `expo-notifications` extraction.

# How

- A singleton `EXNotificationCenterDelegate | NotificationManager` is informed of new notifications. The information is redirected to `EXNotificationDelegates | NotificationListeners`, which in fact are
- `EXNotificationsEmitters | NotificationsEmitters` which serialize the notifications and emit events to JS.

![excalidraw-202012311929-4](https://user-images.githubusercontent.com/1151041/73066742-1b7f2780-3ea7-11ea-83fb-0fd392643b82.png)

> Multiple `EventEmitter`s on the image show the situation that will happen in Expo client or when React instance is restarting. In normal situation there is only one emitter per bridge.

> **Note:** what happens to the notification (if it's presented, dismissed, etc) will be decided in another module/PR (handling notifications). Since handling is a non-trivial task (requires communication back and forth) I think splitting those concerns may make the module easier to maintain.

# Test Plan

Received notifications can be printed to `console` when a listener is registered.
* Fix image source prop uri

I found that the source prop needed to be an object with a `uri` key, the same as the example earlier in the file. I think this is just a typo.

* Fix image source prop uri in unversioned docs too
…6791)

- fixed several imports which no longer worked
- rename ‘GoogleService-info.plist’ -> ‘GoogleService-Info.plist’
- update ios workflow to include GoogleService-Info.plist
- fix REVERSED_CLIENT_ID reference
- fixed several typos
- Add optional firebase usage scenario
* Update webbrowser.md

Fix typo

* Update webbrowser.md

Typo fix

* Update webbrowser.md

Typo fix

* Update webbrowser.md

Typo fix
* Created @expo/html-elements package

* Created Anchor element

* Created Article

* Created Header

* Created Main

* Created Section

* Created Footer

* Created P

* Created B

* Created Strong

* Created Strike

* Created I

* Created Br

* Created Hr

* Created Code

* Created Table elements

* Updated text elements

* Updated Hr

* Created Nav

* Created Small

* Created Lists

* Created Mark

* Create Tfooter

* Update package.json

* Updated docs

* Update README.md

* Update README.md

* Update README.md

* Update packages/html-elements/README.md

Co-Authored-By: Brent Vatne <brentvatne@gmail.com>

* extract target from anchor

* Strike -> Del

* Updated headings docs

* Update README.md

* simplify ts method

* removed ts-ignores

* Ul => UL

* Ol => OL

* Li => LI

* Hr => HR

* Thead => THead

* Tbody => TBody

* Tfoot => TFoot

* Td => TD

* Br => BR

* Tr => TR

* Th => TH

* Em => EM

* Fix NCL screen

* Added E2E tests

* Updated .npmignore to remove tests, snapshots, and jest tsconfig

* Created Q and BlockQuote

* pixels -> value

* Added audio tag

* Update README.md

* update build files

* fix view and text styles

* Created Time

* Update README.md

* Update README.md

* updated tests

* Remove unused tests

* Created Pre

* Update README.md

* Update README.md

* Update README.md

* Update README.md

* internal links

* unify blocks

* Update README.md

Co-authored-by: Brent Vatne <brentvatne@gmail.com>
# Why

Next `expo-notifications` feature.

# How

- `NotificationsHandlerModule` registers at singleton for new notifications/messages
- for each message it _starts up_ a task which emits an event to JS
- in response to the JS event, delegate responds with the appropriate behavior (eg. `shouldShowAlert: true`)
- the behavior is pushed to native side using `NotificationsHandler.handleNotificationAsync` call
- which directs it to the appropriate task
- task handles the behavior (on iOS calls `completionHandler`, on Android it will show the notification once implemented) and finishes
- if for whatever reason delegate didn't respond in 3 seconds, `onTimeout` is called on task, which emits another event to JS (for debugging purposes) and the task finishes

![excalidraw-202012311929-7](https://user-images.githubusercontent.com/1151041/73078318-3ca14180-3ec2-11ea-9220-c7a2f3c1e558.png)

# Test Plan

Tested manually by sending notifications and logging messages that the scheme works both when the delegate responds and when it does not.
# Why

Part of `expo-notifications` rework. BTW fixes #5093.

# How

- on iOS:
  - implemented a requester that returns full information on current notification settings
  - implemented an exported module capable of getting and asking for notification permissions
  - added a method so that developers are able to ask for specific notification permissions
- on Android:
  - implemented a native module that returns a response based on `NotificationManagerCompat`
  - moreover, it also returns `importance` of notifications
- on web:
  - nothing yet, to be researched
- in TS:
  - added methods and type definitions for all the new features

# Test Plan

Added a simple test to `test-suite` and manually checked if the values returned match the settings.
* Created new layout elements

* Updated docs

* Updated small size

* updated heading snapshots

* lint fix

* fix format
@expo expo deleted a comment from ExpoBot Feb 13, 2020
@EvanBacon EvanBacon merged commit 9a7f78e into master Feb 13, 2020
@EvanBacon EvanBacon deleted the @evanbacon/contacts/prevent-invalid-date-from-throwing branch February 13, 2020 04:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expo contacts unparseable date