From 0bd33449f05b83509e5f333b64192c4c0cf908bc Mon Sep 17 00:00:00 2001 From: Teemu Patja Date: Thu, 2 Nov 2017 21:26:06 +0200 Subject: [PATCH 1/9] Handle PR edited event-type Look for claims also when receiving a pull-request edited webhook Fixes: #144 --- src/clj/commiteth/routes/webhooks.clj | 60 ++++++++++++++------------- 1 file changed, 31 insertions(+), 29 deletions(-) diff --git a/src/clj/commiteth/routes/webhooks.clj b/src/clj/commiteth/routes/webhooks.clj index 3c69665..5c739a7 100644 --- a/src/clj/commiteth/routes/webhooks.clj +++ b/src/clj/commiteth/routes/webhooks.clj @@ -59,17 +59,17 @@ (issues/update-open-status issue-id false)) #_(when-let [commit-sha (find-commit-sha owner repo issue-number ["referenced" "closed"])] - (log/debug (format "Issue %s/%s/%s closed with commit %s" - owner repo issue-number commit-sha)) - (log/info "NOT considering event as bounty winner") - ;; TODO: disabled for now since the system is meant to be used - ;; exclusively via pull requests. issue closed event without a PR - ;; closed via merge first means that the referencing commit was - ;; pushed directly to master and thus never accepted by the - ;; maintainer (could be that the bounty hunter had write access - ;; to master, but that scenario should be very rare and better - ;; not to support it) - #_(issues/close commit-sha issue-id))) + (log/debug (format "Issue %s/%s/%s closed with commit %s" + owner repo issue-number commit-sha)) + (log/info "NOT considering event as bounty winner") + ;; TODO: disabled for now since the system is meant to be used + ;; exclusively via pull requests. issue closed event without a PR + ;; closed via merge first means that the referencing commit was + ;; pushed directly to master and thus never accepted by the + ;; maintainer (could be that the bounty hunter had write access + ;; to master, but that scenario should be very rare and better + ;; not to support it) + #_(issues/close commit-sha issue-id))) (defn handle-issue-reopened [{{issue-id :id} :issue}] @@ -142,6 +142,8 @@ (log/debug "Referenced bounty issue found" repo bounty-issue-number) (users/create-user user-id login name nil avatar_url) (let [issue (github/get-issue owner repo bounty-issue-number) + open-or-edit? (contains? #{:opened :edited} event-type) + close? (= :closed event-type) pr-data {:repo_id repo-id :pr_id id :pr_number pr-number @@ -154,24 +156,24 @@ ;; Ethereum address stored, we could post a comment to the ;; Github PR explaining that payout is not possible if the PR is ;; merged - (case event-type - :opened (do - (log/info "PR with reference to bounty issue" - bounty-issue-number "opened") - (pull-requests/save (merge pr-data {:state :opened - :commit_sha head-sha}))) - :closed (if merged? - (do (log/info "PR with reference to bounty issue" - bounty-issue-number "merged") - (pull-requests/save - (merge pr-data {:state :merged - :commit_sha head-sha})) - (issues/update-commit-sha (:id issue) head-sha)) - (do (log/info "PR with reference to bounty issue" - bounty-issue-number "closed with no merge") - (pull-requests/save - (merge pr-data {:state :closed - :commit_sha head-sha})))))))) + (cond + open-or-edit? (do + (log/info "PR with reference to bounty issue" + bounty-issue-number "opened") + (pull-requests/save (merge pr-data {:state :opened + :commit_sha head-sha}))) + close? (if merged? + (do (log/info "PR with reference to bounty issue" + bounty-issue-number "merged") + (pull-requests/save + (merge pr-data {:state :merged + :commit_sha head-sha})) + (issues/update-commit-sha (:id issue) head-sha)) + (do (log/info "PR with reference to bounty issue" + bounty-issue-number "closed with no merge") + (pull-requests/save + (merge pr-data {:state :closed + :commit_sha head-sha})))))))) (defn handle-issue-edited [webhook-payload] From f3be62cd554c5220c31e7c4561a81159fb3a4cd0 Mon Sep 17 00:00:00 2001 From: Teemu Patja Date: Thu, 2 Nov 2017 22:29:11 +0200 Subject: [PATCH 2/9] PR webhook handler fix --- src/clj/commiteth/routes/webhooks.clj | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/clj/commiteth/routes/webhooks.clj b/src/clj/commiteth/routes/webhooks.clj index 5c739a7..f59af0a 100644 --- a/src/clj/commiteth/routes/webhooks.clj +++ b/src/clj/commiteth/routes/webhooks.clj @@ -202,11 +202,12 @@ (defn handle-pull-request [pull-request] - (case (:action pull-request) - "opened" (handle-pull-request-event :opened pull-request) - "closed" (handle-pull-request-event :closed pull-request) - nil) - (ok)) + (let [action (keyword (:action pull-request))] + (when (contains? #{:opened + :edited + :closed} action) + (handle-pull-request-event action pull-request)) + (ok))) (defn validate-secret [webhook-payload raw-payload github-signature] From 81c24b5793dfa71907ae2aff0c21d170f052b9fc Mon Sep 17 00:00:00 2001 From: Teemu Patja Date: Thu, 2 Nov 2017 23:14:42 +0200 Subject: [PATCH 3/9] SQL fix related to PR edited webhook --- resources/sql/queries.sql | 2 ++ 1 file changed, 2 insertions(+) diff --git a/resources/sql/queries.sql b/resources/sql/queries.sql index 0a802f3..3621975 100644 --- a/resources/sql/queries.sql +++ b/resources/sql/queries.sql @@ -227,6 +227,8 @@ VALUES(:pr_id, ON CONFLICT (pr_id) DO UPDATE SET state = :state, + issue_number = :issue_number, + issue_id = :issue_id, updated = timezone('utc'::text, now()), commit_sha = :commit_sha; From d265977e035d5df40910715f368b22ad43eb8572 Mon Sep 17 00:00:00 2001 From: Teemu Patja Date: Fri, 3 Nov 2017 08:53:05 +0200 Subject: [PATCH 4/9] Fix update-address button styling --- src/cljs/commiteth/update_address.cljs | 16 +++++++++------- src/less/style.less | 8 ++++++++ 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/src/cljs/commiteth/update_address.cljs b/src/cljs/commiteth/update_address.cljs index a64d361..6134bde 100644 --- a/src/cljs/commiteth/update_address.cljs +++ b/src/cljs/commiteth/update_address.cljs @@ -33,10 +33,12 @@ :auto-correct "off" :spell-check "false" :max-length 42}]])] - [:button.ui.button (merge {:on-click - #(rf/dispatch [:save-user-address - (:id @user) - @address])} - (when @updating-address - {:class "busy loading"})) - "Update"]]])))) + [:button + (merge {:on-click + #(rf/dispatch [:save-user-address + (:id @user) + @address]) + :class (str "ui button small update-address-button" + (when @updating-address + " busy loading"))}) + "UPDATE"]]])))) diff --git a/src/less/style.less b/src/less/style.less index ed174db..d6887d8 100644 --- a/src/less/style.less +++ b/src/less/style.less @@ -39,6 +39,14 @@ background-color: #57a7ed!important; } +.update-address-button { + background-color: #57a7ed!important; + &:hover { + background-color: #57a7ed!important; + } +} + + .login-button { background-color: rgba(255,255,255,0.2)!important; } From e39d07c37fd4330187e72cd08821f9001dbf1669 Mon Sep 17 00:00:00 2001 From: Teemu Patja Date: Fri, 3 Nov 2017 09:50:15 +0200 Subject: [PATCH 5/9] Improve flash message styling & invalid address error message * more readable flash message popup * close flash message popup by clicking outside * show input in "invalid address" error message --- src/clj/commiteth/routes/services.clj | 6 ++++-- src/cljs/commiteth/core.cljs | 1 + src/cljs/commiteth/handlers.cljs | 8 +++++--- src/less/style.less | 10 +++++++++- 4 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/clj/commiteth/routes/services.clj b/src/clj/commiteth/routes/services.clj index f9f9bbc..5789df1 100644 --- a/src/clj/commiteth/routes/services.clj +++ b/src/clj/commiteth/routes/services.clj @@ -235,8 +235,10 @@ :body-params [user-id :- Long, address :- String] :summary "Update user address" (if-not (eth/valid-address? address) - {:status 400 - :body "Invalid Ethereum address"} + (do + (log/debug "POST /address: invalid input" address) + {:status 400 + :body (str "Invalid Ethereum address '" address "'")}) (let [result (users/update-user-address user-id address)] diff --git a/src/cljs/commiteth/core.cljs b/src/cljs/commiteth/core.cljs index 3a12f68..cabdc01 100644 --- a/src/cljs/commiteth/core.cljs +++ b/src/cljs/commiteth/core.cljs @@ -285,4 +285,5 @@ (load-interceptors!) (hook-browser-navigation!) (load-data true) + (.addEventListener js/window "click" #(rf/dispatch [:clear-flash-message])) (on-js-load)) diff --git a/src/cljs/commiteth/handlers.cljs b/src/cljs/commiteth/handlers.cljs index cffac1e..48eccf8 100644 --- a/src/cljs/commiteth/handlers.cljs +++ b/src/cljs/commiteth/handlers.cljs @@ -324,9 +324,11 @@ (dispatch [:set-flash-message :success "Address saved"])) - :on-error #(dispatch [:set-flash-message - :error - (:response %)]) + :on-error #(do + (println %) + (dispatch [:set-flash-message + :error + (:response %)])) :finally #(dispatch [:clear-updating-address]) :params {:user-id user-id :address address}}})) diff --git a/src/less/style.less b/src/less/style.less index d6887d8..5b7e159 100644 --- a/src/less/style.less +++ b/src/less/style.less @@ -607,11 +607,14 @@ &.error { background-color: #e96868; } + display: flex; + flex-direction: column; color: #fff; border-radius: 8px; z-index: 100; - padding: 1.4em 1em 1.4em; + padding: 40px 60px 40px; + height: auto; i { position: absolute; margin: 0; @@ -619,6 +622,11 @@ top: 1.5em; cursor: pointer; } + p { + font-family: "PostGrotesk-Book"; + font-weight: 500; + font-size: 15px; + } } From d806e38433beab87e9e0bdc400d863d59b37757c Mon Sep 17 00:00:00 2001 From: Teemu Patja Date: Fri, 3 Nov 2017 12:35:42 +0200 Subject: [PATCH 6/9] Remove PR -> issue link if comment removed If "Fixes: #NN" is removed from PR title/description, no longer consider it a claim for any issue. --- resources/sql/queries.sql | 7 ++ src/clj/commiteth/db/pull_requests.clj | 6 ++ src/clj/commiteth/routes/webhooks.clj | 91 ++++++++++++++++---------- 3 files changed, 68 insertions(+), 36 deletions(-) diff --git a/resources/sql/queries.sql b/resources/sql/queries.sql index 3621975..902a244 100644 --- a/resources/sql/queries.sql +++ b/resources/sql/queries.sql @@ -232,6 +232,13 @@ SET updated = timezone('utc'::text, now()), commit_sha = :commit_sha; + +-- :name remove-pull-request! :! :n +-- :doc remove a PR by id +DELETE from pull_requests +WHERE pr_id = :pr_id; + + -- Bounties ------------------------------------------------------------------------ diff --git a/src/clj/commiteth/db/pull_requests.clj b/src/clj/commiteth/db/pull_requests.clj index 3213f73..14a31fa 100644 --- a/src/clj/commiteth/db/pull_requests.clj +++ b/src/clj/commiteth/db/pull_requests.clj @@ -16,3 +16,9 @@ (jdbc/with-db-connection [con-db *db*] (db/save-pull-request! con-db (assoc pull-request :state state))))) + +(defn remove + [pr-id] + (jdbc/with-db-connection [con-db *db*] + (db/remove-pull-request! con-db + {:pr_id pr-id}))) diff --git a/src/clj/commiteth/routes/webhooks.clj b/src/clj/commiteth/routes/webhooks.clj index f59af0a..8f93a0e 100644 --- a/src/clj/commiteth/routes/webhooks.clj +++ b/src/clj/commiteth/routes/webhooks.clj @@ -111,6 +111,44 @@ issue-number))) +(defn handle-claim + [user-id login name avatar_url owner repo repo-id bounty-issue-number pr-id pr-number head-sha merged? event-type] + (users/create-user user-id login name nil avatar_url) + (let [issue (github/get-issue owner repo bounty-issue-number) + open-or-edit? (contains? #{:opened :edited} event-type) + close? (= :closed event-type) + pr-data {:repo_id repo-id + :pr_id pr-id + :pr_number pr-number + :user_id user-id + :issue_number bounty-issue-number + :issue_id (:id issue) + :state event-type}] + + ;; TODO: in the opened case if the submitting user has no + ;; Ethereum address stored, we could post a comment to the + ;; Github PR explaining that payout is not possible if the PR is + ;; merged + (cond + open-or-edit? (do + (log/info "PR with reference to bounty issue" + bounty-issue-number "opened") + (pull-requests/save (merge pr-data {:state :opened + :commit_sha head-sha}))) + close? (if merged? + (do (log/info "PR with reference to bounty issue" + bounty-issue-number "merged") + (pull-requests/save + (merge pr-data {:state :merged + :commit_sha head-sha})) + (issues/update-commit-sha (:id issue) head-sha)) + (do (log/info "PR with reference to bounty issue" + bounty-issue-number "closed with no merge") + (pull-requests/save + (merge pr-data {:state :closed + :commit_sha head-sha}))))))) + + (defn handle-pull-request-event ;; when a PR is opened, only consider it as a claim if: ;; * PR references an existing bounty-issue @@ -127,7 +165,7 @@ login :login avatar_url :avatar_url name :name} :user - id :id + pr-id :id merged? :merged {head-sha :sha} :head pr-number :number @@ -135,45 +173,26 @@ pr-title :title} :pull_request}] (log/debug "handle-pull-request-event" event-type owner repo repo-id login pr-body pr-title) (log/debug (extract-issue-number pr-body pr-title)) - (when-let [bounty-issue-number (->> + (if-let [bounty-issue-number (->> (extract-issue-number pr-body pr-title) (first) (ensure-bounty-issue owner repo))] - (log/debug "Referenced bounty issue found" repo bounty-issue-number) - (users/create-user user-id login name nil avatar_url) - (let [issue (github/get-issue owner repo bounty-issue-number) - open-or-edit? (contains? #{:opened :edited} event-type) - close? (= :closed event-type) - pr-data {:repo_id repo-id - :pr_id id - :pr_number pr-number - :user_id user-id - :issue_number bounty-issue-number - :issue_id (:id issue) - :state event-type}] + (do + (log/debug "Referenced bounty issue found" owner repo bounty-issue-number) + (handle-claim user-id + login name + avatar_url + owner repo + repo-id + bounty-issue-number + pr-id + pr-number + head-sha + merged? + event-type)) + (when (= :edited event-type) + (pull-requests/remove pr-id)))) - ;; TODO: in the opened case if the submitting user has no - ;; Ethereum address stored, we could post a comment to the - ;; Github PR explaining that payout is not possible if the PR is - ;; merged - (cond - open-or-edit? (do - (log/info "PR with reference to bounty issue" - bounty-issue-number "opened") - (pull-requests/save (merge pr-data {:state :opened - :commit_sha head-sha}))) - close? (if merged? - (do (log/info "PR with reference to bounty issue" - bounty-issue-number "merged") - (pull-requests/save - (merge pr-data {:state :merged - :commit_sha head-sha})) - (issues/update-commit-sha (:id issue) head-sha)) - (do (log/info "PR with reference to bounty issue" - bounty-issue-number "closed with no merge") - (pull-requests/save - (merge pr-data {:state :closed - :commit_sha head-sha})))))))) (defn handle-issue-edited [webhook-payload] From 820337e92b484059a3afe5657832243d31aa90aa Mon Sep 17 00:00:00 2001 From: Teemu Patja Date: Fri, 3 Nov 2017 13:14:38 +0200 Subject: [PATCH 7/9] Update github comment text --- src/clj/commiteth/github/core.clj | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/clj/commiteth/github/core.clj b/src/clj/commiteth/github/core.clj index 1dceb31..72bb8dc 100644 --- a/src/clj/commiteth/github/core.clj +++ b/src/clj/commiteth/github/core.clj @@ -227,7 +227,8 @@ (contract-addr-text contract-address) "%s\n" (network-text) - "To claim this bounty sign up at %s\n" + "To claim this bounty sign up at %s\ and make sure to update your Ethereum address " + "in `My Payment Details` so that the bounty is correctly allocated." (if (on-testnet?) "To fund it, send test ETH or test ERC20/ERC223 tokens to the contract address." "To fund it, send ETH or ERC20/ERC223 tokens to the contract address.")) From 39151c9ff9d6f20a5709355c0b692e1cdba74d5f Mon Sep 17 00:00:00 2001 From: Teemu Patja Date: Fri, 3 Nov 2017 13:32:15 +0200 Subject: [PATCH 8/9] Fix compile error --- src/clj/commiteth/github/core.clj | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/clj/commiteth/github/core.clj b/src/clj/commiteth/github/core.clj index 72bb8dc..761ff8b 100644 --- a/src/clj/commiteth/github/core.clj +++ b/src/clj/commiteth/github/core.clj @@ -227,8 +227,8 @@ (contract-addr-text contract-address) "%s\n" (network-text) - "To claim this bounty sign up at %s\ and make sure to update your Ethereum address " - "in `My Payment Details` so that the bounty is correctly allocated." + "To claim this bounty sign up at %sand make sure to update your Ethereum address " + "in `My Payment Details` so that the bounty is correctly allocated.\n" (if (on-testnet?) "To fund it, send test ETH or test ERC20/ERC223 tokens to the contract address." "To fund it, send ETH or ERC20/ERC223 tokens to the contract address.")) From 29a5b808c34208827ece676cb89f1eb4c187827e Mon Sep 17 00:00:00 2001 From: Teemu Patja Date: Fri, 3 Nov 2017 13:43:35 +0200 Subject: [PATCH 9/9] Typo fix --- src/clj/commiteth/github/core.clj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/clj/commiteth/github/core.clj b/src/clj/commiteth/github/core.clj index 761ff8b..21b8ba6 100644 --- a/src/clj/commiteth/github/core.clj +++ b/src/clj/commiteth/github/core.clj @@ -227,7 +227,7 @@ (contract-addr-text contract-address) "%s\n" (network-text) - "To claim this bounty sign up at %sand make sure to update your Ethereum address " + "To claim this bounty sign up at %s and make sure to update your Ethereum address " "in `My Payment Details` so that the bounty is correctly allocated.\n" (if (on-testnet?) "To fund it, send test ETH or test ERC20/ERC223 tokens to the contract address."