{"action":"created","issue":{"url":"https://api.github.com/repos/openspending/os-import/issues/29","labels_url":"https://api.github.com/repos/openspending/os-import/issues/29/labels{/name}","comments_url":"https://api.github.com/repos/openspending/os-import/issues/29/comments","events_url":"https://api.github.com/repos/openspending/os-import/issues/29/events","html_url":"https://github.com/openspending/os-import/pull/29","id":106049790,"number":29,"title":"Step 2 UI with tests. Closes #12.","user":{"login":"gvidon","id":58164,"avatar_url":"https://avatars.githubusercontent.com/u/58164?v=3","gravatar_id":"","url":"https://api.github.com/users/gvidon","html_url":"https://github.com/gvidon","followers_url":"https://api.github.com/users/gvidon/followers","following_url":"https://api.github.com/users/gvidon/following{/other_user}","gists_url":"https://api.github.com/users/gvidon/gists{/gist_id}","starred_url":"https://api.github.com/users/gvidon/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gvidon/subscriptions","organizations_url":"https://api.github.com/users/gvidon/orgs","repos_url":"https://api.github.com/users/gvidon/repos","events_url":"https://api.github.com/users/gvidon/events{/privacy}","received_events_url":"https://api.github.com/users/gvidon/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":1,"created_at":"2015-09-11T16:48:10Z","updated_at":"2015-09-13T09:42:59Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/openspending/os-import/pulls/29","html_url":"https://github.com/openspending/os-import/pull/29","diff_url":"https://github.com/openspending/os-import/pull/29.diff","patch_url":"https://github.com/openspending/os-import/pull/29.patch"},"body":"## Short recap\r\n* Sample of user data on top of the page\r\n* forms for editing fields properties\r\n* restrictions on concepts\r\n* navigating between steps\r\n* tested\r\n* restructured tests dir a little bit.\r\n\r\n## Details\r\n* Tests are ok https://travis-ci.org/openspending/os-import\r\n* html/css are quite simple\r\n* columns forms area width is limited by CSS to demonstrate horizontal scrolling.\r\n\r\nDeployed this branch to heroku http://os-import.herokuapp.com/"},"comment":{"url":"https://api.github.com/repos/openspending/os-import/issues/comments/139856982","html_url":"https://github.com/openspending/os-import/pull/29#issuecomment-139856982","issue_url":"https://api.github.com/repos/openspending/os-import/issues/29","id":139856982,"user":{"login":"pwalsh","id":791678,"avatar_url":"https://avatars.githubusercontent.com/u/791678?v=3","gravatar_id":"","url":"https://api.github.com/users/pwalsh","html_url":"https://github.com/pwalsh","followers_url":"https://api.github.com/users/pwalsh/followers","following_url":"https://api.github.com/users/pwalsh/following{/other_user}","gists_url":"https://api.github.com/users/pwalsh/gists{/gist_id}","starred_url":"https://api.github.com/users/pwalsh/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pwalsh/subscriptions","organizations_url":"https://api.github.com/users/pwalsh/orgs","repos_url":"https://api.github.com/users/pwalsh/repos","events_url":"https://api.github.com/users/pwalsh/events{/privacy}","received_events_url":"https://api.github.com/users/pwalsh/received_events","type":"User","site_admin":false},"created_at":"2015-09-13T09:42:58Z","updated_at":"2015-09-13T09:42:58Z","body":"@gvidon \r\n\r\nI've done a code review now. I'm going to list some specific things related to the feature here, and open a separate issue for issues of design and architecture.\r\n\r\n* If I go straight to the route for step 2 `/map` I get some errors in the console, and I'm not directed instead to step one (ie: there is no management of the user flow)\r\n* If I enter data and then reload the page, I lose everything I added (there is no local persistence of data entered - this is really important in a multi-step user flow)\r\n* Step 2 changes are not persisted at all (even without a reload as above) - I can change \"description\", \"data type\", and \"concept\" for any field, then move back to step 1, and then move back again to step 2, and my changes are gone\r\n* Probably related to above, when I look at the data package object in the console, the mapping object is empty, even when I have added concepts etc. in the UI (see screenshot below)\r\n* There is no validation on user actions on both the \"data type\" and \"concept\" selectors. For example:\r\n  * I can easily set a column that has a string like \"Australia\" in it to be data type integer\r\n  * I can easily set a column that has a string like \"Australia\" in it to be concept Date/Time\r\n* The mock UI described in the issue for step 2 shows data and the controls in a delineated table that is left aligned, and there is some vertical and horizontal consistency. The task here was not to implement \"design\", but still, what is implemented is very hard to read as things do not line up horizontally, there are no lines to delineate cells, etc.\r\n* Likewise, the mock UI in the issue has a clear separator with he word \"description\". This makes it easy to distinguish between the data and the controls - please do implement the same\r\n* The UI has a horizontal scroll for the data/controls layout even when it does not need one (3 columns)\r\n* The issue asks for \"Unit tests for discrete logic (eg: updating mapping etc)\", in addition to functional tests. As far as I see, all tests user zombie to perform functional tests via the UI layer, but I am really looking to see how the core logic is tested without the UI layer. The ability to unit test logic helps me see that there is clear separation between the view layer and the logic layer of the app.\r\n\r\n<img width=\"1280\" alt=\"no_persistent_mapping\" src=\"https://cloud.githubusercontent.com/assets/791678/9836106/f0b9edf8-5a14-11e5-840a-940b2ba9e8b4.png\">\r\n"}}