GitHub PullRequestReviewCommentEvent at 2014-07-23 13:10:39
Repository: datawrapper
Actor: fangel
Payload: {"action":"created","comment":{"url":"https://api.github.com/repos/datawrapper/datawrapper/pulls/comments/15286052","id":15286052,"diff_hunk":"@@ -80,6 +80,9 @@ public static function hookRegistered($hookName) {\n     // returns the URL of a published chart (args: chart)\n     const GET_PUBLISHED_URL = 'get_pulished_url';\n \n+    // returns a regex that can identify the id of published charts\n+    const GET_PUBLISHED_URL_PATTERN = 'get_published_url_pattern';\n+","path":"lib/hooks/DatawrapperHooks.php","position":6,"original_position":6,"commit_id":"fd60e58d45948fdf1eb37fcbf60ec93a5616d978","original_commit_id":"fd60e58d45948fdf1eb37fcbf60ec93a5616d978","user":{"login":"fangel","id":19275,"avatar_url":"https://avatars.githubusercontent.com/u/19275?","gravatar_id":"67a19eaa056f7226264891ffa5dad074","url":"https://api.github.com/users/fangel","html_url":"https://github.com/fangel","followers_url":"https://api.github.com/users/fangel/followers","following_url":"https://api.github.com/users/fangel/following{/other_user}","gists_url":"https://api.github.com/users/fangel/gists{/gist_id}","starred_url":"https://api.github.com/users/fangel/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/fangel/subscriptions","organizations_url":"https://api.github.com/users/fangel/orgs","repos_url":"https://api.github.com/users/fangel/repos","events_url":"https://api.github.com/users/fangel/events{/privacy}","received_events_url":"https://api.github.com/users/fangel/received_events","type":"User","site_admin":false},"body":"Gotcha. I wasn't quite sure what rule was here.","created_at":"2014-07-23T13:10:39Z","updated_at":"2014-07-23T13:10:39Z","html_url":"https://github.com/datawrapper/datawrapper/pull/109#discussion_r15286052","pull_request_url":"https://api.github.com/repos/datawrapper/datawrapper/pulls/109","_links":{"self":{"href":"https://api.github.com/repos/datawrapper/datawrapper/pulls/comments/15286052"},"html":{"href":"https://github.com/datawrapper/datawrapper/pull/109#discussion_r15286052"},"pull_request":{"href":"https://api.github.com/repos/datawrapper/datawrapper/pulls/109"}}},"pull_request":{"url":"https://api.github.com/repos/datawrapper/datawrapper/pulls/109","id":18538250,"html_url":"https://github.com/datawrapper/datawrapper/pull/109","diff_url":"https://github.com/datawrapper/datawrapper/pull/109.diff","patch_url":"https://github.com/datawrapper/datawrapper/pull/109.patch","issue_url":"https://api.github.com/repos/datawrapper/datawrapper/issues/109","number":109,"state":"open","title":"Feature: add oEmbed-endpoint for charts","user":{"login":"fangel","id":19275,"avatar_url":"https://avatars.githubusercontent.com/u/19275?","gravatar_id":"67a19eaa056f7226264891ffa5dad074","url":"https://api.github.com/users/fangel","html_url":"https://github.com/fangel","followers_url":"https://api.github.com/users/fangel/followers","following_url":"https://api.github.com/users/fangel/following{/other_user}","gists_url":"https://api.github.com/users/fangel/gists{/gist_id}","starred_url":"https://api.github.com/users/fangel/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/fangel/subscriptions","organizations_url":"https://api.github.com/users/fangel/orgs","repos_url":"https://api.github.com/users/fangel/repos","events_url":"https://api.github.com/users/fangel/events{/privacy}","received_events_url":"https://api.github.com/users/fangel/received_events","type":"User","site_admin":false},"body":"Hi,\r\n\r\nWe are in the process of building an automated embedding-system for our CMS. The automation relies on oEmbed and oEmbed discovery.\r\nAnd since we sometimes have to embed Datawrapper-contents, we thought it would be awesome if Datawrapper could support oEmbed and oEmbed Discovery (basically a `<link>` on the exported charts.\r\n\r\nWe (Casper Thomsen) have discussed adding such a functionality with Mirko Lorenz via e-mail, and settled on us trying to add the functionality. So here's our attempt at adding the functionality.s of building an automated embedding-system for our CMS. The automation relies on oEmbed and oEmbed discovery.\r\nAnd since we sometimes have to embed Datawrapper-contents, we thought it would be awesome if Datawrapper could support oEmbed and oEmbed Discovery (basically a `<link>` on the exported charts.\r\n\r\nWe (Casper Thomsen) have discussed adding such a functionality with Mirko Lorenz via e-mail, and settled on us trying to add the functionality. So here's our attempt at adding the functionality.\r\n\r\nIt consists of a small change to the `chart.twig`-template that adds the `<link>`, and then a new API-endpoint (located in `lib/api/oembed.php`) at the path `api/oembed`.\r\n\r\nThe oEmbed is performed by checking if the URL to embed matches the URL to see if it's a chart URL, and if it is, extract the id. If the chart is public, then it generates a oEmbed document with an iframe in it.\r\n\r\nWe have updated the API tests to check if the API-endpoint works. To get this to work, I had to also modify the tests such that the created user is activated and authenticated. The chart has the user added as an author, and then the chart is published. I've also added clean-up functionality such that the created user and chart is removed again.\r\nDuring my process of adding in the test of the user-deletion endpoint I stumbled across a bug where a user couldn't be removed if the user was given by an ID from a non-admin user, because it used strict equality, but there was a type-mismatch as the provided user-id is represented as a string (from its nature if coming from the URL).\r\n\r\nTODO:\r\n- [ ] Ensure that the oEmbed functionality works on CDN paths.\r\n\r\nKind Regards\r\nMorten Fangel\r\nTV2 DANMARK","created_at":"2014-07-17T16:16:09Z","updated_at":"2014-07-23T13:10:39Z","closed_at":null,"merged_at":null,"merge_commit_sha":"81eec868be05900a875b9bc0c1e1c80a716a7f48","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/datawrapper/datawrapper/pulls/109/commits","review_comments_url":"https://api.github.com/repos/datawrapper/datawrapper/pulls/109/comments","review_comment_url":"https://api.github.com/repos/datawrapper/datawrapper/pulls/comments/{number}","comments_url":"https://api.github.com/repos/datawrapper/datawrapper/issues/109/comments","statuses_url":"https://api.github.com/repos/datawrapper/datawrapper/statuses/fd60e58d45948fdf1eb37fcbf60ec93a5616d978","head":{"label":"fangel:feature/add-oembed-endpoint-for-charts","ref":"feature/add-oembed-endpoint-for-charts","sha":"fd60e58d45948fdf1eb37fcbf60ec93a5616d978","user":{"login":"fangel","id":19275,"avatar_url":"https://avatars.githubusercontent.com/u/19275?","gravatar_id":"67a19eaa056f7226264891ffa5dad074","url":"https://api.github.com/users/fangel","html_url":"https://github.com/fangel","followers_url":"https://api.github.com/users/fangel/followers","following_url":"https://api.github.com/users/fangel/following{/other_user}","gists_url":"https://api.github.com/users/fangel/gists{/gist_id}","starred_url":"https://api.github.com/users/fangel/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/fangel/subscriptions","organizations_url":"https://api.github.com/users/fangel/orgs","repos_url":"https://api.github.com/users/fangel/repos","events_url":"https://api.github.com/users/fangel/events{/privacy}","received_events_url":"https://api.github.com/users/fangel/received_events","type":"User","site_admin":false},"repo":{"id":21948319,"name":"datawrapper","full_name":"fangel/datawrapper","owner":{"login":"fangel","id":19275,"avatar_url":"https://avatars.githubusercontent.com/u/19275?","gravatar_id":"67a19eaa056f7226264891ffa5dad074","url":"https://api.github.com/users/fangel","html_url":"https://github.com/fangel","followers_url":"https://api.github.com/users/fangel/followers","following_url":"https://api.github.com/users/fangel/following{/other_user}","gists_url":"https://api.github.com/users/fangel/gists{/gist_id}","starred_url":"https://api.github.com/users/fangel/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/fangel/subscriptions","organizations_url":"https://api.github.com/users/fangel/orgs","repos_url":"https://api.github.com/users/fangel/repos","events_url":"https://api.github.com/users/fangel/events{/privacy}","received_events_url":"https://api.github.com/users/fangel/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/fangel/datawrapper","description":"An open source data visualization platform helping everyone to create simple, correct and embeddable charts in minutes.","fork":true,"url":"https://api.github.com/repos/fangel/datawrapper","forks_url":"https://api.github.com/repos/fangel/datawrapper/forks","keys_url":"https://api.github.com/repos/fangel/datawrapper/keys{/key_id}","collaborators_url":"https://api.github.com/repos/fangel/datawrapper/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/fangel/datawrapper/teams","hooks_url":"https://api.github.com/repos/fangel/datawrapper/hooks","issue_events_url":"https://api.github.com/repos/fangel/datawrapper/issues/events{/number}","events_url":"https://api.github.com/repos/fangel/datawrapper/events","assignees_url":"https://api.github.com/repos/fangel/datawrapper/assignees{/user}","branches_url":"https://api.github.com/repos/fangel/datawrapper/branches{/branch}","tags_url":"https://api.github.com/repos/fangel/datawrapper/tags","blobs_url":"https://api.github.com/repos/fangel/datawrapper/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/fangel/datawrapper/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/fangel/datawrapper/git/refs{/sha}","trees_url":"https://api.github.com/repos/fangel/datawrapper/git/trees{/sha}","statuses_url":"https://api.github.com/repos/fangel/datawrapper/statuses/{sha}","languages_url":"https://api.github.com/repos/fangel/datawrapper/languages","stargazers_url":"https://api.github.com/repos/fangel/datawrapper/stargazers","contributors_url":"https://api.github.com/repos/fangel/datawrapper/contributors","subscribers_url":"https://api.github.com/repos/fangel/datawrapper/subscribers","subscription_url":"https://api.github.com/repos/fangel/datawrapper/subscription","commits_url":"https://api.github.com/repos/fangel/datawrapper/commits{/sha}","git_commits_url":"https://api.github.com/repos/fangel/datawrapper/git/commits{/sha}","comments_url":"https://api.github.com/repos/fangel/datawrapper/comments{/number}","issue_comment_url":"https://api.github.com/repos/fangel/datawrapper/issues/comments/{number}","contents_url":"https://api.github.com/repos/fangel/datawrapper/contents/{+path}","compare_url":"https://api.github.com/repos/fangel/datawrapper/compare/{base}...{head}","merges_url":"https://api.github.com/repos/fangel/datawrapper/merges","archive_url":"https://api.github.com/repos/fangel/datawrapper/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/fangel/datawrapper/downloads","issues_url":"https://api.github.com/repos/fangel/datawrapper/issues{/number}","pulls_url":"https://api.github.com/repos/fangel/datawrapper/pulls{/number}","milestones_url":"https://api.github.com/repos/fangel/datawrapper/milestones{/number}","notifications_url":"https://api.github.com/repos/fangel/datawrapper/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/fangel/datawrapper/labels{/name}","releases_url":"https://api.github.com/repos/fangel/datawrapper/releases{/id}","created_at":"2014-07-17T15:58:37Z","updated_at":"2014-07-14T19:52:33Z","pushed_at":"2014-07-23T08:59:50Z","git_url":"git://github.com/fangel/datawrapper.git","ssh_url":"git@github.com:fangel/datawrapper.git","clone_url":"https://github.com/fangel/datawrapper.git","svn_url":"https://github.com/fangel/datawrapper","homepage":"http://datawrapper.de","size":16295,"stargazers_count":0,"watchers_count":0,"language":null,"has_issues":false,"has_downloads":true,"has_wiki":true,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"datawrapper:master","ref":"master","sha":"8936887b44b3bf9ed9f6ab73d0cd27cfb62d8db1","user":{"login":"datawrapper","id":1931523,"avatar_url":"https://avatars.githubusercontent.com/u/1931523?","gravatar_id":"c95de35ab9066c3e6439acb4508c7416","url":"https://api.github.com/users/datawrapper","html_url":"https://github.com/datawrapper","followers_url":"https://api.github.com/users/datawrapper/followers","following_url":"https://api.github.com/users/datawrapper/following{/other_user}","gists_url":"https://api.github.com/users/datawrapper/gists{/gist_id}","starred_url":"https://api.github.com/users/datawrapper/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/datawrapper/subscriptions","organizations_url":"https://api.github.com/users/datawrapper/orgs","repos_url":"https://api.github.com/users/datawrapper/repos","events_url":"https://api.github.com/users/datawrapper/events{/privacy}","received_events_url":"https://api.github.com/users/datawrapper/received_events","type":"Organization","site_admin":false},"repo":{"id":4922017,"name":"datawrapper","full_name":"datawrapper/datawrapper","owner":{"login":"datawrapper","id":1931523,"avatar_url":"https://avatars.githubusercontent.com/u/1931523?","gravatar_id":"c95de35ab9066c3e6439acb4508c7416","url":"https://api.github.com/users/datawrapper","html_url":"https://github.com/datawrapper","followers_url":"https://api.github.com/users/datawrapper/followers","following_url":"https://api.github.com/users/datawrapper/following{/other_user}","gists_url":"https://api.github.com/users/datawrapper/gists{/gist_id}","starred_url":"https://api.github.com/users/datawrapper/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/datawrapper/subscriptions","organizations_url":"https://api.github.com/users/datawrapper/orgs","repos_url":"https://api.github.com/users/datawrapper/repos","events_url":"https://api.github.com/users/datawrapper/events{/privacy}","received_events_url":"https://api.github.com/users/datawrapper/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/datawrapper/datawrapper","description":"An open source data visualization platform helping everyone to create simple, correct and embeddable charts in minutes.","fork":false,"url":"https://api.github.com/repos/datawrapper/datawrapper","forks_url":"https://api.github.com/repos/datawrapper/datawrapper/forks","keys_url":"https://api.github.com/repos/datawrapper/datawrapper/keys{/key_id}","collaborators_url":"https://api.github.com/repos/datawrapper/datawrapper/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/datawrapper/datawrapper/teams","hooks_url":"https://api.github.com/repos/datawrapper/datawrapper/hooks","issue_events_url":"https://api.github.com/repos/datawrapper/datawrapper/issues/events{/number}","events_url":"https://api.github.com/repos/datawrapper/datawrapper/events","assignees_url":"https://api.github.com/repos/datawrapper/datawrapper/assignees{/user}","branches_url":"https://api.github.com/repos/datawrapper/datawrapper/branches{/branch}","tags_url":"https://api.github.com/repos/datawrapper/datawrapper/tags","blobs_url":"https://api.github.com/repos/datawrapper/datawrapper/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/datawrapper/datawrapper/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/datawrapper/datawrapper/git/refs{/sha}","trees_url":"https://api.github.com/repos/datawrapper/datawrapper/git/trees{/sha}","statuses_url":"https://api.github.com/repos/datawrapper/datawrapper/statuses/{sha}","languages_url":"https://api.github.com/repos/datawrapper/datawrapper/languages","stargazers_url":"https://api.github.com/repos/datawrapper/datawrapper/stargazers","contributors_url":"https://api.github.com/repos/datawrapper/datawrapper/contributors","subscribers_url":"https://api.github.com/repos/datawrapper/datawrapper/subscribers","subscription_url":"https://api.github.com/repos/datawrapper/datawrapper/subscription","commits_url":"https://api.github.com/repos/datawrapper/datawrapper/commits{/sha}","git_commits_url":"https://api.github.com/repos/datawrapper/datawrapper/git/commits{/sha}","comments_url":"https://api.github.com/repos/datawrapper/datawrapper/comments{/number}","issue_comment_url":"https://api.github.com/repos/datawrapper/datawrapper/issues/comments/{number}","contents_url":"https://api.github.com/repos/datawrapper/datawrapper/contents/{+path}","compare_url":"https://api.github.com/repos/datawrapper/datawrapper/compare/{base}...{head}","merges_url":"https://api.github.com/repos/datawrapper/datawrapper/merges","archive_url":"https://api.github.com/repos/datawrapper/datawrapper/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/datawrapper/datawrapper/downloads","issues_url":"https://api.github.com/repos/datawrapper/datawrapper/issues{/number}","pulls_url":"https://api.github.com/repos/datawrapper/datawrapper/pulls{/number}","milestones_url":"https://api.github.com/repos/datawrapper/datawrapper/milestones{/number}","notifications_url":"https://api.github.com/repos/datawrapper/datawrapper/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/datawrapper/datawrapper/labels{/name}","releases_url":"https://api.github.com/repos/datawrapper/datawrapper/releases{/id}","created_at":"2012-07-06T08:54:49Z","updated_at":"2014-07-21T11:40:47Z","pushed_at":"2014-07-23T04:57:18Z","git_url":"git://github.com/datawrapper/datawrapper.git","ssh_url":"git@github.com:datawrapper/datawrapper.git","clone_url":"https://github.com/datawrapper/datawrapper.git","svn_url":"https://github.com/datawrapper/datawrapper","homepage":"http://datawrapper.de","size":29015,"stargazers_count":357,"watchers_count":357,"language":"PHP","has_issues":true,"has_downloads":true,"has_wiki":true,"forks_count":95,"mirror_url":null,"open_issues_count":41,"forks":95,"open_issues":41,"watchers":357,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/datawrapper/datawrapper/pulls/109"},"html":{"href":"https://github.com/datawrapper/datawrapper/pull/109"},"issue":{"href":"https://api.github.com/repos/datawrapper/datawrapper/issues/109"},"comments":{"href":"https://api.github.com/repos/datawrapper/datawrapper/issues/109/comments"},"review_comments":{"href":"https://api.github.com/repos/datawrapper/datawrapper/pulls/109/comments"},"review_comment":{"href":"https://api.github.com/repos/datawrapper/datawrapper/pulls/comments/{number}"},"commits":{"href":"https://api.github.com/repos/datawrapper/datawrapper/pulls/109/commits"},"statuses":{"href":"https://api.github.com/repos/datawrapper/datawrapper/statuses/fd60e58d45948fdf1eb37fcbf60ec93a5616d978"}}}}