gh app #723

carlad · 2018-03-29T14:16:46Z

Addresses https://github.com/travis-pro/team-teal/issues/2608

current tests have been updated to pass, but a specific one for the new service needs to be completed

…h-app

rkh

This generally looks really good. I understood where the confusion around attribute names and endpoints comes from: The current version isn't exposing the installation via the API (which kinda makes sense, as it doesn't have any relevant attributes yet from my understanding). But in order to do this, it has to pretend like the GitHub installation id is an attribute on the owner, which it isn't under the hood. I do think treating installations as proper entities even in the API is a more elegant solution, and also leads to clearer API endpoints. See my inline comments.

lib/travis/api/v3/queries/organization.rb

+
+    def find_by_github_installation_id(github_installation_id)
+      installation = Models::Installation.where(github_id: github_installation_id).first
+      Models::Organization.find(installation.owner_id) if installation.owner_type == "Organization"


lib/travis/api/v3/renderer/owner.rb

+    def github_installation_id
+      installation = query(:installation).for_owner(@model).first
+      installation.github_id if installation
+    end


lib/travis/api/v3/routes.rb

@@ -86,7 +86,7 @@ module Routes
    end

    resource :owner do
-      route '/owner/({owner.login}|{user.login}|{organization.login}|github_id/{owner.github_id})'
+      route '/owner/({owner.login}|{user.login}|{organization.login}|github_id/{owner.github_id}|github_installation_id/{owner.github_installation_id})'


rkh

This looks really good, almost perfect.

I think it does need a little more cleanup (see inline comments) and then we're good to go.

Some changes also have messy indentation.

lib/travis/api/v3/models/repository.rb

+    def managed_by_installation
+      return true if read_attribute(:managed_by_installation_on)
+      return false
+    end


lib/travis/api/v3/queries/installation.rb

@@ -0,0 +1,10 @@
+module Travis::API::V3
+  class Queries::Installation < Query
+  	params :id


lib/travis/api/v3/queries/organization.rb

@@ -1,6 +1,6 @@
 module Travis::API::V3
  class Queries::Organization < Query
-    params :id, :login, :github_id
+    params :id, :login, :github_id, :github_installation_id


lib/travis/api/v3/queries/user.rb

@@ -1,7 +1,7 @@
 module Travis::API::V3
  class Queries::User < Query
    setup_sidekiq(:user_sync, queue: :sync, class_name: "Travis::GithubSync::Worker")
-    params :id, :login, :email, :github_id, :is_syncing
+    params :id, :login, :email, :github_id, :is_syncing, :github_installation_id


lib/travis/api/v3/renderer/installation.rb

+module Travis::API::V3
+  class Renderer::Installation < ModelRenderer
+    representation(:minimal,  :id, :github_id)
+    representation(:standard, *representations[:minimal], :owner_type, :owner_id)


lib/travis/api/v3/renderer/repository.rb

@@ -1,7 +1,7 @@
 module Travis::API::V3
  class Renderer::Repository < ModelRenderer
    representation(:minimal,  :id, :name, :slug)
-    representation(:standard, :id, :name, :slug, :description, :github_language, :active, :private, :owner, :default_branch, :starred)
+    representation(:standard, :id, :name, :slug, :description, :github_language, :active, :private, :owner, :default_branch, :starred, :managed_by_installation, :active_on_org)


lib/travis/api/v3/routes.rb

+    resource :installation do
+      route '/installation/{installation.github_id}'
+      get :find
+    end


lib/travis/api/v3/queries/installation.rb

+  	params :id
+
+  	def find
+      return Models::Installation.where(github_id: id) if id


…ronment variable

rkh · 2018-04-06T17:45:34Z

I don't know why the tests are failing now with id not being set. Cannot reproduce this on staging or locally and the error does not make sense to me.

lib/travis/api/v3/models/organization.rb

@@ -7,6 +7,12 @@ def repositories
      Models::Repository.where(owner_type: 'Organization', owner_id: id)
    end

+    # has_one :installation, as: :owner


lib/travis/api/v3/models/user.rb

@@ -34,5 +34,10 @@ def permission?(roles, options = {})
      scope.any?
    end

+    # has_one :installation, as: :owner


spec/v3/services/installation/find_spec.rb

+    }}
+  end
+
+  describe "authenticated as user with access" do


This reverts commit 321d73d.

Carla Drago added 6 commits March 29, 2018 15:10

add gh repo attributes

f4c1e26

change column to migrated_to_com

f064d01

update repo flag

594c10d

update names

