diff --git a/app/assets/stylesheets/settings.scss b/app/assets/stylesheets/settings.scss index 8adf83f1e..4a9e6d2e8 100755 --- a/app/assets/stylesheets/settings.scss +++ b/app/assets/stylesheets/settings.scss @@ -29,7 +29,7 @@ $input-width: 650px; background: #a8f1e9; } } - &.github-repo-row-selected { + &.github-repo-row-featured { background: $light-green; color: #0f0f0f; } diff --git a/app/controllers/github_repos_controller.rb b/app/controllers/github_repos_controller.rb index 115d5e12d..3b63005f6 100644 --- a/app/controllers/github_repos_controller.rb +++ b/app/controllers/github_repos_controller.rb @@ -5,77 +5,65 @@ class GithubReposController < ApplicationController def index authorize GithubRepo - client = create_octokit_client + known_repositories_ids = current_user.github_repos.featured.distinct.pluck(:github_id_code) - existing_user_repos = current_user.github_repos.where(featured: true). - distinct.pluck(:github_id_code) - - @repos = client.repositories.map do |repo| - repo.selected = existing_user_repos.include?(repo.id) - repo - end + # NOTE: this will invoke autopaging, by issuing multiple calls to GitHub + # to fetch all of the user's repositories. This could eventually become slow + @repos = fetch_repositories_from_github(known_repositories_ids) rescue Octokit::Unauthorized => e - render json: { error: "Github Unauthorized: #{e.message}", status: 401 }, status: :unauthorized - end - - def create - authorize GithubRepo - @repo = GithubRepo.find_or_create(fetched_repo_params(fetch_repo)) - current_user.touch(:github_repos_updated_at) - if @repo.valid? - redirect_to "/settings/integrations", notice: "GitHub repo added" - else - redirect_to "/settings/integrations", - error: "There was an error adding your Github repo" - end - end - - def update - @repo = GithubRepo.find(params[:id]) - current_user.touch(:github_repos_updated_at) - authorize @repo - if @repo.update(featured: false) - redirect_to "/settings/integrations", notice: "GitHub repo added" - else - redirect_to "/settings/integrations", - error: "There was an error removing your Github repo" - end + render json: { error: "GitHub Unauthorized: #{e.message}", status: 401 }, status: :unauthorized end def update_or_create authorize GithubRepo params[:github_repo] = JSON.parse(params[:github_repo]) - fetched_repo = fetch_repo + + fetched_repo = fetch_repository_from_github(repo_params[:github_id_code]) unless fetched_repo - render json: "error: Could not find Github repo", status: :not_found + render json: { error: "GitHub repository not found", status: 404 }, status: :not_found return end - repo = GithubRepo.find_or_create(fetched_repo_params(fetched_repo)) + repo = GithubRepo.upsert(current_user, fetched_repo_params(fetched_repo)) current_user.touch(:github_repos_updated_at) if repo.valid? render json: { featured: repo.featured } else - render json: "error: #{repo.errors.full_messages}" + render json: { error: repo.errors.full_messages, status: 422 }, status: :unprocessable_entity end end private + # TODO: use Github::UserClient or something def create_octokit_client current_user_token = current_user.identities.where(provider: "github").last.token - client = Octokit::Client.new(access_token: current_user_token) - client&.repositories&.sort_by!(&:name) - client + Octokit::Client.new(access_token: current_user_token) + end + + def fetch_repositories_from_github(known_repositories_ids) + client = create_octokit_client + + client.repositories(visibility: :public).map do |repo| + repo.featured = known_repositories_ids.include?(repo.id) + repo + end.sort_by(&:name) + end + + def fetch_repository_from_github(repository_id) + client = create_octokit_client + + client.repository(repository_id) + rescue Octokit::NotFound + nil end def fetched_repo_params(fetched_repo) { github_id_code: fetched_repo.id, - user_id: current_user.id, name: fetched_repo.name, description: fetched_repo.description, language: fetched_repo.language, @@ -89,14 +77,6 @@ class GithubReposController < ApplicationController } end - def fetch_repo - client = create_octokit_client - - client.repositories.detect do |repo| - repo.id == repo_params[:github_id_code].to_i - end - end - def repo_params permitted_attributes(GithubRepo) end diff --git a/app/controllers/stories_controller.rb b/app/controllers/stories_controller.rb index 59c339fc2..dcd3cebb3 100644 --- a/app/controllers/stories_controller.rb +++ b/app/controllers/stories_controller.rb @@ -207,8 +207,11 @@ class StoriesController < ApplicationController redirect_if_view_param return if performed? + assign_user_github_repositories + set_surrogate_key_header "articles-user-#{@user.id}" set_user_json_ld + render template: "users/show" end @@ -310,6 +313,10 @@ class StoriesController < ApplicationController order("published_at DESC").page(@page).per(user_signed_in? ? 2 : SIGNED_OUT_RECORD_COUNT)) end + def assign_user_github_repositories + @github_repositories = @user.github_repos.featured.order(stargazers_count: :desc, name: :asc) + end + def stories_by_timeframe if %w[week month year infinity].include?(params[:timeframe]) @stories.where("published_at > ?", Timeframer.new(params[:timeframe]).datetime). diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 36349ad15..412e2cd84 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -315,10 +315,7 @@ class UsersController < ApplicationController end def handle_integrations_tab - return unless current_user.identities.where(provider: "github").any? - - @client = Octokit::Client. - new(access_token: current_user.identities.where(provider: "github").last.token) + @github_repositories_show = current_user.authenticated_through?(:github) end def handle_billing_tab diff --git a/app/javascript/githubRepos/__tests__/__snapshots__/singleRepo.test.jsx.snap b/app/javascript/githubRepos/__tests__/__snapshots__/singleRepo.test.jsx.snap index 31e6ca280..8e08e3959 100644 --- a/app/javascript/githubRepos/__tests__/__snapshots__/singleRepo.test.jsx.snap +++ b/app/javascript/githubRepos/__tests__/__snapshots__/singleRepo.test.jsx.snap @@ -25,7 +25,27 @@ exports[` when it is a fork should render and match the snapshot 1 `; -exports[` when it is not already selected should render and match the snapshot 1`] = ` +exports[` when it is featured should render and match the snapshot 1`] = ` + +`; + +exports[` when it is not already featured should render and match the snapshot 1`] = `
@@ -44,23 +64,3 @@ exports[` when it is not already selected should render and match
`; - -exports[` when it is selected should render and match the snapshot 1`] = ` -
-
- - dev.to -
-
-`; diff --git a/app/javascript/githubRepos/__tests__/singleRepo.test.jsx b/app/javascript/githubRepos/__tests__/singleRepo.test.jsx index c8445ccfb..54dfa38da 100644 --- a/app/javascript/githubRepos/__tests__/singleRepo.test.jsx +++ b/app/javascript/githubRepos/__tests__/singleRepo.test.jsx @@ -7,13 +7,13 @@ import { SingleRepo } from '../singleRepo'; global.fetch = fetch; describe('', () => { - describe('when it is not already selected', () => { + describe('when it is not already featured', () => { const subject = ( ); @@ -22,30 +22,30 @@ describe('', () => { expect(tree).toMatchSnapshot(); }); - it('should have a state of { selected: false }', () => { + it('should have a state of { featured: false }', () => { const context = shallow(subject); - expect(context.state()).toEqual({ selected: false }); + expect(context.state()).toEqual({ featured: false }); }); }); - describe('when it is selected', () => { + describe('when it is featured', () => { const subject = ( - + ); it('should render and match the snapshot', () => { const tree = render(subject); expect(tree).toMatchSnapshot(); }); - it('should have a state of { selected: true }', () => { + it('should have a state of { featured: true }', () => { const context = shallow(subject); - expect(context.state()).toEqual({ selected: true }); + expect(context.state()).toEqual({ featured: true }); }); }); describe('when it is a fork', () => { const subject = ( - + ); it('should render and match the snapshot', () => { diff --git a/app/javascript/githubRepos/githubRepos.jsx b/app/javascript/githubRepos/githubRepos.jsx index e4e1cd27a..a9cb67b03 100644 --- a/app/javascript/githubRepos/githubRepos.jsx +++ b/app/javascript/githubRepos/githubRepos.jsx @@ -19,8 +19,8 @@ export class GithubRepos extends Component { }, credentials: 'same-origin', }) - .then(response => response.json()) - .then(json => { + .then((response) => response.json()) + .then((json) => { this.setState({ repos: json }); }) .catch(() => { @@ -30,12 +30,12 @@ export class GithubRepos extends Component { render() { const { repos, erroredOut } = this.state; - const allRepos = repos.map(repo => ( + const allRepos = repos.map((repo) => ( )); @@ -55,4 +55,4 @@ export class GithubRepos extends Component { } } -GithubRepos.displayName = 'GitHub Repos Wrapper'; +GithubRepos.displayName = 'GitHub Repositories Wrapper'; diff --git a/app/javascript/githubRepos/singleRepo.jsx b/app/javascript/githubRepos/singleRepo.jsx index 9034f0403..2ec1dba48 100644 --- a/app/javascript/githubRepos/singleRepo.jsx +++ b/app/javascript/githubRepos/singleRepo.jsx @@ -4,8 +4,8 @@ import PropTypes from 'prop-types'; export class SingleRepo extends Component { constructor(props) { super(props); - const { selected } = this.props; - this.state = { selected }; + const { featured } = this.props; + this.state = { featured }; } forkLabel = () => { @@ -17,7 +17,7 @@ export class SingleRepo extends Component { }; submitRepo = () => { - const { selected } = this.state; + const { featured } = this.state; const { githubIdCode } = this.props; const submitButton = document.getElementById( @@ -30,7 +30,7 @@ export class SingleRepo extends Component { const formData = new FormData(); const formAttributes = { github_id_code: githubIdCode, - featured: !selected, + featured: !featured, }; formData.append('github_repo', JSON.stringify(formAttributes)); @@ -42,23 +42,23 @@ export class SingleRepo extends Component { body: formData, credentials: 'same-origin', }) - .then(response => response.json()) - .then(json => { + .then((response) => response.json()) + .then((json) => { submitButton.disabled = false; - this.setState({ selected: json.featured }); + this.setState({ featured: json.featured }); }); }; githubRepoClassName = () => { - const { selected } = this.state; - if (selected) { - return 'github-repo-row github-repo-row-selected'; + const { featured } = this.state; + if (featured) { + return 'github-repo-row github-repo-row-featured'; } return 'github-repo-row'; }; render() { - const { selected } = this.state; + const { featured } = this.state; const { name, githubIdCode } = this.props; return (
@@ -69,7 +69,7 @@ export class SingleRepo extends Component { id={`github-repo-button-${githubIdCode}`} onClick={this.submitRepo} > - {selected ? 'REMOVE' : 'SELECT'} + {featured ? 'REMOVE' : 'SELECT'} {name} {this.forkLabel()} @@ -85,5 +85,5 @@ SingleRepo.propTypes = { name: PropTypes.string.isRequired, githubIdCode: PropTypes.number.isRequired, fork: PropTypes.bool.isRequired, - selected: PropTypes.bool.isRequired, + featured: PropTypes.bool.isRequired, }; diff --git a/app/models/github_repo.rb b/app/models/github_repo.rb index 3b0c52b7f..84a7c6652 100644 --- a/app/models/github_repo.rb +++ b/app/models/github_repo.rb @@ -2,18 +2,27 @@ class GithubRepo < ApplicationRecord belongs_to :user serialize :info_hash, Hash + validates :name, :url, :github_id_code, presence: true - validates :url, uniqueness: true + validates :url, url: true, uniqueness: true validates :github_id_code, uniqueness: true + scope :featured, -> { where(featured: true) } + after_save :clear_caches before_destroy :clear_caches - def self.find_or_create(params) - repo = where(github_id_code: params[:github_id_code]). + # Update existing repository or create a new one with given params. + # Repository is searched by either GitHub ID or URL. + def self.upsert(user, **params) + repo = user.github_repos. + where(github_id_code: params[:github_id_code]). or(where(url: params[:url])). - first_or_initialize + first + repo ||= new(params.merge(user_id: user.id)) + repo.update(params) + repo end diff --git a/app/models/user.rb b/app/models/user.rb index cc332913d..d49cee617 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -451,6 +451,13 @@ class User < ApplicationRecord search_score end + def authenticated_through?(provider_name) + return false unless Authentication::Providers.available?(provider_name) + return false unless Authentication::Providers.enabled?(provider_name) + + identities.exists?(provider: provider_name) + end + def rate_limiter RateLimitChecker.new(self) end diff --git a/app/policies/github_repo_policy.rb b/app/policies/github_repo_policy.rb index 79a585ace..f51f93052 100644 --- a/app/policies/github_repo_policy.rb +++ b/app/policies/github_repo_policy.rb @@ -1,27 +1,13 @@ class GithubRepoPolicy < ApplicationPolicy def index? - !user_is_banned? - end - - def create? - !user_is_banned? - end - - def update? - !user_is_banned? && user_is_owner? + !user_is_banned? && user.authenticated_through?(:github) end def update_or_create? - !user_is_banned? + !user_is_banned? && user.authenticated_through?(:github) end def permitted_attributes %i[github_id_code featured] end - - private - - def user_is_owner? - record.user_id == user.id - end end diff --git a/app/views/github_repos/index.json.jbuilder b/app/views/github_repos/index.json.jbuilder index 043125863..fc6c9cdb9 100644 --- a/app/views/github_repos/index.json.jbuilder +++ b/app/views/github_repos/index.json.jbuilder @@ -1,5 +1,5 @@ json.array! @repos.each do |repo| json.github_id_code repo.id - json.extract!(repo, :name, :fork, :selected) + json.extract!(repo, :name, :fork, :featured) end diff --git a/app/views/users/_github_repositories_area.html.erb b/app/views/users/_github_repositories_area.html.erb new file mode 100644 index 000000000..753e55664 --- /dev/null +++ b/app/views/users/_github_repositories_area.html.erb @@ -0,0 +1,21 @@ +<% repositories.each do |repo| %> + +
+

" alt="github logo"><%= repo.name %>

+
+
+ <% if repo.description.present? %> + <%= EmojiConverter.call(repo.description) %> + <% end %> + +
+
+<% end %> diff --git a/app/views/users/_integrations.html.erb b/app/views/users/_integrations.html.erb index e82890499..6d4eac73a 100644 --- a/app/views/users/_integrations.html.erb +++ b/app/views/users/_integrations.html.erb @@ -1,13 +1,4 @@ -
-

Pin a few of your GitHub repos to your profile

- <% if @client %> -
-
-
-
- <%= javascript_packs_with_chunks_tag "githubRepos", defer: true %> - <% end %> -
+<%= render partial: "users/integrations_github_repositories", locals: { show_integration: @github_repositories_show } %>

Generate a personal blog from your <%= community_name %> posts

diff --git a/app/views/users/_integrations_github_repositories.html.erb b/app/views/users/_integrations_github_repositories.html.erb new file mode 100644 index 000000000..a04209b81 --- /dev/null +++ b/app/views/users/_integrations_github_repositories.html.erb @@ -0,0 +1,12 @@ +<% if show_integration %> +
+

Pin a few of your GitHub repositories to your profile

+ +
+
+
+
+ + <%= javascript_packs_with_chunks_tag "githubRepos", defer: true %> +
+<% end %> diff --git a/app/views/users/_response_templates.html.erb b/app/views/users/_response_templates.html.erb index 67d380de5..72ac69d5b 100644 --- a/app/views/users/_response_templates.html.erb +++ b/app/views/users/_response_templates.html.erb @@ -8,7 +8,7 @@
<% @response_templates.each do |response_template| %> -
"> +
">
<%= response_template.title %> diff --git a/app/views/users/_sidebar_additional.html.erb b/app/views/users/_sidebar_additional.html.erb index ca60e4743..d52d6ae97 100644 --- a/app/views/users/_sidebar_additional.html.erb +++ b/app/views/users/_sidebar_additional.html.erb @@ -1,28 +1,10 @@ diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb index 459941bec..f3f22aa55 100644 --- a/app/views/users/show.html.erb +++ b/app/views/users/show.html.erb @@ -206,7 +206,7 @@
<% cache "user-profile-sidebar-additional-#{@user.id}-#{@user.github_repos_updated_at}-#{@user.badge_achievements_count}-#{@user.organization_info_updated_at}", expires_in: 2.days do %> - <%= render "users/sidebar_additional" %> + <%= render partial: "users/sidebar_additional", locals: { repositories: @github_repositories } %> <% end %>
diff --git a/config/routes.rb b/config/routes.rb index 62b23b9ef..794feee34 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -195,7 +195,7 @@ Rails.application.routes.draw do end resources :stripe_active_cards, only: %i[create update destroy] resources :live_articles, only: [:index] - resources :github_repos, only: %i[index create update] do + resources :github_repos, only: %i[index] do collection do post "/update_or_create", to: "github_repos#update_or_create" end diff --git a/spec/models/github_repo_spec.rb b/spec/models/github_repo_spec.rb index 5f4c83a2b..b9cac7d5b 100644 --- a/spec/models/github_repo_spec.rb +++ b/spec/models/github_repo_spec.rb @@ -2,34 +2,79 @@ require "rails_helper" RSpec.describe GithubRepo, type: :model do let(:user) { create(:user, :with_identity, identities: ["github"]) } - let(:repo) { build(:github_repo, user_id: user.id) } + let(:repo) { create(:github_repo, user: user) } - before { omniauth_mock_github_payload } - - it { is_expected.to validate_presence_of(:name) } - it { is_expected.to validate_presence_of(:url) } - it { is_expected.to validate_uniqueness_of(:url) } - it { is_expected.to validate_uniqueness_of(:github_id_code) } - - it "saves" do - repo.save - expect(repo).to be_valid + before do + omniauth_mock_github_payload end - describe "::find_or_create" do - it "creates a new object if one doesn't already exists" do - params = { name: Faker::Book.title, user_id: user.id, github_id_code: rand(1000), - url: Faker::Internet.url } - described_class.find_or_create(params) - expect(described_class.count).to eq(1) + describe "validations" do + describe "builtin validations" do + subject { repo } + + it { is_expected.to validate_presence_of(:github_id_code) } + it { is_expected.to validate_presence_of(:name) } + it { is_expected.to validate_presence_of(:url) } + it { is_expected.to validate_uniqueness_of(:github_id_code) } + it { is_expected.to validate_uniqueness_of(:url) } + it { is_expected.to validate_url_of(:url) } + end + end + + context "when callbacks are triggered after save" do + let(:repo) { build(:github_repo, user: user) } + + describe "clearing caches" do + it "updates the user's updated_at" do + old_updated_at = user.updated_at + + Timecop.travel(1.minute.from_now) do + repo.save + end + + expect(user.reload.updated_at.to_i > old_updated_at.to_i).to be(true) + end + + it "busts the correct caches" do + allow(CacheBuster).to receive(:bust) + + repo.save + + expect(CacheBuster).to have_received(:bust).with(user.path) + expect(CacheBuster).to have_received(:bust).with("#{user.path}?i=i") + expect(CacheBuster).to have_received(:bust).with("#{user.path}/?i=i") + end + end + end + + describe ".upsert" do + let(:params) do + { + github_id_code: rand(1000), + name: Faker::Book.title, + url: Faker::Internet.url + } end - it "returns existing object if it already exists" do - repo.save - before_update_id = repo.id - params = { github_id_code: repo.github_id_code } - updated_repo = described_class.find_or_create(params) - expect(updated_repo.id).to eq(before_update_id) + it "creates a new repo" do + expect do + described_class.upsert(user, params) + end.to change(described_class, :count).by(1) + end + + it "creates a repo for the given user" do + repo = described_class.upsert(user, params) + + expect(repo.user_id).to eq(user.id) + end + + it "returns an existing repo updated with new params" do + new_name = Faker::Book.title + + new_repo = described_class.upsert(user, url: repo.url, name: new_name) + + expect(new_repo.id).to eq(repo.id) + expect(repo.reload.name).to eq(new_name) end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index b80be8fbf..f251f9877 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1123,4 +1123,28 @@ RSpec.describe User, type: :model do expect(described_class.mascot_account).to eq(user) end end + + describe "#authenticated_through?" do + let(:provider) { Authentication::Providers.available.first } + + it "returns false if provider is not known" do + expect(user.authenticated_through?(:unknown)).to be(false) + end + + it "returns false if provider is not enabled" do + providers = Authentication::Providers.available - [provider] + allow(Authentication::Providers).to receive(:enabled).and_return(providers) + + expect(user.authenticated_through?(provider)).to be(false) + end + + it "returns false if the user has no related identity" do + expect(user.authenticated_through?(provider)).to be(false) + end + + it "returns true if the user has related identity" do + user = create(:user, :with_identity, identities: [provider]) + expect(user.authenticated_through?(provider)).to be(true) + end + end end diff --git a/spec/policies/github_repo_policy_spec.rb b/spec/policies/github_repo_policy_spec.rb index eae4204f8..097222d79 100644 --- a/spec/policies/github_repo_policy_spec.rb +++ b/spec/policies/github_repo_policy_spec.rb @@ -3,37 +3,35 @@ require "rails_helper" RSpec.describe GithubRepoPolicy, type: :policy do subject { described_class.new(user, github_repo) } - let_it_be(:github_repo) { create(:github_repo) } - let(:valid_attributes) { %i[github_id_code] } - context "when user is not signed in" do let(:user) { nil } + let(:github_repo) { build(:github_repo, user: user) } it { within_block_is_expected.to raise_error(Pundit::NotAuthorizedError) } end - context "when user is not the owner" do - let!(:user) { create(:user) } + context "when the user is not authenticated through GitHub" do + let(:user) { build(:user) } + let(:github_repo) { build(:github_repo, user: user) } - it { is_expected.to permit_actions(%i[create]) } - it { is_expected.to forbid_actions(%i[update]) } - - context "when user is banned" do - before { user.add_role(:banned) } - - it { is_expected.to forbid_actions(%i[create update]) } - end + it { is_expected.to forbid_actions(%i[index update_or_create]) } end - context "when user is the owner" do - let(:user) { github_repo.user } + context "when the user is authenticated through GitHub" do + let(:user) { create(:user, :with_identity, identities: %i[github]) } + let(:github_repo) { build(:github_repo, user: user) } - it { is_expected.to permit_actions(%i[create update]) } - - context "when user is banned" do - let(:user) { build(:user, :banned) } - - it { is_expected.to forbid_actions(%i[create update]) } + before do + omniauth_mock_github_payload end + + it { is_expected.to permit_actions(%i[index update_or_create]) } + end + + context "when user is banned" do + let(:user) { build(:user, :banned) } + let(:github_repo) { build(:github_repo, user: user) } + + it { is_expected.to forbid_actions(%i[index update_or_create]) } end end diff --git a/spec/requests/github_repos_spec.rb b/spec/requests/github_repos_spec.rb index 22ae06456..14499fe91 100644 --- a/spec/requests/github_repos_spec.rb +++ b/spec/requests/github_repos_spec.rb @@ -2,13 +2,14 @@ require "rails_helper" RSpec.describe "GithubRepos", type: :request do let(:user) { create(:user, :with_identity, identities: ["github"]) } - let(:repo) { build(:github_repo, user_id: user.id) } + let(:repo) { build(:github_repo, user: user) } let(:my_octokit_client) { instance_double(Octokit::Client) } let(:stubbed_github_repos) do repo_params = repo.attributes.merge( id: repo.github_id_code, html_url: Faker::Internet.url, ) + [OpenStruct.new(repo_params)] end let(:headers) do @@ -20,82 +21,58 @@ RSpec.describe "GithubRepos", type: :request do before do omniauth_mock_github_payload + allow(Octokit::Client).to receive(:new).and_return(my_octokit_client) allow(my_octokit_client).to receive(:repositories) { stubbed_github_repos } + allow(my_octokit_client).to receive(:repository) { stubbed_github_repos.first } end describe "GET /github_repos" do context "when user is unauthorized" do - it "returns unauthorized" do + it "returns unauthorized if the user is not signed in" do get github_repos_path, headers: headers expect(response).to have_http_status(:unauthorized) end + + it "returns unauthorized if the user not has authenticated through GitHub" do + user = create(:user) + sign_in user + + expect do + get github_repos_path, headers: headers + end.to raise_error(Pundit::NotAuthorizedError) + end end context "when user is authorized" do before { sign_in user } + it "returns unauthorized if the user is not authorized to perform the GitHub API call" do + allow(Octokit::Client).to receive(:new).and_raise(Octokit::Unauthorized) + + get github_repos_path, headers: headers + expect(response).to have_http_status(:unauthorized) + expect(response.parsed_body["error"]).to include("GitHub Unauthorized") + end + it "returns 200 on success" do get github_repos_path, headers: headers expect(response).to have_http_status(:ok) end - it "returns 401 if github raises an unauthorized error" do - allow(Octokit::Client).to receive(:new).and_raise(Octokit::Unauthorized) - - get github_repos_path, headers: headers - expect(response).to have_http_status(:unauthorized) - expect(response.parsed_body["error"]).to include("Github Unauthorized") - end - - it "returns repos with the correct json representation" 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["selected"]).to be(false) + expect(response_repo["featured"]).to be(false) end end end - describe "POST /github_repos" do - before { sign_in user } - - it "returns a 302" do - params = { github_repo: { github_id_code: repo.github_id_code } } - post github_repos_path, params: params - - expect(response).to have_http_status(:found) - end - - it "creates a new GithubRepo object" do - params = { github_repo: { github_id_code: repo.github_id_code } } - expect do - post github_repos_path, params: params - end.to change(GithubRepo, :count).by(1) - end - end - - describe "PUT /github_repos/:id" do - before do - repo.save - sign_in user - end - - it "returns a 302" do - put "/github_repos/#{repo.id}" - expect(response).to have_http_status(:found) - end - - it "unfeatures the requested GithubRepo" do - put "/github_repos/#{repo.id}" - expect(GithubRepo.first.featured).to eq(false) - end - end - describe "POST /github_repos/update_or_create" do before { sign_in user } @@ -109,13 +86,12 @@ RSpec.describe "GithubRepos", type: :request do expect(response.content_type).to eq("application/json") end - it "returns 404 and json response on error" do - allow(my_octokit_client).to receive(:repositories).and_return([]) + it "returns 404 if no repository is found" do + allow(my_octokit_client).to receive(:repository).and_raise(Octokit::NotFound) - params = { github_repo: "{}" } + params = { github_repo: github_repo.to_json } post update_or_create_github_repos_path(params), headers: headers expect(response).to have_http_status(:not_found) - expect(response.body).to include("Could not find Github repo") end it "updates the current user github_repos_updated_at" do @@ -124,7 +100,7 @@ RSpec.describe "GithubRepos", type: :request do Timecop.travel(5.minutes.from_now) do params = { github_repo: github_repo.to_json } post update_or_create_github_repos_path(params), headers: headers - expect(user.reload.github_repos_updated_at > previous_date).to be(true) + expect(user.reload.github_repos_updated_at.to_i > previous_date.to_i).to be(true) end end diff --git a/spec/requests/internal/negative_reactions_spec.rb b/spec/requests/internal/negative_reactions_spec.rb index d5e7a1509..f19bc5903 100644 --- a/spec/requests/internal/negative_reactions_spec.rb +++ b/spec/requests/internal/negative_reactions_spec.rb @@ -46,7 +46,7 @@ RSpec.describe "/internal/negative_reactions", type: :request do get "/internal/negative_reactions" expect(response.body).to include(moderator.username) expect(response.body).to include(user_reaction.reactable.username) - expect(response.body).to include(article_reaction.reactable.title) + expect(response.body).to include(CGI.escapeHTML(article_reaction.reactable.title)) end end end diff --git a/spec/requests/user/user_profile_spec.rb b/spec/requests/user/user_profile_spec.rb index 95166c225..22ef2a221 100644 --- a/spec/requests/user/user_profile_spec.rb +++ b/spec/requests/user/user_profile_spec.rb @@ -128,20 +128,40 @@ RSpec.describe "UserProfiles", type: :request do end end - context "when github repo" do - before do - repo = build(:github_repo, user: user) - params = { name: Faker::Book.title, user_id: user.id, github_id_code: repo.github_id_code, - url: Faker::Internet.url, description: "A book bot :robot:", featured: true, - stargazers_count: 1 } - updated_repo = GithubRepo.find_or_create(params) - - user.github_repos = [updated_repo] + context "when displaying a GitHub repository on the profile" do + let(:github_user) { create(:user, :with_identity, identities: %i[github]) } + let(:params) do + { + description: "A book bot :robot:", + featured: true, + github_id_code: build(:github_repo).github_id_code, + name: Faker::Book.title, + stargazers_count: 1, + url: Faker::Internet.url + } end - it "renders emoji in description of pinned github repo" do - get "/#{user.username}" - expect(response.body).to include "A book bot 🤖" + before do + omniauth_mock_github_payload + end + + it "renders emoji in description of featured repository" do + GithubRepo.upsert(github_user, params) + + get "/#{github_user.username}" + expect(response.body).to include("A book bot 🤖") + end + + it "does not show a non featured repository" do + GithubRepo.upsert(github_user, params.merge(featured: false)) + + get "/#{github_user.username}" + expect(response.body).not_to include("A book bot 🤖") + end + + it "does not render anything if the user has not authenticated through GitHub" do + get "/#{github_user.username}" + expect(response.body).not_to include("github-repos-container") end end end diff --git a/spec/requests/user/user_settings_spec.rb b/spec/requests/user/user_settings_spec.rb index 7b2771c76..ef18fefef 100644 --- a/spec/requests/user/user_settings_spec.rb +++ b/spec/requests/user/user_settings_spec.rb @@ -150,6 +150,23 @@ RSpec.describe "UserSettings", type: :request do expect(response.body).to include("Connect GitHub Account") end end + + describe ":integrations" do + it "renders the repositories container if the user has authenticated through GitHub" do + user = create(:user, :with_identity, identities: [:github]) + sign_in user + + get user_settings_path(tab: :integrations) + expect(response.body).to include("github-repos-container") + end + + it "does not render anything if the user has not authenticated through GitHub" do + sign_in user + + get user_settings_path(tab: :integrations) + expect(response.body).not_to include("github-repos-container") + end + end end describe "PUT /update/:id" do