diff --git a/app/controllers/github_repos_controller.rb b/app/controllers/github_repos_controller.rb index 854e6526b..042890d65 100644 --- a/app/controllers/github_repos_controller.rb +++ b/app/controllers/github_repos_controller.rb @@ -41,7 +41,10 @@ class GithubReposController < ApplicationController client = Github::OauthClient.for_user(current_user) repos = client.repositories(visibility: :public).map do |repo| - repo.featured = known_repositories.delete_if { |known| known.github_id_code == repo.id }.present? + if (known_index = known_repositories.find_index { |known| known.github_id_code == repo.id }) + repo.featured = true + known_repositories.delete_at(known_index) + end repo end diff --git a/spec/requests/github_repos_spec.rb b/spec/requests/github_repos_spec.rb index 974b891bc..29c87749b 100644 --- a/spec/requests/github_repos_spec.rb +++ b/spec/requests/github_repos_spec.rb @@ -9,14 +9,21 @@ RSpec.describe "GithubRepos", type: :request do end end let(:user) { create(:user, :with_identity, identities: ["github"]) } - let(:repo) { build(:github_repo, user: user) } + # Using explicit name attributes soordering in controller is stable + let(:repo1) { build(:github_repo, user: user, name: "A", featured: false) } + let(:repo2) { build(:github_repo, user: user, name: "B", featured: true) } let(:stubbed_github_repos) do - repo_params = repo.attributes.merge( - id: repo.github_id_code, + repo1_params = repo1.attributes.merge( + id: repo1.github_id_code, html_url: Faker::Internet.url, ) - [OpenStruct.new(repo_params)] + repo2_params = repo2.attributes.merge( + id: repo2.github_id_code, + html_url: Faker::Internet.url, + ) + + [OpenStruct.new(repo1_params), OpenStruct.new(repo2_params)] end let(:github_client) do instance_double( @@ -76,10 +83,13 @@ RSpec.describe "GithubRepos", type: :request do it "returns repositories with the correct JSON representation" do get github_repos_path, headers: headers - response_repo = response.parsed_body.first - expect(response_repo["name"]).to eq(repo.name) - expect(response_repo["fork"]).to eq(repo.fork) - expect(response_repo["featured"]).to be(false) + response_repo1, response_repo2 = response.parsed_body + expect(response_repo1["name"]).to eq(repo1.name) + expect(response_repo1["fork"]).to eq(repo1.fork) + expect(response_repo1["featured"]).to be(false) + expect(response_repo2["name"]).to eq(repo2.name) + expect(response_repo2["fork"]).to eq(repo2.fork) + expect(response_repo2["featured"]).to be(true) end it "deletes repositories which are no longer publicly accessible" do