COVID-19-electronic-health-system / Corona-tracker

An easy-to-use PWA to monitor the user's wellness and learn about COVID-19.
https://coronatracker.me/
MIT License
235 stars 101 forks source link

Redux-subscribe #671

Closed fallon7284 closed 4 years ago

fallon7284 commented 4 years ago

⚠️ IMPORTANT: Please do not create a Pull Request without creating an issue first.

All changes need to be discusssed before proceeding. Failure to do so may result in the pull request being rejected.

Please be sure to review our Code of Conduct, Contributing Guidelines, and Support documentation before submitting a pull request.


Please include the issue number the pull request fixes by replacing YOUR-ISSUE-HERE in the text below.

Fixes #537

Summary

Adds subscribed phone number state to onboard reducer. Also error and success state for rendering snackbar dialogs.

New flow for subscribe action, to allow conditional rendering based on whether number is subscribed: Call subscribe/unsubscribe API. Receive response. If call succeeds, add/remove number from gaia, then add/remove it from redux store. Update success state string (either "subscribed" or "unsubscribed") in store, triggering rendering of proper snackbar dialog. If response fails, update error object in store, triggering error snackbar dialog. On dialog close, clear error/success status in store. Render subscribe/unsubscribe buttons based on existence of subscribedNumber state in redux store.

Fetch subscribed number on app load, add to Redux store.

Details

This update allows for conditional rendering of subscribe/unsubscribe buttons and phone number input field, allowing the user to be sure whether they have successfully subscribed previously, and at which number. Stops user from subscribing same number twice, or unsubscribing a number that isn't subscribed. Also decouples async api calls and error handling from UI code.

Test Plan (required)

I have not written tests for this code. I'm not well versed in testing and I don't really know how to approach testing this update, and I'm a bit busy. If anyone wants to talk through that with me, I'd be happy to look at adding tests.

subscribe

Final Checklist

fallon7284 commented 4 years ago

I guess there's already a test on this component and it fails now because of the reliance on the redux store. I'll see if I can figure out what I need to do to update the test.

fallon7284 commented 4 years ago

Without seeing what you're actually describing, I can't be certain, but I think I know what you're referring to, and it's a feature, not a bug. The "snackbar" alert component that the developer who originally built the More component used closes in one of two ways: with a click, or with a 6 second timeout -- line 282.
Screen Shot 2020-04-29 at 7 06 23 PM I tried to preserve as best as possible the previous design of the component. A couple things weren't working as I think the original developer intended: one was that the asynchronous subscribe call, when resolved, would trigger a close() call on the more component. This UX choice made sense to me: a user clicked on more in order to subscribe or unsubscribe, and when that action resolved, the more dialog would be closed. But because the success snackbar alert was a child of More, when More closed, the alert disappeared. This was a little bit awkward visually, there wasnt quite enough time to read the Subscribed successfully! You will be automatically unsubscribed in one day, and will receive three texts in that timespan. If you enter again, you will receive double the notifications - so please do not! This is very early alpha :) alert before it disappeared. I decided to tweak it -- while maintaining the original developer's intention of closing the More component after subscribe/unsubscribe -- by making the click that closes the alert also close the More dialog, rather than the More dialog, and with it it's child, the alert, close itself too quickly. Successful subscribe, see the alert, close the alert and with it the more dialog. In any case, assuming this is what you're describing, we could easily either 1) not have the More dialog close when the Alert is clicked away or 2) not have the alert close automatically after 6 seconds. I don't think it has anything to do with the error object being held in the redux store, and it isn't obvious to me what would be a better way to have the resolution of these API calls trigger alerts describing the different kinds of success or failure outcomes of the API call.

fallon7284 commented 4 years ago

You got it.

fallon7284 commented 4 years ago

actually let me quickly fix a couple PropTypes errors before merging

fallon7284 commented 4 years ago

Should be good to go.

@acthelemann @SomeMoosery @pavel-ilin @AdhamAH If you guys like the work I'm doing I just want to point out I'm still looking for a first professional role after my bootcamp experience and I'm struggling to get interviews, so I'd love the opportunity to talk more about what you all think I can do to be a better candidate and if anyone would like to refer me that would probably be really valuable for me.

AdhamAH commented 4 years ago

@fallon7284 I am like you looking for work! If any one has an input to help would be great. @SomeMoosery @acthelemann 😅

acthelemann commented 4 years ago

I can't really help you guys on how to be better. I've had some pretty bad experiences trying to find jobs myself. Doing project work like this should help.

Where I work currently is on a hiring freeze, but I still may be able to refer you. If not now then later. Feel free to message me on Discord if you want more details.

acthelemann commented 4 years ago

It looks like something was merged in so this will need to be updated again

SomeMoosery commented 4 years ago

Unfortunately I can't be much help either in the way of direct jobs (Connor and I work at the same place i.e. my company is on a hiring freeze as well).

But, I'm willing to offer any help I can for software engineering interviews such as Leetcode problems or system design related questions. I'm a bit rusty at both, but I've spent some significant time studying these in the past and have picked up some tricks / patterns. Arbitrary / annoying as they may be (as they clearly have very little bearing on the quality of a developer), they're prevalent in most interviews nowadays and I imagine will be even more so now that interviews are all virtual.