diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 637d70d6..22c9ecd4 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -7,13 +7,13 @@ jobs: continue-on-error: ${{ matrix.failure-allowed }} strategy: matrix: - ruby-version: ['2.7.8', '3.0.6', '3.1.4', '3.2.2'] + ruby-version: ['3.0.7', '3.1.6', '3.2.5', '3.3.4'] failure-allowed: [false] include: - ruby-version: 'truffleruby' failure-allowed: true steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Set up Ruby uses: ruby/setup-ruby@v1 with: diff --git a/.readthedocs.yaml b/.readthedocs.yaml new file mode 100644 index 00000000..67062932 --- /dev/null +++ b/.readthedocs.yaml @@ -0,0 +1,18 @@ +# Read the Docs configuration file for Sphinx projects +# See https://docs.readthedocs.io/en/stable/config-file/v2.html for details +# Required +version: 2 + +# Set the OS, Python version and other tools you might need +build: + os: ubuntu-22.04 + tools: + python: "3.12" + +# Build documentation in the "docs/" directory with Sphinx +sphinx: + configuration: docs/conf.py + +python: + install: + - requirements: docs/requirements.txt diff --git a/.ruby-version b/.ruby-version index e4604e3a..619b5376 100644 --- a/.ruby-version +++ b/.ruby-version @@ -1 +1 @@ -3.2.1 +3.3.3 diff --git a/Gemfile b/Gemfile index 0dbd0ba3..23d3cd29 100644 --- a/Gemfile +++ b/Gemfile @@ -5,8 +5,8 @@ if ENV["CUSTOM_RUBY_VERSION"] end gem 'octokit' -gem 'sinatra', '3.0.5' -gem 'sinatra-contrib', '3.0.5' +gem 'sinatra', '4.0.0' +gem 'sinatra-contrib', '4.0.0' gem 'openssl' gem 'puma' gem 'sidekiq' diff --git a/Gemfile.lock b/Gemfile.lock index 9b71f558..baefa922 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,94 +1,107 @@ GEM remote: https://rubygems.org/ specs: - addressable (2.8.4) - public_suffix (>= 2.0.2, < 6.0) + addressable (2.8.7) + public_suffix (>= 2.0.2, < 7.0) airrecord (1.0.12) faraday (>= 1.0, < 3.0) faraday-net_http_persistent net-http-persistent - bibtex-ruby (6.0.0) + base64 (0.2.0) + bibtex-ruby (6.1.0) latex-decode (~> 0.0) - cgi (0.3.6) - charlock_holmes (0.7.7) + racc (~> 1.7) + bigdecimal (3.1.8) + cgi (0.4.1) + charlock_holmes (0.7.9) chronic (0.10.2) - concurrent-ruby (1.2.2) - connection_pool (2.4.0) - crack (0.4.5) + concurrent-ruby (1.3.4) + connection_pool (2.4.1) + crack (1.0.0) + bigdecimal rexml - diff-lcs (1.5.0) - dotenv (2.8.1) - faraday (2.7.4) - faraday-net_http (>= 2.0, < 3.1) - ruby2_keywords (>= 0.0.4) + diff-lcs (1.5.1) + dotenv (3.1.4) + faraday (2.12.0) + faraday-net_http (>= 2.0, < 3.4) + json + logger faraday-follow_redirects (0.1.0) faraday (>= 2, < 3) - faraday-net_http (3.0.2) + faraday-net_http (3.3.0) + net-http faraday-net_http_persistent (2.1.0) faraday (~> 2.5) net-http-persistent (~> 4.0) - faraday-retry (2.1.0) + faraday-retry (2.2.1) faraday (~> 2.0) - github-linguist (7.25.0) + github-linguist (8.0.1) cgi charlock_holmes (~> 0.7.7) mini_mime (~> 1.0) rugged (~> 1.0) - hashdiff (1.0.1) - issue (1.0.0) - openssl - rack + hashdiff (1.1.1) + issue (1.0.2) + openssl (~> 3.2) + rack (~> 3.1) + json (2.7.2) latex-decode (0.4.0) - licensee (9.16.0) - dotenv (~> 2.0) - octokit (>= 4.20, < 7.0) + licensee (9.17.1) + dotenv (>= 2, < 4) + octokit (>= 4.20, < 10.0) reverse_markdown (>= 1, < 3) rugged (>= 0.24, < 2.0) thor (>= 0.19, < 2.0) - mini_mime (1.1.2) - mini_portile2 (2.8.1) + logger (1.6.1) + mini_mime (1.1.5) + mini_portile2 (2.8.7) multi_json (1.15.0) - mustermann (3.0.0) + mustermann (3.0.3) ruby2_keywords (~> 0.0.1) + net-http (0.4.1) + uri net-http-persistent (4.0.2) connection_pool (~> 2.2) - nio4r (2.5.9) - nokogiri (1.14.3) - mini_portile2 (~> 2.8.0) + nio4r (2.7.3) + nokogiri (1.16.7) + mini_portile2 (~> 2.8.2) racc (~> 1.4) - octokit (6.1.1) + octokit (9.1.0) faraday (>= 1, < 3) sawyer (~> 0.9) - openssl (3.1.0) - public_suffix (5.0.1) - puma (6.2.1) + openssl (3.2.0) + public_suffix (6.0.1) + puma (6.4.3) nio4r (~> 2.0) - racc (1.6.2) - rack (2.2.6.4) - rack-protection (3.0.5) - rack + racc (1.8.1) + rack (3.1.7) + rack-protection (4.0.0) + base64 (>= 0.1.0) + rack (>= 3.0.0, < 4) + rack-session (2.0.0) + rack (>= 3.0.0) rack-test (2.1.0) rack (>= 1.3) - redis-client (0.14.1) + redis-client (0.22.2) connection_pool reverse_markdown (2.1.1) nokogiri - rexml (3.2.5) - rspec (3.12.0) - rspec-core (~> 3.12.0) - rspec-expectations (~> 3.12.0) - rspec-mocks (~> 3.12.0) - rspec-core (3.12.1) - rspec-support (~> 3.12.0) - rspec-expectations (3.12.2) + rexml (3.3.7) + rspec (3.13.0) + rspec-core (~> 3.13.0) + rspec-expectations (~> 3.13.0) + rspec-mocks (~> 3.13.0) + rspec-core (3.13.1) + rspec-support (~> 3.13.0) + rspec-expectations (3.13.3) diff-lcs (>= 1.2.0, < 2.0) - rspec-support (~> 3.12.0) - rspec-mocks (3.12.5) + rspec-support (~> 3.13.0) + rspec-mocks (3.13.1) diff-lcs (>= 1.2.0, < 2.0) - rspec-support (~> 3.12.0) - rspec-support (3.12.0) + rspec-support (~> 3.13.0) + rspec-support (3.13.1) ruby2_keywords (0.0.5) - rugged (1.6.3) + rugged (1.7.2) sawyer (0.9.2) addressable (>= 2.3.5) faraday (>= 0.17.3, < 3) @@ -98,25 +111,28 @@ GEM multi_json (~> 1.15) rexml (~> 3.2, >= 3.2.5) thor (~> 1.2, >= 1.2.1) - sidekiq (7.0.8) + sidekiq (7.3.2) concurrent-ruby (< 2) connection_pool (>= 2.3.0) + logger rack (>= 2.2.4) - redis-client (>= 0.11.0) - sinatra (3.0.5) + redis-client (>= 0.22.2) + sinatra (4.0.0) mustermann (~> 3.0) - rack (~> 2.2, >= 2.2.4) - rack-protection (= 3.0.5) + rack (>= 3.0.0, < 4) + rack-protection (= 4.0.0) + rack-session (>= 2.0.0, < 3) tilt (~> 2.0) - sinatra-contrib (3.0.5) - multi_json + sinatra-contrib (4.0.0) + multi_json (>= 0.0.2) mustermann (~> 3.0) - rack-protection (= 3.0.5) - sinatra (= 3.0.5) + rack-protection (= 4.0.0) + sinatra (= 4.0.0) tilt (~> 2.0) - thor (1.2.1) - tilt (2.1.0) - webmock (3.18.1) + thor (1.3.2) + tilt (2.4.0) + uri (0.13.1) + webmock (3.23.1) addressable (>= 2.8.0) crack (>= 0.3.2) hashdiff (>= 0.4.0, < 2.0.0) @@ -141,9 +157,9 @@ DEPENDENCIES rspec serrano sidekiq - sinatra (= 3.0.5) - sinatra-contrib (= 3.0.5) + sinatra (= 4.0.0) + sinatra-contrib (= 4.0.0) webmock BUNDLED WITH - 2.3.4 + 2.5.14 diff --git a/README.md b/README.md index d87ba458..91575853 100644 --- a/README.md +++ b/README.md @@ -1,8 +1,8 @@ # Buffy -A service to provide a bot helping scientific journals manage submission reviews. +A service to provide a bot helping scientific journals and communities manage submission reviews. -Buffy automates common editorial tasks like those needed by [The Journal of Open Source Software](https://joss.theoj.org/) or [rOpenSci](https://ropensci.org/). +Buffy automates common editorial tasks like those needed by [The Journal of Open Source Software](https://joss.theoj.org/), [rOpenSci](https://ropensci.org/), [JuliaCon](https://proceedings.juliacon.org/) or [pyOpenSci](https://www.pyopensci.org/). [![Build Status](https://github.com/openjournals/buffy/actions/workflows/tests.yml/badge.svg)](https://github.com/openjournals/buffy/actions/workflows/tests.yml) [![Documentation Status](https://readthedocs.org/projects/buffy/badge/?version=latest)](https://buffy.readthedocs.io/en/latest/?badge=latest) diff --git a/app/lib/doi_checker.rb b/app/lib/doi_checker.rb index f2fdeac5..e0e400c8 100644 --- a/app/lib/doi_checker.rb +++ b/app/lib/doi_checker.rb @@ -9,30 +9,52 @@ def initialize(entries=[]) end def check_dois - doi_summary = {ok: [], missing: [], invalid: []} + doi_summary = {ok: [], skip: [], missing: [], invalid: []} if @entries.any? @entries.each do |entry| - if entry.has_field?('doi') && !entry.doi.empty? + # handle special cases first + if special_case = handle_special_case(entry) + doi_validity = special_case + elsif entry.has_field?('doi') && !entry.doi.empty? + # Validate entries with DOIs doi_validity = validate_doi(entry.doi.value) - doi_summary[doi_validity[:validity]].push(doi_validity[:msg]) - # If there's no DOI present, check Crossref to see if we can find a candidate DOI for this entry. elsif entry.has_field?('title') - candidate_doi = crossref_lookup(entry.title.value) - if candidate_doi == "CROSSREF-ERROR" - truncated_title = entry.title.to_s[0,50] - truncated_title += "..." if truncated_title.length < entry.title.to_s.length - doi_summary[:missing].push("Errored finding suggestions for \"#{truncated_title}\", please try later") - elsif candidate_doi - doi_summary[:missing].push("#{candidate_doi} may be a valid DOI for title: #{entry.title}") - end + # Try and find candidate entries if doi absent, but title present + doi_validity = handle_missing_doi(entry) + else + doi_validity = {validity: :missing, msg: "Entry without DOI or title found"} end + + doi_summary[doi_validity[:validity]].push(doi_validity[:msg]) end end doi_summary end + # any special case should return false if not applicable, and an object like + # {:validity => :ok, :msg => "whatever"} otherwise. + # Add additional special cases as private methods and chain in a tidy sequence plz <3 + def handle_special_case(entry) + acm_105555_prefix(entry) || false + end + + + # If there's no DOI present, check Crossref to see if we can find a candidate DOI for this entry. + def handle_missing_doi(entry) + candidate_doi = crossref_lookup(entry.title.value) + truncated_title = entry.title.to_s[0,50] + truncated_title += "..." if truncated_title.length < entry.title.to_s.length + if candidate_doi == "CROSSREF-ERROR" + { validity: :missing, msg: "Errored finding suggestions for \"#{truncated_title}\", please try later" } + elsif candidate_doi + { validity: :missing, msg: "#{candidate_doi} may be a valid DOI for title: #{truncated_title}" } + else + { validity: :skip, msg: "No DOI given, and none found for title: #{truncated_title}" } + end + end + def validate_doi(doi_string) return { validity: :invalid, msg: "Empty DOI string" } if doi_string.nil? || doi_string.empty? @@ -108,4 +130,16 @@ def levenshtein_distance(s, t) def similar?(string_1, string_2) levenshtein_distance(string_1, string_2) < 3 end + + private + + def acm_105555_prefix(entry) + if entry.has_field?('doi') && entry.doi.include?("10.5555/") + { validity: :invalid, msg: "#{entry.doi} is INVALID - 10.5555 is a known broken prefix, replace with https://dl.acm.org/doi/{doi} in the {url} field" } + elsif entry.has_field?('url') && entry.url.include?("https://dl.acm.org/doi/10.5555") + { validity: :skip, msg: "#{entry.url} - non-DOI with 10.5555 correctly placed in the url field, editor should ensure this resolves" } + else + false + end + end end diff --git a/app/lib/paper_file.rb b/app/lib/paper_file.rb index b711ee55..e45a4119 100644 --- a/app/lib/paper_file.rb +++ b/app/lib/paper_file.rb @@ -64,7 +64,7 @@ def self.find(search_path) if Dir.exist? search_path Find.find(search_path).each do |path| - if path =~ /paper\.tex$|paper\.md$/ + if path =~ /\/paper\.tex$|\/paper\.md$/ paper_path = path break end diff --git a/app/lib/responder.rb b/app/lib/responder.rb index 38767810..f094dfb9 100644 --- a/app/lib/responder.rb +++ b/app/lib/responder.rb @@ -181,6 +181,7 @@ def locals from_context = Sinatra::IndifferentHash[ issue_id: context.issue_id, issue_author: context.issue_author, + issue_title: context.issue_title, repo: context.repo, sender: context.sender, bot_name: bot_name ] diff --git a/app/lib/responder_registry.rb b/app/lib/responder_registry.rb index 9e87ce7e..e4db2745 100644 --- a/app/lib/responder_registry.rb +++ b/app/lib/responder_registry.rb @@ -93,7 +93,7 @@ def self.available_responders responder_class = Object.const_get(name) available_responders[responder_class.key] = responder_class rescue NameError => err - logger.warn("There is a mismatch in a Responder class name/module: #{err.message}") + Logger.new(STDOUT).warn("There is a mismatch in a Responder class name/module: #{err.message}") end end diff --git a/app/lib/utilities.rb b/app/lib/utilities.rb index 52679e7f..d16271a2 100644 --- a/app/lib/utilities.rb +++ b/app/lib/utilities.rb @@ -23,9 +23,4 @@ def run_cloc(local_path) status.success? ? result : nil end - def run_gitinspector(local_path) - result, stderr, status = Open3.capture3("PYTHONIOENCODING=utf-8 gitinspector -f** #{local_path}") - status.success? ? result : nil - end - end diff --git a/app/responders/reminders_responder.rb b/app/responders/reminders_responder.rb index 0d2e07d8..dead474e 100644 --- a/app/responders/reminders_responder.rb +++ b/app/responders/reminders_responder.rb @@ -17,7 +17,7 @@ def process_message(message) human = "@#{user_login(context.sender)}" if human == "me" - unless targets.include?(human) + unless targets.include?(human.downcase) respond("#{human} doesn't seem to be a reviewer or author for this submission.") return false end @@ -25,11 +25,12 @@ def process_message(message) schedule_at = target_time(size, unit) if schedule_at - if user_login(human) == user_login(context.sender) + if user_login(human).downcase == user_login(context.sender).downcase + human = "@#{user_login(context.sender)}" msg = ":wave: #{human}, please take a look at the state of the submission (this is an automated reminder)." AsyncMessageWorker.perform_at(schedule_at, serializable(locals), msg) else - ReviewReminderWorker.perform_at(schedule_at, serializable(locals), human, authors_list.include?(human)) + ReviewReminderWorker.perform_at(schedule_at, serializable(locals), human, authors_list.include?(human.downcase)) end respond("Reminder set for #{human} in #{size} #{unit}") else @@ -38,16 +39,16 @@ def process_message(message) end def targets - (authors_list + reviewers_list + ["@#{user_login(context.sender)}"]).uniq + (authors_list + reviewers_list + ["@#{user_login(context.sender).downcase}"]).uniq end def reviewers_list - @reviewers_list ||= reviewers_value.inject([]) {|re, value| re + read_value_from_body(value).split(",").map(&:strip)} + @reviewers_list ||= reviewers_value.inject([]) {|re, value| re + read_value_from_body(value).split(",").map(&:strip).map(&:downcase)} @reviewers_list.compact.uniq end def authors_list - @authors_list ||= authors_value.inject([]) {|au, value| au + read_value_from_body(value).split(",").map(&:strip)} + @authors_list ||= authors_value.inject([]) {|au, value| au + read_value_from_body(value).split(",").map(&:strip).map(&:downcase)} @authors_list.compact.uniq end diff --git a/app/responders/repo_checks_responder.rb b/app/responders/repo_checks_responder.rb index 56ce8300..0909d4e7 100644 --- a/app/responders/repo_checks_responder.rb +++ b/app/responders/repo_checks_responder.rb @@ -6,7 +6,7 @@ class RepoChecksResponder < Responder def define_listening @event_action = "issue_comment.created" - @event_regex = /\A@#{bot_name} check repository(?: from branch ([\/\w-]+))?\.?\s*$/i + @event_regex = /\A@#{bot_name} check repository(?: from branch ([\/\.\w-]+))?\.?\s*$/i end def process_message(message) diff --git a/app/responders/reviewer_checklist_comment_responder.rb b/app/responders/reviewer_checklist_comment_responder.rb index ebdfb126..708e273b 100644 --- a/app/responders/reviewer_checklist_comment_responder.rb +++ b/app/responders/reviewer_checklist_comment_responder.rb @@ -22,7 +22,7 @@ def process_message(message) end def sender_in_reviewers_list? - reviewers.include?("@#{context.sender}") + reviewers.include?("@#{context.sender.downcase}") end def update_checklists_links @@ -38,7 +38,7 @@ def update_checklists_links end def reviewers - @reviewers ||= read_value_from_body("reviewers-list").split(",").map(&:strip) + @reviewers ||= read_value_from_body("reviewers-list").split(",").map(&:strip).map(&:downcase) end def checklists_mapping diff --git a/app/responders/reviewers_list_responder.rb b/app/responders/reviewers_list_responder.rb index 5feb0104..a3392e16 100644 --- a/app/responders/reviewers_list_responder.rb +++ b/app/responders/reviewers_list_responder.rb @@ -6,7 +6,7 @@ class ReviewersListResponder < Responder def define_listening @event_action = "issue_comment.created" - @event_regex = /\A@#{bot_name} (add|remove) (\S+) (to reviewers|from reviewers|as reviewer)\.?\s*$/i + @event_regex = /\A@#{bot_name} (add|remove) +(\S+) +(to reviewers|from reviewers|as reviewer)\.?\s*$/i end def process_message(message) diff --git a/app/responses/doi_checks.erb b/app/responses/doi_checks.erb index cd18696b..529ea2f0 100644 --- a/app/responses/doi_checks.erb +++ b/app/responses/doi_checks.erb @@ -1,14 +1,18 @@ ``` Reference check summary (note 'MISSING' DOIs are suggestions that need verification): <% doi_summary.each do |type, messages| -%> - -<%= type.to_s.upcase %> DOIs - +<% if type.to_s === "ok" %> +✅ <%= type.to_s.upcase %> DOIs +<% elsif type.to_s === "skip" %> +🟡 <%= type.to_s.upcase %> DOIs +<% else %> +❌ <%= type.to_s.upcase %> DOIs +<% end %> <% if messages.empty? -%> - None <% else -%> -<% messages.each do |message| -%> -- <%= message%> +<% messages.each do |message| -%> +- <%= message %> <% end -%> <% end -%> <% end -%> diff --git a/app/workers/buffy_worker.rb b/app/workers/buffy_worker.rb index d5aae24f..71f774d6 100644 --- a/app/workers/buffy_worker.rb +++ b/app/workers/buffy_worker.rb @@ -2,6 +2,7 @@ require 'erb' require 'faraday' require 'sidekiq' +require 'ostruct' require_relative '../lib/defaults' require_relative '../lib/github' diff --git a/app/workers/repo_checks_worker.rb b/app/workers/repo_checks_worker.rb index a922f062..5f40018c 100644 --- a/app/workers/repo_checks_worker.rb +++ b/app/workers/repo_checks_worker.rb @@ -30,20 +30,13 @@ def repo_summary message = "```\nSoftware report:\n" cloc_result = run_cloc(path) - gitinspector_result = run_gitinspector(path) if cloc_result message << "#{cloc_result}" else message << "cloc failed to run analysis of the source code" end - message << "\n\n" - if gitinspector_result - message << "#{gitinspector_result}" - else - message << "gitinspector failed to run statistical information for the repository" - end message << "\n```" respond(message) @@ -52,8 +45,8 @@ def repo_summary def detect_languages repo = Rugged::Repository.new(path) project = Linguist::Repository.new(repo, repo.head.target_id) - - top_3 = project.languages.keys.take(3) + ordered_languages = project.languages.sort_by { |_, size| size }.reverse + top_3 = ordered_languages.first(3).map {|l,s| l} label_issue(top_3) unless top_3.empty? end diff --git a/docs/conf.py b/docs/conf.py index a699493f..c328c734 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -31,6 +31,7 @@ extensions = [ 'sphinx.ext.mathjax', 'recommonmark', + 'sphinx_rtd_theme', ] # Add any paths that contain templates here, relative to this directory. diff --git a/docs/index.rst b/docs/index.rst index 0b0fee46..0e943094 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -2,7 +2,7 @@ Buffy ===== Buffy is an editorial bots generator, a service to provide a bot helping scientific journals manage submission reviews. -Buffy is a configurable Ruby application that –once deployed as a web service listening to incoming GitHub webhooks– provides a bot that interacts during the peer-review process with editors, reviewers and authors to help them perform actions on the review, the software being reviewed and its corresponding paper, automating common editorial tasks like those needed by the `Journal of Open Source Software `_, `rOpenSci `_ or `Scipy `_. +Buffy is a configurable Ruby application that –once deployed as a web service listening to incoming GitHub webhooks– provides a bot that interacts during the peer-review process with editors, reviewers and authors to help them perform actions on the review, the software being reviewed and its corresponding paper, automating common editorial tasks like those needed by the `Journal of Open Source Software `_, `rOpenSci `_, the `Journal of Open Source Education `_ or `Scipy `_. Buffy is an Open Source project, `the code `_ is hosted at GitHub and released under a MIT license. diff --git a/docs/installation.md b/docs/installation.md index a1f27b88..8edfb1ed 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -1,17 +1,18 @@ Installation ============ -Buffy works listening to events received from GitHub and deciding if/how to reply by passing the received payload to different Responders. -You can fork Buffy and configure the responders you want to use for any specific repository. There is no need for the Buffy fork to be located in the same GitHub user/organization as the repo where it will be used. To have Buffy ready and listening to events you can install it locally or deploy it to a server or service platform. -You'll need the following components: +Buffy functions by monitoring events that are sent from a GitHub repository (e.g., openjournals/joss-reviews). Based on the information in these events, Buffy determines whether and how to respond by passing the event data to different [Responders](https://api.rubyonrails.org/v4.1/classes/ActionController/Responder.html). -- A GitHub user to use as the bot with admin permissions on the target repo (usually a member of the organization owning the repo). +You can fork Buffy and configure the responders you want to use for a particular repository, and the fork doesn't necessarily +need to be hosted under the same GitHub user or organization (as the repository where it will be used). For Buffy to be operational, it must be running either through a local installation or deployment to a platform. The following components are necessary for this setup: + +- A GitHub user with administrative permissions on the target repository (typically a member of the organization that owns the repository) is required to act as the bot. - An instance of Buffy running -- A webhook configured in the GitHub repo's settings to send events to Buffy +- A webhook configured in the settings of the GitHub repository that will send events to Buffy (e.g., a `reviews` repository). ### Create the bot GitHub user -This will be the user responding to commands in the reviews repo. +This will be the "user" responding to the commands issued from a reviews repository. **1.** [Sign up at GitHub](https://github.com/join) and create the bot user: @@ -35,18 +36,16 @@ This will be the user responding to commands in the reviews repo. Some applications and services must be available to use by Buffy: - **[Redis](https://redis.io/)**: To process background jobs Buffy needs `redis` installed. -- **[Gitinspector](https://github.com/ejwa/gitinspector)**: The *Respository Checks Responder* performs a statistical analysis using it. - **[cloc](https://github.com/AlDanial/cloc)**: The *Respository Checks Responder* can analyze source code, to run this check `cloc` is used. #### Deployment -We will use here [Heroku](https://www.heroku.com) as an example service to deploy Buffy but you can use any other server or platform. +As an example, we will use [Heroku](https://www.heroku.com) to deploy Buffy. However, any other server or platform can also be used. -**1.** Create a new app in heroku linked to the url of your fork of Buffy. Automatically Heroku will use the `heroku/ruby` buildpack. +**1.** To begin, create a new app in Heroku linked to the URL of your Buffy fork. Heroku will automatically use the `heroku/ruby` buildpack. -- To process background jobs Buffy needs `redis` installed, several add-ons providing it are available: Heroku Redis, RedisGreen, Redis To Go, etc. -- To install the `cloc` dependency there's a buildpack for Heroku available [here](https://github.com/openjournals/heroku-buildpack-cloc). -- Gitinspector can be installed [using npm](https://www.npmjs.com/package/gitinspector). To do so in Heroku, the `heroku/nodejs` buildpack can be added. +- To process background jobs, Buffy needs a `redis` add-on, such as Heroku Redis or RedisGreen etc. +- You can use [this Heroku buildpack](https://github.com/openjournals/heroku-buildpack-cloc) to install the `cloc` dependency. **2.** In the app settings add the following Config Vars: @@ -64,23 +63,24 @@ We will use here [Heroku](https://www.heroku.com) as an example service to deplo There are detailed instructions in the Deploy section of your Heroku app. -**4.** You should now have a public URL with Buffy running. You can test that pointing your browser to `/status`, like for example: `https://your-new-buffy-deploy.herokuapp.com/status` It should return a simple *up and running* message. +**4.** At this point, you should have a public URL pointing to your new Buffy app! To confirm this, you can test it by visiting https://your-new-buffy-deploy.herokuapp.com/status. On success, you should see a basic (*up and running*) message confirming that Buffy is up and running. ### Configure a webhook to send events from GitHub to Buffy -**1.** Go to the settings page of the repository where you want to use buffy. Add a new webhook. +**1.** Navigate to the settings page of the repository that Buffy will be listening to and add a new webhook. ![Add webhook](./images/new_webhook.png "Add webhook") -**2.** Configure the new webhook with: +**2.** Set up the new webhook with the following configuration: Payload URL: /dispatch path at your public buffy url Content type: application/json Secret: The BUFFY_GH_SECRET_TOKEN you configured in the previous step Select individual events to trigger: **issue comments** and **issues** + ![New webhook](./images/webhook.png "New webhook") -If everything went well you should have now your bot responding on the reviews issues. Try `@botname help` for example. +Assuming everything went smoothly, your Buffy instance should now be responding to review issues. You can test this by sending the `@botname help` command from a reviews issue to verify that the bot is functioning as expected. diff --git a/docs/responders/github_action.md b/docs/responders/github_action.md index 2bb50225..7fbe5c5e 100644 --- a/docs/responders/github_action.md +++ b/docs/responders/github_action.md @@ -1,7 +1,7 @@ GitHub Action ============= -This responder triggers workflow run on a GitHub Action using the GitHub API. Optionally if the call is successful (not the result of the workflow run but the call to trigger it) a reply message can be posted as a comment in the issue. +This `buffy` responder dispatches an event to trigger a GitHub Action workflow in a repository, where the workflow is defined by a `.github/workflows/*.yaml` file. If desired, upon a successful event dispatch to trigger the workflow (not the outcome of the workflow run), a reply message can be posted as a comment in the (corresponding review) issue. Allows [labeling](../labeling). ## Listens to @@ -17,7 +17,7 @@ For example, if you configure the command to be _compile pdf_, it will respond t ## Requirements -Some parameters are required for the responder to work: the `command` to invoke it, and the `workflow_repo` and `workflow_name` values to identify the action to run. All can be set using the settings YAML file. +Some parameters are required for the responder to work: the `command` to invoke it, and the `workflow_repo` and `workflow_name` values to identify the action to run. All can be set using the respective settings YAML file (e.g., `buffy/config/settings-.yml`). ## Settings key @@ -54,22 +54,29 @@ If you want to run multiple responders, use an array of these subparams. ## Examples **A complete example:** + +The following snippet sets up `buffy` to react to a `@editorialbot generate pdf` command issued during a `joss` review: + ```yaml ... github_action: - only: editors - command: compile pdf - description: Generates a PDF based on the paper.md file in the repository - workflow_repo: openjournals/reviews - workflow_name: compile-pdf.yml - inputs: - file: paper.md - data-from-issue: - - branch - - target_repository - mapping: - repository: target_repository - number: issue_id + - draft_paper: + command: generate pdf + workflow_repo: openjournals/joss-papers + workflow_name: draft-paper.yml + workflow_ref: master + description: Generates the pdf paper + data_from_issue: + - branch + - target-repository + - issue_id + mapping: + repository_url: target-repository ... ``` -Once the responder is invoked it triggers the _compile-pdf.yml_ workflow on the _openjournals/reviews_ repository passing to it the _file_, _repository_, _branch_ and _number_ inputs. + +Once invoked, this `github_action` responder triggers the [_draft-paper_](https://github.com/openjournals/joss-papers/blob/main/.github/workflows/draft-paper.yml) workflow on the [_openjournals/joss-papers_](https://github.com/openjournals/joss-papers) repository (see the [actions tab](https://github.com/openjournals/joss-papers/actions)). + +The `data_from_issue` field lists the values of _branch_, _target-repository_, and _issue-id_ (which `buffy` fetches from a review issue body) that serve as input arguments for this action. The optional `mapping` field indicates that the value _target-repository_ is mapped to the _repository_url_ variable. + +For additional use cases, please refer to the complete [`settings-production.yml`](https://github.com/openjournals/buffy/blob/joss/config/settings-production.yml) file located under the `joss` branch of `buffy`. diff --git a/docs/responders/repo_checks.md b/docs/responders/repo_checks.md index 9212a995..5f05b3ac 100644 --- a/docs/responders/repo_checks.md +++ b/docs/responders/repo_checks.md @@ -39,7 +39,7 @@ The body of the issue should have the url of the repository marked with HTML com The following values are valid for the `:checks` list: -* `repo summary`: This check performs an analysis of the source code and list authorship, contributions and file types information. +* `repo summary`: This check performs an analysis of the source code and list file types information. * `languages`: This will detect the languages used in the repository and tagged the issue with the top three used languages. * `wordcount`: This will count the number of words in the paper file. * `license`: This will look for an Open Source License in the target repo and reply an error message if no license is found. diff --git a/docs/using_templates.md b/docs/using_templates.md index 92e63805..5354c572 100644 --- a/docs/using_templates.md +++ b/docs/using_templates.md @@ -47,6 +47,7 @@ The content of a template can include placeholders to be filled with the actual When rendering a template, Buffy will use a hash of `key:value` pairs. When a placeholder is found in the template, it will look up for the corresponding key in the hash and insert the value in the template. The hash will always include _at least_: - **issue_id**: The id of the issue - **issue_author**: The handle of the user that opened the issue +- **issue_title**: The title of the issue - **repo**: the name of the repository - **sender**: the handle of the user creating the comment/issue triggering the responder - **bot_name**: the name of the bot user responding diff --git a/package.json b/package.json deleted file mode 100644 index 9bfa221c..00000000 --- a/package.json +++ /dev/null @@ -1,9 +0,0 @@ -{ - "name": "buffy-dependencies", - "description": "Some dependencies to install on deployment", - "scripts": { - "preinstall": "npm install -g gitinspector" - }, - "author": "Juanjo Bazán", - "license": "MIT" -} diff --git a/spec/doi_checker_spec.rb b/spec/doi_checker_spec.rb index 5614c5cd..a5b9937b 100644 --- a/spec/doi_checker_spec.rb +++ b/spec/doi_checker_spec.rb @@ -11,6 +11,7 @@ expect(doi_summary[:ok]).to be_empty expect(doi_summary[:invalid]).to be_empty expect(doi_summary[:missing]).to be_empty + expect(doi_summary[:skip]).to be_empty end it "should classify as invalid entries with invalid DOI" do @@ -25,6 +26,7 @@ expect(doi_summary[:invalid].size).to eq(1) expect(doi_summary[:invalid].first).to eq(validity[:msg]) expect(doi_summary[:missing]).to be_empty + expect(doi_summary[:skip]).to be_empty end it "should classify as ok entries with valid DOI" do @@ -39,6 +41,7 @@ expect(doi_summary[:ok].first).to eq(validity[:msg]) expect(doi_summary[:invalid]).to be_empty expect(doi_summary[:missing]).to be_empty + expect(doi_summary[:skip]).to be_empty end it "should classify as missing entries without DOI but with a candidate crossref entry" do @@ -50,6 +53,7 @@ doi_summary = doi_checker.check_dois expect(doi_summary[:ok]).to be_empty expect(doi_summary[:invalid]).to be_empty + expect(doi_summary[:skip]).to be_empty expect(doi_summary[:missing].size).to eq(1) expect(doi_summary[:missing][0]).to eq("10.maybe/doi may be a valid DOI for title: No DOI") end @@ -63,6 +67,7 @@ doi_summary = doi_checker.check_dois expect(doi_summary[:ok]).to be_empty expect(doi_summary[:invalid]).to be_empty + expect(doi_summary[:skip]).to be_empty expect(doi_summary[:missing].size).to eq(1) expect(doi_summary[:missing][0]).to eq('Errored finding suggestions for "No DOI", please try later') end @@ -78,12 +83,14 @@ doi_summary = doi_checker.check_dois expect(doi_summary[:ok]).to be_empty expect(doi_summary[:invalid]).to be_empty + expect(doi_summary[:skip]).to be_empty expect(doi_summary[:missing].size).to eq(1) expect(doi_summary[:missing][0]).to eq("Errored finding suggestions for \"#{expected_title}\", please try later") end - it "should ignore entries no DOI and no crossref alternative" do - missing_doi = BibTeX::Entry.new({title: "No DOI"}) + it "should report entries with no DOI and no crossref alternative as missing DOIs" do + title = "No DOI" + missing_doi = BibTeX::Entry.new({title: title}) doi_checker = DOIChecker.new([missing_doi]) expect(doi_checker).to receive(:crossref_lookup).with("No DOI").and_return(nil) @@ -93,6 +100,46 @@ expect(doi_summary[:ok]).to be_empty expect(doi_summary[:invalid]).to be_empty expect(doi_summary[:missing]).to be_empty + expect(doi_summary[:skip].size).to eq(1) + expect(doi_summary[:skip][0]).to eq("No DOI given, and none found for title: #{title}") + end + + it "should report entries with no DOI or title as missing both" do + entry = BibTeX::Entry.new(journal: "A Well Respected Journal") + doi_checker = DOIChecker.new([entry]) + + doi_summary = doi_checker.check_dois + expect(doi_summary[:ok]).to be_empty + expect(doi_summary[:invalid]).to be_empty + expect(doi_summary[:skip]).to be_empty + expect(doi_summary[:missing][0]).to eq("Entry without DOI or title found") + end + end + + describe "#handle_special_case" do + it "should treat DOIs with a 10.5555 prefix as invalid" do + entry = BibTeX::Entry.new(doi: "10.5555/xxxxxxx.yyyyyyyyy") + validity = subject.handle_special_case(entry) + expect(validity[:validity]).to eq(:invalid) + expect(validity[:msg]).to include("replace with https://dl.acm.org/doi") + end + + it "should treat URLs with a 10.5555 prefix as a skip" do + entry = BibTeX::Entry.new(url: "https://dl.acm.org/doi/10.5555/2827719.2827740") + validity = subject.handle_special_case(entry) + expect(validity[:validity]).to eq(:skip) + expect(validity[:msg]).to eq("https://dl.acm.org/doi/10.5555/2827719.2827740 - non-DOI with 10.5555 correctly placed in the url field, editor should ensure this resolves") + end + + it "should handle special cases separately from normal DOI checking" do + entry = BibTeX::Entry.new(doi: "10.5555/xxxxxxx.yyyyyyyyy") + doi_checker = DOIChecker.new([entry]) + + doi_summary = doi_checker.check_dois + expect(doi_summary[:ok]).to be_empty + expect(doi_summary[:missing]).to be_empty + expect(doi_summary[:skip]).to be_empty + expect(doi_summary[:invalid][0]).to include("is INVALID - 10.5555 is a known broken prefix, replace with https://dl.acm.org/doi/") end end @@ -203,4 +250,4 @@ end end -end \ No newline at end of file +end diff --git a/spec/paper_file_spec.rb b/spec/paper_file_spec.rb index 5aaf23eb..4727b968 100644 --- a/spec/paper_file_spec.rb +++ b/spec/paper_file_spec.rb @@ -144,7 +144,7 @@ it "should return a nil PaperFile if no paper file found" do expect(Dir).to receive(:exist?).with("/repo/path/").and_return(true) - allow(Find).to receive(:find).with("/repo/path/").and_return(["lib/papers.pdf", "./docs", "app"]) + allow(Find).to receive(:find).with("/repo/path/").and_return(["lib/papers.pdf", "lib/other_paper.md", "./docs", "app"]) paper_file = PaperFile.find("/repo/path/") diff --git a/spec/responder_spec.rb b/spec/responder_spec.rb index 4fceb109..801ffd97 100644 --- a/spec/responder_spec.rb +++ b/spec/responder_spec.rb @@ -335,6 +335,7 @@ before do @responder = described_class.new({ env: {bot_github_user: 'botsci'} }, {}) @responder.context = OpenStruct.new(issue_id: 5, + issue_title: "Test submission", issue_author: "opener", repo: "openjournals/buffy", sender: "user33", @@ -342,18 +343,18 @@ end it "should include basic config info" do - expected_locals = Sinatra::IndifferentHash[issue_id: 5, issue_author: "opener", bot_name: "botsci", repo: "openjournals/buffy", sender: "user33"] + expected_locals = Sinatra::IndifferentHash[issue_id: 5, issue_author: "opener", issue_title: "Test submission", bot_name: "botsci", repo: "openjournals/buffy", sender: "user33"] expect(@responder.locals).to eq(expected_locals) end it "should add info from the issue body if requested" do @responder.params = {data_from_issue: ["reviewer"]} - expected_locals = Sinatra::IndifferentHash[issue_id: 5, issue_author: "opener", bot_name: "botsci", repo: "openjournals/buffy", sender: "user33", "reviewer" => "@xuanxu"] + expected_locals = Sinatra::IndifferentHash[issue_id: 5, issue_author: "opener", issue_title: "Test submission", bot_name: "botsci", repo: "openjournals/buffy", sender: "user33", "reviewer" => "@xuanxu"] expect(@responder.locals).to eq(expected_locals) end it "should add info from the event_regex if present" do - expected_locals = Sinatra::IndifferentHash[issue_id: 5, issue_author: "opener", bot_name: "botsci", repo: "openjournals/buffy", sender: "user33", match_data_1: "@xuanxu"] + expected_locals = Sinatra::IndifferentHash[issue_id: 5, issue_author: "opener", issue_title: "Test submission", bot_name: "botsci", repo: "openjournals/buffy", sender: "user33", match_data_1: "@xuanxu"] @responder.event_regex = /\A@bot assign (.*) as editor\z/i @responder.match_data = @responder.event_regex.match("@bot assign @xuanxu as editor") diff --git a/spec/responders/add_and_remove_user_checklist_responder_spec.rb b/spec/responders/add_and_remove_user_checklist_responder_spec.rb index 19ce085a..4c10af5f 100644 --- a/spec/responders/add_and_remove_user_checklist_responder_spec.rb +++ b/spec/responders/add_and_remove_user_checklist_responder_spec.rb @@ -29,6 +29,7 @@ before do @responder.context = OpenStruct.new(issue_id: 5, issue_author: "opener", + issue_title: "New paper", repo: "openjournals/buffy", sender: "user33", issue_body: "Test Review\n\n ... description ..." + @@ -43,7 +44,7 @@ msg = "@botsci add checklist for @arfon" @responder.match_data = @responder.event_regex.match(msg) - expected_locals = { issue_id: 5, issue_author: "opener", bot_name: "botsci", repo: "openjournals/buffy", sender: "user33", match_data_1: "add", match_data_2: "@arfon" } + expected_locals = { issue_id: 5, issue_author: "opener", issue_title: "New paper", bot_name: "botsci", repo: "openjournals/buffy", sender: "user33", match_data_1: "add", match_data_2: "@arfon" } expected_checklist = "\n" + "\n## Review checklist for @arfon" + "\n[] A" + diff --git a/spec/responders/basic_command_responder_spec.rb b/spec/responders/basic_command_responder_spec.rb index 3d943243..7ef34303 100644 --- a/spec/responders/basic_command_responder_spec.rb +++ b/spec/responders/basic_command_responder_spec.rb @@ -33,6 +33,7 @@ @responder = subject.new({env: {bot_github_user: "botsci"}}, params) @responder.context = OpenStruct.new(issue_id: 15, issue_author: "opener", + issue_title: "New paper", repo: "tests", sender: "rev33", issue_body: "Test Review\n\n ... X...") @@ -43,7 +44,7 @@ expect(@responder).to receive(:respond).with("Here you have the list of editors") expect(@responder).to receive(:respond).with("msg 1") expect(@responder).to receive(:respond).with("msg 2") - expected_params = { bot_name: "botsci", issue_author: "opener", issue_id: 15, repo: "tests", sender: "rev33", "x"=>"X" } + expected_params = { bot_name: "botsci", issue_author: "opener", issue_title: "New paper", issue_id: 15, repo: "tests", sender: "rev33", "x"=>"X" } expect(@responder).to receive(:render_external_template). with("editor_list.md", expected_params). and_return("editor 1 & editor 2") diff --git a/spec/responders/check_references_responder_spec.rb b/spec/responders/check_references_responder_spec.rb index 82762d1a..d5484778 100644 --- a/spec/responders/check_references_responder_spec.rb +++ b/spec/responders/check_references_responder_spec.rb @@ -26,7 +26,7 @@ end describe "#process_message" do - let(:expected_locals) { {"bot_name" => "botsci", "issue_author" => nil, "issue_id" => nil, "repo" => nil, "sender" => nil} } + let(:expected_locals) { {"bot_name" => "botsci", "issue_author" => nil, "issue_title" => nil, "issue_id" => nil, "repo" => nil, "sender" => nil} } it "should respond an error message if no url" do expect(@responder).to receive(:respond).with("I couldn't find the URL for the target repository") diff --git a/spec/responders/external_service_responder_spec.rb b/spec/responders/external_service_responder_spec.rb index 8c3a925f..f568344a 100644 --- a/spec/responders/external_service_responder_spec.rb +++ b/spec/responders/external_service_responder_spec.rb @@ -31,6 +31,7 @@ @responder = subject.new(settings, params) @responder.context = OpenStruct.new(issue_id: 33, issue_author: "opener", + issue_title: "Test title", repo: "openjournals/testing", sender: "xuanxu") disable_github_calls_for(@responder) @@ -53,11 +54,8 @@ end it "should pass right info to the worker" do - params = { name: "test-service", command: "run tests", url: "http://testing.openjournals.org" } - locals = { bot_name: "botsci", issue_author: "opener", issue_id: 33, repo: "openjournals/testing", sender: "xuanxu" } - expected_params = { "name" => "test-service", "command" => "run tests", "url" => "http://testing.openjournals.org", "extra" => {"restrict_access" => true} } - expected_locals = { "bot_name" => "botsci", "issue_author" => "opener", "issue_id" => 33, "repo" => "openjournals/testing", "sender" => "xuanxu"} + expected_locals = { "bot_name" => "botsci", "issue_author" => "opener", "issue_title" => "Test title","issue_id" => 33, "repo" => "openjournals/testing", "sender" => "xuanxu"} expect(ExternalServiceWorker).to receive(:perform_async).with(expected_params, expected_locals) @responder.process_message("") end diff --git a/spec/responders/external_start_review_responder_spec.rb b/spec/responders/external_start_review_responder_spec.rb index fc0e77a9..14c8c457 100644 --- a/spec/responders/external_start_review_responder_spec.rb +++ b/spec/responders/external_start_review_responder_spec.rb @@ -69,6 +69,7 @@ "editor_login" => "arfon", "editor_username" => "@arfon", "issue_author" => "opener", + "issue_title" => "[PRE REVIEW]: TesT", "issue_id" => 33, "repo" => "openjournals/testing", "reviewers_logins" => "xuanxu,karthik", diff --git a/spec/responders/goodbye_responder_spec.rb b/spec/responders/goodbye_responder_spec.rb index c45144f1..b308738c 100644 --- a/spec/responders/goodbye_responder_spec.rb +++ b/spec/responders/goodbye_responder_spec.rb @@ -64,6 +64,7 @@ @responder = subject.new({env: {bot_github_user: "botsci"}}, { template_file: "thanks.md", data_from_issue: ["reviewer"] }) @responder.context = OpenStruct.new(issue_id: 5, issue_author: "opener", + issue_title: "Test paper", repo: "openjournals/buffy", sender: "user33", issue_body: "Test Software Review\n\n@xuanxu") @@ -71,7 +72,7 @@ end it "should populate locals" do - expected_locals = { issue_id: 5, issue_author: "opener", bot_name: "botsci", repo: "openjournals/buffy", sender: "user33", "reviewer" => "@xuanxu" } + expected_locals = { issue_id: 5, issue_author: "opener", issue_title: "Test paper", bot_name: "botsci", repo: "openjournals/buffy", sender: "user33", "reviewer" => "@xuanxu" } expect(@responder).to receive(:respond_external_template).with("thanks.md", expected_locals) @responder.process_message("") @@ -96,6 +97,7 @@ @responder = subject.new(settings, {external_service: params}) @responder.context = OpenStruct.new(issue_id: 33, issue_author: "opener", + issue_title: "Test paper", repo: "openjournals/testing", sender: "xuanxu", issue_body: "Test Review\n\nABC123") @@ -108,7 +110,7 @@ it "should pass right info to the worker" do expected_params = { "name" => "test-service", "url" => "http://testing.openjournals.org", "data_from_issue" => ["extra-data"] } - expected_locals = { "extra-data" => "ABC123", "bot_name" => "botsci", "issue_author" => "opener", "issue_id" => 33, "repo" => "openjournals/testing", "sender" => "xuanxu" } + expected_locals = { "extra-data" => "ABC123", "bot_name" => "botsci", "issue_author" => "opener", "issue_title" => "Test paper", "issue_id" => 33, "repo" => "openjournals/testing", "sender" => "xuanxu" } expect(ExternalServiceWorker).to receive(:perform_async).with(expected_params, expected_locals) @responder.process_message("") end diff --git a/spec/responders/reminders_responder_spec.rb b/spec/responders/reminders_responder_spec.rb index 34cdbcf8..993d8152 100644 --- a/spec/responders/reminders_responder_spec.rb +++ b/spec/responders/reminders_responder_spec.rb @@ -5,7 +5,7 @@ before do settings = { env: { bot_github_user: "botsci" }} @responder = RemindersResponder.new(settings, {}) - @responder.context = OpenStruct.new(issue_body: "", sender: "editor21") + @responder.context = OpenStruct.new(issue_body: "", issue_title: "", sender: "editor21") disable_github_calls_for(@responder) end @@ -56,6 +56,19 @@ @responder.process_message(msg) end + it "should be case insensitive with the editor's GitHub handles" do + msg = "@botsci remind @eDItoR21 in 5 weeks" + @responder.match_data = @responder.event_regex.match(msg) + in_five_weeks = Chronic.parse("in 5 weeks") + expect(@responder).to receive(:target_time).with("5", "weeks").and_return(in_five_weeks) + expected_msg = ":wave: @editor21, please take a look at the state of the submission (this is an automated reminder)." + expect(AsyncMessageWorker).to receive(:perform_at).with(in_five_weeks, @responder.locals, expected_msg) + expect(ReviewReminderWorker).to_not receive(:perform_at) + expect(@responder).to receive(:respond).with("Reminder set for @editor21 in 5 weeks") + + @responder.process_message(msg) + end + it "should respond success message and schedule worker run for 'me'" do msg = "@botsci remind me in 15 days" @responder.match_data = @responder.event_regex.match(msg) @@ -88,6 +101,26 @@ expect(ReviewReminderWorker).to receive(:perform_at).with(in_four_days, @responder.locals, "@author", true) @responder.process_message(msg) end + + it "should be case insensitive with the reviewers GitHub handles" do + msg = "@botsci remind @ReVieWEr42 in 3 weeks" + @responder.match_data = @responder.event_regex.match(msg) + expect(ReviewReminderWorker).to receive(:perform_at) + expect(@responder).to receive(:respond).with("Reminder set for @ReVieWEr42 in 3 weeks") + + @responder.process_message(msg) + end + + it "should be case insensitive with the authors GitHub handles" do + msg = "@botsci remind @AUTHor in 4 days" + @responder.match_data = @responder.event_regex.match(msg) + + in_four_days = Chronic.parse("in 4 days") + expect(@responder).to receive(:target_time).with("4", "days").and_return(in_four_days) + + expect(ReviewReminderWorker).to receive(:perform_at).with(in_four_days, @responder.locals, "@AUTHor", true) + @responder.process_message(msg) + end end describe "configurable targets" do diff --git a/spec/responders/repo_checks_responder_spec.rb b/spec/responders/repo_checks_responder_spec.rb index 051e69dc..2ffdc02a 100644 --- a/spec/responders/repo_checks_responder_spec.rb +++ b/spec/responders/repo_checks_responder_spec.rb @@ -20,7 +20,7 @@ expect(@responder.event_regex).to match("@botsci check repository") expect(@responder.event_regex).to match("@botsci check repository \r\n") expect(@responder.event_regex).to match("@botsci check repository\r\nmore") - expect(@responder.event_regex).to match("@botsci check repository from branch custom-branch") + expect(@responder.event_regex).to match("@botsci check repository from branch custom-branch_6.0") expect(@responder.event_regex).to match("@botsci check repository from branch custom/branch") expect(@responder.event_regex).to match("@botsci check repository from branch development \r\n") expect(@responder.event_regex).to_not match("@botsci check repository from branch ") @@ -28,7 +28,7 @@ end describe "#process_message" do - let(:expected_locals) { {"bot_name" => "botsci", "issue_author" => nil, "issue_id" => nil, "repo" => nil, "sender" => nil} } + let(:expected_locals) { {"bot_name" => "botsci", "issue_author" => nil, "issue_title" => nil, "issue_id" => nil, "repo" => nil, "sender" => nil} } it "should respond an error message if no url" do @responder.params[:url_field] = "url" diff --git a/spec/responders/reviewer_checklist_comment_responder_spec.rb b/spec/responders/reviewer_checklist_comment_responder_spec.rb index a3cb912e..dd43a441 100644 --- a/spec/responders/reviewer_checklist_comment_responder_spec.rb +++ b/spec/responders/reviewer_checklist_comment_responder_spec.rb @@ -29,6 +29,7 @@ before do @responder.context = OpenStruct.new(issue_id: 5, issue_author: "opener", + issue_title: "New paper", repo: "openjournals/buffy", comment_id: 111222, issue_body: "Test Submission\n\n ... description ... \n\n" + @@ -42,7 +43,7 @@ it "should add user checklist for reviewer" do @responder.context[:sender] = "reviewer1" - expected_locals = { issue_id: 5, issue_author: "opener", bot_name: "botsci", repo: "openjournals/buffy", sender: "reviewer1" } + expected_locals = { issue_id: 5, issue_author: "opener", bot_name: "botsci", issue_title: "New paper", repo: "openjournals/buffy", sender: "reviewer1" } expected_checklist = "Checklist for @reviewer1 \n[] A" expect(@responder).to receive(:render_external_template).with("checklist.md", expected_locals).and_return(expected_checklist) @@ -51,6 +52,18 @@ @responder.process_message(@msg) end + it "should be case insensitive for the reviewer's username" do + @responder.context[:sender] = "ReVIEwer1" + + expected_locals = { issue_id: 5, issue_author: "opener", issue_title: "New paper", bot_name: "botsci", repo: "openjournals/buffy", sender: "ReVIEwer1" } + expected_checklist = "Checklist for @ReVIEwer1 \n[] A" + + expect(@responder).to receive(:render_external_template).with("checklist.md", expected_locals).and_return(expected_checklist) + expect(@responder).to receive(:update_comment).with(111222, expected_checklist) + expect(@responder).to_not receive(:respond) + @responder.process_message(@msg) + end + it "should not add user checklist if sender is not a reviewer" do @responder.context[:sender] = "nonreviewer" @@ -63,7 +76,7 @@ before do @responder.context[:sender] = "reviewer1" - expected_locals = { issue_id: 5, issue_author: "opener", bot_name: "botsci", repo: "openjournals/buffy", sender: "reviewer1" } + expected_locals = { issue_id: 5, issue_author: "opener", issue_title: "New paper", bot_name: "botsci", repo: "openjournals/buffy", sender: "reviewer1" } expected_checklist = "Checklist for @reviewer1 \n[] A" expect(@responder).to receive(:render_external_template).with("checklist.md", expected_locals).and_return(expected_checklist) diff --git a/spec/responders/reviewers_list_responder_spec.rb b/spec/responders/reviewers_list_responder_spec.rb index 0a6976cf..f231ac1e 100644 --- a/spec/responders/reviewers_list_responder_spec.rb +++ b/spec/responders/reviewers_list_responder_spec.rb @@ -15,6 +15,9 @@ it "should define regex" do expect(@responder.event_regex).to match("@botsci add @arfon to reviewers") + expect(@responder.event_regex).to match("@botsci add @arfon to reviewers") + expect(@responder.event_regex).to match("@botsci add @arfon to reviewers") + expect(@responder.event_regex).to match("@botsci add @arfon to reviewers") expect(@responder.event_regex).to match("@botsci add @arfon as reviewer") expect(@responder.event_regex).to match("@botsci add me as reviewer") expect(@responder.event_regex).to match("@botsci remove me from reviewers") @@ -22,6 +25,8 @@ expect(@responder.event_regex).to match("@botsci remove @arfon from reviewers \r\n") expect(@responder.event_regex).to match("@botsci remove @arfon from reviewers \r\n more ") expect(@responder.event_regex).to_not match("@botsci add to reviewers") + expect(@responder.event_regex).to_not match("@botsci add@arfon to reviewers") + expect(@responder.event_regex).to_not match("@botsci add @arfonto reviewers") expect(@responder.event_regex).to_not match("@botsci remove from reviewers") end end diff --git a/spec/responders/set_value_responder_spec.rb b/spec/responders/set_value_responder_spec.rb index 019a5e9a..c0de248a 100644 --- a/spec/responders/set_value_responder_spec.rb +++ b/spec/responders/set_value_responder_spec.rb @@ -154,7 +154,7 @@ describe "with config option: template_file" do before do @responder = subject.new({env: {bot_github_user: "botsci"}}, { name: "version", if_missing: "error", template_file: "version_changed.md"}) - @responder.context = OpenStruct.new(issue_id: 5, issue_author: "opener", repo: "openjournals/buffy", sender: "user33") + @responder.context = OpenStruct.new(issue_id: 5, issue_author: "opener", issue_title: "Title!", repo: "openjournals/buffy", sender: "user33") disable_github_calls_for(@responder) @msg = "@botsci set v0.0.33-alpha as version" @responder.match_data = @responder.event_regex.match(@msg) @@ -168,6 +168,7 @@ value: "v0.0.33-alpha", bot_name: "botsci", issue_author: "opener", + issue_title: "Title!", issue_id: 5, match_data_1: "v0.0.33-alpha", repo: "openjournals/buffy", diff --git a/spec/responders/update_comment_responder_spec.rb b/spec/responders/update_comment_responder_spec.rb index 2bbe5a0f..46afa2cc 100644 --- a/spec/responders/update_comment_responder_spec.rb +++ b/spec/responders/update_comment_responder_spec.rb @@ -29,6 +29,7 @@ before do @responder.context = OpenStruct.new(issue_id: 5, issue_author: "opener", + issue_title: "Title test", sender: "editor33", repo: "openjournals/buffy", comment_id: 111222, @@ -39,7 +40,7 @@ context "#process_message" do it "should update original comment" do - expected_locals = { issue_id: 5, issue_author: "opener", bot_name: "botsci", repo: "openjournals/buffy", sender: "editor33" } + expected_locals = { issue_id: 5, issue_author: "opener", issue_title: "Title test", bot_name: "botsci", repo: "openjournals/buffy", sender: "editor33" } expected_checklist = "Final tasks: \n[ ] A\n[ ] B" expect(@responder).to receive(:render_external_template).with("final-checklist.md", expected_locals).and_return(expected_checklist) diff --git a/spec/responders/welcome_responder_spec.rb b/spec/responders/welcome_responder_spec.rb index 7672f5a0..dab1f029 100644 --- a/spec/responders/welcome_responder_spec.rb +++ b/spec/responders/welcome_responder_spec.rb @@ -67,6 +67,7 @@ @responder = subject.new({env: {bot_github_user: "botsci"}}, { template_file: "test.md", data_from_issue: ["reviewer"] }) @responder.context = OpenStruct.new(issue_id: 5, issue_author: "opener", + issue_title: "Test paper", repo: "openjournals/buffy", sender: "user33", issue_body: "Test Software Review\n\n@xuanxu") @@ -74,7 +75,7 @@ end it "should populate locals" do - expected_locals = { issue_id: 5, issue_author: "opener", bot_name: "botsci", repo: "openjournals/buffy", sender: "user33", "reviewer" => "@xuanxu" } + expected_locals = { issue_id: 5, issue_author: "opener", issue_title: "Test paper", bot_name: "botsci", repo: "openjournals/buffy", sender: "user33", "reviewer" => "@xuanxu" } expect(@responder).to receive(:respond_external_template).with("test.md", expected_locals) @responder.process_message("") @@ -154,6 +155,7 @@ @responder = subject.new(settings, {external_service: params}) @responder.context = OpenStruct.new(issue_id: 33, issue_author: "opener", + issue_title: "Test paper", repo: "openjournals/testing", sender: "xuanxu", issue_body: "Test Review\n\nABC123") @@ -166,7 +168,7 @@ it "should pass right info to the worker" do expected_params = { "name" => "test-service", "url" => "http://testing.openjournals.org", "data_from_issue" => ["extra-data"] } - expected_locals = { "extra-data" => "ABC123", "bot_name" => "botsci", "issue_author" => "opener", "issue_id" => 33, "repo" => "openjournals/testing", "sender" => "xuanxu" } + expected_locals = { "extra-data" => "ABC123", "bot_name" => "botsci", "issue_author" => "opener", "issue_title" => "Test paper", "issue_id" => 33, "repo" => "openjournals/testing", "sender" => "xuanxu" } expect(ExternalServiceWorker).to receive(:perform_async).with(expected_params, expected_locals) @responder.process_message("") end @@ -177,6 +179,7 @@ @settings = { env: {bot_github_user: "botsci"} } @context = OpenStruct.new(issue_id: 33, issue_author: "opener", + issue_title: "Title!", repo: "openjournals/testing", sender: "xuanxu", issue_body: "Test Review\n\nABC123") diff --git a/spec/responders/wrong_command_responder_spec.rb b/spec/responders/wrong_command_responder_spec.rb index 29e60e66..5fb55f4c 100644 --- a/spec/responders/wrong_command_responder_spec.rb +++ b/spec/responders/wrong_command_responder_spec.rb @@ -28,6 +28,7 @@ @responder = subject.new({env: { bot_github_user: "botsci" }, responders: { help: {} }}, {}) @responder.context = OpenStruct.new(issue_id: 15, issue_author: "opener", + issue_title: "Test paper", repo: "tests", sender: "rev33", issue_body: "Test Review\n\n ... X...") @@ -51,7 +52,7 @@ it "should respond with custom template" do @responder.params = { template_file: "wrong_command.md" } - expected_params = { bot_name: "botsci", issue_author: "opener", issue_id: 15, repo: "tests", sender: "rev33", match_data_1: "blah blah" } + expected_params = { bot_name: "botsci", issue_author: "opener", issue_title: "Test paper", issue_id: 15, repo: "tests", sender: "rev33", match_data_1: "blah blah" } expect(@responder).to receive(:render_external_template). with("wrong_command.md", expected_params). and_return("I don't understand `blah blah`") @@ -62,7 +63,7 @@ it "should give precedence to template over custom message" do @responder.params = { template_file: "wrong_command.md", message: "what?"} - expected_params = { bot_name: "botsci", issue_author: "opener", issue_id: 15, repo: "tests", sender: "rev33", match_data_1: "blah blah" } + expected_params = { bot_name: "botsci", issue_author: "opener", issue_title: "Test paper", issue_id: 15, repo: "tests", sender: "rev33", match_data_1: "blah blah" } expect(@responder).to receive(:render_external_template). with("wrong_command.md", expected_params). and_return("I don't understand `blah blah`") diff --git a/spec/utilities_spec.rb b/spec/utilities_spec.rb index 1dd9516c..7a14543f 100644 --- a/spec/utilities_spec.rb +++ b/spec/utilities_spec.rb @@ -81,22 +81,4 @@ class TestingUtilities end end - describe "#run_gitinspector" do - it "should run cloc return true" do - expect(Open3).to receive(:capture3). - with("PYTHONIOENCODING=utf-8 gitinspector -f** tmp/543/repo"). - and_return(["OK", "", OpenStruct.new(success?: true)]) - - expect(subject.run_gitinspector("tmp/543/repo")).to be_truthy - end - - it "should return nil if command fails" do - expect(Open3).to receive(:capture3). - with("PYTHONIOENCODING=utf-8 gitinspector -f** tmp/543/repo"). - and_return(["", "stats failed", OpenStruct.new(success?: false)]) - - expect(subject.run_gitinspector("tmp/543/repo")).to be_falsy - end - end - end diff --git a/spec/workers/repo_checks_worker_spec.rb b/spec/workers/repo_checks_worker_spec.rb index 28075553..3a7eb99b 100644 --- a/spec/workers/repo_checks_worker_spec.rb +++ b/spec/workers/repo_checks_worker_spec.rb @@ -43,7 +43,6 @@ describe "#repo_summary" do before do allow(@worker).to receive(:run_cloc).and_return("Ruby 50%, Julia 50%") - allow(@worker).to receive(:run_gitinspector).and_return("Author: Buffy Summers") end it "should include cloc report" do @@ -51,28 +50,17 @@ @worker.repo_summary end - it "should include gitinspector report" do - expect(@worker).to receive(:respond).with(/Author: Buffy Summers/) - @worker.repo_summary - end - it "should include error message if cloc fails" do expect(@worker).to receive(:run_cloc).and_return(nil) expect(@worker).to receive(:respond).with(/cloc failed to run/) @worker.repo_summary end - - it "should include gitinspector report" do - expect(@worker).to receive(:run_gitinspector).and_return(nil) - expect(@worker).to receive(:respond).with(/gitinspector failed to run/) - @worker.repo_summary - end end describe "#detect_languages" do before do repo = OpenStruct.new(head: OpenStruct.new(target_id: 33)) - expected_languages = OpenStruct.new(languages: {"Ruby"=>176110, "HTML"=>664, "TeX"=>475, "Go"=>21}) + expected_languages = OpenStruct.new(languages: {"Go"=>21, "HTML"=>664, "Ruby"=>176110, "TeX"=>475, "XML" => 100}) allow(Rugged::Repository).to receive(:new).and_return(repo) allow(Linguist::Repository).to receive(:new).with(repo, 33).and_return(expected_languages) end