{"action":"created","issue":{"url":"https://api.github.com/repos/tidepool-org/chrome-uploader/issues/332","repository_url":"https://api.github.com/repos/tidepool-org/chrome-uploader","labels_url":"https://api.github.com/repos/tidepool-org/chrome-uploader/issues/332/labels{/name}","comments_url":"https://api.github.com/repos/tidepool-org/chrome-uploader/issues/332/comments","events_url":"https://api.github.com/repos/tidepool-org/chrome-uploader/issues/332/events","html_url":"https://github.com/tidepool-org/chrome-uploader/pull/332","id":181722153,"number":332,"title":"Add checks for Data Storage","user":{"login":"hntrdglss","id":434672,"avatar_url":"https://avatars.githubusercontent.com/u/434672?v=3","gravatar_id":"","url":"https://api.github.com/users/hntrdglss","html_url":"https://github.com/hntrdglss","followers_url":"https://api.github.com/users/hntrdglss/followers","following_url":"https://api.github.com/users/hntrdglss/following{/other_user}","gists_url":"https://api.github.com/users/hntrdglss/gists{/gist_id}","starred_url":"https://api.github.com/users/hntrdglss/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/hntrdglss/subscriptions","organizations_url":"https://api.github.com/users/hntrdglss/orgs","repos_url":"https://api.github.com/users/hntrdglss/repos","events_url":"https://api.github.com/users/hntrdglss/events{/privacy}","received_events_url":"https://api.github.com/users/hntrdglss/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":2,"created_at":"2016-10-07T17:12:12Z","updated_at":"2016-10-14T11:26:23Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/tidepool-org/chrome-uploader/pulls/332","html_url":"https://github.com/tidepool-org/chrome-uploader/pull/332","diff_url":"https://github.com/tidepool-org/chrome-uploader/pull/332.diff","patch_url":"https://github.com/tidepool-org/chrome-uploader/pull/332.patch"},"body":"Hey, @krystophv and @jebeck! Very excited to submit my first pull request. The functional changes here are pretty minimal, but I feel like I learned a lot about the structure of the Uploader, data models, and account types through this enhancement. Here are a few points I'd like to specifically call out for feedback:\r\n\r\n- **Naming conventions**: if you have suggestions for naming anything differently (in particular the `DataStorageCheck` [page](https://github.com/tidepool-org/chrome-uploader/compare/hntrdglss/no-dsa-check?expand=1#diff-f43d6abf0dd446d37fa39faa2fbd71fbR22) and [component](https://github.com/tidepool-org/chrome-uploader/compare/hntrdglss/no-dsa-check?expand=1#diff-6adb8bfb0bd1a9b07a1faf0491490354R26)), please let me know.\r\n\r\n- **Memberships**: from what I've seen, it looks like an account always has it's own ID in the memberships array. I did a check for [`memberships.length > 1`](https://github.com/tidepool-org/chrome-uploader/compare/hntrdglss/no-dsa-check?expand=1#diff-487ce994eadf6647e8b5c39b770c9ec9R41) on accounts that do not have a patient object in their profile. That seemed like the simplest and least computational check, but if this isn't always assured, I can change it to something like `_.difference(_.keys(memberships), loggedInUser).length`, so we check the array for user IDs not of the current user.\r\n\r\n- **CSS Override**: I ran into a CSS specificity issue on the \"Set up data storage\" button, due to [this selector](https://github.com/tidepool-org/chrome-uploader/blob/master/styles/core/buttons.module.less#L22). It was overriding my [padding and margin declarations](https://github.com/tidepool-org/chrome-uploader/compare/hntrdglss/no-dsa-check?expand=1#diff-e36fff856d6e3db38812efd11885c47eR36) on the `A` element. Rather than using `!important`, I set the button style on a [`DIV` and wrapped that in the link](https://github.com/tidepool-org/chrome-uploader/compare/hntrdglss/no-dsa-check?expand=1#diff-6adb8bfb0bd1a9b07a1faf0491490354R38). I couldn't find any similar instances of this situation—looks like this is a rather unique button—but let me know if I should resolve this differently. (I also tried `a.buttonCta`, but that resulted in an error. I guess element-based selectors aren't allowed in CSS Modules.)\r\n\r\n- **Updates to fixtures**: I had done some work with updating the fixtures to include a clinic account and no-DSA variants, but that started to convolute the code changes for this specific functionality. It could probably tie in to some work on [this tech debt](https://trello.com/c/mmMR0qpw) card. I'll submit that in a separate PR.\r\n\r\nThanks!"},"comment":{"url":"https://api.github.com/repos/tidepool-org/chrome-uploader/issues/comments/253774912","html_url":"https://github.com/tidepool-org/chrome-uploader/pull/332#issuecomment-253774912","issue_url":"https://api.github.com/repos/tidepool-org/chrome-uploader/issues/332","id":253774912,"user":{"login":"jebeck","id":1588547,"avatar_url":"https://avatars.githubusercontent.com/u/1588547?v=3","gravatar_id":"","url":"https://api.github.com/users/jebeck","html_url":"https://github.com/jebeck","followers_url":"https://api.github.com/users/jebeck/followers","following_url":"https://api.github.com/users/jebeck/following{/other_user}","gists_url":"https://api.github.com/users/jebeck/gists{/gist_id}","starred_url":"https://api.github.com/users/jebeck/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jebeck/subscriptions","organizations_url":"https://api.github.com/users/jebeck/orgs","repos_url":"https://api.github.com/users/jebeck/repos","events_url":"https://api.github.com/users/jebeck/events{/privacy}","received_events_url":"https://api.github.com/users/jebeck/received_events","type":"User","site_admin":false},"created_at":"2016-10-14T11:26:23Z","updated_at":"2016-10-14T11:26:23Z","body":"Please add the requested action creator and reducer unit tests before merging, but otherwise good 😎"}}
{"actor":{"display_login":"jebeck"}}