Fix sync bugs and fail runs on Teamwork errors

README.md

@@ -21,7 +21,7 @@ Create the next environment vars in your repository:
 * `TEAMWORK_URI`: The URL of your installation (e.g.: https://yourcompany.teamwork.com)
 * `TEAMWORK_API_TOKEN`: The API token to authenticate the workflow. Follow [this guide](https://support.teamwork.com/projects/using-teamwork/locating-your-api-key) to find your URL and API key.
 
-**Please Note:** The Teamwork account associated with this API key is the account which these comments will be created under. If this user does not have permission to access the project, this action will be ignored.
+**Please Note:** The Teamwork account associated with this API key is the account which these comments will be created under. If this user does not have permission to access a task's project, that task will be skipped and the workflow run will be marked as failed.
 
 `GITHUB_TOKEN` doesn't need to be setup in the repository, this var is always available during the workflows execution.
 
@@ -59,7 +59,7 @@ jobs:
 ## Usage
 When creating a new PR, write in the description of the PR the URL of the task. The action will automatically add a comment in the task. Optionally, you may choose for a lightweight comment that will only include the PR title, URL and the user that performed the action, to enable this option set the `LIGHTWEIGHT_COMMENT` input to `true`.
 
-Please note, the comment will be created in Teamwork under the account you have attached to this action. If the API key of the user you are using does not have permissions to access certain projects, the comment will not be created.
+Please note, the comment will be created in Teamwork under the account you have attached to this action. If the API key of the user you are using does not have permissions to access certain projects, the comment will not be created and the workflow run will report an error.
 
 ![GitHub pr comment](./.github/assets/github_pr_comment.png)
 

src/ensure.sh

@@ -1,18 +1,8 @@
 #!/usr/bin/env bash
 
 ensure::env_variable_exist() {
-  if [[ -z "${!1}" ]]; then
-    echoerr "The env variable $1 is required."
-    exit 1
-  fi
-}
-
-ensure::total_args() {
-  local -r received_args=$(( $# - 1 ))
-  local -r expected_args=$1
-
-  if ((received_args != expected_args)); then
-    echoerr "Illegal number of parameters, $expected_args expected but $received_args found"
+  if [[ -z "${!1:-}" ]]; then
+    log::error "The env variable $1 is required."
     exit 1
   fi
 }

src/github.sh

@@ -8,12 +8,9 @@ github::get_action() {
   jq --raw-output .action "$GITHUB_EVENT_PATH"
 }
 
-github::get_pr_number() {
-  jq --raw-output .pull_request.number "$GITHUB_EVENT_PATH"
-}
-
 github::get_pr_body() {
-  jq --raw-output .pull_request.body "$GITHUB_EVENT_PATH" | sed -r 's/^#{1,3}\ /#### /g'
+  jq --raw-output .pull_request.body "$GITHUB_EVENT_PATH" |\
+    awk '/^[[:space:]]*(```|~~~)/ { fenced = !fenced } !fenced { sub(/^#{1,3} /, "#### ") } { print }'
 }
 
 github::get_head_ref() {
@@ -24,10 +21,6 @@ github::get_base_ref() {
   jq --raw-output .pull_request.base.ref "$GITHUB_EVENT_PATH"
 }
 
-github::get_repository_full_name() {
-  jq --raw-output .pull_request.head.repo.full_name "$GITHUB_EVENT_PATH"
-}
-
 github::get_pr_url() {
   jq --raw-output .pull_request.html_url "$GITHUB_EVENT_PATH"
 }
@@ -55,7 +48,3 @@ github::get_review_state() {
 github::get_review_comment() {
   jq --raw-output '.review.body // empty' "$GITHUB_EVENT_PATH"
 }
-
-github::print_all_data() {
-  cat "$GITHUB_EVENT_PATH"
-}

src/main.sh

@@ -12,6 +12,7 @@ main() {
   # Ensure env vars and args exist
   ensure::env_variable_exist "GITHUB_REPOSITORY"
   ensure::env_variable_exist "GITHUB_EVENT_PATH"
+  ensure::env_variable_exist "GITHUB_EVENT_NAME"
 
   export GITHUB_TOKEN="$1"
   export TEAMWORK_URI="$2"
@@ -48,37 +49,52 @@ main() {
 
   log::message "Event: $event - Action: $action"
 
+  # Pick the handler before touching the API, so unsupported events make no
+  # requests at all
+  local handler
+  if [ "$event" == "pull_request" ] && [ "$action" == "opened" ]; then
+    handler=teamwork::pull_request_opened
+  elif [ "$event" == "pull_request" ] && [ "$action" == "closed" ]; then
+    handler=teamwork::pull_request_closed
+  elif [ "$event" == "pull_request_review" ] && [ "$action" == "submitted" ]; then
+    handler=teamwork::pull_request_review_submitted
+  elif [ "$event" == "pull_request_review" ] && [ "$action" == "dismissed" ]; then
+    handler=teamwork::pull_request_review_dismissed
+  elif [ "$ENV" == "test" ]; then # always run pull_request_opened at the very least when in test
+    handler=teamwork::pull_request_opened
+  else
+    log::message "Operation not allowed"
+    exit 0
+  fi
+
+  TEAMWORK_FAILED_REQUESTS=0
+
   local project_id
   IFS=',' read -r -a task_ids <<< "$task_ids_str"
   for task_id in "${task_ids[@]}"; do
     log::message "Task found with the id: $task_id"
 
     export TEAMWORK_TASK_ID=$task_id
-    project_id="$(teamwork::get_project_id_from_task "$task_id")"
+    if ! project_id="$(teamwork::get_project_id_from_task "$task_id")" || [[ ! $project_id =~ ^[0-9]+$ ]]; then
+      teamwork::request_failed "Failed to get the project of task $task_id, check the task exists and the API token's user can access it"
+      continue
+    fi
     export TEAMWORK_PROJECT_ID=$project_id
 
     ignored_project_ids=("${IGNORE_PROJECT_IDS:-}")
     if utils::in_array "$project_id" "${ignored_project_ids[*]}"
     then
         log::message "ignored due to IGNORE_PROJECT_IDS"
-        exit 0
+        continue
     fi
 
-    if [ "$event" == "pull_request" ] && [ "$action" == "opened" ]; then
-      teamwork::pull_request_opened
-    elif [ "$event" == "pull_request" ] && [ "$action" == "closed" ]; then
-      teamwork::pull_request_closed
-    elif [ "$event" == "pull_request_review" ] && [ "$action" == "submitted" ]; then
-      teamwork::pull_request_review_submitted
-    elif [ "$event" == "pull_request_review" ] && [ "$action" == "dismissed" ]; then
-      teamwork::pull_request_review_dismissed
-    elif [ "$ENV" == "test" ]; then # always run pull_request_opened at the very least when in test
-      teamwork::pull_request_opened
-    else
-      log::message "Operation not allowed"
-      exit 0
-    fi
+    "$handler"
   done
 
-  exit $?
+  if ((TEAMWORK_FAILED_REQUESTS > 0)); then
+    log::error "$TEAMWORK_FAILED_REQUESTS Teamwork request(s) failed, see the errors above"
+    exit 1
+  fi
+
+  exit 0
 }

src/teamwork.sh

@@ -6,7 +6,9 @@ teamwork::get_task_id_from_body() {
 
   pat='tasks\/([0-9]{1,})'
   while [[ $body =~ $pat ]]; do
-    task_ids+=( "${BASH_REMATCH[1]}" )
+    if ! utils::in_array "${BASH_REMATCH[1]}" "${task_ids[*]}"; then
+      task_ids+=( "${BASH_REMATCH[1]}" )
+    fi
     body=${body#*"${BASH_REMATCH[0]}"}
   done
 
@@ -16,20 +18,28 @@ teamwork::get_task_id_from_body() {
   echo "$task_ids_str"
 }
 
-# Calls the Teamwork API and prints the response body.
+# Calls the Teamwork API and prints the response body. Fails on network errors,
+# timeouts and HTTP error statuses, printing the error body in the latter case.
 teamwork::request() {
   local -r method=$1
   local -r path=$2
   local -r payload=${3:-}
 
-  local args=(-sS -X "$method" -u "$TEAMWORK_API_TOKEN"':')
+  local args=(-sS --fail-with-body --max-time 30 -X "$method" -u "$TEAMWORK_API_TOKEN"':')
   if [ -n "$payload" ]; then
     args+=(-H 'Content-Type: application/json; charset=utf-8' -d "$payload")
   fi
 
   curl "${args[@]}" "$TEAMWORK_URI$path"
 }
 
+# Logs a failed request and records it, so the run fails once every task has
+# been processed.
+teamwork::request_failed() {
+  log::error "$1"
+  TEAMWORK_FAILED_REQUESTS=$((TEAMWORK_FAILED_REQUESTS + 1))
+}
+
 teamwork::get_project_id_from_task() {
   local -r task_id=$1
 
@@ -38,17 +48,15 @@ teamwork::get_project_id_from_task() {
     return
   fi
 
-  response=$(
-    teamwork::request GET "/projects/api/v1/tasks/$task_id.json" |\
-      jq -r '.["todo-item"]["project-id"]'
-  )
-  echo "$response"
+  teamwork::request GET "/projects/api/v1/tasks/$task_id.json" |\
+    jq -r '.["todo-item"]["project-id"]'
 }
 
 # Resolves a workflow stage name to its "<workflowId> <stageId>" pair within the
 # task's project. Matching is a case-sensitive substring match against the stage
 # name (mirroring the legacy board-column behaviour), and the first match wins.
-# Echoes nothing when no stage name is provided or no stage matches.
+# Echoes nothing when no stage name is provided or no stage matches, and fails
+# when the stages can't be fetched.
 teamwork::get_workflow_stage() {
   local -r stage_name=$1
 
@@ -77,7 +85,11 @@ teamwork::move_task_to_stage() {
     return
   fi
 
-  local -r stage=$(teamwork::get_workflow_stage "$stage_name")
+  local stage
+  if ! stage=$(teamwork::get_workflow_stage "$stage_name"); then
+    teamwork::request_failed "Failed to fetch the workflow stages of project $TEAMWORK_PROJECT_ID"
+    return
+  fi
   if [ -z "$stage" ]; then
     log::message "Failed to find a matching workflow stage for '$stage_name'"
     return
@@ -91,8 +103,11 @@ teamwork::move_task_to_stage() {
     return
   fi
 
-  response=$(teamwork::request PATCH "/projects/api/v3/tasks/$TEAMWORK_TASK_ID/workflows/$workflow_id.json" \
-      "{ \"stageId\": $stage_id, \"positionAfterTask\": -1 }")
+  if ! response=$(teamwork::request PATCH "/projects/api/v3/tasks/$TEAMWORK_TASK_ID/workflows/$workflow_id.json" \
+      "{ \"stageId\": $stage_id, \"positionAfterTask\": -1 }"); then
+    teamwork::request_failed "Failed to move task $TEAMWORK_TASK_ID to the '$stage_name' workflow stage: $response"
+    return
+  fi
 
   log::message "$response"
 }
@@ -101,12 +116,18 @@ teamwork::add_comment() {
   local -r body=$1
 
   if [ "$ENV" == "test" ]; then
-    log::message "Test - Simulate request. Task ID: $TEAMWORK_TASK_ID - Comment: ${body//\"/}"
+    log::message "Test - Simulate request. Task ID: $TEAMWORK_TASK_ID - Comment: $body"
     return
   fi
 
-  response=$(teamwork::request POST "/tasks/$TEAMWORK_TASK_ID/comments.json" \
-      "{ \"comment\": { \"body\": \"${body//\"/}\", \"notify\": true, \"content-type\": \"text\", \"isprivate\": false } }")
+  # jq escapes the body (quotes, backslashes, newlines...) into valid JSON
+  local -r payload=$(jq -n --arg body "$body" \
+    '{ comment: { body: $body, notify: true, "content-type": "text", isprivate: false } }')
+
+  if ! response=$(teamwork::request POST "/tasks/$TEAMWORK_TASK_ID/comments.json" "$payload"); then
+    teamwork::request_failed "Failed to add a comment to task $TEAMWORK_TASK_ID: $response"
+    return
+  fi
 
   log::message "$response"
 }
@@ -120,8 +141,11 @@ teamwork::add_tag() {
   fi
 
   if [ "$AUTOMATIC_TAGGING" == true ]; then
-    response=$(teamwork::request PUT "/tasks/$TEAMWORK_TASK_ID/tags.json" \
-        "{ \"tags\": { \"content\": \"${tag_name//\"/}\" } }")
+    if ! response=$(teamwork::request PUT "/tasks/$TEAMWORK_TASK_ID/tags.json" \
+        "{ \"tags\": { \"content\": \"${tag_name//\"/}\" } }"); then
+      teamwork::request_failed "Failed to add the '$tag_name' tag to task $TEAMWORK_TASK_ID: $response"
+      return
+    fi
 
     log::message "$response"
   fi
@@ -136,8 +160,11 @@ teamwork::remove_tag() {
   fi
 
   if [ "$AUTOMATIC_TAGGING" == true ]; then
-    response=$(teamwork::request PUT "/tasks/$TEAMWORK_TASK_ID/tags.json" \
-        "{ \"tags\": { \"content\": \"${tag_name//\"/}\" },\"removeProvidedTags\":\"true\" }")
+    if ! response=$(teamwork::request PUT "/tasks/$TEAMWORK_TASK_ID/tags.json" \
+        "{ \"tags\": { \"content\": \"${tag_name//\"/}\" },\"removeProvidedTags\":\"true\" }"); then
+      teamwork::request_failed "Failed to remove the '$tag_name' tag from task $TEAMWORK_TASK_ID: $response"
+      return
+    fi
 
     log::message "$response"
   fi

tests/cases/closed-merged-lightweight/expected

@@ -4,7 +4,7 @@ GET /projects/api/v1/tasks/1001.json -> 200
 
 POST /tasks/1001/comments.json -> 201
   comment:
-    body: "PR [**Add OAuth login**](https://github.com/acme/webapp/pull/42) merged by **octocat**"
+    body: "PR [**\"Add OAuth login\"**](https://github.com/acme/webapp/pull/42) merged by **octocat**"
     content-type: "text"
     isprivate: false
     notify: true

tests/cases/closed-merged/expected

@@ -2,9 +2,16 @@ exit code: 0
 
 GET /projects/api/v1/tasks/1001.json -> 200
 
-POST /tasks/1001/comments.json -> 400
-  invalid JSON: Invalid control character at: line 1 column 25 (char 24)
-  "{ \"comment\": { \"body\": \"\n**octocat** merged a PR: **Add OAuth login**\n[https://github.com/acme/webapp/pull/42](https://github.com/acme/webapp/pull/42)\n\", \"notify\": true, \"content-type\": \"text\", \"isprivate\": false } }"
+POST /tasks/1001/comments.json -> 201
+  comment:
+    body:
+      |
+      | **octocat** merged a PR: **Add OAuth login**
+      | [https://github.com/acme/webapp/pull/42](https://github.com/acme/webapp/pull/42)
+      |
+    content-type: "text"
+    isprivate: false
+    notify: true
 
 PUT /tasks/1001/tags.json -> 200
   tags:

tests/cases/closed-unmerged-lightweight/expected

@@ -4,7 +4,7 @@ GET /projects/api/v1/tasks/1001.json -> 200
 
 POST /tasks/1001/comments.json -> 201
   comment:
-    body: "PR [**Add OAuth login**](https://github.com/acme/webapp/pull/42) closed without merging by **octocat**"
+    body: "PR [**\"Add OAuth login\"**](https://github.com/acme/webapp/pull/42) closed without merging by **octocat**"
     content-type: "text"
     isprivate: false
     notify: true

tests/cases/closed-unmerged/expected

@@ -2,9 +2,16 @@ exit code: 0
 
 GET /projects/api/v1/tasks/1001.json -> 200
 
-POST /tasks/1001/comments.json -> 400
-  invalid JSON: Invalid control character at: line 1 column 25 (char 24)
-  "{ \"comment\": { \"body\": \"\n**octocat** closed a PR without merging: **Add OAuth login**\n[https://github.com/acme/webapp/pull/42](https://github.com/acme/webapp/pull/42)\n\", \"notify\": true, \"content-type\": \"text\", \"isprivate\": false } }"
+POST /tasks/1001/comments.json -> 201
+  comment:
+    body:
+      |
+      | **octocat** closed a PR without merging: **Add OAuth login**
+      | [https://github.com/acme/webapp/pull/42](https://github.com/acme/webapp/pull/42)
+      |
+    content-type: "text"
+    isprivate: false
+    notify: true
 
 PUT /tasks/1001/tags.json -> 200
   removeProvidedTags: "true"

tests/cases/comment-fails/expected

@@ -1,10 +1,15 @@
-exit code: 0
+exit code: 1
+
+stderr:
+  curl: (22) The requested URL returned error: 500
+  Failed to add a comment to task 1003: {"MESSAGE": "Internal server error"}
+  1 Teamwork request(s) failed, see the errors above
 
 GET /projects/api/v1/tasks/1003.json -> 200
 
 POST /tasks/1003/comments.json -> 500
   comment:
-    body: "PR [**Add OAuth login**](https://github.com/acme/webapp/pull/42) opened by **octocat**"
+    body: "PR [**\"Add OAuth login\"**](https://github.com/acme/webapp/pull/42) opened by **octocat**"
     content-type: "text"
     isprivate: false
     notify: true

tests/cases/deprecated-board-column-closed/expected

@@ -4,7 +4,7 @@ GET /projects/api/v1/tasks/1001.json -> 200
 
 POST /tasks/1001/comments.json -> 201
   comment:
-    body: "PR [**Add OAuth login**](https://github.com/acme/webapp/pull/42) closed without merging by **octocat**"
+    body: "PR [**\"Add OAuth login\"**](https://github.com/acme/webapp/pull/42) closed without merging by **octocat**"
     content-type: "text"
     isprivate: false
     notify: true

tests/cases/deprecated-board-column/expected

@@ -4,7 +4,7 @@ GET /projects/api/v1/tasks/1001.json -> 200
 
 POST /tasks/1001/comments.json -> 201
   comment:
-    body: "PR [**Add OAuth login**](https://github.com/acme/webapp/pull/42) opened by **octocat**"
+    body: "PR [**\"Add OAuth login\"**](https://github.com/acme/webapp/pull/42) opened by **octocat**"
     content-type: "text"
     isprivate: false
     notify: true

tests/cases/ignored-project/expected

@@ -1,3 +1,12 @@
 exit code: 0
 
 GET /projects/api/v1/tasks/1001.json -> 200
+
+GET /projects/api/v1/tasks/1002.json -> 200
+
+POST /tasks/1002/comments.json -> 201
+  comment:
+    body: "PR [**\"Add OAuth login\"**](https://github.com/acme/webapp/pull/42) opened by **octocat**"
+    content-type: "text"
+    isprivate: false
+    notify: true