From 990ac190950dfba813d3fb1f9a7d96c68c9aeb09 Mon Sep 17 00:00:00 2001 From: Vaughn Weiss Date: Fri, 17 Nov 2023 15:35:44 -0700 Subject: [PATCH 1/2] Install rubocop --- Gemfile | 1 + Gemfile.lock | 30 ++++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/Gemfile b/Gemfile index 813041b..bfcffa7 100644 --- a/Gemfile +++ b/Gemfile @@ -9,6 +9,7 @@ gem "pg", "~> 1.1" gem "puma", "~> 5.0" gem "rack-cors" gem "rails", "~> 7.0.8" +gem "rubocop-rails", require: false gem "tzinfo-data", platforms: %i[ mingw mswin x64_mingw jruby ] # Build JSON APIs with ease [https://github.com/rails/jbuilder] diff --git a/Gemfile.lock b/Gemfile.lock index 7ee3170..7f6c269 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -66,6 +66,7 @@ GEM i18n (>= 1.6, < 2) minitest (>= 5.1) tzinfo (~> 2.0) + ast (2.4.2) bootsnap (1.17.0) msgpack (~> 1.2) builder (3.2.4) @@ -97,6 +98,8 @@ GEM irb (1.9.0) rdoc reline (>= 0.3.8) + json (2.6.3) + language_server-protocol (3.17.0.3) loofah (2.22.0) crass (~> 1.0.2) nokogiri (>= 1.12.0) @@ -122,6 +125,10 @@ GEM nio4r (2.6.0) nokogiri (1.15.4-x86_64-darwin) racc (~> 1.4) + parallel (1.23.0) + parser (3.2.2.4) + ast (~> 2.4.1) + racc pg (1.5.4) psych (5.1.1.1) stringio @@ -161,11 +168,14 @@ GEM rake (>= 12.2) thor (~> 1.0) zeitwerk (~> 2.5) + rainbow (3.1.1) rake (13.1.0) rdoc (6.6.0) psych (>= 4.0.0) + regexp_parser (2.8.2) reline (0.4.0) io-console (~> 0.5) + rexml (3.2.6) rspec-core (3.12.2) rspec-support (~> 3.12.0) rspec-expectations (3.12.3) @@ -183,6 +193,24 @@ GEM rspec-mocks (~> 3.12) rspec-support (~> 3.12) rspec-support (3.12.1) + rubocop (1.57.2) + json (~> 2.3) + language_server-protocol (>= 3.17.0) + parallel (~> 1.10) + parser (>= 3.2.2.4) + rainbow (>= 2.2.2, < 4.0) + regexp_parser (>= 1.8, < 3.0) + rexml (>= 3.2.5, < 4.0) + rubocop-ast (>= 1.28.1, < 2.0) + ruby-progressbar (~> 1.7) + unicode-display_width (>= 2.4.0, < 3.0) + rubocop-ast (1.30.0) + parser (>= 3.2.1.0) + rubocop-rails (2.22.1) + activesupport (>= 4.2.0) + rack (>= 1.1) + rubocop (>= 1.33.0, < 2.0) + ruby-progressbar (1.13.0) shoulda-matchers (5.3.0) activesupport (>= 5.2.0) sprockets (4.2.1) @@ -197,6 +225,7 @@ GEM timeout (0.4.1) tzinfo (2.0.6) concurrent-ruby (~> 1.0) + unicode-display_width (2.5.0) websocket-driver (0.7.6) websocket-extensions (>= 0.1.0) websocket-extensions (0.1.5) @@ -217,6 +246,7 @@ DEPENDENCIES rack-cors rails (~> 7.0.8) rspec-rails (~> 6.0.0) + rubocop-rails shoulda-matchers (~> 5.0) tzinfo-data From 768944859d5e619a9bd8a9ebfb04458085773ade Mon Sep 17 00:00:00 2001 From: Vaughn Weiss Date: Fri, 17 Nov 2023 15:59:16 -0700 Subject: [PATCH 2/2] Add rubocop --- .../{rspec_tests.yml => workflow.yml} | 5 +- .rubocop.yml | 27 +++++++++++ Gemfile | 34 ++++++------- app/controllers/application_controller.rb | 2 + app/controllers/graphql_controller.rb | 5 +- app/graphql/types/member_type.rb | 2 +- app/graphql/types/mutation_type.rb | 12 ++--- app/graphql/types/query_type.rb | 19 ++++---- app/models/member.rb | 6 ++- spec/factories/member.rb | 2 + .../mutations/members/create_member_spec.rb | 40 ++++++++-------- spec/graphql/queries/member_spec.rb | 48 ++++++++++--------- spec/models/member_spec.rb | 4 +- spec/support/factory_bot.rb | 2 + 14 files changed, 128 insertions(+), 80 deletions(-) rename .github/workflows/{rspec_tests.yml => workflow.yml} (95%) create mode 100644 .rubocop.yml diff --git a/.github/workflows/rspec_tests.yml b/.github/workflows/workflow.yml similarity index 95% rename from .github/workflows/rspec_tests.yml rename to .github/workflows/workflow.yml index 1ad9276..1047aa7 100644 --- a/.github/workflows/rspec_tests.yml +++ b/.github/workflows/workflow.yml @@ -9,7 +9,7 @@ on: - main jobs: - rspec: + test: runs-on: ubuntu-latest services: @@ -49,6 +49,9 @@ jobs: bundle exec rails db:create bundle exec rails db:migrate + - name: Run Rubocop + run: rubocop + - name: Run RSpec env: RAILS_ENV: test diff --git a/.rubocop.yml b/.rubocop.yml new file mode 100644 index 0000000..45ff438 --- /dev/null +++ b/.rubocop.yml @@ -0,0 +1,27 @@ +require: rubocop-rails + +AllCops: + Exclude: + # App files + - "app/channels/application_cable/**/*" + - "app/graphql/kiln_backend_schema.rb" + - "app/jobs/application_job.rb" + - "app/mailers/application_mailer.rb" + - "app/models/application_record.rb" + - "bin/**/*" + - "config.ru" + - "config/**/*" + - "db/**/*" + - "Rakefile" + - "spec/rails_helper.rb" + - "spec/spec_helper.rb" + # Added files to review + - "app/controllers/graphql_controller.rb" + - "app/graphql/types/base_*" + +Documentation: + Enabled: false + +Metrics/BlockLength: + Exclude: + - "spec/**/*" diff --git a/Gemfile b/Gemfile index bfcffa7..27f498c 100644 --- a/Gemfile +++ b/Gemfile @@ -1,16 +1,18 @@ -source "https://rubygems.org" +# frozen_string_literal: true + +source 'https://rubygems.org' git_source(:github) { |repo| "https://github.com/#{repo}.git" } -ruby "3.2.1" +ruby '3.2.1' -gem "bootsnap", require: false -gem "graphql" -gem "pg", "~> 1.1" -gem "puma", "~> 5.0" -gem "rack-cors" -gem "rails", "~> 7.0.8" -gem "rubocop-rails", require: false -gem "tzinfo-data", platforms: %i[ mingw mswin x64_mingw jruby ] +gem 'bootsnap', require: false +gem 'graphql' +gem 'pg', '~> 1.1' +gem 'puma', '~> 5.0' +gem 'rack-cors' +gem 'rails', '~> 7.0.8' +gem 'rubocop-rails', require: false +gem 'tzinfo-data', platforms: %i[mingw mswin x64_mingw jruby] # Build JSON APIs with ease [https://github.com/rails/jbuilder] # gem "jbuilder" @@ -28,16 +30,16 @@ gem "tzinfo-data", platforms: %i[ mingw mswin x64_mingw jruby ] # gem "image_processing", "~> 1.2" group :development, :test do - gem "debug", platforms: %i[ mri mingw x64_mingw ] - gem "factory_bot_rails" - gem "faker" - gem "rspec-rails", "~> 6.0.0" + gem 'debug', platforms: %i[mri mingw x64_mingw] + gem 'factory_bot_rails' + gem 'faker' + gem 'rspec-rails', '~> 6.0.0' end group :development do - gem "graphiql-rails" + gem 'graphiql-rails' end group :test do - gem "shoulda-matchers", "~> 5.0" + gem 'shoulda-matchers', '~> 5.0' end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 4ac8823..13c271f 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -1,2 +1,4 @@ +# frozen_string_literal: true + class ApplicationController < ActionController::API end diff --git a/app/controllers/graphql_controller.rb b/app/controllers/graphql_controller.rb index 2d6884c..2e2ef51 100644 --- a/app/controllers/graphql_controller.rb +++ b/app/controllers/graphql_controller.rb @@ -14,10 +14,11 @@ def execute # Query context goes here, for example: # current_user: current_user, } - result = KilnBackendSchema.execute(query, variables: variables, context: context, operation_name: operation_name) + result = KilnBackendSchema.execute(query, variables:, context:, operation_name:) render json: result rescue StandardError => e raise e unless Rails.env.development? + handle_error_in_development(e) end @@ -47,6 +48,6 @@ def handle_error_in_development(e) logger.error e.message logger.error e.backtrace.join("\n") - render json: { errors: [{ message: e.message, backtrace: e.backtrace }], data: {} }, status: 500 + render json: { errors: [{ message: e.message, backtrace: e.backtrace }], data: {} }, status: :internal_server_error end end diff --git a/app/graphql/types/member_type.rb b/app/graphql/types/member_type.rb index 38e49c3..3f4420a 100644 --- a/app/graphql/types/member_type.rb +++ b/app/graphql/types/member_type.rb @@ -2,7 +2,7 @@ module Types class MemberType < Types::BaseObject - description "A Kiln Collective member" + description 'A Kiln Collective member' field :id, ID, null: false field :first_name, String diff --git a/app/graphql/types/mutation_type.rb b/app/graphql/types/mutation_type.rb index dfc0faf..dc74e7e 100644 --- a/app/graphql/types/mutation_type.rb +++ b/app/graphql/types/mutation_type.rb @@ -2,24 +2,24 @@ module Types class MutationType < Types::BaseObject - field :create_member, MemberType, null: true, description: "Create a new member" do + field :create_member, MemberType, null: true, description: 'Create a new member' do argument :first_name, String, required: true argument :last_name, String, required: true argument :title, String, required: true end def create_member(first_name:, last_name:, title:) Member.create( - first_name: first_name, - last_name: last_name, - title: title + first_name:, + last_name:, + title: ) end - field :delete_member, Boolean, null: false, description: "Delete a member permanently" do + field :delete_member, Boolean, null: false, description: 'Delete a member permanently' do argument :id, ID, required: true end def delete_member(id:) - member = Member.find_by(id: id) + member = Member.find_by(id:) return false unless member member.destroy diff --git a/app/graphql/types/query_type.rb b/app/graphql/types/query_type.rb index 660188e..6ee331e 100644 --- a/app/graphql/types/query_type.rb +++ b/app/graphql/types/query_type.rb @@ -2,16 +2,17 @@ module Types class QueryType < Types::BaseObject - field :node, Types::NodeType, null: true, description: "Fetches an object given its ID." do - argument :id, ID, required: true, description: "ID of the object." + field :node, Types::NodeType, null: true, description: 'Fetches an object given its ID.' do + argument :id, ID, required: true, description: 'ID of the object.' end def node(id:) context.schema.object_from_id(id, context) end - field :nodes, [Types::NodeType, null: true], null: true, description: "Fetches a list of objects given a list of IDs." do - argument :ids, [ID], required: true, description: "IDs of the objects." + field :nodes, [Types::NodeType, { null: true }], null: true, + description: 'Fetches a list of objects given a list of IDs.' do + argument :ids, [ID], required: true, description: 'IDs of the objects.' end def nodes(ids:) @@ -21,16 +22,16 @@ def nodes(ids:) # Add root-level fields here. # They will be entry points for queries on your schema. - field :members, [MemberType], null: false, description: "Fetches all members" + field :members, [MemberType], null: false, description: 'Fetches all members' def members - Member.all.order("first_name ASC") + Member.all.order('first_name ASC') end - field :member, MemberType, null: true, description: "Fetches a member by first_name" do - argument :first_name, String, required: true, description: "First name of the member" + field :member, MemberType, null: true, description: 'Fetches a member by first_name' do + argument :first_name, String, required: true, description: 'First name of the member' end def member(first_name:) - Member.find_by(first_name: first_name) + Member.find_by(first_name:) end end end diff --git a/app/models/member.rb b/app/models/member.rb index 5d83988..91a3e05 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -1,4 +1,6 @@ +# frozen_string_literal: true + class Member < ApplicationRecord - validates_presence_of :first_name - validates_presence_of :last_name + validates :first_name, presence: true + validates :last_name, presence: true end diff --git a/spec/factories/member.rb b/spec/factories/member.rb index 48fa65a..1e4b12d 100644 --- a/spec/factories/member.rb +++ b/spec/factories/member.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + FactoryBot.define do factory :member do first_name { Faker::Name.first_name } diff --git a/spec/graphql/mutations/members/create_member_spec.rb b/spec/graphql/mutations/members/create_member_spec.rb index 9444836..9a0ee92 100644 --- a/spec/graphql/mutations/members/create_member_spec.rb +++ b/spec/graphql/mutations/members/create_member_spec.rb @@ -1,9 +1,11 @@ -require "rails_helper" +# frozen_string_literal: true + +require 'rails_helper' module Mutations RSpec.describe Member, type: :request do - describe "Member create mutation" do - let(:mutation) { + describe 'Member create mutation' do + let(:mutation) do <<~GQL mutation { createMember( @@ -16,54 +18,54 @@ module Mutations } } GQL - } + end let(:result) { KilnBackendSchema.execute(mutation).as_json } - it "returns the created Member" do - data = result["data"]["createMember"] + it 'returns the created Member' do + data = result['data']['createMember'] expect(data).to include( { - "id" => be_present, - "firstName" => "Test" + 'id' => be_present, + 'firstName' => 'Test' } ) expect(Member.count).to be 1 end end - describe "Member delete mutation" do + describe 'Member delete mutation' do let!(:result) { KilnBackendSchema.execute(mutation).as_json } - context "when member exists" do + context 'when member exists' do let(:member) { create(:member) } - let(:mutation) { + let(:mutation) do <<~GQL mutation { deleteMember(id: "#{member.id}") } GQL - } + end - it "returns true" do - data = result["data"]["deleteMember"] + it 'returns true' do + data = result['data']['deleteMember'] expect(data).to be true expect(Member.count).to be 0 end end - context "when member does not exist" do - let(:mutation) { + context 'when member does not exist' do + let(:mutation) do <<~GQL mutation { deleteMember(id: "999") } GQL - } + end - it "returns false" do - expect(result["data"]["deleteMember"]).to be false + it 'returns false' do + expect(result['data']['deleteMember']).to be false end end end diff --git a/spec/graphql/queries/member_spec.rb b/spec/graphql/queries/member_spec.rb index f473bb4..e3ad5b3 100644 --- a/spec/graphql/queries/member_spec.rb +++ b/spec/graphql/queries/member_spec.rb @@ -1,10 +1,12 @@ -require "rails_helper" +# frozen_string_literal: true + +require 'rails_helper' module Queries RSpec.describe Member, type: :request do - describe "Member query" do + describe 'Member query' do let!(:member) { create(:member) } - let(:query) { + let(:query) do <<~GQL query { member(firstName: "#{member.first_name}") { @@ -15,26 +17,26 @@ module Queries } } GQL - } + end let(:result) { KilnBackendSchema.execute(query).as_json } - it "returns the expected Member" do - data = result["data"]["member"] + it 'returns the expected Member' do + data = result['data']['member'] expect(data).to include( - "id" => member.id.to_s, - "firstName" => member.first_name, - "lastName" => member.last_name, - "title" => member.title + 'id' => member.id.to_s, + 'firstName' => member.first_name, + 'lastName' => member.last_name, + 'title' => member.title ) end end end - describe "Members query" do + describe 'Members query' do let!(:members) { create_list(:member, 2) } - let(:query) { + let(:query) do <<~GQL query { members { @@ -45,25 +47,25 @@ module Queries } } GQL - } + end let(:result) { KilnBackendSchema.execute(query).as_json } - it "returns all Members" do - data = result["data"]["members"] + it 'returns all Members' do + data = result['data']['members'] expect(data).to include( { - "id" => members[0].id.to_s, - "firstName" => members[0].first_name, - "lastName" => members[0].last_name, - "title" => members[0].title + 'id' => members[0].id.to_s, + 'firstName' => members[0].first_name, + 'lastName' => members[0].last_name, + 'title' => members[0].title }, { - "id" => members[1].id.to_s, - "firstName" => members[1].first_name, - "lastName" => members[1].last_name, - "title" => members[1].title + 'id' => members[1].id.to_s, + 'firstName' => members[1].first_name, + 'lastName' => members[1].last_name, + 'title' => members[1].title } ) end diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index 1197c99..8891b40 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -1,7 +1,9 @@ +# frozen_string_literal: true + require 'rails_helper' RSpec.describe Member, type: :model do - describe "validations" do + describe 'validations' do it { should validate_presence_of(:first_name) } it { should validate_presence_of(:last_name) } end diff --git a/spec/support/factory_bot.rb b/spec/support/factory_bot.rb index c7890e4..2e7665c 100644 --- a/spec/support/factory_bot.rb +++ b/spec/support/factory_bot.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + RSpec.configure do |config| config.include FactoryBot::Syntax::Methods end