Refactoring GitHub Repos functionality - step 1 (#7764)
* Rename find_or_create to upsert and improve validation and testing * Add User.authenticated_through? * Refactor settings/integrations * Refactor profile github repositories rendering * Refactor repos fetching * Only fetch a single repo and improve error messages * Remove unused code * Cleanups * Fix specs * Remove trailing whitespace * Fix spec * Trigger Travis
This commit is contained in:
parent
d10888bfa5
commit
81fff248dd
26 changed files with 339 additions and 267 deletions
|
|
@ -29,7 +29,7 @@ $input-width: 650px;
|
|||
background: #a8f1e9;
|
||||
}
|
||||
}
|
||||
&.github-repo-row-selected {
|
||||
&.github-repo-row-featured {
|
||||
background: $light-green;
|
||||
color: #0f0f0f;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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).
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -25,7 +25,27 @@ exports[`<SingleRepo /> when it is a fork should render and match the snapshot 1
|
|||
</div>
|
||||
`;
|
||||
|
||||
exports[`<SingleRepo /> when it is not already selected should render and match the snapshot 1`] = `
|
||||
exports[`<SingleRepo /> when it is featured should render and match the snapshot 1`] = `
|
||||
<div
|
||||
class="github-repo-row github-repo-row-featured"
|
||||
>
|
||||
<div
|
||||
class="github-repo-row-name"
|
||||
>
|
||||
<button
|
||||
class="cta"
|
||||
id="github-repo-button-123"
|
||||
onClick={[Function]}
|
||||
type="button"
|
||||
>
|
||||
REMOVE
|
||||
</button>
|
||||
dev.to
|
||||
</div>
|
||||
</div>
|
||||
`;
|
||||
|
||||
exports[`<SingleRepo /> when it is not already featured should render and match the snapshot 1`] = `
|
||||
<div
|
||||
class="github-repo-row"
|
||||
>
|
||||
|
|
@ -44,23 +64,3 @@ exports[`<SingleRepo /> when it is not already selected should render and match
|
|||
</div>
|
||||
</div>
|
||||
`;
|
||||
|
||||
exports[`<SingleRepo /> when it is selected should render and match the snapshot 1`] = `
|
||||
<div
|
||||
class="github-repo-row github-repo-row-selected"
|
||||
>
|
||||
<div
|
||||
class="github-repo-row-name"
|
||||
>
|
||||
<button
|
||||
class="cta"
|
||||
id="github-repo-button-123"
|
||||
onClick={[Function]}
|
||||
type="button"
|
||||
>
|
||||
REMOVE
|
||||
</button>
|
||||
dev.to
|
||||
</div>
|
||||
</div>
|
||||
`;
|
||||
|
|
|
|||
|
|
@ -7,13 +7,13 @@ import { SingleRepo } from '../singleRepo';
|
|||
global.fetch = fetch;
|
||||
|
||||
describe('<SingleRepo />', () => {
|
||||
describe('when it is not already selected', () => {
|
||||
describe('when it is not already featured', () => {
|
||||
const subject = (
|
||||
<SingleRepo
|
||||
githubIdCode={123}
|
||||
name="dev.to"
|
||||
fork={false}
|
||||
selected={false}
|
||||
featured={false}
|
||||
/>
|
||||
);
|
||||
|
||||
|
|
@ -22,30 +22,30 @@ describe('<SingleRepo />', () => {
|
|||
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 = (
|
||||
<SingleRepo githubIdCode={123} name="dev.to" fork={false} selected />
|
||||
<SingleRepo githubIdCode={123} name="dev.to" fork={false} featured />
|
||||
);
|
||||
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 = (
|
||||
<SingleRepo githubIdCode={123} name="dev.to" fork selected={false} />
|
||||
<SingleRepo githubIdCode={123} name="dev.to" fork featured={false} />
|
||||
);
|
||||
|
||||
it('should render and match the snapshot', () => {
|
||||
|
|
|
|||
|
|
@ -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) => (
|
||||
<SingleRepo
|
||||
githubIdCode={repo.github_id_code}
|
||||
name={repo.name}
|
||||
fork={repo.fork}
|
||||
selected={repo.selected}
|
||||
featured={repo.featured}
|
||||
/>
|
||||
));
|
||||
|
||||
|
|
@ -55,4 +55,4 @@ export class GithubRepos extends Component {
|
|||
}
|
||||
}
|
||||
|
||||
GithubRepos.displayName = 'GitHub Repos Wrapper';
|
||||
GithubRepos.displayName = 'GitHub Repositories Wrapper';
|
||||
|
|
|
|||
|
|
@ -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 (
|
||||
<div className={this.githubRepoClassName()}>
|
||||
|
|
@ -69,7 +69,7 @@ export class SingleRepo extends Component {
|
|||
id={`github-repo-button-${githubIdCode}`}
|
||||
onClick={this.submitRepo}
|
||||
>
|
||||
{selected ? 'REMOVE' : 'SELECT'}
|
||||
{featured ? 'REMOVE' : 'SELECT'}
|
||||
</button>
|
||||
{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,
|
||||
};
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
21
app/views/users/_github_repositories_area.html.erb
Normal file
21
app/views/users/_github_repositories_area.html.erb
Normal file
|
|
@ -0,0 +1,21 @@
|
|||
<% repositories.each do |repo| %>
|
||||
<a class="widget" href="<%= repo.url %>" target="_blank" rel="noopener">
|
||||
<header>
|
||||
<h4><span class="emoji"><img src="<%= asset_path("github-logo.svg") %>" alt="github logo"></span><%= repo.name %></h4>
|
||||
</header>
|
||||
<div class="widget-body">
|
||||
<% if repo.description.present? %>
|
||||
<%= EmojiConverter.call(repo.description) %>
|
||||
<% end %>
|
||||
<div class="widget-footer">
|
||||
<% if repo.fork %>
|
||||
<span class="widget-accent">fork</span>
|
||||
<% end %>
|
||||
<%= repo.language %>
|
||||
<% if repo.stargazers_count.to_i > 0 %>
|
||||
<img src="<%= asset_path("star.svg") %>" alt="star icon"> <%= repo.stargazers_count %>
|
||||
<% end %>
|
||||
</div>
|
||||
</div>
|
||||
</a>
|
||||
<% end %>
|
||||
|
|
@ -1,13 +1,4 @@
|
|||
<div class="crayons-card grid gap-6 p-6 mb-6">
|
||||
<h3>Pin a few of your GitHub repos to your profile</h3>
|
||||
<% if @client %>
|
||||
<div id="github-repos-container">
|
||||
<div class="github-repos loading-repos">
|
||||
</div>
|
||||
</div>
|
||||
<%= javascript_packs_with_chunks_tag "githubRepos", defer: true %>
|
||||
<% end %>
|
||||
</div>
|
||||
<%= render partial: "users/integrations_github_repositories", locals: { show_integration: @github_repositories_show } %>
|
||||
|
||||
<div class="crayons-card grid gap-6 p-6 mb-6">
|
||||
<h3>Generate a personal blog from your <%= community_name %> posts</h3>
|
||||
|
|
|
|||
12
app/views/users/_integrations_github_repositories.html.erb
Normal file
12
app/views/users/_integrations_github_repositories.html.erb
Normal file
|
|
@ -0,0 +1,12 @@
|
|||
<% if show_integration %>
|
||||
<div class="crayons-card grid gap-6 p-6 mb-6">
|
||||
<h3>Pin a few of your GitHub repositories to your profile</h3>
|
||||
|
||||
<div id="github-repos-container">
|
||||
<div class="github-repos loading-repos">
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<%= javascript_packs_with_chunks_tag "githubRepos", defer: true %>
|
||||
</div>
|
||||
<% end %>
|
||||
|
|
@ -8,7 +8,7 @@
|
|||
<div class="github-repos-container -mt-4">
|
||||
<div class="github-repos">
|
||||
<% @response_templates.each do |response_template| %>
|
||||
<div class="github-repo-row <%= "github-repo-row-selected" if response_template.id == params[:id].to_i %>">
|
||||
<div class="github-repo-row <%= "github-repo-row-featured" if response_template.id == params[:id].to_i %>">
|
||||
<div class="github-repo-row-name flex items-center crayons-card crayons-card--secondary p-2 pl-4 mt-2">
|
||||
<div class="flex-1">
|
||||
<%= response_template.title %>
|
||||
|
|
|
|||
|
|
@ -1,28 +1,10 @@
|
|||
<div id="sidebar-wrapper-right" class="sidebar-wrapper sidebar-wrapper-right">
|
||||
<div class="sidebar-bg" id="sidebar-bg-right"></div>
|
||||
<div class="sidebar-bg" id="sidebar-bg-right">
|
||||
</div>
|
||||
|
||||
<div class="side-bar sidebar-additional showing" id="sidebar-additional">
|
||||
<%= render "users/organizations_area" %>
|
||||
<% @user.github_repos.where(featured: true).order(stargazers_count: :desc, name: :asc).each do |repo| %>
|
||||
<a class="widget" href="<%= repo.url %>" target="_blank" rel="noopener">
|
||||
<header>
|
||||
<h4><span class="emoji"><img src="<%= asset_path("github-logo.svg") %>" alt="github logo"></span><%= repo.name %></h4>
|
||||
</header>
|
||||
<div class="widget-body">
|
||||
<% if repo.description.present? %>
|
||||
<%= EmojiConverter.call(repo.description) %>
|
||||
<% end %>
|
||||
<div class="widget-footer">
|
||||
<% if repo.fork %>
|
||||
<span class="widget-accent">fork</span>
|
||||
<% end %>
|
||||
<%= repo.language %>
|
||||
<% if repo.stargazers_count.to_i > 0 %>
|
||||
<img src="<%= asset_path("star.svg") %>" alt="star icon"> <%= repo.stargazers_count %>
|
||||
<% end %>
|
||||
</div>
|
||||
</div>
|
||||
</a>
|
||||
<% end %>
|
||||
<%= render partial: "users/github_repositories_area", locals: { repositories: repositories } %>
|
||||
<%= render "articles/badges_area" %>
|
||||
</div>
|
||||
</div>
|
||||
|
|
|
|||
|
|
@ -206,7 +206,7 @@
|
|||
</div>
|
||||
</div>
|
||||
<% 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 %>
|
||||
</div>
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue