diff --git a/.rubocop.yml b/.rubocop.yml new file mode 100644 index 0000000..c389bc6 --- /dev/null +++ b/.rubocop.yml @@ -0,0 +1,35 @@ +inherit_from: .rubocop_todo.yml + +require: + - rubocop-capybara + - rubocop-graphql + - rubocop-minitest + - rubocop-performance + - rubocop-rails + +AllCops: + TargetRubyVersion: 3.2 + NewCops: enable + Exclude: + - "db/**/*" + - "config/**/*" + - "bin/**" + - 'node_modules/**/*' + - 'tmp/**/*' + - 'vendor/**/*' + - '.git/**/*' + - 'Gemfile' + - 'Rakefile' + - 'config.ru' + +Metrics/BlockLength: + Exclude: + - "test/**/*" + +Metrics/ClassLength: + Exclude: + - "test/**/*" + +Metrics/MethodLength: + Exclude: + - "test/**/*.rb" diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml new file mode 100644 index 0000000..dfecc96 --- /dev/null +++ b/.rubocop_todo.yml @@ -0,0 +1,126 @@ +# This configuration was generated by +# `rubocop --auto-gen-config` +# on 2024-08-12 20:10:38 UTC using RuboCop version 1.65.0. +# The point is for the user to remove these configuration records +# one by one as the offenses are removed from the code base. +# Note that changes in the inspected code, or installation of new +# versions of RuboCop, may require this file to be generated again. + +# Offense count: 3 +GraphQL/ArgumentDescription: + Exclude: + - 'app/graphql/types/query_type.rb' + +# Offense count: 28 +GraphQL/FieldDescription: + Exclude: + - 'app/graphql/types/details_type.rb' + - 'app/graphql/types/search_event_type.rb' + - 'app/graphql/types/standard_identifiers_type.rb' + - 'app/graphql/types/term_type.rb' + +# Offense count: 1 +# Configuration parameters: Include. +# Include: **/graphql/**/*_schema.rb +GraphQL/MaxComplexitySchema: + Exclude: + - 'app/graphql/tacos_schema.rb' + +# Offense count: 1 +# Configuration parameters: Include. +# Include: **/graphql/**/*_schema.rb +GraphQL/MaxDepthSchema: + Exclude: + - 'app/graphql/tacos_schema.rb' + +# Offense count: 7 +GraphQL/ObjectDescription: + Exclude: + - 'app/graphql/mutations/base_mutation.rb' + - 'app/graphql/tacos_schema.rb' + - 'app/graphql/types/base_argument.rb' + - 'app/graphql/types/base_connection.rb' + - 'app/graphql/types/base_edge.rb' + - 'app/graphql/types/base_enum.rb' + - 'app/graphql/types/base_field.rb' + - 'app/graphql/types/base_input_object.rb' + - 'app/graphql/types/base_interface.rb' + - 'app/graphql/types/base_object.rb' + - 'app/graphql/types/base_scalar.rb' + - 'app/graphql/types/base_union.rb' + - 'app/graphql/types/details_type.rb' + - 'app/graphql/types/mutation_type.rb' + - 'app/graphql/types/node_type.rb' + - 'app/graphql/types/query_type.rb' + - 'app/graphql/types/search_event_type.rb' + - 'app/graphql/types/standard_identifiers_type.rb' + - 'app/graphql/types/term_type.rb' + +# Offense count: 1 +Lint/DuplicateMethods: + Exclude: + - 'app/graphql/types/query_type.rb' + +# Offense count: 1 +# This cop supports unsafe autocorrection (--autocorrect-all). +# Configuration parameters: AutoCorrect. +Lint/UselessMethodDefinition: + Exclude: + - 'app/graphql/tacos_schema.rb' + +# Offense count: 1 +# Configuration parameters: AllowedMethods, AllowedPatterns, CountRepeatedAttributes. +Metrics/AbcSize: + Max: 18 + +# Offense count: 2 +# Configuration parameters: CountComments, CountAsOne, AllowedMethods, AllowedPatterns. +# AllowedMethods: refine +Metrics/BlockLength: + Max: 60 + +# Offense count: 4 +# Configuration parameters: CountComments, CountAsOne, AllowedMethods, AllowedPatterns. +Metrics/MethodLength: + Max: 16 + +# Offense count: 1 +Minitest/AssertRaisesCompoundBody: + Exclude: + - 'test/tasks/suggested_resource_rake_test.rb' + +# Offense count: 2 +Minitest/MultipleAssertions: + Max: 5 + +# Offense count: 6 +# Configuration parameters: EnforcedStyle, CheckMethodNames, CheckSymbols, AllowedIdentifiers, AllowedPatterns. +# SupportedStyles: snake_case, normalcase, non_integer +# AllowedIdentifiers: capture3, iso8601, rfc1123_date, rfc822, rfc2822, rfc3339, x86_64 +Naming/VariableNumber: + Exclude: + - 'test/models/metrics/algorithms_test.rb' + +# Offense count: 1 +# This cop supports safe autocorrection (--autocorrect). +Performance/StringReplacement: + Exclude: + - 'lib/tasks/suggested_resources.rake' + +# Offense count: 3 +Rails/I18nLocaleTexts: + Exclude: + - 'app/controllers/admin/application_controller.rb' + - 'app/controllers/application_controller.rb' + +# Offense count: 25 +# Configuration parameters: AllowedConstants. +Style/Documentation: + Enabled: false + +# Offense count: 6 +# This cop supports safe autocorrection (--autocorrect). +# Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, AllowedPatterns. +# URISchemes: http, https +Layout/LineLength: + Max: 310 diff --git a/Gemfile b/Gemfile index 39ba4b6..abc2c57 100644 --- a/Gemfile +++ b/Gemfile @@ -89,9 +89,12 @@ group :development do gem 'annotate' # RuboCop is a Ruby static code analyzer (a.k.a. linter) and code formatter. - gem 'rubocop' - gem 'rubocop-capybara' - gem 'rubocop-rails' + gem 'rubocop', require: false + gem 'rubocop-capybara', require: false + gem 'rubocop-graphql', require: false + gem 'rubocop-minitest', require: false + gem 'rubocop-performance', require: false + gem 'rubocop-rails', require: false # Use console on exceptions pages [https://github.com/rails/web-console] gem 'web-console' @@ -112,4 +115,4 @@ group :test do gem 'webmock' end -gem "administrate", "~> 0.20.1" +gem 'administrate', '~> 0.20.1' diff --git a/Gemfile.lock b/Gemfile.lock index edfc3aa..5fca592 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -358,6 +358,14 @@ GEM parser (>= 3.3.1.0) rubocop-capybara (2.21.0) rubocop (~> 1.41) + rubocop-graphql (1.5.4) + rubocop (>= 1.50, < 2) + rubocop-minitest (0.35.1) + rubocop (>= 1.61, < 2.0) + rubocop-ast (>= 1.31.1, < 2.0) + rubocop-performance (1.21.1) + rubocop (>= 1.48.1, < 2.0) + rubocop-ast (>= 1.31.1, < 2.0) rubocop-rails (2.25.1) activesupport (>= 4.2.0) rack (>= 1.1) @@ -477,6 +485,9 @@ DEPENDENCIES rails (~> 7.1.2) rubocop rubocop-capybara + rubocop-graphql + rubocop-minitest + rubocop-performance rubocop-rails selenium-webdriver simplecov diff --git a/Makefile b/Makefile index 050945f..ca21f57 100644 --- a/Makefile +++ b/Makefile @@ -45,4 +45,11 @@ outdated: # List outdated dependencies # Code quality and safety commands #################################### -# coming soon! +lint: + bundle exec rubocop + +lint-models: + bundle exec rubocop app/models + +lint-controllers: + bundle exec rubocop app/controllers diff --git a/app/channels/application_cable/channel.rb b/app/channels/application_cable/channel.rb index d672697..9aec230 100644 --- a/app/channels/application_cable/channel.rb +++ b/app/channels/application_cable/channel.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module ApplicationCable class Channel < ActionCable::Channel::Base end diff --git a/app/channels/application_cable/connection.rb b/app/channels/application_cable/connection.rb index 0ff5442..8d6c2a1 100644 --- a/app/channels/application_cable/connection.rb +++ b/app/channels/application_cable/connection.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module ApplicationCable class Connection < ActionCable::Connection::Base end diff --git a/app/controllers/admin/application_controller.rb b/app/controllers/admin/application_controller.rb index 8449926..dbc907e 100644 --- a/app/controllers/admin/application_controller.rb +++ b/app/controllers/admin/application_controller.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # All Administrate controllers inherit from this # `Administrate::ApplicationController`, making it the ideal place to put # authentication logic or other before_actions. @@ -13,7 +15,7 @@ class ApplicationController < Administrate::ApplicationController def authorize_user return if authorize_action?(resource_name, action_name) - + redirect_to root_path, alert: 'Not authorized' end @@ -23,7 +25,7 @@ def authorize_action?(resource, action) def require_user return if current_user - + redirect_to root_path, alert: 'Please sign in to continue' end diff --git a/app/controllers/admin/detector/suggested_resources_controller.rb b/app/controllers/admin/detector/suggested_resources_controller.rb index 9cebe16..9e794d9 100644 --- a/app/controllers/admin/detector/suggested_resources_controller.rb +++ b/app/controllers/admin/detector/suggested_resources_controller.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Admin module Detector class SuggestedResourcesController < Admin::ApplicationController @@ -8,7 +10,7 @@ class SuggestedResourcesController < Admin::ApplicationController # super # send_foo_updated_email(requested_resource) # end - + # Override this method to specify custom lookup behavior. # This will be used to set the resource for the `show`, `edit`, and `update` # actions. @@ -16,9 +18,9 @@ class SuggestedResourcesController < Admin::ApplicationController # def find_resource(param) # Foo.find_by!(slug: param) # end - + # The result of this lookup will be available as `requested_resource` - + # Override this if you have certain roles that require a subset # this will be used to set the records shown on the `index` action. # @@ -29,7 +31,7 @@ class SuggestedResourcesController < Admin::ApplicationController # resource_class.with_less_stuff # end # end - + # Override `resource_params` if you want to transform the submitted # data before it's persisted. For example, the following would turn all # empty values into nil values. It uses other APIs such as `resource_class` @@ -40,7 +42,7 @@ class SuggestedResourcesController < Admin::ApplicationController # permit(dashboard.permitted_attributes(action_name)). # transform_values { |value| value == "" ? nil : value } # end - + # See https://administrate-demo.herokuapp.com/customizing_controller_actions # for more information end diff --git a/app/controllers/admin/search_events_controller.rb b/app/controllers/admin/search_events_controller.rb index de79091..cd2b0de 100644 --- a/app/controllers/admin/search_events_controller.rb +++ b/app/controllers/admin/search_events_controller.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Admin class SearchEventsController < Admin::ApplicationController # Overwrite any of the RESTful controller actions to implement custom behavior diff --git a/app/controllers/admin/terms_controller.rb b/app/controllers/admin/terms_controller.rb index 9f2780c..778a926 100644 --- a/app/controllers/admin/terms_controller.rb +++ b/app/controllers/admin/terms_controller.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Admin class TermsController < Admin::ApplicationController # Overwrite any of the RESTful controller actions to implement custom behavior diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index 2c4ab7d..8465b41 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Admin class UsersController < Admin::ApplicationController # Overwrite any of the RESTful controller actions to implement custom behavior diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 4bb3e9c..41e9036 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class ApplicationController < ActionController::Base helper Mitlibraries::Theme::Engine.helpers diff --git a/app/controllers/graphql_controller.rb b/app/controllers/graphql_controller.rb index 923c431..951a568 100644 --- a/app/controllers/graphql_controller.rb +++ b/app/controllers/graphql_controller.rb @@ -44,10 +44,10 @@ def prepare_variables(variables_param) end end - def handle_error_in_development(e) - logger.error e.message - logger.error e.backtrace.join("\n") + def handle_error_in_development(error) + logger.error error.message + logger.error error.backtrace.join("\n") - render json: { errors: [{ message: e.message, backtrace: e.backtrace }], data: {} }, status: 500 + render json: { errors: [{ message: error.message, backtrace: error.backtrace }], data: {} }, status: :internal_server_error end end diff --git a/app/controllers/static_controller.rb b/app/controllers/static_controller.rb index 4d46687..c41a44b 100644 --- a/app/controllers/static_controller.rb +++ b/app/controllers/static_controller.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class StaticController < ApplicationController def index; end end diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb index d822729..a08cb73 100644 --- a/app/controllers/users/omniauth_callbacks_controller.rb +++ b/app/controllers/users/omniauth_callbacks_controller.rb @@ -1,21 +1,25 @@ +# frozen_string_literal: true + # Handles authentication response from Omniauth. See # [the Devise docs](https://www.rubydoc.info/gems/devise_token_auth/DeviseTokenAuth/OmniauthCallbacksController) for # additional information about this controller. -class Users::OmniauthCallbacksController < Devise::OmniauthCallbacksController - include FakeAuthConfig +module Users + class OmniauthCallbacksController < Devise::OmniauthCallbacksController + include FakeAuthConfig - def openid_connect - @user = User.from_omniauth(request.env['omniauth.auth']) - sign_in_and_redirect @user, event: :authentication - flash[:notice] = "Welcome, #{@user.email}!" - end + def openid_connect + @user = User.from_omniauth(request.env['omniauth.auth']) + sign_in_and_redirect @user, event: :authentication + flash[:notice] = "Welcome, #{@user.email}!" + end - # Developer authentication is used in local dev and PR builds. - def developer - raise 'Invalid Authentication' unless FakeAuthConfig.fake_auth_enabled? + # Developer authentication is used in local dev and PR builds. + def developer + raise 'Invalid Authentication' unless FakeAuthConfig.fake_auth_enabled? - @user = User.from_omniauth(request.env['omniauth.auth']) - sign_in_and_redirect @user, event: :authentication - flash[:notice] = "Welcome, #{@user.email}!" + @user = User.from_omniauth(request.env['omniauth.auth']) + sign_in_and_redirect @user, event: :authentication + flash[:notice] = "Welcome, #{@user.email}!" + end end end diff --git a/app/dashboards/detector/suggested_resource_dashboard.rb b/app/dashboards/detector/suggested_resource_dashboard.rb index 7fd3240..83c6f6a 100644 --- a/app/dashboards/detector/suggested_resource_dashboard.rb +++ b/app/dashboards/detector/suggested_resource_dashboard.rb @@ -1,7 +1,9 @@ -require "administrate/base_dashboard" +# frozen_string_literal: true + +require 'administrate/base_dashboard' module Detector - class Detector::SuggestedResourceDashboard < Administrate::BaseDashboard + class SuggestedResourceDashboard < Administrate::BaseDashboard # ATTRIBUTE_TYPES # a hash that describes the type of each of the model's fields. # @@ -15,9 +17,9 @@ class Detector::SuggestedResourceDashboard < Administrate::BaseDashboard title: Field::String, url: Field::String, created_at: Field::DateTime, - updated_at: Field::DateTime, + updated_at: Field::DateTime }.freeze - + # COLLECTION_ATTRIBUTES # an array of attributes that will be displayed on the model's index page. # @@ -29,7 +31,7 @@ class Detector::SuggestedResourceDashboard < Administrate::BaseDashboard phrase title ].freeze - + # SHOW_PAGE_ATTRIBUTES # an array of attributes that will be displayed on the model's show page. SHOW_PAGE_ATTRIBUTES = %i[ @@ -41,7 +43,7 @@ class Detector::SuggestedResourceDashboard < Administrate::BaseDashboard created_at updated_at ].freeze - + # FORM_ATTRIBUTES # an array of attributes that will be displayed # on the model's form (`new` and `edit`) pages. @@ -50,7 +52,7 @@ class Detector::SuggestedResourceDashboard < Administrate::BaseDashboard title url ].freeze - + # COLLECTION_FILTERS # a hash that defines filters that can be used while searching via the search # field of the dashboard. @@ -62,7 +64,7 @@ class Detector::SuggestedResourceDashboard < Administrate::BaseDashboard # open: ->(resources) { resources.where(open: true) } # }.freeze COLLECTION_FILTERS = {}.freeze - + # Overwrite this method to customize how suggested resources are displayed # across all pages of the admin dashboard. # diff --git a/app/dashboards/search_event_dashboard.rb b/app/dashboards/search_event_dashboard.rb index cd4e096..922d58a 100644 --- a/app/dashboards/search_event_dashboard.rb +++ b/app/dashboards/search_event_dashboard.rb @@ -1,4 +1,6 @@ -require "administrate/base_dashboard" +# frozen_string_literal: true + +require 'administrate/base_dashboard' class SearchEventDashboard < Administrate::BaseDashboard # ATTRIBUTE_TYPES @@ -12,7 +14,7 @@ class SearchEventDashboard < Administrate::BaseDashboard source: Field::String, term: Field::BelongsTo, created_at: Field::DateTime, - updated_at: Field::DateTime, + updated_at: Field::DateTime }.freeze # COLLECTION_ATTRIBUTES diff --git a/app/dashboards/term_dashboard.rb b/app/dashboards/term_dashboard.rb index b75dbff..c8d1090 100644 --- a/app/dashboards/term_dashboard.rb +++ b/app/dashboards/term_dashboard.rb @@ -1,4 +1,6 @@ -require "administrate/base_dashboard" +# frozen_string_literal: true + +require 'administrate/base_dashboard' class TermDashboard < Administrate::BaseDashboard # ATTRIBUTE_TYPES @@ -12,7 +14,7 @@ class TermDashboard < Administrate::BaseDashboard phrase: Field::String, search_events: Field::HasMany, created_at: Field::DateTime, - updated_at: Field::DateTime, + updated_at: Field::DateTime }.freeze # COLLECTION_ATTRIBUTES diff --git a/app/dashboards/user_dashboard.rb b/app/dashboards/user_dashboard.rb index 7fe9659..001d81e 100644 --- a/app/dashboards/user_dashboard.rb +++ b/app/dashboards/user_dashboard.rb @@ -1,4 +1,6 @@ -require "administrate/base_dashboard" +# frozen_string_literal: true + +require 'administrate/base_dashboard' class UserDashboard < Administrate::BaseDashboard # ATTRIBUTE_TYPES @@ -13,7 +15,7 @@ class UserDashboard < Administrate::BaseDashboard email: Field::String, uid: Field::String, created_at: Field::DateTime, - updated_at: Field::DateTime, + updated_at: Field::DateTime }.freeze # COLLECTION_ATTRIBUTES diff --git a/app/graphql/tacos_schema.rb b/app/graphql/tacos_schema.rb index c195e17..31e661c 100644 --- a/app/graphql/tacos_schema.rb +++ b/app/graphql/tacos_schema.rb @@ -16,7 +16,7 @@ def self.type_error(err, context) end # Union and Interface Resolution - def self.resolve_type(abstract_type, obj, ctx) + def self.resolve_type(_abstract_type, _obj, _ctx) # TODO: Implement this method # to return the correct GraphQL object type for `obj` raise(GraphQL::RequiredImplementationMissingError) @@ -28,13 +28,13 @@ def self.resolve_type(abstract_type, obj, ctx) # Relay-style Object Identification: # Return a string UUID for `object` - def self.id_from_object(object, type_definition, query_ctx) + def self.id_from_object(object, _type_definition, _query_ctx) # For example, use Rails' GlobalID library (https://github.com/rails/globalid): object.to_gid_param end # Given a string UUID, find the object - def self.object_from_id(global_id, query_ctx) + def self.object_from_id(global_id, _query_ctx) # For example, use Rails' GlobalID library (https://github.com/rails/globalid): GlobalID.find(global_id) end diff --git a/app/graphql/types/details_type.rb b/app/graphql/types/details_type.rb index b4d0a5a..32eb554 100644 --- a/app/graphql/types/details_type.rb +++ b/app/graphql/types/details_type.rb @@ -2,17 +2,17 @@ module Types class DetailsType < Types::BaseObject - field :title, String field :authors, [String] - field :date, String - field :publisher, String - field :oa, Boolean - field :oa_status, String field :best_oa_location, String + field :date, String + field :doi, String field :issns, [String] field :journal_name, String - field :doi, String field :link_resolver_url, String + field :oa, Boolean + field :oa_status, String + field :publisher, String + field :title, String def issns @object[:journal_issns]&.split(',') diff --git a/app/graphql/types/mutation_type.rb b/app/graphql/types/mutation_type.rb index 1138619..8bda052 100644 --- a/app/graphql/types/mutation_type.rb +++ b/app/graphql/types/mutation_type.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Types class MutationType < Types::BaseObject end diff --git a/app/graphql/types/query_type.rb b/app/graphql/types/query_type.rb index f77e73f..cd88fce 100644 --- a/app/graphql/types/query_type.rb +++ b/app/graphql/types/query_type.rb @@ -6,19 +6,11 @@ class QueryType < Types::BaseObject 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.' end - def nodes(ids:) - ids.map { |id| context.schema.object_from_id(id, context) } - end - # Add root-level fields here. # They will be entry points for queries on your schema. @@ -28,18 +20,30 @@ def nodes(ids:) argument :source_system, String, required: true end - def log_search_event(search_term:, source_system:) - term = Term.create_or_find_by!(phrase: search_term) - term.search_events.create!(source: source_system) - end - field :lookup_term, TermType, null: true, description: 'Lookup a term to return information about it (bypasses logging)' do argument :search_term, String, required: true end + def node(id:) + context.schema.object_from_id(id, context) + end + + def node(id:) + context.schema.object_from_id(id, context) + end + + def nodes(ids:) + ids.map { |id| context.schema.object_from_id(id, context) } + end + + def log_search_event(search_term:, source_system:) + term = Term.create_or_find_by!(phrase: search_term) + term.search_events.create!(source: source_system) + end + def lookup_term(search_term:) - term = Term.find_by(phrase: search_term) + Term.find_by(phrase: search_term) end end end diff --git a/app/graphql/types/search_event_type.rb b/app/graphql/types/search_event_type.rb index 437e9e3..d1d0d32 100644 --- a/app/graphql/types/search_event_type.rb +++ b/app/graphql/types/search_event_type.rb @@ -2,24 +2,22 @@ module Types class SearchEventType < Types::BaseObject - field :id, ID, null: false - field :term_id, Integer - field :source, String field :created_at, GraphQL::Types::ISO8601DateTime, null: false - field :updated_at, GraphQL::Types::ISO8601DateTime, null: false + field :id, ID, null: false field :phrase, String + field :source, String field :standard_identifiers, [StandardIdentifiersType] + field :term_id, Integer + field :updated_at, GraphQL::Types::ISO8601DateTime, null: false def phrase @object.term.phrase end def standard_identifiers - ids = [] - StandardIdentifiers.new(@object.term.phrase).identifiers.each do |identifier| - ids << { kind: identifier.first, value: identifier.last } + StandardIdentifiers.new(@object.term.phrase).identifiers.map do |identifier| + { kind: identifier.first, value: identifier.last } end - ids end end end diff --git a/app/graphql/types/standard_identifiers_type.rb b/app/graphql/types/standard_identifiers_type.rb index b9776d6..3d1d5d4 100644 --- a/app/graphql/types/standard_identifiers_type.rb +++ b/app/graphql/types/standard_identifiers_type.rb @@ -2,9 +2,9 @@ module Types class StandardIdentifiersType < Types::BaseObject + field :details, DetailsType field :kind, String, null: false field :value, String, null: false - field :details, DetailsType # details does external lookups and should only be run if the fields # have been explicitly requested diff --git a/app/graphql/types/term_type.rb b/app/graphql/types/term_type.rb index 742d1e3..28b5243 100644 --- a/app/graphql/types/term_type.rb +++ b/app/graphql/types/term_type.rb @@ -2,24 +2,22 @@ module Types class TermType < Types::BaseObject - field :id, ID, null: false field :created_at, GraphQL::Types::ISO8601DateTime, null: false - field :updated_at, GraphQL::Types::ISO8601DateTime, null: false - field :phrase, String, null: false + field :id, ID, null: false field :occurence_count, Integer + field :phrase, String, null: false field :search_events, [SearchEventType], null: false field :standard_identifiers, [StandardIdentifiersType] + field :updated_at, GraphQL::Types::ISO8601DateTime, null: false def occurence_count @object.search_events.count end def standard_identifiers - ids = [] - StandardIdentifiers.new(@object.phrase).identifiers.each do |identifier| - ids << { kind: identifier.first, value: identifier.last } + StandardIdentifiers.new(@object.phrase).identifiers.map do |identifier| + { kind: identifier.first, value: identifier.last } end - ids end end end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index de6be79..15b06f0 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -1,2 +1,4 @@ +# frozen_string_literal: true + module ApplicationHelper end diff --git a/app/jobs/application_job.rb b/app/jobs/application_job.rb index d394c3d..bef3959 100644 --- a/app/jobs/application_job.rb +++ b/app/jobs/application_job.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class ApplicationJob < ActiveJob::Base # Automatically retry jobs that encountered a deadlock # retry_on ActiveRecord::Deadlocked diff --git a/app/mailers/application_mailer.rb b/app/mailers/application_mailer.rb index 3c34c81..d84cb6e 100644 --- a/app/mailers/application_mailer.rb +++ b/app/mailers/application_mailer.rb @@ -1,4 +1,6 @@ +# frozen_string_literal: true + class ApplicationMailer < ActionMailer::Base - default from: "from@example.com" - layout "mailer" + default from: 'from@example.com' + layout 'mailer' end diff --git a/app/models/ability.rb b/app/models/ability.rb index 407b086..998253a 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -7,10 +7,11 @@ class Ability # See the wiki for details: # https://github.com/CanCanCommunity/cancancan/blob/develop/docs/define_check_abilities.md def initialize(user) - return unless user.present? + return if user.blank? # Rules will go here. return unless user.admin? + can :manage, :all end end diff --git a/app/models/concerns/fake_auth_config.rb b/app/models/concerns/fake_auth_config.rb index eeda3fb..809c7f0 100644 --- a/app/models/concerns/fake_auth_config.rb +++ b/app/models/concerns/fake_auth_config.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module FakeAuthConfig # Used in an initializer to determine if the application is configured and allowed to use fake authentication. def self.fake_auth_enabled? diff --git a/app/models/detector/journal.rb b/app/models/detector/journal.rb index 61962ae..ebcf50b 100644 --- a/app/models/detector/journal.rb +++ b/app/models/detector/journal.rb @@ -39,7 +39,7 @@ def self.full_term_match(phrase) # # @return [Set of Detector::Journal] A set of ActiveRecord Detector::Journal relations. def self.partial_term_match(phrase) - Journal.all.map { |journal| journal if phrase.downcase.include?(journal.name) }.compact + Journal.all.select { |journal| phrase.downcase.include?(journal.name) } end private diff --git a/app/models/lookup_doi.rb b/app/models/lookup_doi.rb index bf41e55..7e8eba1 100644 --- a/app/models/lookup_doi.rb +++ b/app/models/lookup_doi.rb @@ -38,8 +38,10 @@ def fetch(doi) if resp.status == 200 JSON.parse(resp.to_s) else - Rails.logger.debug("Fact lookup error. DOI #{doi} detected but unpaywall returned no data or otherwise errored") - Rails.logger.debug("URL: #{url(doi)}") + Rails.logger.debug do + "Fact lookup error. DOI #{doi} detected but unpaywall returned no data or otherwise errored" + end + Rails.logger.debug { "URL: #{url(doi)}" } 'Error' end end diff --git a/app/models/lookup_isbn.rb b/app/models/lookup_isbn.rb index b7792a4..d17f5d1 100644 --- a/app/models/lookup_isbn.rb +++ b/app/models/lookup_isbn.rb @@ -26,7 +26,7 @@ def fetch_isbn(isbn) def fetch_authors(isbn_json) return unless isbn_json['authors'] - authors = isbn_json['authors'].map { |a| a['key'] } + authors = isbn_json['authors'].pluck('key') author_names = authors.map do |author| url = [base_url, author, '.json'].join json = parse_response(url) @@ -42,7 +42,7 @@ def parse_response(url) JSON.parse(resp.to_s) else Rails.logger.debug('Fact lookup error: openlibrary returned no data') - Rails.logger.debug("URL: #{url}") + Rails.logger.debug { "URL: #{url}" } 'Error' end end diff --git a/app/models/lookup_issn.rb b/app/models/lookup_issn.rb index a70d6cc..f7dcbd3 100644 --- a/app/models/lookup_issn.rb +++ b/app/models/lookup_issn.rb @@ -31,8 +31,8 @@ def fetch(issn) if resp.status == 200 JSON.parse(resp.to_s) else - Rails.logger.debug("ISSN Lookup error. ISSN #{issn} detected but crossref returned no data") - Rails.logger.debug("URL: #{url(issn)}") + Rails.logger.debug { "ISSN Lookup error. ISSN #{issn} detected but crossref returned no data" } + Rails.logger.debug { "URL: #{url(issn)}" } 'Error' end end diff --git a/app/models/lookup_pmid.rb b/app/models/lookup_pmid.rb index 7a9444f..90df4ad 100644 --- a/app/models/lookup_pmid.rb +++ b/app/models/lookup_pmid.rb @@ -12,7 +12,7 @@ def info(pmid) if metadata.reject { |_k, v| v.empty? }.present? metadata else - Rails.logger.debug("Fact lookup error. PMID #{pmid} detected but ncbi returned no data") + Rails.logger.debug { "Fact lookup error. PMID #{pmid} detected but ncbi returned no data" } nil end end @@ -37,8 +37,8 @@ def fetch(pmid) if resp.status == 200 Nokogiri::XML(resp.to_s) else - Rails.logger.debug("Fact lookup error. PMID #{pmid} detected but ncbi an error status") - Rails.logger.debug("URL: #{url(pmid)}") + Rails.logger.debug { "Fact lookup error. PMID #{pmid} detected but ncbi an error status" } + Rails.logger.debug { "URL: #{url(pmid)}" } 'Error' end end diff --git a/app/models/metrics/algorithms.rb b/app/models/metrics/algorithms.rb index 82535ab..2ba9b24 100644 --- a/app/models/metrics/algorithms.rb +++ b/app/models/metrics/algorithms.rb @@ -41,7 +41,7 @@ def generate(month = nil) matches = if month.present? count_matches(SearchEvent.single_month(month).includes(:term)) else - count_matches(SearchEvent.all.includes(:term)) + count_matches(SearchEvent.includes(:term)) end Metrics::Algorithms.create(month:, doi: matches[:doi], issn: matches[:issn], isbn: matches[:isbn], pmid: matches[:pmid], journal_exact: matches[:journal_exact], diff --git a/app/models/search_event.rb b/app/models/search_event.rb index 8fe5a5f..7042b8f 100644 --- a/app/models/search_event.rb +++ b/app/models/search_event.rb @@ -21,5 +21,5 @@ class SearchEvent < ApplicationRecord # # @param month [DateTime] A DateTime object within the `month` to be filtered. # @return [Array] All SearchEvents for the supplied `month`. - scope :single_month, ->(month) { where(created_at: month.beginning_of_month..month.end_of_month) } + scope :single_month, ->(month) { where(created_at: month.all_month) } end diff --git a/app/models/standard_identifiers.rb b/app/models/standard_identifiers.rb index 0de7f4c..b8fd3ed 100644 --- a/app/models/standard_identifiers.rb +++ b/app/models/standard_identifiers.rb @@ -55,7 +55,7 @@ def strip_invalid_issns # An example calculation is shared at # https://en.wikipedia.org/wiki/International_Standard_Serial_Number#Code_format def validate_issn(candidate) - digits = candidate.gsub('-', '').chars[..6] + digits = candidate.delete('-')[..6].chars check_digit = candidate.last.downcase sum = 0 diff --git a/app/models/user.rb b/app/models/user.rb index a2126ce..99d9d99 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # == Schema Information # # Table name: users @@ -33,7 +35,7 @@ def self.from_omniauth(auth) email = auth.extra.raw_info.email end - User.where(uid: uid).first_or_create do |user| + User.where(uid:).first_or_create do |user| user.uid = uid user.email = email end diff --git a/lib/tasks/journals.rake b/lib/tasks/journals.rake index 75cd577..eff5bc8 100644 --- a/lib/tasks/journals.rake +++ b/lib/tasks/journals.rake @@ -16,7 +16,7 @@ namespace :journals do # In development, use your own. If used in production, use a team Moira list. desc 'Harvest from Open Alex' task :openalex_harvester, %i[email] => :environment do |_task, args| - raise ArgumentError.new, 'Email is required' unless args.email.present? + raise ArgumentError.new, 'Email is required' if args.email.blank? base_url = 'https://api.openalex.org/sources?filter=is_core:true' next_cursor = '*' @@ -81,12 +81,12 @@ namespace :journals do # @param path [String] local file path or URI to a JSON file to load desc 'Load from OpenAlex harvest' task :openalex_loader, %i[file] => :environment do |_task, args| - raise ArgumentError.new, 'File is required' unless args.file.present? + raise ArgumentError.new, 'File is required' if args.file.blank? # does the file look like a path or a URI if URI(args.file).scheme Rails.logger.info("Loading data from remote file #{args.file}") - data = URI.open(args.file, 'rb', &:read) + data = URI.parse(args.file).open('rb', &:read) else Rails.logger.info("Loading data from local file #{args.file}") data = File.read(args.file) diff --git a/lib/tasks/search_event_loader.rake b/lib/tasks/search_event_loader.rake index 88a80a1..d57b0a5 100644 --- a/lib/tasks/search_event_loader.rake +++ b/lib/tasks/search_event_loader.rake @@ -18,8 +18,8 @@ namespace :search_events do # @param source [String] source name to load the data under desc 'Load search_events from csv' task :csv_loader, %i[path source] => :environment do |_task, args| - raise ArgumentError.new, 'Path is required' unless args.path.present? - raise ArgumentError.new, 'Source is required' unless args.source.present? + raise ArgumentError.new, 'Path is required' if args.path.blank? + raise ArgumentError.new, 'Source is required' if args.source.blank? Rails.logger.info("Loading data from #{args.path}") diff --git a/lib/tasks/suggested_resources.rake b/lib/tasks/suggested_resources.rake index 9c0272f..d444272 100644 --- a/lib/tasks/suggested_resources.rake +++ b/lib/tasks/suggested_resources.rake @@ -6,7 +6,7 @@ namespace :suggested_resources do # we do need a way to import records from a CSV file. desc 'Replace all Suggested Resources from CSV' task :reload, [:addr] => :environment do |_task, args| - raise ArgumentError.new, 'URL is required' unless args.addr.present? + raise ArgumentError.new, 'URL is required' if args.addr.blank? raise ArgumentError.new, 'Local files are not supported yet' unless URI(args.addr).scheme diff --git a/test/controllers/graphql_controller_test.rb b/test/controllers/graphql_controller_test.rb index 60ae020..2647775 100644 --- a/test/controllers/graphql_controller_test.rb +++ b/test/controllers/graphql_controller_test.rb @@ -12,14 +12,15 @@ class GraphqlControllerTest < ActionDispatch::IntegrationTest updatedAt } }' } + assert_equal(200, response.status) - json = JSON.parse(response.body) + json = response.parsed_body term_id = Term.last.id assert_equal 'bento', json['data']['logSearchEvent']['source'] assert_equal term_id, json['data']['logSearchEvent']['termId'] - assert_equal Date.today, json['data']['logSearchEvent']['createdAt'].to_date - assert_equal Date.today, json['data']['logSearchEvent']['updatedAt'].to_date + assert_equal Time.zone.today, json['data']['logSearchEvent']['createdAt'].to_date + assert_equal Time.zone.today, json['data']['logSearchEvent']['updatedAt'].to_date end test 'search event query creates a new term if one does not exist' do @@ -32,6 +33,7 @@ class GraphqlControllerTest < ActionDispatch::IntegrationTest updatedAt } }' } + assert_equal(200, response.status) assert_equal Term.count, (initial_term_count + 1) assert_equal 'range life', Term.last.phrase @@ -47,6 +49,7 @@ class GraphqlControllerTest < ActionDispatch::IntegrationTest updatedAt } }' } + assert_equal(200, response.status) assert_equal Term.count, initial_term_count end @@ -61,7 +64,8 @@ class GraphqlControllerTest < ActionDispatch::IntegrationTest } }' } - json = JSON.parse(response.body) + json = response.parsed_body + assert_equal('doi', json['data']['logSearchEvent']['standardIdentifiers'].first['kind']) assert_equal('10.1038/nphys1170', json['data']['logSearchEvent']['standardIdentifiers'].first['value']) end @@ -73,7 +77,8 @@ class GraphqlControllerTest < ActionDispatch::IntegrationTest } }' } - json = JSON.parse(response.body) + json = response.parsed_body + assert_equal('10.1038/nphys1170', json['data']['logSearchEvent']['phrase']) end @@ -94,7 +99,8 @@ class GraphqlControllerTest < ActionDispatch::IntegrationTest } }' } - json = JSON.parse(response.body) + json = response.parsed_body + assert_equal('Measured measurement', json['data']['logSearchEvent']['standardIdentifiers'].first['details']['title']) assert_equal('https://mit.primo.exlibrisgroup.com/discovery/openurl?institution=01MIT_INST&rfr_id=info:sid/mit.tacos.api&vid=01MIT_INST:MIT&rft.atitle=Measured measurement&rft.date=&rft.genre=journal-article&rft.jtitle=Nature Physics&rft_id=info:doi/10.1038/nphys1170', diff --git a/test/integration/admin_dashboard_test.rb b/test/integration/admin_dashboard_test.rb index ba934c7..2f91b81 100644 --- a/test/integration/admin_dashboard_test.rb +++ b/test/integration/admin_dashboard_test.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'test_helper' class AdminDashboardTest < ActionDispatch::IntegrationTest @@ -11,8 +13,10 @@ def teardown test 'admin area redirects to root with prompt to sign in if not already signed in' do get '/admin' + assert_response :redirect follow_redirect! + assert_equal '/', path assert_select 'div.alert', text: 'Please sign in to continue', count: 1 end @@ -20,8 +24,10 @@ def teardown test 'authenticated users without admin status still cannot access admin area' do mock_auth(users(:basic)) get '/admin' + assert_response :redirect follow_redirect! + assert_equal '/', path assert_select 'div.alert', text: 'Not authorized', count: 1 end @@ -29,7 +35,8 @@ def teardown test 'admin users can access admin area' do mock_auth(users(:admin)) get '/admin' - assert_response 200 + + assert_response :ok assert_equal '/admin', path end end diff --git a/test/integration/authentication_test.rb b/test/integration/authentication_test.rb index 1d501e6..0a095b7 100644 --- a/test/integration/authentication_test.rb +++ b/test/integration/authentication_test.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'test_helper' class AuthenticationTest < ActionDispatch::IntegrationTest @@ -24,6 +26,7 @@ def silence_omniauth get '/users/auth/openid_connect/callback' follow_redirect! end + assert_response :success assert_equal(user_count, User.count) end @@ -36,6 +39,7 @@ def silence_omniauth user_count = User.count get '/users/auth/openid_connect/callback' follow_redirect! + assert_response :success assert_equal(user_count + 1, User.count) end @@ -44,6 +48,7 @@ def silence_omniauth user_count = User.count mock_auth(users(:valid)) follow_redirect! + assert_response :success assert_equal(user_count, User.count) end diff --git a/test/models/detector/journal_test.rb b/test/models/detector/journal_test.rb index 06828c8..ccfba12 100644 --- a/test/models/detector/journal_test.rb +++ b/test/models/detector/journal_test.rb @@ -18,7 +18,7 @@ class JournalTest < ActiveSupport::TestCase expected = detector_journals('the_new_england_journal_of_medicine') actual = Detector::Journal.full_term_match('the new england journal of medicine') - assert actual.count == 1 + assert_equal 1, actual.count assert_equal(expected, actual.first) end @@ -26,30 +26,34 @@ class JournalTest < ActiveSupport::TestCase expected = detector_journals('the_new_england_journal_of_medicine') actual = Detector::Journal.full_term_match('The New England Journal of Medicine') - assert actual.count == 1 + assert_equal 1, actual.count assert_equal(expected, actual.first) end test 'exact match within longer term returns no matches' do actual = Detector::Journal.full_term_match('The New England Journal of Medicine, 1999') - assert actual.count.zero? + + assert_predicate actual.count, :zero? end test 'phrase match within longer term returns matches' do actual = Detector::Journal.partial_term_match('words and stuff The New England Journal of Medicine, 1999') - assert actual.count == 1 + + assert_equal 1, actual.count end test 'multple matches can happen with phrase matching within longer terms' do actual = Detector::Journal.partial_term_match('words and stuff Nature medicine, 1999') - assert actual.count == 2 + + assert_equal 2, actual.count end test 'mixed titles are downcased when saved' do mixed_case = 'ThIs Is A tItLe' actual = Detector::Journal.create(name: mixed_case) actual.reload - refute_equal(mixed_case, actual.name) + + assert_not_equal(mixed_case, actual.name) assert_equal(mixed_case.downcase, actual.name) end end diff --git a/test/models/detector/suggested_resource_test.rb b/test/models/detector/suggested_resource_test.rb index 620b35b..57031b9 100644 --- a/test/models/detector/suggested_resource_test.rb +++ b/test/models/detector/suggested_resource_test.rb @@ -25,99 +25,105 @@ class SuggestedResourceTest < ActiveSupport::TestCase new_resource = Detector::SuggestedResource.create(resource) - assert new_resource.fingerprint == 'latest our resource' + assert_equal 'latest our resource', new_resource.fingerprint end test 'fingerprints are recalculated on save' do resource = detector_suggested_resources('jstor') - refute resource.fingerprint == 'A brand new phrase' + + assert_not_equal resource.fingerprint, 'A brand new phrase' resource.phrase = 'This is a brand new phrase' resource.save resource.reload - assert resource.fingerprint == 'a brand is new phrase this' + assert_equal 'a brand is new phrase this', resource.fingerprint end test 'generating fingerprints does not alter the phrase' do resource = detector_suggested_resources('jstor') benchmark = 'This is an updated phrase! ' - refute resource.phrase == benchmark + assert_not_equal resource.phrase, benchmark resource.phrase = benchmark resource.save resource.reload - assert resource.phrase == benchmark + assert_equal resource.phrase, benchmark end test 'fingerprints strip extra spaces' do resource = detector_suggested_resources('jstor') - refute resource.fingerprint == 'i need space' + + assert_not_equal resource.fingerprint, 'i need space' resource.phrase = ' i need space ' resource.save resource.reload - assert resource.fingerprint == 'i need space' + assert_equal 'i need space', resource.fingerprint end test 'fingerprints are coerced to lowercase' do resource = detector_suggested_resources('jstor') - refute resource.fingerprint == 'ftw intercapping' + + assert_not_equal resource.fingerprint, 'ftw intercapping' resource.phrase = 'InterCapping FTW' resource.save resource.reload - assert resource.fingerprint == 'ftw intercapping' + assert_equal 'ftw intercapping', resource.fingerprint end test 'fingerprints remove punctuation and symbols' do resource = detector_suggested_resources('jstor') - refute resource.fingerprint == 'bullets perfect phrase punctuation quoted symbols' + + assert_not_equal resource.fingerprint, 'bullets perfect phrase punctuation quoted symbols' resource.phrase = 'symbols™ + punctuation: * bullets! - "quoted phrase" (perfect) ¥€$' resource.save resource.reload - assert resource.fingerprint == 'bullets perfect phrase punctuation quoted symbols' + assert_equal 'bullets perfect phrase punctuation quoted symbols', resource.fingerprint end test 'fingerprints coerce characters to ASCII' do resource = { title: 'A wide range of characters', url: 'https://example.org', - phrase: 'а а̀ а̂ а̄ ӓ б в г ґ д ђ ѓ е ѐ е̄ е̂ ё є ж з з́ ѕ и і ї ꙇ ѝ и̂ ӣ й ј к л љ м н њ о о̀ о̂ ō ӧ п р с с́'\ - ' т ћ ќ у у̀ у̂ ӯ ў ӱ ф х ц ч џ ш щ ꙏ ъ ъ̀ ы ь ѣ э ю ю̀ я' + phrase: 'а а̀ а̂ а̄ ӓ б в г ґ д ђ ѓ е ѐ е̄ е̂ ё є ж з з́ ѕ и і ї ꙇ ѝ и̂ ӣ й ј к л љ м н њ о о̀ о̂ ō ӧ п р с с́ ' \ + 'т ћ ќ у у̀ у̂ ӯ ў ӱ ф х ц ч џ ш щ ꙏ ъ ъ̀ ы ь ѣ э ю ю̀ я' } new_resource = Detector::SuggestedResource.create(resource) - assert new_resource.fingerprint == 'a b ch d dj dz dzh e f g gh gj i ia ie io iu j k kh kj l lj m n nj o p r s '\ - 'sh shch t ts tsh u v y yi z zh' + assert_equal 'a b ch d dj dz dzh e f g gh gj i ia ie io iu j k kh kj l lj m n nj o p r s ' \ + 'sh shch t ts tsh u v y yi z zh', new_resource.fingerprint end test 'fingerprints remove repeated words' do resource = detector_suggested_resources('jstor') - refute resource.fingerprint == 'double' + + assert_not_equal resource.fingerprint, 'double' resource.phrase = 'double double' resource.save resource.reload - assert resource.fingerprint == 'double' + assert_equal 'double', resource.fingerprint end test 'fingerprints sort words alphabetically' do resource = detector_suggested_resources('jstor') - refute resource.fingerprint == 'delta gamma' + + assert_not_equal resource.fingerprint, 'delta gamma' resource.phrase = 'gamma delta' resource.save resource.reload - assert resource.fingerprint == 'delta gamma' + assert_equal 'delta gamma', resource.fingerprint end end end diff --git a/test/models/fake_auth_test.rb b/test/models/fake_auth_test.rb index d1f9a26..e846fc7 100644 --- a/test/models/fake_auth_test.rb +++ b/test/models/fake_auth_test.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'test_helper' class FakeAuthTest < ActiveSupport::TestCase @@ -7,7 +9,7 @@ class FakeAuthTest < ActiveSupport::TestCase ClimateControl.modify( FAKE_AUTH_ENABLED: 'false' ) do - assert_equal(false, FakeAuthConfig.fake_auth_enabled?) + assert_not_predicate(FakeAuthConfig, :fake_auth_enabled?) end end @@ -16,7 +18,7 @@ class FakeAuthTest < ActiveSupport::TestCase FAKE_AUTH_ENABLED: 'false', HEROKU_APP_NAME: 'tacos-api-pipeline-pr-1' ) do - assert_equal(false, FakeAuthConfig.fake_auth_enabled?) + assert_not_predicate(FakeAuthConfig, :fake_auth_enabled?) end end @@ -25,33 +27,33 @@ class FakeAuthTest < ActiveSupport::TestCase FAKE_AUTH_ENABLED: 'true', HEROKU_APP_NAME: 'tacos-api-pipeline-pr-1' ) do - assert_equal(true, FakeAuthConfig.fake_auth_enabled?) + assert_predicate(FakeAuthConfig, :fake_auth_enabled?) end ClimateControl.modify( FAKE_AUTH_ENABLED: 'true', HEROKU_APP_NAME: 'tacos-api-pipeline-pr-500' ) do - assert_equal(true, FakeAuthConfig.fake_auth_enabled?) + assert_predicate(FakeAuthConfig, :fake_auth_enabled?) end end test 'fakeauth enabled no HEROKU_APP_NAME' do ClimateControl.modify FAKE_AUTH_ENABLED: 'true' do - assert_equal(false, FakeAuthConfig.fake_auth_enabled?) + assert_not_predicate(FakeAuthConfig, :fake_auth_enabled?) end end test 'fakeauth enabled production app name' do ClimateControl.modify FAKE_AUTH_ENABLED: 'true', HEROKU_APP_NAME: 'tacos-prod' do - assert_equal(false, FakeAuthConfig.fake_auth_enabled?) + assert_not_predicate(FakeAuthConfig, :fake_auth_enabled?) end end test 'fakeauth enabled staging app name' do ClimateControl.modify FAKE_AUTH_ENABLED: 'true', HEROKU_APP_NAME: 'tacos-stage' do - assert_equal(false, FakeAuthConfig.fake_auth_enabled?) + assert_not_predicate(FakeAuthConfig, :fake_auth_enabled?) end end end diff --git a/test/models/lookup_doi_test.rb b/test/models/lookup_doi_test.rb index 796932d..20de97d 100644 --- a/test/models/lookup_doi_test.rb +++ b/test/models/lookup_doi_test.rb @@ -9,8 +9,9 @@ class LookupDoiTest < ActiveSupport::TestCase expected_keys = %i[genre title date publisher oa oa_status best_oa_location journal_issns journal_name link_resolver_url] + expected_keys.each do |key| - assert(metadata.keys.include?(key)) + assert_includes(metadata.keys, key) end end end @@ -20,6 +21,7 @@ class LookupDoiTest < ActiveSupport::TestCase metadata = LookupDoi.new.info('10.1038/d41586-023-03497-2') expected_url = 'https://mit.primo.exlibrisgroup.com/discovery/openurl?institution=01MIT_INST&rfr_id=info:sid/mit.tacos.api&vid=01MIT_INST:MIT&rft.atitle=Flashy molecules decode a polymer’s lengthening chain&rft.date=&rft.genre=journal-article&rft.jtitle=Nature&rft_id=info:doi/10.1038/d41586-023-03497-2' + assert_equal(expected_url, metadata[:link_resolver_url]) end end @@ -27,6 +29,7 @@ class LookupDoiTest < ActiveSupport::TestCase test 'non 200 responses' do VCR.use_cassette('doi not found') do metadata = LookupDoi.new.info('123456') + assert_nil(metadata) end end diff --git a/test/models/lookup_isbn_test.rb b/test/models/lookup_isbn_test.rb index 23b2a4c..d48dd6a 100644 --- a/test/models/lookup_isbn_test.rb +++ b/test/models/lookup_isbn_test.rb @@ -8,8 +8,9 @@ class LookupIsbnTest < ActiveSupport::TestCase metadata = LookupIsbn.new.info('978-0-08-102133-0') expected_keys = %i[title date publisher authors link_resolver_url] + expected_keys.each do |key| - assert(metadata.keys.include?(key)) + assert_includes(metadata.keys, key) end end end @@ -19,6 +20,7 @@ class LookupIsbnTest < ActiveSupport::TestCase metadata = LookupIsbn.new.info('978-0-08-102133-0') expected_url = 'https://mit.primo.exlibrisgroup.com/discovery/openurl?institution=01MIT_INST&rfr_id=info:sid/mit.tacos.api&vid=01MIT_INST:MIT&rft.isbn=978-0-08-102133-0' + assert_equal(expected_url, metadata[:link_resolver_url]) end end @@ -26,6 +28,7 @@ class LookupIsbnTest < ActiveSupport::TestCase test 'non 200 responses' do VCR.use_cassette('isbn not found') do metadata = LookupIsbn.new.info('asdf') + assert_nil(metadata) end end diff --git a/test/models/lookup_issn_test.rb b/test/models/lookup_issn_test.rb index 7cbd819..945c4cf 100644 --- a/test/models/lookup_issn_test.rb +++ b/test/models/lookup_issn_test.rb @@ -8,8 +8,9 @@ class LookupIssnTest < ActiveSupport::TestCase metadata = LookupIssn.new.info('1078-8956') expected_keys = %i[publisher journal_issns journal_name link_resolver_url] + expected_keys.each do |key| - assert(metadata.keys.include?(key)) + assert_includes(metadata.keys, key) end end end @@ -19,6 +20,7 @@ class LookupIssnTest < ActiveSupport::TestCase metadata = LookupIssn.new.info('1078-8956') expected_url = 'https://mit.primo.exlibrisgroup.com/discovery/openurl?institution=01MIT_INST&rfr_id=info:sid/mit.tacos.api&vid=01MIT_INST:MIT&rft.issn=1078-8956' + assert_equal(expected_url, metadata[:link_resolver_url]) end end @@ -26,6 +28,7 @@ class LookupIssnTest < ActiveSupport::TestCase test 'non 200 responses' do VCR.use_cassette('issn not found') do metadata = LookupIssn.new.info('asdf') + assert_nil(metadata) end end diff --git a/test/models/lookup_pmid_test.rb b/test/models/lookup_pmid_test.rb index 5da3557..086c2fe 100644 --- a/test/models/lookup_pmid_test.rb +++ b/test/models/lookup_pmid_test.rb @@ -8,8 +8,9 @@ class LookupPmidTest < ActiveSupport::TestCase metadata = LookupPmid.new.info('37953305') expected_keys = %i[title date journal_volume doi journal_name link_resolver_url] + expected_keys.each do |key| - assert(metadata.keys.include?(key)) + assert_includes(metadata.keys, key) end end end @@ -19,6 +20,7 @@ class LookupPmidTest < ActiveSupport::TestCase metadata = LookupPmid.new.info('37953305') expected_url = "https://mit.primo.exlibrisgroup.com/discovery/openurl?institution=01MIT_INST&rfr_id=info:sid/mit.tacos.api&vid=01MIT_INST:MIT&rft.atitle=Flashy molecules decode a polymer's lengthening chain.&rft.date=2023&rft.jtitle=Nature&rft_id=info:doi/10.1038/d41586-023-03497-2" + assert_equal(expected_url, metadata[:link_resolver_url]) end end @@ -26,6 +28,7 @@ class LookupPmidTest < ActiveSupport::TestCase test 'non 200 responses' do VCR.use_cassette('pmid not found') do metadata = LookupPmid.new.info('asdf') + assert_nil(metadata) end end diff --git a/test/models/metrics/algorithms_test.rb b/test/models/metrics/algorithms_test.rb index 314647c..279a081 100644 --- a/test/models/metrics/algorithms_test.rb +++ b/test/models/metrics/algorithms_test.rb @@ -21,32 +21,38 @@ class Algorithms < ActiveSupport::TestCase # Monthlies test 'dois counts are included in monthly aggregation' do aggregate = Metrics::Algorithms.new.generate(DateTime.now) - assert aggregate.doi == 1 + + assert_equal 1, aggregate.doi end test 'issns counts are included in monthly aggregation' do aggregate = Metrics::Algorithms.new.generate(DateTime.now) - assert aggregate.issn == 1 + + assert_equal 1, aggregate.issn end test 'isbns counts are included in monthly aggregation' do aggregate = Metrics::Algorithms.new.generate(DateTime.now) - assert aggregate.isbn == 1 + + assert_equal 1, aggregate.isbn end test 'pmids counts are included in monthly aggregation' do aggregate = Metrics::Algorithms.new.generate(DateTime.now) - assert aggregate.pmid == 1 + + assert_equal 1, aggregate.pmid end test 'journal exact counts are included in monthly aggregation' do aggregate = Metrics::Algorithms.new.generate(DateTime.now) - assert aggregate.journal_exact == 1 + + assert_equal 1, aggregate.journal_exact end test 'unmatched counts are included are included in monthly aggregation' do aggregate = Metrics::Algorithms.new.generate(DateTime.now) - assert aggregate.unmatched == 2 + + assert_equal 2, aggregate.unmatched end test 'creating lots of searchevents leads to correct data for monthly' do @@ -80,42 +86,48 @@ class Algorithms < ActiveSupport::TestCase aggregate = Metrics::Algorithms.new.generate(DateTime.now) - assert doi_expected_count == aggregate.doi - assert issn_expected_count == aggregate.issn - assert isbn_expected_count == aggregate.isbn - assert pmid_expected_count == aggregate.pmid - assert unmatched_expected_count == aggregate.unmatched + assert_equal doi_expected_count, aggregate.doi + assert_equal issn_expected_count, aggregate.issn + assert_equal isbn_expected_count, aggregate.isbn + assert_equal pmid_expected_count, aggregate.pmid + assert_equal unmatched_expected_count, aggregate.unmatched end # Total test 'dois counts are included in total aggregation' do aggregate = Metrics::Algorithms.new.generate - assert aggregate.doi == 1 + + assert_equal 1, aggregate.doi end test 'issns counts are included in total aggregation' do aggregate = Metrics::Algorithms.new.generate - assert aggregate.issn == 1 + + assert_equal 1, aggregate.issn end test 'isbns counts are included in total aggregation' do aggregate = Metrics::Algorithms.new.generate - assert aggregate.isbn == 1 + + assert_equal 1, aggregate.isbn end test 'pmids counts are included in total aggregation' do aggregate = Metrics::Algorithms.new.generate - assert aggregate.pmid == 2 + + assert_equal 2, aggregate.pmid end test 'journal exact counts are included in total aggregation' do aggregate = Metrics::Algorithms.new.generate - assert aggregate.journal_exact == 2 + + assert_equal 2, aggregate.journal_exact end test 'unmatched counts are included are included in total aggregation' do aggregate = Metrics::Algorithms.new.generate - assert aggregate.unmatched == 2 + + assert_equal 2, aggregate.unmatched end test 'creating lots of searchevents leads to correct data for total' do @@ -154,11 +166,11 @@ class Algorithms < ActiveSupport::TestCase aggregate = Metrics::Algorithms.new.generate - assert doi_expected_count == aggregate.doi - assert issn_expected_count == aggregate.issn - assert isbn_expected_count == aggregate.isbn - assert pmid_expected_count == aggregate.pmid - assert journal_exact_count == aggregate.journal_exact - assert unmatched_expected_count == aggregate.unmatched + assert_equal doi_expected_count, aggregate.doi + assert_equal issn_expected_count, aggregate.issn + assert_equal isbn_expected_count, aggregate.isbn + assert_equal pmid_expected_count, aggregate.pmid + assert_equal journal_exact_count, aggregate.journal_exact + assert_equal unmatched_expected_count, aggregate.unmatched end end diff --git a/test/models/search_event_test.rb b/test/models/search_event_test.rb index ec921d2..7e67007 100644 --- a/test/models/search_event_test.rb +++ b/test/models/search_event_test.rb @@ -15,27 +15,31 @@ class SearchEventTest < ActiveSupport::TestCase test 'term is required' do s = search_events('timdex_cool') - assert(s.valid?) + + assert_predicate(s, :valid?) s.term = nil - refute(s.valid?) + + assert_not_predicate(s, :valid?) end test 'source is required' do s = search_events('timdex_cool') - assert(s.valid?) + + assert_predicate(s, :valid?) s.source = nil - refute(s.valid?) + + assert_not_predicate(s, :valid?) end test 'monthly scope returns requested month of SearchEvents' do - assert SearchEvent.all.include?(search_events(:current_month_pmid)) - assert SearchEvent.single_month(Time.now).include?(search_events(:current_month_pmid)) + assert_includes SearchEvent.all, search_events(:current_month_pmid) + assert_includes SearchEvent.single_month(Time.zone.now), search_events(:current_month_pmid) end test 'monthly scope does not return SearchEvents outside the requested month' do - assert SearchEvent.all.include?(search_events(:old_month_pmid)) - refute SearchEvent.single_month(Time.now).include?(search_events(:old_month_pmid)) + assert_includes SearchEvent.all, search_events(:old_month_pmid) + assert_not_includes SearchEvent.single_month(Time.zone.now), search_events(:old_month_pmid) end end diff --git a/test/models/standard_identifiers_test.rb b/test/models/standard_identifiers_test.rb index cdf8154..24df967 100644 --- a/test/models/standard_identifiers_test.rb +++ b/test/models/standard_identifiers_test.rb @@ -17,6 +17,7 @@ class StandardIdentifiersTest < ActiveSupport::TestCase samples.each do |isbn| actual = StandardIdentifiers.new(isbn).identifiers + assert_equal(isbn, actual[:isbn]) end end @@ -28,6 +29,7 @@ class StandardIdentifiersTest < ActiveSupport::TestCase samples.each do |isbn| actual = StandardIdentifiers.new(isbn).identifiers + assert_equal(isbn, actual[:isbn]) end end @@ -37,6 +39,7 @@ class StandardIdentifiersTest < ActiveSupport::TestCase samples.each do |notisbn| actual = StandardIdentifiers.new(notisbn).identifiers + assert_nil(actual[:isbn]) end end @@ -49,12 +52,14 @@ class StandardIdentifiersTest < ActiveSupport::TestCase samples.each do |notisbn| actual = StandardIdentifiers.new(notisbn).identifiers + assert_nil(actual[:isbn]) end end test 'ISSNs detected in a string' do actual = StandardIdentifiers.new('test 0250-6335 test').identifiers + assert_equal('0250-6335', actual[:issn]) end @@ -63,6 +68,7 @@ class StandardIdentifiersTest < ActiveSupport::TestCase samples.each do |issn| actual = StandardIdentifiers.new(issn).identifiers + assert_equal(issn, actual[:issn]) end end @@ -72,12 +78,14 @@ class StandardIdentifiersTest < ActiveSupport::TestCase samples.each do |notissn| actual = StandardIdentifiers.new(notissn).identifiers + assert_nil(actual[:issn]) end end test 'ISSNs need boundaries' do actual = StandardIdentifiers.new('12345-5678 1234-56789').identifiers + assert_nil(actual[:issn]) end @@ -109,6 +117,7 @@ class StandardIdentifiersTest < ActiveSupport::TestCase ] samples.each do |notissn| actual = StandardIdentifiers.new(notissn).identifiers + assert_nil(actual[:issn]) end end @@ -122,6 +131,7 @@ class StandardIdentifiersTest < ActiveSupport::TestCase ] samples.each do |issn| actual = StandardIdentifiers.new(issn).identifiers + assert_equal(issn, actual[:issn]) end end @@ -129,6 +139,7 @@ class StandardIdentifiersTest < ActiveSupport::TestCase test 'doi detected in string' do actual = StandardIdentifiers.new('"Quantum tomography: Measured measurement", Markus Aspelmeyer, nature physics "\ "January 2009, Volume 5, No 1, pp11-12; [ doi:10.1038/nphys1170 ]').identifiers + assert_equal('10.1038/nphys1170', actual[:doi]) end @@ -138,6 +149,7 @@ class StandardIdentifiersTest < ActiveSupport::TestCase samples.each do |doi| actual = StandardIdentifiers.new(doi).identifiers + assert_equal(doi, actual[:doi]) end end @@ -147,12 +159,14 @@ class StandardIdentifiersTest < ActiveSupport::TestCase samples.each do |notdoi| actual = StandardIdentifiers.new(notdoi).identifiers + assert_nil(actual[:notdoi]) end end test 'pmid detected in string' do actual = StandardIdentifiers.new('Citation and stuff PMID: 35648703 more stuff.').identifiers + assert_equal('PMID: 35648703', actual[:pmid]) end @@ -161,6 +175,7 @@ class StandardIdentifiersTest < ActiveSupport::TestCase samples.each do |pmid| actual = StandardIdentifiers.new(pmid).identifiers + assert_equal(pmid, actual[:pmid]) end end @@ -170,6 +185,7 @@ class StandardIdentifiersTest < ActiveSupport::TestCase samples.each do |notpmid| actual = StandardIdentifiers.new(notpmid).identifiers + assert_nil(actual[:pmid]) end end diff --git a/test/models/term_test.rb b/test/models/term_test.rb index dad33f7..5f09fc4 100644 --- a/test/models/term_test.rb +++ b/test/models/term_test.rb @@ -37,7 +37,7 @@ class TermTest < ActiveSupport::TestCase term.destroy assert_equal((term_pre_count - 1), Term.count) - assert(SearchEvent.count < event_pre_count) + assert_operator(SearchEvent.count, :<, event_pre_count) end test 'destroying a SearchEvent does not delete the Term' do @@ -52,6 +52,6 @@ class TermTest < ActiveSupport::TestCase t.reload assert_equal(events_count - 1, t.search_events.count) - assert(t.valid?) + assert_predicate(t, :valid?) end end diff --git a/test/models/user_test.rb b/test/models/user_test.rb index 9697d86..f554370 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # == Schema Information # # Table name: users @@ -14,38 +16,45 @@ class UserTest < ActiveSupport::TestCase test 'user valid with uid and email' do user = users(:valid) - assert user.uid.present? - assert user.email.present? - assert user.valid? + + assert_predicate user.uid, :present? + assert_predicate user.email, :present? + assert_predicate user, :valid? end test 'user invalid without uid' do user = users(:valid) - assert user.valid? + + assert_predicate user, :valid? user.uid = nil user.save + assert_not user.valid? end test 'user invalid without email' do user = users(:valid) - assert user.valid? + + assert_predicate user, :valid? user.email = nil user.save + assert_not user.valid? end test 'admin user is valid' do user = users(:admin) - assert user.admin? - assert user.valid? + + assert_predicate user, :admin? + assert_predicate user, :valid? end test 'non-admin user is valid' do user = users(:basic) + assert_not user.admin? - assert user.valid? + assert_predicate user, :valid? end end diff --git a/test/tasks/suggested_resource_rake_test.rb b/test/tasks/suggested_resource_rake_test.rb index c1efd9e..51e5b6a 100644 --- a/test/tasks/suggested_resource_rake_test.rb +++ b/test/tasks/suggested_resource_rake_test.rb @@ -16,8 +16,9 @@ def setup remote_file = 'http://static.lndo.site/suggested_resources.csv' Rake::Task['suggested_resources:reload'].invoke(remote_file) end - refute_equal records_before, Detector::SuggestedResource.count - refute_equal first_record_before, Detector::SuggestedResource.first + + assert_not_equal records_before, Detector::SuggestedResource.count + assert_not_equal first_record_before, Detector::SuggestedResource.first end test 'reload task errors without a file argument' do @@ -29,7 +30,7 @@ def setup test 'reload errors on a local file' do error = assert_raises(ArgumentError) do - local_file = Rails.root.join('test', 'fixtures', 'files', 'suggested_resources.csv').to_s + local_file = Rails.root.join('test/fixtures/files/suggested_resources.csv').to_s Rake::Task['suggested_resources:reload'].invoke(local_file) end assert_equal 'Local files are not supported yet', error.message @@ -63,6 +64,7 @@ def setup remote_file = 'http://static.lndo.site/suggested_resources_extra.csv' Rake::Task['suggested_resources:reload'].invoke(remote_file) end - refute_equal records_before, Detector::SuggestedResource.count + + assert_not_equal records_before, Detector::SuggestedResource.count end end diff --git a/test/test_helper.rb b/test/test_helper.rb index ba0bc18..2207e14 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -28,7 +28,7 @@ class TestCase SimpleCov.command_name "#{SimpleCov.command_name}-#{worker}" end - parallelize_teardown do |worker| + parallelize_teardown do |_worker| SimpleCov.result end