Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,5 @@ MAX_AGE: 90
PASS_THRESHOLD: 2
BLOCK_THRESHOLD: -1
PROJECT_URL: https://openpolitics.org.uk/manifesto
PROJECT_NAME: The OpenPolitics Manifesto
PROJECT_NAME: The OpenPolitics Manifesto
CLA_URL: https://openpolitics.org.uk/manifesto/cla.html
6 changes: 2 additions & 4 deletions app/controllers/edit_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,8 @@ def commit
# open PR
pull_from = forked ? "#{@current_user.login}:#{new_branch}" : branch_name
@pr = open_pr(pull_from, @branch, @summary, @description)
# Check for CLA
@cla_url = "#{ENV.fetch("SITE_URL")}/cla.html"
r = Faraday.get @cla_url
@has_cla = (r.status == 200)
# Does the proposer need to sign a CLA?
@proposer_needs_to_sign_cla = current_user.try(:needs_to_sign_cla?)
end

private
Expand Down
2 changes: 2 additions & 0 deletions app/controllers/proposals_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ def show
# Get activity list
presenter = ProposalPresenter.new(@proposal)
@activity = presenter.activity_log
# Does the proposer need to sign a CLA?
@proposer_needs_to_sign_cla = current_user.try(:needs_to_sign_cla?) && @is_author
end

def webhook
Expand Down
16 changes: 15 additions & 1 deletion app/controllers/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,16 @@ def show
end

def edit
@has_cla = ENV["CLA_URL"].present?
end

def update
@user.update!(user_params)
if params.has_key?(:cla_accepted)
accept_cla
else
@user.update!(user_params)
end

redirect_to edit_user_path(@user)
end

Expand All @@ -48,4 +54,12 @@ def authorise
def user_params
params.require(:user).permit(:email, :notify_new)
end

def accept_cla
@user.update!(cla_accepted: true)

@user.proposed.each do |pr|

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Floppy any idea how to make this happen in the background or is it not worth worrying about? (There's a noticeable delay, especially if you have lots of PRs open but that's unlikely in normal circumstances I guess.)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, there's a background job runner that will do it. If you do UpdateProposalJob.perform_later pr.number I think that will work.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, that was a lot easier than I was expecting! Thanks.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Helps that I had basically the exact same problem a while back :)

UpdateProposalJob.perform_later pr.number
end
end
end
11 changes: 11 additions & 0 deletions app/models/concerns/github_pull_request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,17 @@ def set_time_build_status
end
end

def set_cla_build_status
status = "groupthink/cla"
if has_cla?
if proposer_needs_to_sign_cla?
set_build_status(:pending, I18n.t("build_status.cla.pending"), status)
else
set_build_status(:success, I18n.t("build_status.cla.accepted"), status)
end
end
end

def merge_pr!
Octokit.merge_pull_request(ENV.fetch("GITHUB_REPO"), number)
true
Expand Down
1 change: 1 addition & 0 deletions app/models/concerns/user_admin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ module UserAdmin
field :role
field :author
field :voter
field :cla_accepted
end
end
end
Expand Down
1 change: 1 addition & 0 deletions app/models/concerns/votable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ def count_votes!
unless closed?
set_vote_build_status
set_time_build_status
set_cla_build_status
end
end

Expand Down
10 changes: 9 additions & 1 deletion app/models/proposal.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,14 @@ def too_new?
age < Rules.min_age
end

def has_cla?
ENV["CLA_URL"].present?
end

def proposer_needs_to_sign_cla?
proposer.try(:needs_to_sign_cla?)
end

def update_state!
state = pr_closed? ? closed_state : open_state
update!(state: state)
Expand Down Expand Up @@ -90,7 +98,7 @@ def closed_state
def open_state
return nil if pr_closed?
return "dead" if too_old?
return "blocked" if blocked?
return "blocked" if blocked? || proposer_needs_to_sign_cla?
if passed?
return too_new? ? "agreed" : "passed"
end
Expand Down
4 changes: 4 additions & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -95,4 +95,8 @@ def vote(proposal)
def to_param
login
end

def needs_to_sign_cla?
ENV["CLA_URL"].present? && !cla_accepted
end
end
18 changes: 7 additions & 11 deletions app/views/edit/commit.html.erb
Original file line number Diff line number Diff line change
@@ -1,24 +1,20 @@
<div class="alert alert-success">
<div class="form-group alert alert-success">
<%= fa_icon 'check-circle' %>
<%= t 'help.success_title' %>
</div>

