Skip to content

Commit

Permalink
Merge pull request #1228 from openjournals/feature-tracks
Browse files Browse the repository at this point in the history
Feature: tracks
  • Loading branch information
xuanxu committed Jun 1, 2023
2 parents 78deb9b + 7386116 commit 39ab2b0
Show file tree
Hide file tree
Showing 25 changed files with 506 additions and 228 deletions.
7 changes: 6 additions & 1 deletion app/controllers/onboardings_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,12 @@ def add_editor
flash[:notice] = "Thanks! An editor in chief will review your info soon"
redirect_to root_path
else
flash[:error] = "Error saving your data: Name, Email, GitHub username and Tracks are mandatory"
if JournalFeatures.tracks?
flash[:error] = "Error saving your data: Name, Email, GitHub username and Tracks are mandatory"
else
flash[:error] = "Error saving your data: Name, Email and GitHub username are mandatory"
end

redirect_to editor_onboardings_path(@onboarding.token)
end
end
Expand Down
13 changes: 9 additions & 4 deletions app/mailers/notifications.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,22 @@ class Notifications < ApplicationMailer
EDITOR_EMAILS = [Rails.application.settings["editor_email"]]

def submission_email(paper)
aeic_emails = paper.track.aeic_emails
aeic_emails = paper.track.aeic_emails if paper.track.present?
@url = "#{Rails.application.settings["url"]}/papers/#{paper.sha}"
@paper = paper
mail(to: aeic_emails, cc: EDITOR_EMAILS, subject: "New submission: #{paper.title}")
if aeic_emails.present?
mail(to: aeic_emails, cc: EDITOR_EMAILS, subject: "New submission: #{paper.title}")
else
mail(to: EDITOR_EMAILS, subject: "New submission: #{paper.title}")
end
end

def notify_new_aeic(paper, old_track, new_track)
aeic_emails = new_track.aeic_emails
@url = "#{Rails.application.settings["url"]}/papers/#{paper.sha}"
@paper = paper
@old_track = old_track
@new_track = new_track
@from_track_info = old_track.present? ? "from the #{old_track.name} track." : ""
@new_track_name = new_track.name
mail(to: aeic_emails, cc: EDITOR_EMAILS, subject: "Track move for submission: #{paper.title}")
end

Expand Down Expand Up @@ -46,4 +50,5 @@ def onboarding_invitation_email(onboarding_invitation)
@onboarding_invitation = onboarding_invitation
mail(to: onboarding_invitation.email, subject: "Invitation to join the #{Rails.application.settings["abbreviation"]} editorial board")
end

end
2 changes: 1 addition & 1 deletion app/models/editor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ class Editor < ApplicationRecord
validates :last_name, presence: true
validates :email, presence: true, unless: Proc.new { |editor| editor.kind == "emeritus" }
validates :login, presence: true, unless: Proc.new { |editor| editor.kind == "emeritus" }
validates :tracks, presence: true, unless: Proc.new { |editor| ["board", "emeritus"].include?(editor.kind) }
validates :tracks, presence: true, unless: Proc.new { |editor| ["board", "emeritus"].include?(editor.kind) || !JournalFeatures.tracks? }

belongs_to :user, optional: true
has_many :papers
Expand Down
5 changes: 3 additions & 2 deletions app/models/paper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ class Paper < ApplicationRecord
validates_presence_of :repository_url, message: "Repository address can't be blank"
validates_presence_of :software_version, message: "Version can't be blank"
validates_presence_of :body, message: "Description can't be blank"
validates_presence_of :track_id, on: :create, message: "You must select a valid subject for the paper"
validates_presence_of :track_id, on: :create, message: "You must select a valid subject for the paper", if: Proc.new { JournalFeatures.tracks? }
validates :kind, inclusion: { in: Rails.application.settings["paper_types"] }, allow_nil: true
validates :submission_kind, inclusion: { in: SUBMISSION_KINDS, message: "You must select a submission type" }, allow_nil: false
validate :check_repository_address, on: :create
Expand Down Expand Up @@ -403,7 +403,8 @@ def create_meta_review_issue(editor_handle, eic, new_track_id=nil)
return false if meta_review_issue_id

set_track_id(new_track_id) if new_track_id.present?
new_labels = ["pre-review", self.track.label]
new_labels = ["pre-review"]
new_labels << self.track.label if self.track && JournalFeatures.tracks?

