{"action":"created","issue":{"url":"https://api.github.com/repos/civicrm/civicrm-core/issues/11783","repository_url":"https://api.github.com/repos/civicrm/civicrm-core","labels_url":"https://api.github.com/repos/civicrm/civicrm-core/issues/11783/labels{/name}","comments_url":"https://api.github.com/repos/civicrm/civicrm-core/issues/11783/comments","events_url":"https://api.github.com/repos/civicrm/civicrm-core/issues/11783/events","html_url":"https://github.com/civicrm/civicrm-core/pull/11783","id":303267501,"number":11783,"title":"(WIP) CRM-21827: Add Membership External Id As New Field To Core","user":{"login":"vinuvarshith","id":36624620,"avatar_url":"https://avatars1.githubusercontent.com/u/36624620?v=4","gravatar_id":"","url":"https://api.github.com/users/vinuvarshith","html_url":"https://github.com/vinuvarshith","followers_url":"https://api.github.com/users/vinuvarshith/followers","following_url":"https://api.github.com/users/vinuvarshith/following{/other_user}","gists_url":"https://api.github.com/users/vinuvarshith/gists{/gist_id}","starred_url":"https://api.github.com/users/vinuvarshith/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/vinuvarshith/subscriptions","organizations_url":"https://api.github.com/users/vinuvarshith/orgs","repos_url":"https://api.github.com/users/vinuvarshith/repos","events_url":"https://api.github.com/users/vinuvarshith/events{/privacy}","received_events_url":"https://api.github.com/users/vinuvarshith/received_events","type":"User","site_admin":false},"labels":[{"id":246037889,"url":"https://api.github.com/repos/civicrm/civicrm-core/labels/master","name":"master","color":"0052cc","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":3,"created_at":"2018-03-07T21:33:12Z","updated_at":"2018-03-10T01:04:14Z","closed_at":null,"author_association":"NONE","pull_request":{"url":"https://api.github.com/repos/civicrm/civicrm-core/pulls/11783","html_url":"https://github.com/civicrm/civicrm-core/pull/11783","diff_url":"https://github.com/civicrm/civicrm-core/pull/11783.diff","patch_url":"https://github.com/civicrm/civicrm-core/pull/11783.patch"},"body":"Overview\r\n----------------------------------------\r\nWe need to add a new field to the core -\r\nMembership External ID\r\nThis field will be helpful in uploading Contributions through Civi's standard contribution importer as we will be able to reference it to a membership.\r\n\r\nBefore\r\n----------------------------------------\r\nCurrently there is no external field to map when working with memberships\r\n\r\nAfter\r\n----------------------------------------\r\nA new field 'External Membership Id' is available for memberships similar to external_identifier field for contacts. \r\n\r\nComments\r\n----------------------------------------\r\nI have added SQL to add the new column to membership table in CRM/Upgrade/Incremental/sql/4.7.32.mysql.tpl\n\n---\n\n * [CRM-21827: Add Membership External ID as a new field to core](https://issues.civicrm.org/jira/browse/CRM-21827)"},"comment":{"url":"https://api.github.com/repos/civicrm/civicrm-core/issues/comments/371988176","html_url":"https://github.com/civicrm/civicrm-core/pull/11783#issuecomment-371988176","issue_url":"https://api.github.com/repos/civicrm/civicrm-core/issues/11783","id":371988176,"user":{"login":"totten","id":1336047,"avatar_url":"https://avatars0.githubusercontent.com/u/1336047?v=4","gravatar_id":"","url":"https://api.github.com/users/totten","html_url":"https://github.com/totten","followers_url":"https://api.github.com/users/totten/followers","following_url":"https://api.github.com/users/totten/following{/other_user}","gists_url":"https://api.github.com/users/totten/gists{/gist_id}","starred_url":"https://api.github.com/users/totten/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/totten/subscriptions","organizations_url":"https://api.github.com/users/totten/orgs","repos_url":"https://api.github.com/users/totten/repos","events_url":"https://api.github.com/users/totten/events{/privacy}","received_events_url":"https://api.github.com/users/totten/received_events","type":"User","site_admin":false},"created_at":"2018-03-10T01:04:14Z","updated_at":"2018-03-10T01:04:14Z","author_association":"OWNER","body":"@vinuvarshith - Welcome to `civicrm-core.git`!\r\n\r\nOn technical grounds, I want to say kudos for working through each part of this. These layers aren't the most accessible things, and it often takes people a good chunk of time to work through them. On first skim, the changes generally look like what we'd expect for adding a new field across layers. Nice job.\r\n\r\nNow a couple things....\r\n\r\n> We need to add a new field to the core -\r\n> Membership External ID\r\n> This field will be helpful in uploading Contributions through Civi's standard contribution importer \r\n> as we will be able to reference it to a membership.\r\n\r\n1) At the risk of being pedantic, the opening line (\"we need to add a new field to the core\") wasn't very persuasive -- not because it's wrong or a bad patch. We've just had some history of adding  fields of debateable import, and there's generally a lot of mileage in the 'custom-data' system, so we try be a bit conservative with adding things to the core model. And I don't important memberships enough to have a strong reaction on this.\r\n\r\nThere are a couple people who might have good feedback on whether this idea is overall an improvement for the product -- e.g. @MegaphoneJon @petednz @lcdservices @Stoob @KarinG @agh1.\r\n\r\n2) There is a small issue where the test-bot was unable to apply the patch on top of the current `master`. (The testbot is a little more picky about applying patches than the. Github UI.) Usually, you can address this by rebasing your branch (e.g. `git pull --rebase origin master`) and force-pushing (`git push -f myuser CRM-21827-add-membership-external-id-as-new-field`)."}}
{"actor":{"display_login":"totten"}}