<div>
<div class="form-group">
<p>
<%= t 'help.success' %>
</p>
<% if @has_cla %>
<p>
<%= t :cla %>
</p>
<p>
<%= link_to "#{t(:view_cla)}".html_safe, @cla_url, class: 'btn btn-primary' %>
</p>
<% end %>
</div>

<div>
<div class="form-group">
<%= link_to "#{fa_icon('globe')} #{t(:return)}".html_safe, @return_to, class: 'btn btn-primary' if @return_to %>
<%= link_to "#{fa_icon('code-fork')} #{t(:view)}".html_safe, proposal_path(@pr), class: 'btn btn-default' %>
<%= link_to "#{fa_icon('list')} #{t(:view_all)}".html_safe, proposals_path, class: 'btn btn-default' %>
</div>

<% if @proposer_needs_to_sign_cla %>
<%= render partial: 'shared/cla_warning', locals: { user: current_user } %>
<% end %>
6 changes: 6 additions & 0 deletions app/views/proposals/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@
</div>
</div>

<% if @proposer_needs_to_sign_cla %>
<div class='row'>
<%= render partial: 'shared/cla_warning', locals: { user: @proposal.proposer } %>
</div>
<% end %>

<hr/>

<div class='row'>
Expand Down
17 changes: 17 additions & 0 deletions app/views/shared/_cla_warning.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<div class="form-group alert alert-warning">
<%= fa_icon 'exclamation-circle' %>
<%= t 'cla_title' %>
</div>

<div class="form-group">
<p>
<%= t 'cla' %>
</p>
<p>
Please <%= link_to 'edit your profile', edit_user_path(user) %> and accept <a href='<%= ENV.fetch("CLA_URL")%>' target='_blank' rel='noopener noreferrer'>the license agreement</a> to unblock your proposal.
</p>
</div>

<div class="form-group">
<%= link_to "#{fa_icon('cog lg')} #{t(:edit_profile)}".html_safe, edit_user_path(user), class: 'btn btn-primary' %>
</div>
29 changes: 29 additions & 0 deletions app/views/users/edit.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,32 @@
</div>
</div>
<% end %>

<% if @has_cla %>
<h2>Contributor License Agreement</h2>

<p>
<%= t :cla %>
</p>

<% if @user.needs_to_sign_cla? %>
<%= form_for @user, html: {class: "form-horizontal", style: 'clear: right'} do |f| %>
<%= hidden_field_tag "cla_accepted", true %>

<div class="form-group">
<div class="col-sm-offset-2 col-sm-10">
<%= link_to "#{t(:view_cla)}".html_safe, ENV.fetch("CLA_URL"), class: 'btn btn-default', target: :_blank, rel: 'noopener noreferrer' %>
<%= f.submit class: "btn btn-primary", value: t(:sign_cla) %>
</div>
</div>
<% end %>
<% else %>
<div class="col-sm-offset-2 col-sm-10">
<%= fa_icon 'check' %>
You have signed the <a href='<%= ENV.fetch("CLA_URL")%>' target='_blank' rel='noopener noreferrer'>contributor license agreement</a>.
</div>
<% end %>

<span style="clear: both">&nbsp;</span>

<% end %>
10 changes: 8 additions & 2 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,12 @@ en:
return: Return to site
view: View your proposal
view_all: View all proposals
edit_profile: Edit profile
signed_out_site_intro: To contribute you need to log in with a free GitHub account. GitHub is a popular code-sharing site which hosts the site content.
signed_in_site_intro: There is no way at the moment to explore github content here. Please go to the project you want to edit on GitHub and follow a link to the editor. Thanks!
cla: This project has a CLA (Contributor License Agreement). You may need to agree to it before your change can be accepted, if you haven't already.
cla_title: Your proposal is blocked until you accept the contributor license agreement.
cla: This manifesto has a Contributor License Agreement (CLA) which you must accept before any of your proposals can be included.
sign_cla: Accept CLA
view_cla: View CLA
help:
edit: Make your changes in the editor below, then hit "Submit changes" at the bottom.
Expand Down Expand Up @@ -70,4 +73,7 @@ en:
time:
too_old: "The change has been open for more than %{max_age} days, and should be closed (age: %{age}d)."
too_new: "The change has not yet been open for %{min_age} days (age: %{age}d)."
success: "The change has been open long enough to be merged (age: %{age}d)."
success: "The change has been open long enough to be merged (age: %{age}d)."
cla:
pending: The contributor license agreement has not been accepted.
accepted: The contributor license agreement has been accepted.
117 changes: 116 additions & 1 deletion spec/models/proposal_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
require "rails_helper"

