{"action":"created","issue":{"url":"https://api.github.com/repos/theforeman/foreman/issues/5240","repository_url":"https://api.github.com/repos/theforeman/foreman","labels_url":"https://api.github.com/repos/theforeman/foreman/issues/5240/labels{/name}","comments_url":"https://api.github.com/repos/theforeman/foreman/issues/5240/comments","events_url":"https://api.github.com/repos/theforeman/foreman/issues/5240/events","html_url":"https://github.com/theforeman/foreman/pull/5240","id":295047123,"number":5240,"title":"WIP: Fixes #22473 - Refactor react_app folder structure","user":{"login":"sharvit","id":1262502,"avatar_url":"https://avatars2.githubusercontent.com/u/1262502?v=4","gravatar_id":"","url":"https://api.github.com/users/sharvit","html_url":"https://github.com/sharvit","followers_url":"https://api.github.com/users/sharvit/followers","following_url":"https://api.github.com/users/sharvit/following{/other_user}","gists_url":"https://api.github.com/users/sharvit/gists{/gist_id}","starred_url":"https://api.github.com/users/sharvit/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sharvit/subscriptions","organizations_url":"https://api.github.com/users/sharvit/orgs","repos_url":"https://api.github.com/users/sharvit/repos","events_url":"https://api.github.com/users/sharvit/events{/privacy}","received_events_url":"https://api.github.com/users/sharvit/received_events","type":"User","site_admin":false},"labels":[{"id":79040689,"url":"https://api.github.com/repos/theforeman/foreman/labels/Needs%20testing","name":"Needs testing","color":"fbca04","default":false},{"id":79040085,"url":"https://api.github.com/repos/theforeman/foreman/labels/Not%20yet%20reviewed","name":"Not yet reviewed","color":"0e8a16","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":10,"created_at":"2018-02-07T08:30:36Z","updated_at":"2018-02-13T05:01:45Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/theforeman/foreman/pulls/5240","html_url":"https://github.com/theforeman/foreman/pull/5240","diff_url":"https://github.com/theforeman/foreman/pull/5240.diff","patch_url":"https://github.com/theforeman/foreman/pull/5240.patch"},"body":"I have been starting from the `PasswordStrength` because it was the easiest (the files are most organized).\r\n\r\n**The current goal is to make this component perfect so we can use it later as a boilerplate.**\r\nBefore continuing to the other components, I want to make sure everyone is agree about the new structure.\r\n\r\n**Goals:**\r\n* Migrate to a domain level folder structure\r\n* Have integration test for the connected component\r\n* Have unit tests for all the other parts\r\n* Make it scaleable\r\n* Introduce redux selectors\r\n\r\n**In order to do so, i refactor the component, the tests, the actions and the reducers.**\r\n\r\nI migrated the actions from `updatePassword` and `checkPasswordsMatch` to `updatePassword`, `updatePasswordConfirmation` and created a selector call `doesPasswordsMatch`.\r\n\r\nI have been trying many different variations and i feel i most like this structure:\r\n```js\r\nPasswordStrength\r\n├── __tests__\r\n│   ├── __snapshots__\r\n│   │   ├── index.test.js.snap\r\n│   │   ├── PasswordStrengthActions.test.js.snap\r\n│   │   ├── PasswordStrengthReducer.test.js.snap\r\n│   │   └── PasswordStrength.test.js.snap\r\n│   ├── index.test.js // integration test\r\n│   ├── PasswordStrengthActions.test.js // actions unit tests\r\n│   ├── PasswordStrengthReducer.test.js // reducer unit tests\r\n│   ├── PasswordStrengthSelectors.test.js // selectors unit tests\r\n│   └── PasswordStrength.test.js // component unit tests\r\n├── index.js // integration file\r\n├── PasswordStrength.js // component\r\n├── PasswordStrength.scss // styles\r\n├── PasswordStrengthActions.js // actions\r\n├── PasswordStrengthConstants.js // consts\r\n├── PasswordStrengthReducer.js // reducers\r\n└── PasswordStrengthSelectors.js // selectors\r\n```\r\n\r\n**Why i moved the tests into the `__tests__` folder?**\r\nThe file list just getting to long and it can get confusing.\r\n\r\n**Why do we need `index.js`? Why not connecting the component into the store from the `PasswordStrength.js`?**\r\n1. When we lose the reference to the unconnected component, it became hard to unit-test them because you must use a store.\r\n2. It feels right when `index.js` responsibility is to integrate the different parts of the domain.\r\n\r\n**How do we scale it to have more inner components?**\r\nCreating a `components` folder and put them inside.\r\n\r\nLooking forward to get some feedback here.\r\n\r\n@amirfefer @ohadlevy @waldenraines @tstrachota @danseethaler @priley86"},"comment":{"url":"https://api.github.com/repos/theforeman/foreman/issues/comments/365151740","html_url":"https://github.com/theforeman/foreman/pull/5240#issuecomment-365151740","issue_url":"https://api.github.com/repos/theforeman/foreman/issues/5240","id":365151740,"user":{"login":"priley86","id":4237045,"avatar_url":"https://avatars2.githubusercontent.com/u/4237045?v=4","gravatar_id":"","url":"https://api.github.com/users/priley86","html_url":"https://github.com/priley86","followers_url":"https://api.github.com/users/priley86/followers","following_url":"https://api.github.com/users/priley86/following{/other_user}","gists_url":"https://api.github.com/users/priley86/gists{/gist_id}","starred_url":"https://api.github.com/users/priley86/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/priley86/subscriptions","organizations_url":"https://api.github.com/users/priley86/orgs","repos_url":"https://api.github.com/users/priley86/repos","events_url":"https://api.github.com/users/priley86/events{/privacy}","received_events_url":"https://api.github.com/users/priley86/received_events","type":"User","site_admin":false},"created_at":"2018-02-13T05:01:45Z","updated_at":"2018-02-13T05:01:45Z","author_association":"NONE","body":"@sharvit i had no issues with this structure. I've started a parallel PR for our v2v efforts [here](https://github.com/priley86/miq_v2v_ui_plugin/pull/43).\r\n\r\n I need to do a bit more homework with the tests you've started here, but was at least able to render the `mapStateToProps` results (giving me some simple integration tests).\r\n\r\nI'd also like to propose we adopt something similar to Ryan Florence's route hierarchy/shared module structure noted above ([link](https://gist.github.com/ryanflorence/daafb1e3cb8ad740b346#shared-modules)). I'm happy with it for now..."}}
{"actor":{"display_login":"priley86"}}