{"action":"created","issue":{"url":"https://api.github.com/repos/Araphel/zoey.news/issues/122","labels_url":"https://api.github.com/repos/Araphel/zoey.news/issues/122/labels{/name}","comments_url":"https://api.github.com/repos/Araphel/zoey.news/issues/122/comments","events_url":"https://api.github.com/repos/Araphel/zoey.news/issues/122/events","html_url":"https://github.com/Araphel/zoey.news/pull/122","id":102879394,"number":122,"title":"Refactor/reorganize data flow","user":{"login":"MajorJ","id":12522788,"avatar_url":"https://avatars.githubusercontent.com/u/12522788?v=3","gravatar_id":"","url":"https://api.github.com/users/MajorJ","html_url":"https://github.com/MajorJ","followers_url":"https://api.github.com/users/MajorJ/followers","following_url":"https://api.github.com/users/MajorJ/following{/other_user}","gists_url":"https://api.github.com/users/MajorJ/gists{/gist_id}","starred_url":"https://api.github.com/users/MajorJ/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/MajorJ/subscriptions","organizations_url":"https://api.github.com/users/MajorJ/orgs","repos_url":"https://api.github.com/users/MajorJ/repos","events_url":"https://api.github.com/users/MajorJ/events{/privacy}","received_events_url":"https://api.github.com/users/MajorJ/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":4,"created_at":"2015-08-24T20:02:14Z","updated_at":"2015-09-02T02:40:37Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/Araphel/zoey.news/pulls/122","html_url":"https://github.com/Araphel/zoey.news/pull/122","diff_url":"https://github.com/Araphel/zoey.news/pull/122.diff","patch_url":"https://github.com/Araphel/zoey.news/pull/122.patch"},"body":"@mattfoulger @jzhang729 @Araphel \r\n\r\nA very large refactor of existing code, with orphaned code culled, data flow reorganized and functionality improved. Summary of changes:\r\n\r\n1) The 'snapshot' chart has been renamed 'barchart', with a number of files modified accordingly. This will also require a re-seed of your charts table if you have any charts of type 'snapshot' remaining in there, or at least a manual delete of those charts.\r\n\r\n2) Many of the view files have been further modularized and/or renamed, in part for clarity/cleanness of code and in part to allow for the functionality in #4, below.\r\n\r\n3) Props now pass along as little data as is required by the child elements. In particular, all the datarows received from the database are no longer passed as props.\r\n\r\n4) Adding/removing keywords and publishers now works much differently. The current state of the publishers/keywords lists are now maintained in chartmenu.js, a new container for the keywords and publishers lists. Now, when you add/remove a publisher or keyword, the store is updated instantly using setState, and the displayed lists also update instantly. \r\n\r\nAs a result, the actions in main.js do not need to themselves update the list of keywords or publishers before requesting new data from the database, so the former add/remove keyword/publisher functions in main.js have been combined into updateKeywords and updatePublishers. A second consequence of this change is that there is no longer any need for functions to add/remove keywords/publishers in the snapshotstore, so those functions and the calls to them have been deleted. This changes have eliminated many of the multiple render calls which were previously seen.\r\n"},"comment":{"url":"https://api.github.com/repos/Araphel/zoey.news/issues/comments/136917562","html_url":"https://github.com/Araphel/zoey.news/pull/122#issuecomment-136917562","issue_url":"https://api.github.com/repos/Araphel/zoey.news/issues/122","id":136917562,"user":{"login":"MajorJ","id":12522788,"avatar_url":"https://avatars.githubusercontent.com/u/12522788?v=3","gravatar_id":"","url":"https://api.github.com/users/MajorJ","html_url":"https://github.com/MajorJ","followers_url":"https://api.github.com/users/MajorJ/followers","following_url":"https://api.github.com/users/MajorJ/following{/other_user}","gists_url":"https://api.github.com/users/MajorJ/gists{/gist_id}","starred_url":"https://api.github.com/users/MajorJ/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/MajorJ/subscriptions","organizations_url":"https://api.github.com/users/MajorJ/orgs","repos_url":"https://api.github.com/users/MajorJ/repos","events_url":"https://api.github.com/users/MajorJ/events{/privacy}","received_events_url":"https://api.github.com/users/MajorJ/received_events","type":"User","site_admin":false},"created_at":"2015-09-02T02:40:37Z","updated_at":"2015-09-02T02:40:37Z","body":"Good catch - let me take another look before merging. The bug you’re seeing might be resolved with a full charts table reset, but I want to be sure.\n\nOn Tue, Sep 1, 2015 at 2:20 PM, jzhang729 <notifications@github.com>\nwrote:\n\n> @MajorJ Ok, I renamed \"snapshot\" to \"barchart\" in my Postico app and everything seems to be working fine. One new bug that I noticed (not sure if it is the result of your refactor) is that when the donut chart is the last chart remaining, it looks like it is retaining legacy data from a previous loaded version. Screen shot attached. Notice that there are 3 colors in the chart but only 1 in the legend.\n> Other than that, I think we can merge your pull request in. Not sure if @mattfoulger and @Araphel want to weigh in at all?\n> ![screen shot 2015-09-01 at 2 19 45 pm](https://cloud.githubusercontent.com/assets/10377863/9616712/875bafb8-50b4-11e5-9d40-a27b2807ac52.png)\n> ---\n> Reply to this email directly or view it on GitHub:\n> https://github.com/Araphel/zoey.news/pull/122#issuecomment-136864398"}}