RSpec.describe Proposal, type: :model do
context "when checking overall state" do
context "when checking overall state without a CLA" do
let(:pr) { create :proposal }

context "when the upstream PR has been closed" do
Expand Down Expand Up @@ -34,6 +34,7 @@
BLOCK_THRESHOLD: "-1",
MIN_AGE: "7",
MAX_AGE: "90",
CLA_URL: nil,
}
ClimateControl.modify env do
example.run
Expand Down Expand Up @@ -76,6 +77,120 @@
end
end

context "when checking overall state with a signed CLA" do
let(:proposer) { create :user, cla_accepted: true }
let(:pr) { create :proposal, proposer: proposer }

context "when the upstream PR is still open" do
around do |example|
env = {
YES_WEIGHT: "1",
NO_WEIGHT: "-1",
BLOCK_WEIGHT: "-1000",
PASS_THRESHOLD: "2",
BLOCK_THRESHOLD: "-1",
MIN_AGE: "7",
MAX_AGE: "90",
CLA_URL: "https://example.org/cla.html",
}
ClimateControl.modify env do
example.run
end
end

before do
allow(pr).to receive(:pr_closed?).and_return(false)
end

it "stores old pull requests as dead" do
allow(pr).to receive_messages(age: 100)
pr.update_state!
expect(pr.state).to eq "dead"
end

it "stores blocked pull requests as blocked" do
allow(pr).to receive_messages(score: -500, age: 10)
pr.update_state!
expect(pr.state).to eq "blocked"
end

it "stores PRs with enough votes and enough time as passed" do
allow(pr).to receive_messages(score: 5, age: 10)
pr.update_state!
expect(pr.state).to eq "passed"
end

it "stores PRs with enough votes and not enough time as agreed" do
allow(pr).to receive_messages(score: 5, age: 5)
pr.update_state!
expect(pr.state).to eq "agreed"
end

it "stores PRs with not enough votes and not enough time as pending" do
allow(pr).to receive_messages(score: 1, age: 5)
pr.update_state!
expect(pr.state).to eq "waiting"
end
end
end

context "when checking overall state with an unsigned CLA" do
let(:proposer) { create :user, cla_accepted: false }
let(:pr) { create :proposal, proposer: proposer }

context "when the upstream PR is still open" do
around do |example|
env = {
YES_WEIGHT: "1",
NO_WEIGHT: "-1",
BLOCK_WEIGHT: "-1000",
PASS_THRESHOLD: "2",
BLOCK_THRESHOLD: "-1",
MIN_AGE: "7",
MAX_AGE: "90",
CLA_URL: "https://example.org/cla.html",
}
ClimateControl.modify env do
example.run
end
end

before do
allow(pr).to receive(:pr_closed?).and_return(false)
end

it "stores old pull requests as dead" do
allow(pr).to receive_messages(age: 100)
pr.update_state!
expect(pr.state).to eq "dead"
end

it "stores blocked pull requests as blocked" do
allow(pr).to receive_messages(score: -500, age: 10)
pr.update_state!
expect(pr.state).to eq "blocked"
end

it "stores PRs with enough votes and enough time as passed" do
allow(pr).to receive_messages(score: 5, age: 10)
pr.update_state!
expect(pr.state).to eq "blocked"
end

it "stores PRs with enough votes and not enough time as agreed" do
allow(pr).to receive_messages(score: 5, age: 5)
pr.update_state!
expect(pr.state).to eq "blocked"
end

it "stores PRs with not enough votes and not enough time as pending" do
allow(pr).to receive_messages(score: 1, age: 5)
pr.update_state!
expect(pr.state).to eq "blocked"
end
end
end

context "with notification of new proposals" do
let!(:proposer) { create :user, voter: true, notify_new: true }
let!(:voter) { create :user, voter: true, notify_new: true }
Expand Down