11f6fc9

revert to active_on_org

4374ff4

add github_installation_id to owner

436d276

current tests have been updated to pass, but a specific one for the new service needs to be completed

carlad changed the title ~~gh app~~ [WIP] gh app Mar 29, 2018

Carla Drago and others added 14 commits March 29, 2018 19:06

comment out breaking test

84822b9

use different migrations branch

6eca455

Merge branch 'cd-gh-app' of github.com:travis-ci/travis-api into cd-g…

60f5a10

…h-app

try pg 9.6

b112d8c

Merge branch 'master' into cd-gh-app

a7ce6a4

remove unnecessary test

ae9b303

update to installations

a2c7e79

use correct migrations branch

24b3433

add test for github_installation_id

37be6f3

add renderer

754282f

update specs for github_installation_id

161b8ec

add owner/github_installation_id endpoint

c14a023

add check for type

6995096

change to github_id

a4414f3

rkh suggested changes Apr 5, 2018

View reviewed changes

carlad added 3 commits April 6, 2018 11:18

remove github_installation_id attribute on owner

231f59c

add installation service end endpoint

8193673

remove redundant attributes on user and org

a225d3f

rkh suggested changes Apr 6, 2018

View reviewed changes

rkh reviewed Apr 6, 2018

View reviewed changes

lib/travis/api/v3/queries/installation.rb Outdated

params :id

def find

return Models::Installation.where(github_id: id) if id

This comment was marked as spam.

Sign in to view

carlad added 3 commits April 6, 2018 13:08

add access control, update param, add renderer expansion check

3aeac04

update to :owner, add and update spec

346a28d

revert to managed_by_installation

505f403

carlad deployed to com-staging April 6, 2018 14:00 Active

add installation to repo renderer

1316eae

carlad deployed to com-staging April 6, 2018 14:12 Active

add association to user and org

820acb1

carlad deployed to com-staging April 6, 2018 14:23 Active

rkh added 3 commits April 6, 2018 17:20

allow overriding the travis-migrations branch being used with an envi…

606c0f5

…ronment variable

minor clean-ups

2ebcde1

v3: allow filtering repositoires by managed_by_installation

4398b3c

rkh deployed to com-staging April 6, 2018 15:40 Active

set travis migrations branch for ci

321d73d

rkh deployed to com-staging April 6, 2018 16:25 Active

Owner#installation: has_one is not wokring properly

5e97ac1

rkh force-pushed the cd-gh-app branch from 5d4564f to 5e97ac1 Compare April 6, 2018 16:33

rkh deployed to com-staging April 6, 2018 16:34 Active

carlad commented Apr 9, 2018

View reviewed changes

lib/travis/api/v3/models/organization.rb Outdated

@@ -7,6 +7,12 @@ def repositories

Models::Repository.where(owner_type: 'Organization', owner_id: id)

end

# has_one :installation, as: :owner

This comment was marked as spam.

Sign in to view

carlad commented Apr 9, 2018

View reviewed changes

lib/travis/api/v3/models/user.rb Outdated

@@ -34,5 +34,10 @@ def permission?(roles, options = {})

scope.any?

end

# has_one :installation, as: :owner

This comment was marked as spam.

Sign in to view

carlad commented Apr 9, 2018

View reviewed changes

spec/v3/services/installation/find_spec.rb Outdated

}}

end

describe "authenticated as user with access" do

This comment was marked as spam.

Sign in to view

Carla Drago and others added 2 commits April 9, 2018 12:24

remove comments, update description

9a62dcc

Merge branch 'master' into cd-gh-app

e3eefce

rkh approved these changes Apr 9, 2018

View reviewed changes

rkh added 2 commits April 9, 2018 14:19

Merge branch 'master' into cd-gh-app

572e712

Revert "set travis migrations branch for ci"

8437e07

This reverts commit 321d73d.

carlad changed the title ~~[WIP] gh app~~ gh app Apr 9, 2018

carlad merged commit 2cef2bd into master Apr 9, 2018

rkh deleted the cd-gh-app branch April 9, 2018 12:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gh app #723

gh app #723

carlad commented Mar 29, 2018 •

edited

rkh left a comment

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

rkh left a comment

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

rkh commented Apr 6, 2018

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

gh app #723

gh app #723

Conversation

carlad commented Mar 29, 2018 • edited

rkh left a comment

Choose a reason for hiding this comment

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

rkh left a comment

Choose a reason for hiding this comment

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

rkh commented Apr 6, 2018

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

carlad commented Mar 29, 2018 •

edited