Skip to content

Commit 111c51f

Browse files
authored
Only create Contracts through Contractable (#12251)
## Summary of the problem <!-- Why are these changes being made? What problem does it solve? Link any related issues to provide more details. --> https://appsignal.com/hack-club/sites/6596247683eb67648f30f807/exceptions/incidents/2394 Bug introduced in #12094 - since `Contract` and `OrganizerPositionInvite` are now decoupled, there are some fields of the `Contract` model that need to be set using info from the `Contractable`. Notably, the `prefills` jsonb includes event data that must be included in the `Contract` before creation, since it is used in the after create callback that sends the payload to Docuseal. While we were doing this in the `send_contract` method on `OrganizerPositionInvite`, we were not doing this when creating a contract using the create action in `ContractsController` (used when sending a contract on a pre-existing invite or organizer position), causing the above bug. ## Describe your changes <!-- Explain your thought process to the solution and provide a quick summary of the changes. --> While this could have been fixed by just grabbing the necessary data from the `Contractable` and passing it to `Contract.new` in `ContractsController`, it's better to maintain that contracts should only be created using methods on the `Contractable`. This way, changes can be easily made to what data is passed to the `Contract`, and any extra code (such as updating the `OrganizerPosition` or Airtable can be done in one place. This does involve updating the `OrganizerPositionInvite` after it has been sent and accepted - I've fixed a validation to not fail when this happens, but it would also be possible to modify the code so that this model is essentially "frozen" after being accepted, and only update the `is_signee` field before accepting.
1 parent 9b09f84 commit 111c51f

File tree

10 files changed

+43
-45
lines changed

10 files changed

+43
-45
lines changed

app/controllers/contracts_controller.rb

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,6 @@
33
class ContractsController < ApplicationController
44
before_action :set_contract, only: [:void, :resend_to_user, :resend_to_cosigner]
55

6-
# This is only used for sending a contract for an OrganizerPositionInvite via
7-
# the team page.
8-
def create
9-
@contract = Contract.new(contract_params)
10-
authorize @contract, policy_class: ContractPolicy
11-
@contract.save!
12-
flash[:success] = "Contract sent successfully."
13-
redirect_back(fallback_location: event_team_path(@contract.event))
14-
end
15-
166
def void
177
authorize @contract, policy_class: ContractPolicy
188
@contract.mark_voided!
@@ -48,8 +38,4 @@ def set_contract
4838
@contract = Contract.find(params[:id])
4939
end
5040

51-
def contract_params
52-
params.require(:contract).permit(:type, :contractable_id, :contractable_type, :cosigner_email, :include_videos)
53-
end
54-
5541
end

app/controllers/organizer_position_invites_controller.rb

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ class OrganizerPositionInvitesController < ApplicationController
44
include SetEvent
55
include ChangePositionRole
66

7-
before_action :set_opi, only: [:show, :accept, :reject, :cancel, :resend]
7+
before_action :set_opi, except: [:new, :create]
88
before_action :set_event, only: [:new, :create]
99
before_action :hide_footer, only: :show
1010

@@ -112,6 +112,15 @@ def resend
112112
redirect_to event_team_path @invite.event
113113
end
114114

115+
def send_contract
116+
authorize @invite
117+
118+
@invite.send_contract(cosigner_email: params[:cosigner_email].presence, include_videos: params[:include_videos])
119+
120+
flash[:success] = "Contract successfully sent"
121+
redirect_to event_team_path @invite.event
122+
end
123+
115124
private
116125

117126
def set_opi

app/models/organizer_position_invite.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ class OrganizerPositionInvite < ApplicationRecord
8282

8383
belongs_to :organizer_position, optional: true
8484

85-
validate :not_already_organizer
85+
validate :not_already_organizer, if: -> { event_changed? || user_changed? }
8686
validate :not_already_invited, on: :create
8787
validates :accepted_at, absence: true, if: -> { rejected_at.present? }
8888
validates :rejected_at, absence: true, if: -> { accepted_at.present? }
@@ -221,7 +221,7 @@ def send_contract(cosigner_email: nil, include_videos: false)
221221

222222
def on_contract_signed(contract)
223223
if contract.is_a?(Contract::FiscalSponsorship)
224-
deliver
224+
deliver if organizer_position.nil?
225225

226226
# Unfreeze the event if this is the first signed contract
227227
if event.contracts.signed.count == 1

app/policies/organizer_position_invite_policy.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,10 @@ def change_position_role?
3737
admin_or_manager? && !record.signee?
3838
end
3939

40+
def send_contract?
41+
user&.admin?
42+
end
43+
4044
private
4145

4246
def admin_or_manager?

app/views/contract/_form.html.erb

Lines changed: 0 additions & 24 deletions
This file was deleted.
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<%# locals: (opi:) %>
2+
3+
<%= form_with(local: true, data: { turbo: false }, url: send_contract_organizer_position_invite_path(id: opi.id)) do |form| %>
4+
<p class="muted">
5+
This will send <%= opi.user.name %> a fiscal sponsorship agreement
6+
for <%= opi.event.name %>. <strong>If they are under 18, <%= opi.user.name %> will need to have a parent sign on their behalf.</strong>
7+
<p>
8+
9+
<div class="field field--checkbox">
10+
<%= form.check_box :include_videos %>
11+
<%= form.label :include_videos, "Would you like us to include the onboarding videos?" %>
12+
</div>
13+
14+
<div class="field">
15+
<%= form.label :cosigner_email, "Cosigned by (email address)" %>
16+
<%= form.text_field :cosigner_email %>
17+
</div>
18+
19+
<div class="actions">
20+
<%= form.submit "Send contract" %>
21+
</div>
22+
<% end %>

app/views/organizer_position_invites/_organizer_position_invite.html.erb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@
5454
<% end %>
5555
<section class="modal modal--scroll bg-snow" data-behavior="modal" role="dialog" id="send_contract_<%= organizer_position_invite.id %>">
5656
<%= modal_header "Send a contract" %>
57-
<%= render "contract/form", contract: Contract::FiscalSponsorship.new(contractable: organizer_position_invite) %>
57+
<%= render "organizer_position_invites/contract_form", opi: organizer_position_invite %>
5858
</section>
5959
<% end %>
6060
</div>

app/views/organizer_positions/_organizer_position.html.erb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@
140140
<% end %>
141141
<section class="modal modal--scroll bg-snow" data-behavior="modal" role="dialog" id="send_contract_<%= organizer_position.organizer_position_invite.id %>">
142142
<%= modal_header "Send a contract" %>
143-
<%= render "contract/form", contract: Contract::FiscalSponsorship.new(contractable: organizer_position.organizer_position_invite) %>
143+
<%= render "organizer_position_invites/contract_form", opi: organizer_position.organizer_position_invite %>
144144
</section>
145145
<% end %>
146146
</div>

app/views/organizer_positions/_organizer_position_row.html.erb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@
6767
<% end %>
6868
<section class="modal modal--scroll bg-snow" data-behavior="modal" role="dialog" id="send_contract_<%= organizer_position.organizer_position_invite.id %>">
6969
<%= modal_header "Send a contract" %>
70-
<%= render "contract/form", contract: Contract::FiscalSponsorship.new(contractable: organizer_position.organizer_position_invite) %>
70+
<%= render "organizer_position_invites/contract_form", opi: organizer_position.organizer_position_invite %>
7171
</section>
7272
<% end %>
7373
</div>

config/routes.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -332,10 +332,11 @@
332332
post "resend"
333333
member do
334334
post "change_position_role"
335+
post "send_contract"
335336
end
336337
end
337338

338-
resources :contracts, only: [:create] do
339+
resources :contracts, only: [] do
339340
member do
340341
post "void"
341342
post "resend_to_user"

0 commit comments

Comments
 (0)