issue = GITHUB.create_issue(Rails.application.settings["reviews"],
"[PRE REVIEW]: #{self.title}",
Expand Down
2 changes: 1 addition & 1 deletion app/views/aeic_dashboard/_menu.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
<%= link_to "Onboarding", onboardings_path, class: current_class?(onboardings_path) %>
</div>

<% if [aeic_dashboard_path, editors_path, invitations_path].include?(request.path) %>
<% if [aeic_dashboard_path, editors_path, invitations_path].include?(request.path) && JournalFeatures.tracks? %>
<div class="float-right">
<%= select_tag :track_id, options_from_collection_for_select(Track.all, "id", "name", params[:track_id].presence), include_blank: "All tracks", class: "form-control left", style: "font-size:81%", onchange: "top.location.href='?track_id=' + this.options[this.selectedIndex].value;" %>
</div>
Expand Down
2 changes: 2 additions & 0 deletions app/views/editors/_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@
<%= f.text_field :url, class: "form-control" %>
</div>

<% if JournalFeatures.tracks? %>
<div class="form-group">
<%= f.label :tracks %>
<div class="row">
Expand All @@ -99,6 +100,7 @@
</div>
</div>
</div>
<% end %>

<div class="form-group">
<%= f.label :description %>
Expand Down
2 changes: 2 additions & 0 deletions app/views/editors/profile.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@
</div>
</div>

<% if JournalFeatures.tracks? %>
<div class="form-group">
<%= f.label :tracks %>
<div class="row">
Expand All @@ -88,6 +89,7 @@
</div>
</div>
</div>
<% end %>

<div class="form-group">
<%= f.label :description %>
Expand Down
2 changes: 1 addition & 1 deletion app/views/home/_menu.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
<% end %>
</div>

<% if [dashboard_incoming_path, dashboard_in_progress_path, dashboard_all_path].include?(request.path) %>
<% if [dashboard_incoming_path, dashboard_in_progress_path, dashboard_all_path].include?(request.path) && JournalFeatures.tracks? %>
<div class="float-right">
<%= select_tag :track_id, options_from_collection_for_select(Track.all, "id", "name", params[:track_id].presence), include_blank: "All tracks", class: "form-control left", style: "font-size:81%", onchange: "top.location.href='?track_id=' + this.options[this.selectedIndex].value;" %>
</div>
Expand Down
2 changes: 1 addition & 1 deletion app/views/notifications/notify_new_aeic.html.erb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
A paper has just been moved to your track (<%= @new_track.name %>) from the <%= @old_track.name %> track.<br />
A paper has just been moved to your track (<%= @new_track_name %>) <%= @from_track_info %><br />
<br />
Title: <%= @paper.title %><br />
Repo: <%= @paper.repository_url %><br />
Expand Down
2 changes: 1 addition & 1 deletion app/views/notifications/notify_new_aeic.text.erb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
A paper has just been moved to your track (<%= @new_track.name %>) from the <%= @old_track.name %> track.
A paper has just been moved to your track (<%= @new_track_name %>) <%= @from_track_info %>

Title: <%= @paper.title %>
Repo: <%= @paper.repository_url %>
Expand Down
2 changes: 2 additions & 0 deletions app/views/onboardings/editor.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
</div>
</div>

<% if JournalFeatures.tracks? %>
<div class="form-group">
<%= f.label :tracks %>
<div class="row">
Expand All @@ -72,6 +73,7 @@
</div>
</div>
</div>
<% end %>

<div class="form-group">
<%= f.label :description %>
Expand Down
2 changes: 1 addition & 1 deletion app/views/papers/_actions.html.erb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<% if current_user.aeic? %>
<% if current_user.aeic? && JournalFeatures.tracks? %>
<div class="card" id="track-info">
<div class="card-header">Track</div>
<div class="card-body">
Expand Down
2 changes: 2 additions & 0 deletions config/settings-development.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,5 @@ submission_enquiry: |-
Briefly describe your software, its research application and any questions you have about the JOSS submission criteria.
paper_types: []
bot_username: editorialbot
features:
tracks: true
2 changes: 2 additions & 0 deletions config/settings-production.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,5 @@ submission_enquiry: |-
Briefly describe your software, its research application and any questions you have about the JOSS submission criteria.
paper_types: []
bot_username: editorialbot
features:
tracks: true
2 changes: 2 additions & 0 deletions config/settings-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,5 @@ submission_enquiry: |-
Briefly describe your software, its research application and any questions you have about the JOSS submission criteria.
paper_types: []
bot_username: editorialbot
features:
tracks: true
5 changes: 5 additions & 0 deletions lib/journal_features.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module JournalFeatures
def self.tracks?
return !!Rails.application.settings.dig(:features, :tracks)
end
end
2 changes: 1 addition & 1 deletion spec/factories/editors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
url { "http://placekitten.com" }
description { "Person McEditor is an editor" }
availability_comment { "OOO until March 1" }
track_ids { ["board", "emeritus"].include?(kind) ? [] : [create(:track).id]}
track_ids { (["board", "emeritus"].include?(kind) ? [] : [create(:track).id]) if JournalFeatures.tracks? }

factory :board_editor do
kind { "board" }
Expand Down
89 changes: 62 additions & 27 deletions spec/models/paper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,22 +39,38 @@
expect(paper.submitting_author).to eq(user)
end

it "must have a track assigned on creation" do
no_track_params = { title: 'Test paper',
body: 'A test paper description',
repository_url: 'http://github.com/arfon/fidgit',
software_version: 'v1.0.0',
submitting_author: create(:user),
submission_kind: 'new' }

valid_params = no_track_params.merge track: create(:track)

paper = Paper.create(no_track_params)
expect(paper).to_not be_valid
expect(paper.errors.full_messages.first).to eq("Track You must select a valid subject for the paper")

paper = Paper.create(valid_params)
expect(paper).to be_valid
it "must have a track assigned on creation if tracks are enabled" do
enable_feature(:tracks) do
no_track_params = { title: 'Test paper',
body: 'A test paper description',
repository_url: 'http://github.com/arfon/fidgit',
software_version: 'v1.0.0',
submitting_author: create(:user),
submission_kind: 'new' }

valid_params = no_track_params.merge track: create(:track)

paper = Paper.create(no_track_params)
expect(paper).to_not be_valid
expect(paper.errors.full_messages.first).to eq("Track You must select a valid subject for the paper")

paper = Paper.create(valid_params)
expect(paper).to be_valid
end
end

it "does not have a track assigned on creation if tracks are disabled" do
disable_feature(:tracks) do
no_track_params = { title: 'Test paper',
body: 'A test paper description',
repository_url: 'http://github.com/arfon/fidgit',
software_version: 'v1.0.0',
submitting_author: create(:user),
submission_kind: 'new' }

paper = Paper.create(no_track_params)
expect(paper).to be_valid
end
end

# Scopes
Expand Down Expand Up @@ -265,20 +281,39 @@
expect(paper.reload.track).to eq(track2)
end

it "should label GH issue with track label" do
editor = create(:editor, login: "arfon")
user = create(:user, editor: editor)
submitting_author = create(:user)
it "should label GH issue with track label if tracks are enabled" do
enable_feature(:tracks) do
editor = create(:editor, login: "arfon")
user = create(:user, editor: editor)
submitting_author = create(:user)

paper = create(:submitted_paper_with_sha, submitting_author: submitting_author)
fake_issue = Object.new
allow(fake_issue).to receive(:number).and_return(1)
paper = create(:submitted_paper_with_sha, submitting_author: submitting_author)
fake_issue = Object.new
allow(fake_issue).to receive(:number).and_return(1)

track = create(:track)
expected_labels = { labels: "pre-review,#{track.label}" }
expect(GITHUB).to receive(:create_issue).with(anything, anything, anything, expected_labels).and_return(fake_issue)

paper.start_meta_review!('arfon', editor, track.id)
end
end

it "should not label GH issue with track label if tracks are disabled" do
disable_feature(:tracks) do
editor = create(:editor, login: "arfon")
user = create(:user, editor: editor)
submitting_author = create(:user)

track = create(:track)
expected_labels = { labels: "pre-review,#{track.label}" }
expect(GITHUB).to receive(:create_issue).with(anything, anything, anything, expected_labels).and_return(fake_issue)
paper = create(:submitted_paper_with_sha, submitting_author: submitting_author)
fake_issue = Object.new
allow(fake_issue).to receive(:number).and_return(1)

paper.start_meta_review!('arfon', editor, track.id)
expected_labels = { labels: "pre-review" }
expect(GITHUB).to receive(:create_issue).with(anything, anything, anything, expected_labels).and_return(fake_issue)

paper.start_meta_review!('arfon', editor)
end
end
end

Expand Down
18 changes: 18 additions & 0 deletions spec/support/common_actions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,22 @@ def skip_paper_repo_url_check
allow(Open3).to receive(:capture3).with("git ls-remote https://github.com/openjournals/joss").and_return(["", "", ok ])
allow(Open3).to receive(:capture3).with("git ls-remote https://github.com/openjournals/joss-reviews").and_return(["", "", ok ])
end

def disable_feature(feat, &block)
previous_value = Rails.application.settings.dig(:features, feat.to_sym)
Rails.application.settings[:features][feat.to_sym] = false
if block_given?
yield
Rails.application.settings[:features][feat.to_sym] = previous_value
end
end

def enable_feature(feat, &block)
previous_value = Rails.application.settings.dig(:features, feat.to_sym)
Rails.application.settings[:features][feat.to_sym] = true
if block_given?
yield
Rails.application.settings[:features][feat.to_sym] = previous_value
end
end
end
29 changes: 28 additions & 1 deletion spec/system/dashboard_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,34 @@
expect(page).to have_content("Paper Rejected")
end

feature "Filter by track" do
scenario "Dropdown is not visible if tracks are disabled" do
disable_feature(:tracks) do
expect(page).to_not have_css("select#track_id")

visit dashboard_incoming_path
expect(page).to_not have_css("select#track_id")

visit "/dashboard/#{Editor.last.login}"
expect(page).to_not have_css("select#track_id")

visit dashboard_in_progress_path
expect(page).to_not have_css("select#track_id")

visit "/dashboard"
expect(page).to_not have_css("select#track_id")

visit dashboard_all_path
expect(page).to_not have_css("select#track_id")
end
end

feature "Filter by track, if tracks are enabled" do
around(:example) do |ex|
enable_feature(:tracks) do
ex.run
end
end

scenario "Dropdown is visible only in incoming/in_progress/all_papers tabs" do
expect(page).to_not have_css("select#track_id")

Expand Down

0 comments on commit 39ab2b0

Please sign in to comment.