diff --git a/.env.example b/.env.example index f670cc990..d5921a766 100644 --- a/.env.example +++ b/.env.example @@ -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 \ No newline at end of file +PROJECT_NAME: The OpenPolitics Manifesto +CLA_URL: https://openpolitics.org.uk/manifesto/cla.html \ No newline at end of file diff --git a/app/controllers/edit_controller.rb b/app/controllers/edit_controller.rb index 25549966f..a19d4fb67 100644 --- a/app/controllers/edit_controller.rb +++ b/app/controllers/edit_controller.rb @@ -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 diff --git a/app/controllers/proposals_controller.rb b/app/controllers/proposals_controller.rb index 95d8df793..3c3f8b856 100644 --- a/app/controllers/proposals_controller.rb +++ b/app/controllers/proposals_controller.rb @@ -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 diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 4a019ae82..27b6a5eb2 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -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 @@ -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| + UpdateProposalJob.perform_later pr.number + end + end end diff --git a/app/models/concerns/github_pull_request.rb b/app/models/concerns/github_pull_request.rb index 904059ffc..2e234cd34 100644 --- a/app/models/concerns/github_pull_request.rb +++ b/app/models/concerns/github_pull_request.rb @@ -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 diff --git a/app/models/concerns/user_admin.rb b/app/models/concerns/user_admin.rb index 7ce3f0a52..f79efc146 100644 --- a/app/models/concerns/user_admin.rb +++ b/app/models/concerns/user_admin.rb @@ -21,6 +21,7 @@ module UserAdmin field :role field :author field :voter + field :cla_accepted end end end diff --git a/app/models/concerns/votable.rb b/app/models/concerns/votable.rb index 6b964a558..4688882fb 100644 --- a/app/models/concerns/votable.rb +++ b/app/models/concerns/votable.rb @@ -43,6 +43,7 @@ def count_votes! unless closed? set_vote_build_status set_time_build_status + set_cla_build_status end end diff --git a/app/models/proposal.rb b/app/models/proposal.rb index cb18134c3..fa9f4788e 100644 --- a/app/models/proposal.rb +++ b/app/models/proposal.rb @@ -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) @@ -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 diff --git a/app/models/user.rb b/app/models/user.rb index 78584f802..29ba677ae 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -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 diff --git a/app/views/edit/commit.html.erb b/app/views/edit/commit.html.erb index 825c6eedb..08cf1fde5 100644 --- a/app/views/edit/commit.html.erb +++ b/app/views/edit/commit.html.erb @@ -1,24 +1,20 @@ -
+
<%= fa_icon 'check-circle' %> <%= t 'help.success_title' %>
-
+

<%= t 'help.success' %>

- <% if @has_cla %> -

- <%= t :cla %> -

-

- <%= link_to "#{t(:view_cla)}".html_safe, @cla_url, class: 'btn btn-primary' %> -

- <% end %>
-
+
<%= 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' %>
+ +<% if @proposer_needs_to_sign_cla %> + <%= render partial: 'shared/cla_warning', locals: { user: current_user } %> +<% end %> diff --git a/app/views/proposals/show.html.erb b/app/views/proposals/show.html.erb index 7f016a45e..3402c5939 100644 --- a/app/views/proposals/show.html.erb +++ b/app/views/proposals/show.html.erb @@ -38,6 +38,12 @@
+<% if @proposer_needs_to_sign_cla %> +
+ <%= render partial: 'shared/cla_warning', locals: { user: @proposal.proposer } %> +
+<% end %> +
diff --git a/app/views/shared/_cla_warning.html.erb b/app/views/shared/_cla_warning.html.erb new file mode 100644 index 000000000..7e8b0ef08 --- /dev/null +++ b/app/views/shared/_cla_warning.html.erb @@ -0,0 +1,17 @@ +
+ <%= fa_icon 'exclamation-circle' %> + <%= t 'cla_title' %> +
+ +
+

+ <%= t 'cla' %> +

+

+ Please <%= link_to 'edit your profile', edit_user_path(user) %> and accept the license agreement to unblock your proposal. +

+
+ +
+ <%= link_to "#{fa_icon('cog lg')} #{t(:edit_profile)}".html_safe, edit_user_path(user), class: 'btn btn-primary' %> +
diff --git a/app/views/users/edit.html.erb b/app/views/users/edit.html.erb index 65bd66d3a..51751a31a 100644 --- a/app/views/users/edit.html.erb +++ b/app/views/users/edit.html.erb @@ -28,3 +28,32 @@
<% end %> + +<% if @has_cla %> +

Contributor License Agreement

+ +

+ <%= t :cla %> +

+ + <% if @user.needs_to_sign_cla? %> + <%= form_for @user, html: {class: "form-horizontal", style: 'clear: right'} do |f| %> + <%= hidden_field_tag "cla_accepted", true %> + +
+
+ <%= 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) %> +
+
+ <% end %> + <% else %> +
+ <%= fa_icon 'check' %> + You have signed the contributor license agreement. +
+ <% end %> + +   + +<% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index f3d869e9d..428915be5 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -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. @@ -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)." \ No newline at end of file + 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. \ No newline at end of file diff --git a/spec/models/proposal_spec.rb b/spec/models/proposal_spec.rb index 2c87ffca4..86782d647 100644 --- a/spec/models/proposal_spec.rb +++ b/spec/models/proposal_spec.rb @@ -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 @@ -34,6 +34,7 @@ BLOCK_THRESHOLD: "-1", MIN_AGE: "7", MAX_AGE: "90", + CLA_URL: nil, } ClimateControl.modify env do example.run @@ -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 }