{"action":"created","issue":{"url":"https://api.github.com/repos/Automattic/wp-calypso/issues/9624","repository_url":"https://api.github.com/repos/Automattic/wp-calypso","labels_url":"https://api.github.com/repos/Automattic/wp-calypso/issues/9624/labels{/name}","comments_url":"https://api.github.com/repos/Automattic/wp-calypso/issues/9624/comments","events_url":"https://api.github.com/repos/Automattic/wp-calypso/issues/9624/events","html_url":"https://github.com/Automattic/wp-calypso/pull/9624","id":191450188,"number":9624,"title":"Clean/site list/sites dropdown","user":{"login":"brunoscopelliti","id":3489757,"avatar_url":"https://avatars.githubusercontent.com/u/3489757?v=3","gravatar_id":"","url":"https://api.github.com/users/brunoscopelliti","html_url":"https://github.com/brunoscopelliti","followers_url":"https://api.github.com/users/brunoscopelliti/followers","following_url":"https://api.github.com/users/brunoscopelliti/following{/other_user}","gists_url":"https://api.github.com/users/brunoscopelliti/gists{/gist_id}","starred_url":"https://api.github.com/users/brunoscopelliti/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/brunoscopelliti/subscriptions","organizations_url":"https://api.github.com/users/brunoscopelliti/orgs","repos_url":"https://api.github.com/users/brunoscopelliti/repos","events_url":"https://api.github.com/users/brunoscopelliti/events{/privacy}","received_events_url":"https://api.github.com/users/brunoscopelliti/received_events","type":"User","site_admin":false},"labels":[{"id":301697869,"url":"https://api.github.com/repos/Automattic/wp-calypso/labels/OSS%20Citizen","name":"OSS Citizen","color":"5319e7","default":false},{"id":285010018,"url":"https://api.github.com/repos/Automattic/wp-calypso/labels/[Status]%20Needs%20Review","name":"[Status] Needs Review","color":"fbca04","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":9,"created_at":"2016-11-24T07:28:15Z","updated_at":"2016-12-16T20:33:03Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/Automattic/wp-calypso/pulls/9624","html_url":"https://github.com/Automattic/wp-calypso/pull/9624","diff_url":"https://github.com/Automattic/wp-calypso/pull/9624.diff","patch_url":"https://github.com/Automattic/wp-calypso/pull/9624.patch"},"body":"Hi everybody,\r\n\r\n@gwwar , as you suggested I am opening a pull request to track our work on this request; this is not ready to be merged now.\r\n\r\nI've also a couple of questions for you;\r\nI understand that you might be working on completely different things right now, so let me give you a little more context before asking the questions (sorry for the long text).\r\n\r\nI am working on the `sites-dropdown` component.\r\nIt permits the user to select what site should be considered as *primary*. It uses `site-selector` component for the selection logic, and considers as initially selected the site that is currently the user's primary site (indepentently from what is the currently selected site in the redux store).\r\n\r\nSince the component receives the currently primary site as `selected` prop, we can't use `getSelectedSite` from 'state/ui/selectors' to get the site that should be initially selected.\r\n\r\nCurrently I am trying using `getSite` from 'state/sites/selectors'.\r\nIts signature is `( state, siteId ) => Site`. So it works on the basis of the `siteId`.\r\n\r\n```js\r\nconst mapStateToProps = ( state, ownProps ) => ( {\r\n\tprimarySite: getSite( state, ownProps.selected ),\r\n} );\r\n\r\nexport default connect( mapStateToProps )( SitesDropdown );\r\n```\r\n\r\nHere, the problem is that sometimes the `sites-dropdown` component is used, specifing as `selected` prop the `siteSlug`, instead of that the `siteId`.\r\nIn these cases this PR, at the current state, introduces a bug.\r\n\r\nFrom a quick search in the codebase I found that `sites-dropdown` is used only in the following components/pages.\r\n\r\n```txt\r\n# providing selected prop as slug\r\ncomponents/site-selector-modal\r\nme/help/help-contact-form\r\n\r\n# providing selected prop as siteId\r\nme/account/main.jsx\r\n```\r\n\r\nMy first feeling was that we should uniform, and probably use always the `siteId` (since *selectors* work exclusively with it).\r\nI've started doing this but I felt that I was just increasing confusion around `siteId`, and `slug`; I've now reverted those changes.\r\nFor example `site-selector` when an option is selected, executes `onSiteSelect` passing as argument the `slug` (not the `siteId`), and this depend more deeper by the `site` component.\r\n\r\n```js\r\nconst SiteSelector = React.createClass( {\r\n    ...\r\n    onSiteSelect( event, siteSlug ) {\r\n        const handledByHost = this.props.onSiteSelect( siteSlug );\r\n        ...\r\n```\r\n\r\nAt this regard my first question is: what do you consider the best way to identify a site (siteId or slug)?\r\nThe codebase is not completely uniform here; for example `site-selector` works mostly with the slug name. I guess this is due to the fact that *selectors* are a relatively new introduction, and you want to have uniformity one day.\r\nEventually I would be happy to help.\r\n\r\nA completely different approach would be to make `getRawSite` work both with `siteId`, both with `slug` transparently.\r\nThat's surely less work, but I am not sure it is the right thing to do. What do you think?\r\n\r\n```js\r\nexport const getRawSite = ( state, siteId ) => {\r\n\treturn state.sites.items[ siteId ] || null;\r\n};\r\n```\r\n\r\nCan you think to other alternatives which haven't come to my mind?\r\n"},"comment":{"url":"https://api.github.com/repos/Automattic/wp-calypso/issues/comments/267689382","html_url":"https://github.com/Automattic/wp-calypso/pull/9624#issuecomment-267689382","issue_url":"https://api.github.com/repos/Automattic/wp-calypso/issues/9624","id":267689382,"user":{"login":"brunoscopelliti","id":3489757,"avatar_url":"https://avatars.githubusercontent.com/u/3489757?v=3","gravatar_id":"","url":"https://api.github.com/users/brunoscopelliti","html_url":"https://github.com/brunoscopelliti","followers_url":"https://api.github.com/users/brunoscopelliti/followers","following_url":"https://api.github.com/users/brunoscopelliti/following{/other_user}","gists_url":"https://api.github.com/users/brunoscopelliti/gists{/gist_id}","starred_url":"https://api.github.com/users/brunoscopelliti/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/brunoscopelliti/subscriptions","organizations_url":"https://api.github.com/users/brunoscopelliti/orgs","repos_url":"https://api.github.com/users/brunoscopelliti/repos","events_url":"https://api.github.com/users/brunoscopelliti/events{/privacy}","received_events_url":"https://api.github.com/users/brunoscopelliti/received_events","type":"User","site_admin":false},"created_at":"2016-12-16T20:33:03Z","updated_at":"2016-12-16T20:33:03Z","body":"I think this PR is now ready to be reviewed; I've left a few comments/questions too.\r\n\r\n@gwwar I've read in the other thread that you're going to take a vacation... enjoy it! I'll appreciate your feedback on this, when you will be back.\r\n"}}
{"actor":{"display_login":"brunoscopelliti"}}