Skip to content
This repository has been archived by the owner on Jan 16, 2022. It is now read-only.

removes events #430

Merged
merged 4 commits into from
Aug 18, 2021
Merged

removes events #430

merged 4 commits into from
Aug 18, 2021

Conversation

redreceipt
Copy link
Member

@redreceipt redreceipt commented Jul 14, 2021

Screen Shot 2021-08-17 at 4 58 17 PM

@codecov
Copy link

codecov bot commented Jul 14, 2021

Codecov Report

Merging #430 (ea6b9cb) into master (a3b6598) will decrease coverage by 13.07%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           master     #430       +/-   ##
===========================================
- Coverage   58.73%   45.65%   -13.08%     
===========================================
  Files           7        3        -4     
  Lines         126       92       -34     
  Branches       27       18        -9     
===========================================
- Hits           74       42       -32     
+ Misses         50       48        -2     
  Partials        2        2               

Continue to review full report at Codecov.

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

@redreceipt redreceipt marked this pull request as ready for review August 17, 2021 20:58
@redreceipt redreceipt enabled auto-merge (squash) August 17, 2021 20:59
@@ -109,11 +108,6 @@ const App = () => (
stackPresentation: 'push',
})}
/>
<Screen
name="Event"
Copy link
Member

Choose a reason for hiding this comment

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

should we put in a redirect from Event to content? I know if you try to navigate to a route that doesn't exist, you'll get an app crash

@redreceipt
Copy link
Member Author

redreceipt commented Aug 17, 2021 via email

@conrad-vanl
Copy link
Member

They had an event opened before they upgraded to this version of the app
The API had an event as the action to link to (like in a feature) and either the app had cached content or the api wasn't properly updated
A push notification was sent out with /event in the url

I'm happy approving without this change....I was hoping react-navigation had an easy way to do redirects......

@redreceipt
Copy link
Member Author

redreceipt commented Aug 17, 2021 via email

@redreceipt
Copy link
Member Author

@conrad-vanl react-navigation 6 has built-in deep link support complete with a catch-all that we should probably move to. But I don't it's worth waiting on this PR considering I believe Willow is the only app that uses official events.

@redreceipt redreceipt requested a review from conrad-vanl August 18, 2021 00:34
@redreceipt redreceipt merged commit 1fc060e into master Aug 18, 2021
@redreceipt redreceipt deleted the events branch August 18, 2021 02:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants