diff --git a/app/controllers/github_repos_controller.rb b/app/controllers/github_repos_controller.rb index 2efbe5e97..e5d3084ba 100644 --- a/app/controllers/github_repos_controller.rb +++ b/app/controllers/github_repos_controller.rb @@ -1,14 +1,23 @@ class GithubReposController < ApplicationController def create @client = create_octokit_client - @repo = GithubRepo.find_or_create(github_repo_params[:url], fetched_repo_params) - redirect_to "/settings/integrations", notice: "GitHub repo added" + @repo = GithubRepo.find_or_create(fetched_repo_params) + 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]) - @repo.update(featured: false) - redirect_to "/settings/integrations", notice: "GitHub repo added" + 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 end private diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 58c020bc5..0dfb697a5 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -29,14 +29,14 @@ class UsersController < ApplicationController @user = current_user @tab_list = tab_list(@user) @tab = params["user"]["tab"] || "profile" - if @user.update(user_params) - RssReader.new.delay.fetch_user(@user) if @user.feed_url.present? - notice = "Your profile was successfully updated." - follow_hiring_tag(@user) - redirect_to "/settings/#{@tab}", notice: notice - else - render :edit - end + if @user.update(user_params) + RssReader.new.delay.fetch_user(@user) if @user.feed_url.present? + notice = "Your profile was successfully updated." + follow_hiring_tag(@user) + redirect_to "/settings/#{@tab}", notice: notice + else + render :edit + end end def onboarding_update diff --git a/app/models/github_repo.rb b/app/models/github_repo.rb index e63710992..3f7895402 100644 --- a/app/models/github_repo.rb +++ b/app/models/github_repo.rb @@ -1,31 +1,37 @@ class GithubRepo < ApplicationRecord belongs_to :user - validates :name, :url, presence: true + validates :name, :url, :github_id_code, presence: true validates :url, uniqueness: true validates :github_id_code, uniqueness: true after_save :clear_caches before_destroy :clear_caches - def self.find_or_create(github_url, params = {}) - repo = where(url: github_url).first_or_initialize + def self.find_or_create(params) + repo = where(github_id_code: params[:github_id_code]).or(where(url: params[:url])). + first_or_initialize repo.update(params) repo end def self.update_to_latest + # TODO: this is a very intensive process. Definitely not a good approach on the long run. where("updated_at < ?", 1.day.ago).find_each do |repo| user_token = User.find_by_id(repo.user_id).identities.where(provider: "github").last.token client = Octokit::Client.new(access_token: user_token) - fetched_repo = client.repositories.select do |fresh_repo| - if repo[:github_id_code] - fresh_repo.id == repo[:github_id_code] - else - fresh_repo.html_url == repo[:url] - end - end.first - repo.update( + + fetched_repo = if repo[:github_id_code] + client.repositories.select do |fresh_repo| + fresh_repo.id == repo[:github_id_code] + end.first + else + client.repositories.select do |fresh_repo| + fresh_repo.html_url == repo[:url] + end.first + end + + repo.update!( github_id_code: fetched_repo.id, name: fetched_repo.name, description: fetched_repo.description, diff --git a/app/views/users/_integrations.html.erb b/app/views/users/_integrations.html.erb index fad2060e8..978e07d64 100644 --- a/app/views/users/_integrations.html.erb +++ b/app/views/users/_integrations.html.erb @@ -2,7 +2,7 @@ <% if @client %>
<% @client.repositories.each do |repo| %> - <% if (user_repo = GithubRepo.find_by_name(repo.name)) && user_repo.featured %> + <% if (user_repo = GithubRepo.find_by(github_id_code: repo.id)) && user_repo.featured %>
<%= repo.name %> <%= "fork".html_safe if repo.fork %> diff --git a/spec/labor/markdown_fixer_spec.rb b/spec/labor/markdown_fixer_spec.rb new file mode 100644 index 000000000..652f50db6 --- /dev/null +++ b/spec/labor/markdown_fixer_spec.rb @@ -0,0 +1,39 @@ +require "rails_helper" + +RSpec.describe MarkdownFixer do + let(:sample_title) { Faker::Book.title } + + def create_sample_markdown(title) + <<~HEREDOC + --- + title: #{title} + --- + HEREDOC + end + + describe "::add_quotes_to_title" do + it "escapes simple title" do + test = described_class.fix_all(create_sample_markdown(sample_title)) + expect(test).to eq create_sample_markdown(%("#{sample_title}")) + end + + it "does not escape titles that came pre-wrapped in single quotes" do + legacy_title = "'#{sample_title}'" + test = described_class.fix_all(create_sample_markdown(legacy_title)) + expect(test).to eq create_sample_markdown(legacy_title) + end + + it "does not escape titles that came pre-wrapped in double quotes" do + legacy_title = "\"#{sample_title}\"" + test = described_class.fix_all(create_sample_markdown(legacy_title)) + expect(test).to eq create_sample_markdown(legacy_title) + end + + it "handles complex title" do + legacy_title = %(Book review: "#{sample_title}", part 1 I'm #deep) + expected_title = "\"Book review: \\\"#{sample_title}\\\", part 1 I'm #deep\"" + test = described_class.fix_all(create_sample_markdown(legacy_title)) + expect(test).to eq create_sample_markdown(expected_title) + end + end +end diff --git a/spec/models/github_repo_spec.rb b/spec/models/github_repo_spec.rb index ac446023f..cdc214cae 100644 --- a/spec/models/github_repo_spec.rb +++ b/spec/models/github_repo_spec.rb @@ -1,14 +1,14 @@ require "rails_helper" RSpec.describe GithubRepo, type: :model do + let(:user) { create(:user) } + let(:repo) { build(:github_repo, user_id: user.id) } + 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) } - let(:user) { create(:user) } - let(:repo) { build(:github_repo, user_id: user.id) } - it "saves" do repo.save expect(repo).to be_valid @@ -16,24 +16,30 @@ RSpec.describe GithubRepo, type: :model do 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 } - described_class.find_or_create("#{Faker::Internet.url}1", params) + 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) end it "returns existing object if it already exists" do repo.save before_update_id = repo.id - params = { name: Faker::Book.title } - updated_repo = described_class.find_or_create(repo.url, params) + 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) end end describe "::update_to_latest" do let(:my_ocktokit_client) { instance_double(Octokit::Client) } + let(:url_of_repos_without_github_id) { Faker::Internet.url } + let(:repo_without_github_id) do + create(:github_repo, user_id: user.id, url: url_of_repos_without_github_id) + end let(:stubbed_github_repos) do - [OpenStruct.new(repo.attributes.merge(id: repo.github_id_code, html_url: repo.url))] + [OpenStruct.new(repo.attributes.merge(id: repo.github_id_code, html_url: repo.url)), + OpenStruct.new(repo.attributes.merge(id: rand(10000), html_url: url_of_repos_without_github_id))] end before do @@ -46,15 +52,16 @@ RSpec.describe GithubRepo, type: :model do old_updated_at = repo.updated_at Timecop.freeze(Date.today + 3) do described_class.update_to_latest - expect(old_updated_at).not_to eq(GithubRepo.first.updated_at) + expect(old_updated_at).not_to eq(GithubRepo.find(repo.id).updated_at) end end it "uses repo's url as reference if id isn't provided" do - repo.update(github_id_code: nil) + repo_without_github_id.update_columns(github_id_code: nil) + old_updated_at = repo_without_github_id.updated_at Timecop.freeze(Date.today + 3) do described_class.update_to_latest - expect(repo.updated_at).not_to eq(GithubRepo.first.updated_at) + expect(old_updated_at).not_to eq(GithubRepo.find(repo_without_github_id.id).updated_at) end end end diff --git a/spec/requests/github_repos_spec.rb b/spec/requests/github_repos_spec.rb index b72af73fe..91ebe94f2 100644 --- a/spec/requests/github_repos_spec.rb +++ b/spec/requests/github_repos_spec.rb @@ -5,7 +5,7 @@ RSpec.describe "GithubRepos", type: :request do let(:repo) { build(:github_repo, user_id: user.id) } let(:my_ocktokit_client) { instance_double(Octokit::Client) } let(:stubbed_github_repos) do - [OpenStruct.new(repo.attributes.merge(id: repo.github_id_code, html_url: repo.url))] + [OpenStruct.new(repo.attributes.merge(id: repo.github_id_code, html_url: Faker::Internet.url))] end before do