From c02a103c7fe2d41c027c68393bb311e1ef37333a Mon Sep 17 00:00:00 2001 From: Anna Buianova Date: Wed, 20 May 2020 01:00:01 +0300 Subject: [PATCH] Refactored creating sponsorships (#7946) * Refactored sponsorships levels and purchasing sponsorships * Fix partnerships tests * Refactored creating partnerships, removed unused functionality * Display flashes in /partnerships * Fix views --- app/controllers/partnerships_controller.rb | 63 ++++---- app/models/sponsorship.rb | 17 +-- app/views/partnerships/_flashes.html.erb | 11 ++ app/views/partnerships/_form.html.erb | 35 +---- app/views/partnerships/index.html.erb | 2 + app/views/partnerships/show.html.erb | 2 + spec/models/sponsorship_spec.rb | 2 +- spec/requests/partnerships_spec.rb | 169 ++++++++------------- 8 files changed, 120 insertions(+), 181 deletions(-) create mode 100644 app/views/partnerships/_flashes.html.erb diff --git a/app/controllers/partnerships_controller.rb b/app/controllers/partnerships_controller.rb index 45d9a87c7..7aebf1305 100644 --- a/app/controllers/partnerships_controller.rb +++ b/app/controllers/partnerships_controller.rb @@ -10,41 +10,39 @@ class PartnershipsController < ApplicationController def show skip_authorization - @organizations = current_user&.admin_organizations end def create - @organization = Organization.find(sponsorship_params[:organization_id]) - authorize @organization, :admin_of_org? + organization = Organization.find(sponsorship_params[:organization_id]) + authorize organization, :admin_of_org? - @level = sponsorship_params[:level] - @number_of_credits_needed = Sponsorship::CREDITS[@level] - if @level == "media" && @number_of_credits_needed.nil? - @number_of_credits_needed = sponsorship_params[:amount].to_i + level = sponsorship_params[:level] + number_of_credits_needed = Sponsorship::CREDITS[level].to_i + + if %w[devrel media].include?(level) + flash[:error] = "#{level.capitalize} sponsorship is not a self-serving one" + elsif organization.credits.unspent.size < number_of_credits_needed + flash[:error] = "Not enough credits" + else + sponsorable = Tag.find_by!(name: sponsorship_params[:tag_name]) if level == "tag" + + purchase_sponsorship( + organization: organization, + level: level, + cost: number_of_credits_needed, + sponsorable: sponsorable, + ) + + Slack::Messengers::Sponsorship.call( + user: current_user, + organization: organization, + level: level, + tag: sponsorable, + ) + + flash[:notice] = "You purchased a sponsorship" end - @available_org_credits = @organization.credits.unspent - - # NOTE: this should probably be a redirect with a notice - raise "Not enough credits" unless @available_org_credits.size >= @number_of_credits_needed - - tag_sponsorship = @level == "tag" - @tag = Tag.find_by!(name: sponsorship_params[:tag_name]) if tag_sponsorship - - sponsorable = tag_sponsorship ? @tag : nil - purchase_sponsorship( - organization: @organization, - level: @level, - cost: @number_of_credits_needed, - sponsorable: sponsorable, - ) - - Slack::Messengers::Sponsorship.call( - user: current_user, - organization: @organization, - level: @level, - tag: @tag, - ) redirect_back(fallback_location: partnerships_path) end @@ -58,15 +56,14 @@ class PartnershipsController < ApplicationController # NOTE: this should probably end up in a service object at some point def purchase_sponsorship(organization:, level:, cost:, sponsorable: nil) - expires_at = Sponsorship::LEVELS_WITH_EXPIRATION.include?(level) ? 1.month.from_now : nil create_params = { user: current_user, level: level, status: :pending, - expires_at: expires_at + sponsorable: sponsorable } - if sponsorable - create_params[:sponsorable] = sponsorable + # set expires_at for gold-silver-bronze and tag sponsorships + if Sponsorship::METAL_LEVELS.include?(level) || sponsorable create_params[:expires_at] = 1.month.from_now end diff --git a/app/models/sponsorship.rb b/app/models/sponsorship.rb index f6ba11524..35cbb3f35 100644 --- a/app/models/sponsorship.rb +++ b/app/models/sponsorship.rb @@ -1,6 +1,6 @@ class Sponsorship < ApplicationRecord LEVELS = %w[gold silver bronze tag media devrel].freeze - LEVELS_WITH_EXPIRATION = %w[gold silver bronze].freeze + METAL_LEVELS = %w[gold silver bronze].freeze STATUSES = %w[none pending live].freeze # media has no fixed amount of credits CREDITS = { @@ -20,14 +20,11 @@ class Sponsorship < ApplicationRecord validates :status, inclusion: { in: STATUSES } validates :url, url: { allow_blank: true, no_local: true, schemes: %w[http https] } validate :validate_tag_uniqueness, if: proc { level.to_s == "tag" } - validate :validate_level_uniqueness, if: proc { LEVELS_WITH_EXPIRATION.include?(level) } + validate :validate_level_uniqueness, if: proc { METAL_LEVELS.include?(level) } - scope :gold, -> { where(level: :gold) } - scope :silver, -> { where(level: :silver) } - scope :bronze, -> { where(level: :bronze) } - scope :tag, -> { where(level: :tag) } - scope :media, -> { where(level: :media) } - scope :devrel, -> { where(level: :devrel) } + LEVELS.each do |level| + scope level, -> { where(level: level) } + end scope :live, -> { where(status: :live) } scope :pending, -> { where(status: :pending) } @@ -45,8 +42,8 @@ class Sponsorship < ApplicationRecord def validate_level_uniqueness return unless self.class.where(organization: organization). - where("level IN (?) AND expires_at > ? AND id != ?", LEVELS_WITH_EXPIRATION, Time.current, id.to_i).exists? + where("level IN (?) AND expires_at > ? AND id != ?", METAL_LEVELS, Time.current, id.to_i).exists? - errors.add(:level, "You can have only one sponsorship of #{LEVELS_WITH_EXPIRATION.join(', ')}") + errors.add(:level, "You can have only one sponsorship of #{METAL_LEVELS.join(', ')}") end end diff --git a/app/views/partnerships/_flashes.html.erb b/app/views/partnerships/_flashes.html.erb new file mode 100644 index 000000000..0a8ee49a3 --- /dev/null +++ b/app/views/partnerships/_flashes.html.erb @@ -0,0 +1,11 @@ +<% if flash[:notice] %> +
+ <%= flash[:notice] %> +
+<% end %> + +<% if flash[:error].present? %> +
+ <%= flash[:error] %> +
+<% end %> diff --git a/app/views/partnerships/_form.html.erb b/app/views/partnerships/_form.html.erb index 0c7f91649..32e119868 100644 --- a/app/views/partnerships/_form.html.erb +++ b/app/views/partnerships/_form.html.erb @@ -34,10 +34,10 @@ <% else %>
- <% if Sponsorship::LEVELS_WITH_EXPIRATION.include?(level) %> + <% if Sponsorship::METAL_LEVELS.include?(level) %>

@<%= org.slug %>

This organization account has <%= org.credits.unspent.size %> credits available
- <% sponsorships = org.sponsorships.unexpired.where(level: Sponsorship::LEVELS_WITH_EXPIRATION) %> + <% sponsorships = org.sponsorships.unexpired.where(level: Sponsorship::METAL_LEVELS) %> <% level_sponsorship = sponsorships.where(level: level).take %> <% if level_sponsorship %>
@@ -86,39 +86,10 @@ <%= select_tag "tag_name", options_for_select(tag_names) %> <% end %> - <%# NOTE: this currently can't be reached %> - <% elsif level == "media" %> -

@<%= org.slug %>

-
This organization account has <%= org.credits.unspent.size %> credits available
-
- <%= form_tag "/partnerships" do %> - <%= hidden_field_tag(:organization_id, org.id) %> - <%= hidden_field_tag(:level, level) %> - <%= number_field_tag(:amount, nil, placeholder: "Number of credits") %> - -

- - Credits will be allotted flexibly use in <%= community_name %>-produced podcasts, videos and other media content. You will be notified as it is being distributed. - -

- <% end %> - <%# NOTE: this currently can't be reached %> - <% elsif level == "devrel" %> -

@<%= org.slug %>

-
This organization account has <%= org.credits.unspent.size %> credits available
-
- <%= form_tag "/partnerships" do %> - <%= hidden_field_tag(:organization_id, org.id) %> - <%= hidden_field_tag(:level, level) %> - -

- You will be contacted by the <%= community_name %> team within one day of signing up for this service. -

- <% end %> <% end %>
<% end %> <% end %> <% else %> -

Contact <%= email_link(:business) %> to sign up

+

Contact <%= email_link(:business) %> to sign up

<% end %> diff --git a/app/views/partnerships/index.html.erb b/app/views/partnerships/index.html.erb index 18467c7e5..72523cd7d 100644 --- a/app/views/partnerships/index.html.erb +++ b/app/views/partnerships/index.html.erb @@ -7,6 +7,8 @@
+ <%= render "flashes" %> +

Partner With " />

🚀 Reach

diff --git a/app/views/partnerships/show.html.erb b/app/views/partnerships/show.html.erb index 156bebfe1..1944bd653 100644 --- a/app/views/partnerships/show.html.erb +++ b/app/views/partnerships/show.html.erb @@ -7,6 +7,8 @@

+ <%= render "flashes" %> +

👈 back to partnerships list

<% if params[:option] == "gold-sponsor" %> <%= render "gold_sponsor_copy" %> diff --git a/spec/models/sponsorship_spec.rb b/spec/models/sponsorship_spec.rb index d49ee65db..814cf2038 100644 --- a/spec/models/sponsorship_spec.rb +++ b/spec/models/sponsorship_spec.rb @@ -17,7 +17,7 @@ RSpec.describe Sponsorship, type: :model do describe "constants" do it "has the correct values for constants" do expect(Sponsorship::LEVELS).to eq(%w[gold silver bronze tag media devrel]) - expect(Sponsorship::LEVELS_WITH_EXPIRATION).to eq(%w[gold silver bronze]) + expect(Sponsorship::METAL_LEVELS).to eq(%w[gold silver bronze]) expect(Sponsorship::STATUSES).to eq(%w[none pending live]) expected_credits = { gold: 6000, silver: 500, bronze: 100, tag: 300, devrel: 500 }.with_indifferent_access expect(Sponsorship::CREDITS).to eq(expected_credits) diff --git a/spec/requests/partnerships_spec.rb b/spec/requests/partnerships_spec.rb index 815b525a8..972ff8d9a 100644 --- a/spec/requests/partnerships_spec.rb +++ b/spec/requests/partnerships_spec.rb @@ -126,7 +126,7 @@ RSpec.describe "Partnerships", type: :request do let(:user) { create(:user) } let(:org) { create(:organization) } - context "when user is logged in as an admin and has enough credits" do + context "when user is logged in as an admin" do before do create(:organization_membership, user: user, organization: org, type_of_user: "admin") sign_in user @@ -135,7 +135,7 @@ RSpec.describe "Partnerships", type: :request do # context "when purchasing a gold sponsorship" is skipped due # to the high amount of required credits - context "when purchasing a silver sponsorship" do + context "when purchasing a silver sponsorship and has enough credits" do let(:params) { { level: :silver, organization_id: org.id } } before do @@ -179,129 +179,88 @@ RSpec.describe "Partnerships", type: :request do context "when purchasing a bronze sponsorship" do let(:params) { { level: :bronze, organization_id: org.id } } - before do - Credit.add_to(org, Sponsorship::CREDITS[:bronze]) - end + context "when enough credits" do + before do + Credit.add_to(org, Sponsorship::CREDITS[:bronze]) + end - it "creates a new sponsorship" do - expect do - post partnerships_path, params: params - expect(response).to redirect_to(partnerships_path) - end.to change(org.sponsorships, :count).by(1) - end + it "creates a new sponsorship" do + expect do + post partnerships_path, params: params + expect(response).to redirect_to(partnerships_path) + end.to change(org.sponsorships, :count).by(1) + end - it "subscribes with the correct info" do - Timecop.freeze(Time.current) do + it "creates a flash notice" do post partnerships_path, params: params - sponsorship = org.sponsorships.bronze.last - expect(sponsorship.status).to eq("pending") - expect(sponsorship.expires_at.to_i).to eq(1.month.from_now.to_i) - expect(sponsorship.sponsorable).to be(nil) - expect(sponsorship.instructions).to be_blank - expect(sponsorship.instructions_updated_at).to be(nil) + expect(flash[:notice]).to eq("You purchased a sponsorship") + end + + it "subscribes with the correct info" do + Timecop.freeze(Time.current) do + post partnerships_path, params: params + sponsorship = org.sponsorships.bronze.last + expect(sponsorship.status).to eq("pending") + expect(sponsorship.expires_at.to_i).to eq(1.month.from_now.to_i) + expect(sponsorship.sponsorable).to be(nil) + expect(sponsorship.instructions).to be_blank + expect(sponsorship.instructions_updated_at).to be(nil) + end + end + + it "detracts the correct amount of credits" do + expect do + post partnerships_path, params: params + end.to change(org.credits.spent, :size).by(Sponsorship::CREDITS[:bronze]) + credit = org.credits.spent.last + expect(credit.purchase.is_a?(Sponsorship)).to be(true) + end + + it "queues a slack message to be sent" do + sidekiq_assert_enqueued_with(job: Slack::Messengers::Worker) do + post partnerships_path, params: params + end end end - it "detracts the correct amount of credits" do - expect do - post partnerships_path, params: params - end.to change(org.credits.spent, :size).by(Sponsorship::CREDITS[:bronze]) - credit = org.credits.spent.last - expect(credit.purchase.is_a?(Sponsorship)).to be(true) - end + context "when not enough credits" do + it "doesn't create a new sponsorship" do + expect do + post partnerships_path, params: params + end.not_to change(org.sponsorships, :count) + end - it "queues a slack message to be sent" do - sidekiq_assert_enqueued_with(job: Slack::Messengers::Worker) do + it "redirects with a flash notice" do post partnerships_path, params: params + expect(response).to redirect_to(partnerships_path) + expect(flash[:error]).to eq("Not enough credits") end end end - context "when purchasing a devrel sponsorship" do - let(:params) { { level: :devrel, organization_id: org.id } } + %i[media devrel].each do |level| + context "when purchasing a #{level} sponsorship" do + let(:params) { { level: level, organization_id: org.id } } - before do - Credit.add_to(org, Sponsorship::CREDITS[:devrel]) - end + before do + Credit.add_to(org, Sponsorship::CREDITS[level].to_i) + end - it "creates a new sponsorship" do - expect do + it "doesn't create a Sponsorship" do + expect do + post partnerships_path, params: params + end.not_to change(Sponsorship, :count) + end + + it "redirects with a notice" do post partnerships_path, params: params expect(response).to redirect_to(partnerships_path) - end.to change(org.sponsorships, :count).by(1) - end - - it "subscribes with the correct info" do - Timecop.freeze(Time.current) do - post partnerships_path, params: params - sponsorship = org.sponsorships.devrel.last - expect(sponsorship.status).to eq("pending") - expect(sponsorship.expires_at).to be(nil) - expect(sponsorship.sponsorable).to be(nil) - expect(sponsorship.instructions).to be_blank - expect(sponsorship.instructions_updated_at).to be(nil) - end - end - - it "detracts the correct amount of credits" do - expect do - post partnerships_path, params: params - end.to change(org.credits.spent, :size).by(Sponsorship::CREDITS[:devrel]) - credit = org.credits.spent.last - expect(credit.purchase.is_a?(Sponsorship)).to be(true) - end - - it "queues a slack message to be sent" do - sidekiq_assert_enqueued_with(job: Slack::Messengers::Worker) do - post partnerships_path, params: params + expect(flash[:error]).to eq("#{level.capitalize} sponsorship is not a self-serving one") end end end - context "when purchasing a media sponsorship" do - let(:params) do - { level: :media, organization_id: org.id, amount: 10 } - end - - before do - Credit.add_to(org, params[:amount]) - end - - it "creates a new sponsorship" do - expect do - post partnerships_path, params: params - expect(response).to redirect_to(partnerships_path) - end.to change(org.sponsorships, :count).by(1) - end - - it "subscribes with the correct info" do - Timecop.freeze(Time.current) do - post partnerships_path, params: params - sponsorship = org.sponsorships.media.last - expect(sponsorship.status).to eq("pending") - expect(sponsorship.expires_at).to be(nil) - expect(sponsorship.sponsorable).to be(nil) - expect(sponsorship.instructions).to be_blank - expect(sponsorship.instructions_updated_at).to be(nil) - end - end - - it "detracts the correct amount of credits" do - expect do - post partnerships_path, params: params - end.to change(org.credits.spent, :size).by(params[:amount]) - credit = org.credits.spent.last - expect(credit.purchase.is_a?(Sponsorship)).to be(true) - end - - it "queues a slack message to be sent" do - sidekiq_assert_enqueued_with(job: Slack::Messengers::Worker) do - post partnerships_path, params: params - end - end - end - - context "when purchasing a tag sponsorship" do + context "when purchasing a tag sponsorship and has enough credits" do let(:tag) { create(:tag) } let(:params) { { level: :tag, organization_id: org.id, tag_name: tag.name } }