- Fixes #3455: Gitlab and Github integration cause application reload…

… when using [Enter]

- Fixes #3456: Wrong API-Key for Gitlab does not throw error message
- Fixes #3458: Gitlab and Github integration should provide visual feedback during adding issue.
- Fixes #3459: Provide error message if user provided wrong information for Github and Gitlab
- Refactoring: Multiple loads of GitLab/GitHub GraphQL schema causes Memory leak.

Gemfile

@@ -123,7 +123,6 @@ gem 'acts_as_list'
 
 # integrations
 gem 'clearbit'
-gem 'graphql-client'
 gem 'net-ldap'
 gem 'slack-notifier'
 gem 'zendesk_api'

Gemfile.lock

@@ -218,10 +218,6 @@ GEM
       activesupport (>= 4.2.0)
     gmail_xoauth (0.4.2)
       oauth (>= 0.3.6)
-    graphql (1.11.6)
-    graphql-client (0.16.0)
-      activesupport (>= 3.0)
-      graphql (~> 1.8)
     guard (2.15.0)
       formatador (>= 0.2.4)
       listen (>= 2.7, < 4.0)
@@ -617,7 +613,6 @@ DEPENDENCIES
   faker
   github_changelog_generator
   gmail_xoauth
-  graphql-client
   guard
   guard-livereload
   guard-symlink

app/assets/javascripts/app/controllers/_integration/github.coffee

@@ -51,8 +51,7 @@ class Form extends App.Controller
           )
           return
 
-        config.schema = data.response
-        App.Setting.set('github_config', config)
+        App.Setting.set('github_config', config, notify: true)
 
       error: (data, status) ->
 

app/assets/javascripts/app/controllers/_integration/gitlab.coffee

@@ -51,8 +51,7 @@ class Form extends App.Controller
           )
           return
 
-        config.schema = data.response
-        App.Setting.set('gitlab_config', config)
+        App.Setting.set('gitlab_config', config, notify: true)
 
       error: (data, status) ->
 

app/assets/javascripts/app/controllers/ticket_zoom/sidebar_git_issue.coffee

@@ -4,7 +4,8 @@ class App.SidebarGitIssue extends App.Controller
 
   constructor: ->
     super
-    @issueLinks = []
+    @issueLinks         = []
+    @issueLinkData      = []
     @providerIdentifier = @provider.toLowerCase()
 
   sidebarItem: =>
@@ -13,7 +14,7 @@ class App.SidebarGitIssue extends App.Controller
       name: @providerIdentifier
       badgeCallback: @badgeRender
       sidebarHead: @provider
-      sidebarCallback: @showObjects
+      sidebarCallback: @reloadIssues
       sidebarActions: [
         {
           title:    'Link issue'
@@ -26,8 +27,8 @@ class App.SidebarGitIssue extends App.Controller
 
   shown: ->
     return if !@ticket
-    return if !@ticket.id
-    @showIssues()
+
+    @listIssues()
 
   metaBadge: =>
     counter = ''
@@ -45,6 +46,7 @@ class App.SidebarGitIssue extends App.Controller
     @badgeRenderLocal()
 
   badgeRenderLocal: =>
+    return if !@badgeEl
     @badgeEl.html(App.view('generic/sidebar_tabs_item')(@metaBadge()))
 
   linkIssue: =>
@@ -54,86 +56,141 @@ class App.SidebarGitIssue extends App.Controller
       taskKey: @taskKey
       container: @el.closest('.content')
       callback: (link, ui) =>
-        if @ticket && @ticket.id
-          @saveTicketIssues = true
-        ui.close()
-        @showIssues([link])
+        @getIssues(
+          links: [link]
+          success: (result) =>
+            if !_.contains(@issueLinks, link)
+              @issueLinks.push(result[0].url)
+              @issueLinkData = @issueLinkData.concat(result)
+
+            if @ticket && @ticket.id
+              @saveIssues(
+                ticket_id: @ticket.id
+                links: @issueLinks
+                success: =>
+                  ui.close()
+                  @renderIssues()
+                error: (message = 'Unable to save issue') =>
+                  ui.showAlert(App.i18n.translatePlain(message))
+                  form = ui.el.find('.js-result')
+                  @formEnable(form)
+              )
+            else
+              ui.close()
+              @renderIssues()
+          error: (message = 'Unable to load issues') =>
+            ui.showAlert(App.i18n.translatePlain(message))
+            form = ui.el.find('.js-result')
+            @formEnable(form)
+        )
     )
 
-  showObjects: (el) =>
-    @el = el
+  reloadIssues: (el) =>
+    if el
+      @el = el
 
-    # show placeholder
-    if @ticket && @ticket.preferences && @ticket.preferences[@providerIdentifier] && @ticket.preferences[@providerIdentifier].issue_links
-      @issueLinks = @ticket.preferences[@providerIdentifier].issue_links
-    queryParams = @queryParam()
+    return @renderIssues() if !@ticket
 
-    # TODO: what is 'gitlab_issue_links'
-    if queryParams && queryParams.gitlab_issue_links
-      @issueLinks.push queryParams.gitlab_issue_links
-    @showIssues()
+    ticketLinks = @ticket?.preferences?[@providerIdentifier]?.issue_links || []
+    return @renderIssues() if _.isEqual(@issueLinks, ticketLinks)
 
-  showIssues: (issueLinks) =>
-    if issueLinks
-      @issueLinks = _.uniq(@issueLinks.concat(issueLinks))
+    @issueLinks = ticketLinks
+    @listIssues(true)
 
-    # show placeholder
-    if _.isEmpty(@issueLinks)
-      @html("<div>#{App.i18n.translateInline('No linked issues')}</div>")
+  renderIssues: =>
+    if _.isEmpty(@issueLinkData)
+      @showEmpty()
       return
 
-    # AJAX call to show items
+    list = $(App.view('ticket_zoom/sidebar_git_issue')(
+      issues: @issueLinkData
+    ))
+    list.delegate('.js-delete', 'click', (e) =>
+      e.preventDefault()
+      issueLink = $(e.currentTarget).attr 'data-issue-id'
+      @deleteIssue(issueLink)
+    )
+    @html(list)
+    @badgeRenderLocal()
+
+  listIssues: (force = false) =>
+    return @renderIssues() if !force && @fetchFullActive && @fetchFullActive > new Date().getTime() - 5000
+    @fetchFullActive = new Date().getTime()
+
+    return @renderIssues() if _.isEmpty(@issueLinks)
+
+    @getIssues(
+      links: @issueLinks
+      success: (result) =>
+        @issueLinks    = result.map((element) -> element.url)
+        @issueLinkData = result
+        @renderIssues()
+      error: =>
+        @showError(App.i18n.translateInline('Unable to load issues'))
+    )
+
+  getIssues: (params) ->
     @ajax(
       id:    "#{@providerIdentifier}-#{@taskKey}"
       type:  'POST'
       url:   "#{@apiPath}/integration/#{@providerIdentifier}"
-      data: JSON.stringify(links: @issueLinks)
-      success: (data, status, xhr) =>
+      data: JSON.stringify(links: params.links)
+      success: (data, status, xhr) ->
         if data.response
-          @showList(data.response)
-          if @saveTicketIssues
-            @saveTicketIssues = false
-            @issueLinks = data.response.map((issue) -> issue.url)
-            @updateTicket(@ticket.id, @issueLinks)
-          return
-        @showError('Unable to load data...')
 
-      error: (xhr, status, error) =>
+          # some issues redirect to pull requests like
+          # https://github.com/zammad/zammad/issues/1574
+          # in this case throw error
+          return params.error('Unable to load issues') if _.isEmpty(data.response)
 
-        # do not close window if request is aborted
+          params.success(data.response)
+        else
+          params.error(data.message)
+      error: (xhr, status, error) ->
         return if status is 'abort'
 
-        # show error message
-        @showError('Unable to load data...')
+        params.error()
     )
 
-  showList: (issues) =>
-    list = $(App.view('ticket_zoom/sidebar_git_issue')(
-      issues: issues
-    ))
-    list.delegate('.js-delete', 'click', (e) =>
-      e.preventDefault()
-      issueLink = $(e.currentTarget).attr 'data-issue-id'
-      @delete(issueLink)
+  saveIssues: (params) ->
+    App.Ajax.request(
+      id:    "#{@providerIdentifier}-update-#{params.ticket_id}"
+      type:  'POST'
+      url:   "#{@apiPath}/integration/#{@providerIdentifier}_ticket_update"
+      data:  JSON.stringify(ticket_id: params.ticket_id, issue_links: params.links)
+      success: (data, status, xhr) ->
+        params.success(data)
+      error: (xhr, status, details) ->
+        return if status is 'abort'
+
+        params.error()
     )
-    @html(list)
+
+  deleteIssue: (link) ->
+    @issueLinks    = _.filter(@issueLinks, (element) -> element isnt link)
+    @issueLinkData = _.filter(@issueLinkData, (element) -> element.url isnt link)
+
+    if @ticket && @ticket.id
+      @saveIssues(
+        ticket_id: @ticket.id
+        links: @issueLinks
+        success: =>
+          @renderIssues()
+        error: (message = 'Unable to save issue') =>
+          @showError(App.i18n.translateInline(message))
+      )
+    else
+      @renderIssues()
+
+  showEmpty: ->
+    @html("<div>#{App.i18n.translateInline('No linked issues')}</div>")
     @badgeRenderLocal()
 
   showError: (message) =>
     @html App.i18n.translateInline(message)
 
   reload: =>
-    @showIssues()
-
-  delete: (issueLink) =>
-    localLinks = []
-    for localLink in @issueLinks
-      if issueLink.toString() isnt localLink.toString()
-        localLinks.push localLink
-    @issueLinks = localLinks
-    if @ticket && @ticket.id
-      @updateTicket(@ticket.id, @issueLinks)
-    @showIssues()
+    @reloadIssues()
 
   postParams: (args) =>
     return if !args.ticket
@@ -143,25 +200,3 @@ class App.SidebarGitIssue extends App.Controller
     args.ticket.preferences ||= {}
     args.ticket.preferences[@providerIdentifier] ||= {}
     args.ticket.preferences[@providerIdentifier].issue_links = @issueLinks
-
-  updateTicket: (ticket_id, issueLinks) =>
-    App.Ajax.request(
-      id:    "#{@providerIdentifier}-update-#{ticket_id}"
-      type:  'POST'
-      url:   "#{@apiPath}/integration/#{@providerIdentifier}_ticket_update"
-      data:  JSON.stringify(ticket_id: ticket_id, issue_links: issueLinks)
-      success: (data, status, xhr) =>
-        @badgeRenderLocal()
-      error: (xhr, status, details) =>
-
-        # do not close window if request is aborted
-        return if status is 'abort'
-
-        # show error message
-        @log 'errors', details
-        @notify(
-          type:    'error'
-          msg:     App.i18n.translateContent(details.error_human || details.error || 'Unable to update object!')
-          timeout: 6000
-        )
-    )

app/assets/javascripts/app/views/integration/git_issue_link_modal.jst.eco

@@ -1,3 +1,3 @@
-<form class="flex horizontal js-result">
+<div class="flex horizontal js-result">
   <input type="text" name="link" value="" autocomplete="off" placeholder="<%= @placeholder %>" class="form-control"/>
-</form>
+</div>

app/controllers/integration/github_controller.rb

@@ -5,9 +5,11 @@ class Integration::GitHubController < ApplicationController
 
   def verify
     github = ::GitHub.new(params[:endpoint], params[:api_token])
+
+    github.verify!
+
     render json: {
-      result:   'ok',
-      response: github.schema.to_json,
+      result: 'ok',
     }
   rescue => e
     logger.error e
@@ -21,7 +23,7 @@ def verify
   def query
     config = Setting.get('github_config')
 
-    github = ::GitHub.new(config['endpoint'], config['api_token'], schema: config['schema'])
+    github = ::GitHub.new(config['endpoint'], config['api_token'])
 
     render json: {
       result:   'ok',

app/controllers/integration/gitlab_controller.rb

@@ -5,9 +5,11 @@ class Integration::GitLabController < ApplicationController
 
   def verify
     gitlab = ::GitLab.new(params[:endpoint], params[:api_token])
+
+    gitlab.verify!
+
     render json: {
-      result:   'ok',
-      response: gitlab.schema.to_json,
+      result: 'ok',
     }
   rescue => e
     logger.error e
@@ -21,7 +23,7 @@ def verify
   def query
     config = Setting.get('gitlab_config')
 
-    gitlab = ::GitLab.new(config['endpoint'], config['api_token'], schema: config['schema'])
+    gitlab = ::GitLab.new(config['endpoint'], config['api_token'])
 
     render json: {
       result:   'ok',

lib/github.rb

@@ -1,14 +1,14 @@
 # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/
 
 class GitHub
-  extend Forwardable
-
   attr_reader :client
 
-  def_delegator :client, :schema
+  def initialize(endpoint, api_token)
+    @client = GitHub::HttpClient.new(endpoint, api_token)
+  end
 
-  def initialize(*args, **kargs)
-    @client = GitHub::Client.new(*args, **kargs)
+  def verify!
+    GitHub::Credentials.new(client).verify!
   end
 
   def issues_by_urls(urls)