Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

P2P matching & Form #730

Merged
merged 36 commits into from
Sep 25, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
6538209
Add SystemSetting to enable peer-to-peer matching
indiebrain Sep 22, 2020
7c4d225
Add contribution show view
indiebrain Sep 22, 2020
eb3bea3
Create methods to anonymize PII contact info
eabousaif Sep 23, 2020
3c967a7
Add box to show view and modify anonymize method
eabousaif Sep 23, 2020
d9d4e46
Enhance anonymize name method and display info in show page
eabousaif Sep 23, 2020
77edc60
Masks user name
acherukuri Sep 23, 2020
1882769
Adds claim this ask form
acherukuri Sep 23, 2020
ab37473
String interpolate contribution type in button
eabousaif Sep 23, 2020
f692602
View contribution when P2P matching enabled
acherukuri Sep 23, 2020
d6be53c
Renames claim ask routes & form
acherukuri Sep 23, 2020
3cf7a3c
Removes gitignore changes
acherukuri Sep 23, 2020
1d74214
Avoid regex for anonymizing name & email
acherukuri Sep 23, 2020
bb1c2e8
Creates person record if absent
acherukuri Sep 24, 2020
16e9f33
Does null check for person contact method
acherukuri Sep 24, 2020
fc97485
Sends P2P email & logs communication
acherukuri Sep 24, 2020
0e60800
Updates contribution state & hides the claim button if matched
acherukuri Sep 24, 2020
91b07ce
Adds TODO for handling match race condition
acherukuri Sep 24, 2020
9f79a66
Includes user message in the email
acherukuri Sep 24, 2020
69e6b6f
Minor fix to button
eabousaif Sep 24, 2020
006a0ea
Minor button fix
eabousaif Sep 24, 2020
4dee09a
Refactors contributions controller
acherukuri Sep 24, 2020
8ffb5fb
Adds an alert if the contribution has no email
acherukuri Sep 24, 2020
2337e58
Revert "Add SystemSetting to enable peer-to-peer matching"
acherukuri Sep 24, 2020
ce12d1d
Uses P2P exchange type setting
acherukuri Sep 24, 2020
22de765
Extracts claim logic to nested resource
acherukuri Sep 24, 2020
4eff8d5
Reverts mailcatcher settings in dev ENV
acherukuri Sep 24, 2020
5e3747c
Fixes indentation in P2P mailer
acherukuri Sep 24, 2020
ac5c9a2
Renames contributions helper to claims helper
acherukuri Sep 24, 2020
a6abc95
Removes unnecessary safe nav operators
acherukuri Sep 24, 2020
0e5fc04
Uses keyword args for alias & message in P2P mailer
acherukuri Sep 25, 2020
a0bb4e8
Extract anonymize name & email logic to Util class
acherukuri Sep 25, 2020
0ca77df
Uses keyword args for create person from P2P params method
acherukuri Sep 25, 2020
26b0e59
Address comments from https://github.com/rubyforgood/mutual-aid/pull/…
acherukuri Sep 25, 2020
82ae80e
Prefer model method, controller logic over helper module
solebared Sep 25, 2020
bab988e
Restricts preferred contact to just email for now
acherukuri Sep 25, 2020
f7774ce
Saves person email if absent in system
acherukuri Sep 25, 2020
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: 3 additions & 0 deletions app/blueprints/contribution_blueprint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ class ContributionBlueprint < Blueprinter::Base
field :respond_path do |contribution, options|
options[:respond_path]&.call(contribution.id)
end
field :view_path do |contribution, options|
options[:view_path]&.call(contribution.id)
end
field :match_path do |contribution, options|
options[:match_path]&.call(contribution.id)
end
Expand Down
51 changes: 51 additions & 0 deletions app/controllers/claims_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
class ClaimsController < ApplicationController
def new
contribution = Listing.find(params[:contribution_id])
if contribution.person&.email.blank?
flash.now[:alert] = "We are sorry, this contributor hasn't provided
an email address yet and can't communicate this way".squish
end
render locals: {
contribution: contribution,
preferred_contact_method_id: current_person&.preferred_contact_method&.id,
preferred_contact_info: current_person&.preferred_contact_info,
}
end

# FIXME: extract into an interactor
def create
# TODO: Need to handle race conditions to prevent creating multiple matches for same contribution.
solebared marked this conversation as resolved.
Show resolved Hide resolved
contribution = Listing.find(params[:contribution_id])
solebared marked this conversation as resolved.
Show resolved Hide resolved
ActiveRecord::Base.transaction do
if current_person.blank?
Person.create_from_peer_to_peer_params!(current_user, name: claim_params[:peer_alias],
preferred_contact_method_id: claim_params[:preferred_contact_method_id],
contact_info: claim_params[:preferred_contact_info])
elsif current_person.present? && current_person.email.blank?
current_person.update!(email: claim_params[:preferred_contact_info])
end
Match.create_match_for_contribution!(contribution, current_user)
contribution.matched!
end
notify_peer_and_log_communication!(contribution)
redirect_to contribution_path(params[:contribution_id]), notice: 'Claim was successful!'
end

private

def claim_params
params.require(:claim).permit(:peer_alias, :preferred_contact_method_id, :preferred_contact_info, :message)
end

def notify_peer_and_log_communication!(contribution)
peer_to_peer_email = PeerToPeerMatchMailer.peer_to_peer_email(contribution,
peer_alias: claim_params[:peer_alias],
message: claim_params[:message])
delivery_status = deliver_now_with_error_handling(peer_to_peer_email, "peer_to_peer_email")
CommunicationLog.log_email(peer_to_peer_email, delivery_status, current_user.person, nil, current_user)
end

def current_person
current_user.person
end
end
11 changes: 11 additions & 0 deletions app/controllers/contributions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,17 @@ def index
end
end

def show
contribution = Listing.find(params[:id])

render(
:show,
locals: {
contribution: contribution,
}
)
end

def combined_form
end

Expand Down
2 changes: 1 addition & 1 deletion app/controllers/submissions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def create
delivery_status = deliver_now_with_error_handling(autoemail, "new_submission_confirmation_email")

# store email that was sent
CommunicationLog.log_submission_email(autoemail, delivery_status, @submission, nil, current_user)
CommunicationLog.log_email(autoemail, delivery_status, @submission.person, nil, current_user)

redirect_to submissions_path, notice: 'Submission successfully created.'
else
Expand Down
4 changes: 4 additions & 0 deletions app/javascript/pages/browse/TileListItem.vue
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@
<div class="buttonSpacing" v-if="respond_path">
<a :href="respond_path" class="button icon-list is-primary"><span class=""> Respond</span></a>
</div>
<div class="buttonSpacing" v-if="view_path">
<a :href="view_path" class="button icon-list is-primary"><span class=""> View</span></a>
</div>
</div>
</li>
</template>
Expand All @@ -59,6 +62,7 @@ export default {
contact_types: {type: Array, default: () => []},
profile_path: String,
respond_path: String,
view_path: String,
match_path: String
},
components: {
Expand Down
24 changes: 24 additions & 0 deletions app/lib/anonymize.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
class Anonymize
Copy link
Collaborator

Choose a reason for hiding this comment

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

cool!

def self.name(name)
return if name.blank?

names = name.split(" ")
names.map do |n|
initial = n.first
n = '*' * n.length
initial + n[1..-1]
end.join(" ")
end

def self.email(email)
return if email.blank?

at_index = email.index("@")
words = email.split(".")
top_level_domain = words.pop

masked_email = "*" * words.join(".").length
masked_email[at_index] = "@"
masked_email.concat(".", top_level_domain)
end
end
14 changes: 14 additions & 0 deletions app/mailers/peer_to_peer_match_mailer.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
class PeerToPeerMatchMailer < ApplicationMailer
def peer_to_peer_email(contribution, peer_alias:, message:)
peer_email_address = contribution.person.email
email_subject = if contribution.ask?
"#{peer_alias} is offering to meet your ask"
elsif contribution.offer? # TODO: check if community resource when added
"#{peer_alias} would like to accept your offer"
end

mail(to: peer_email_address, subject: email_subject) do |format|
format.html { render locals: { message: message } }
end
end
end
4 changes: 3 additions & 1 deletion app/models/browse_filter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,9 @@ def contributions
def options
return {} unless context

{ respond_path: ->(id) { context.respond_contribution_path(id)} }
options = { respond_path: ->(id) { context.respond_contribution_path(id)} }
options[:view_path] = ->(id) { context.contribution_path(id) } if SystemSetting.current_settings&.peer_to_peer?
options
end

private
Expand Down
4 changes: 2 additions & 2 deletions app/models/communication_log.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ class CommunicationLog < ApplicationRecord

scope :needs_follow_up, ->(boolean=nil){ where(needs_follow_up: boolean || true) }

def self.log_submission_email(email_object, delivery_status, submission, delivery_method=nil, current_user=nil)
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is a great change

def self.log_email(email_object, delivery_status, person, delivery_method=nil, current_user=nil)
solebared marked this conversation as resolved.
Show resolved Hide resolved
self.create!(delivery_method: delivery_method || ContactMethod.email,
delivery_status: delivery_status,
person: submission.person,
person: person,
sent_at: Time.current,
subject: email_object.subject,
body: email_object.html_part&.body || email_object.body.raw_source,
Expand Down
18 changes: 18 additions & 0 deletions app/models/match.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,24 @@ def self.follow_up_status(follow_up_status)
result
end

# FIXME: extract into an interactor
def self.create_match_for_contribution!(contribution, current_user)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we add the following comment above this method:
# FIXME: extract into an interactor.

I think this kind of use-case specific logic fits nicely into the interactor pattern which, among other benefits, helps to keep models from getting bloated. Unfortunately we don't have an example in the codebase i can point at currently (though we have pulling in the active_interaction gem), so probably makes sense to refactor this later.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That is really good to know. Do you mind if I try to implement active_interaction? I wasn't sure if you wanted to set up any conventions for interactors.

Copy link
Collaborator

@solebared solebared Sep 25, 2020

Choose a reason for hiding this comment

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

Oh yes, please go ahead if you have the time and are motivated! 💯

I don't have any strong conventions in mind, the gem is pretty good.

In a separate WIP branch, i've placed them in app/interactions. The example there uses the run! form which assumes the interactor isn't trying to return any meaningful data back to the controller; there's the regular run form for when a result object is useful).

One challenge you might run into is that claims_controller makes use of deliver_now_with_error_handling which won't be available from within an interactor. My intention on that other branch is to extract that method into a utility that can be used from anywhere instead of having to be included as a concern.

match_params = if contribution.ask?
{ receiver: contribution, provider: create_offer_for_ask!(contribution, current_user) }
elsif contribution.offer? # TODO: check if community resource type when it's added
{ receiver: create_ask_for_offer!(contribution, current_user), provider: contribution }
end
Match.create!(match_params)
end

def self.create_offer_for_ask!(ask, current_user)
Offer.create!(person: current_user.person, service_area: ask.service_area)
end

def self.create_ask_for_offer!(offer, current_user)
Ask.create!(person: current_user.person, service_area: offer.service_area)
end
Comment on lines +54 to +60
Copy link
Collaborator

Choose a reason for hiding this comment

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

@maebeale , is this sufficient info on the auto-created Ask|Offer? Should we add the category as well?

Copy link
Collaborator

Choose a reason for hiding this comment

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

ooo. yes, we should add Category to the autogenerated Ask/Offer


def category
receiver.all_tags_to_s
end
Expand Down
17 changes: 17 additions & 0 deletions app/models/person.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,19 @@ class Person < ApplicationRecord

validate :preferred_contact_method_present!

# FIXME: extract into an interactor
def self.create_from_peer_to_peer_params!(current_user, name:, preferred_contact_method_id:, contact_info:)
contact_method = ContactMethod.find(preferred_contact_method_id)
person_params = { name: name,
preferred_contact_method: contact_method,
user: current_user }

contact_method_name = contact_method.name == "Email" ? :email : :phone
person_params[contact_method_name] = contact_info

create!(person_params)
end

def name_and_email
"#{name} (#{email})"
end
Expand Down Expand Up @@ -54,6 +67,10 @@ def offer_tag_list
offers.any? ? offers&.map(&:all_tags_unique) : []
end

def anonymized_name_and_email
"#{Anonymize.name(name)} #{Anonymize.email(email)}"
end

private def preferred_contact_method_present!
return unless preferred_contact_method
field = preferred_contact_method.field
Expand Down
1 change: 0 additions & 1 deletion app/models/system_setting.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ def self.current_settings
self.last
end


def dispatch_assisted?
exchange_type == "dispatch_assisted"
end
Expand Down
21 changes: 21 additions & 0 deletions app/views/claims/new.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<section>
<%= content_tag(:h1, "Claim This #{contribution.type}", class: "title") %>
<div class="box">
<p class="mb-2">
Information you enter here will be delivered directly to the other party.
Remember to protect yourself and never provide sensitive personal info to
strangers.
</p>

<%= simple_form_for :claim, url: contribution_claims_path(contribution) do |f| %>
<%= f.input :peer_alias, as: :string, label: false, placeholder: "Alias or handle (do not use your real name here)", required: true %>
<%= f.input :preferred_contact_method_id, collection: Array(ContactMethod.email), label: "Preferred contact method", required: true %>
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

<%= f.input :preferred_contact_info, as: :string, placeholder: "Add your phone number or email address", input_html: { value: preferred_contact_info }, required: true %>
<%= f.input :message, as: :text, required: true, placeholder: "Add some introductory text here. We do not share your contact information
with the other party. Please only share information that you are comfortable
sharing to facilitate this conversation.".squish %>
<%= f.button :submit, "I understand, let's go!", class: "is-primary" %>
<% end %>
</div>
<%= link_to "<i class=\"fas fa-arrow-left\"></i> Nevermind, I'm not ready".html_safe, :back %>
</section>
15 changes: 15 additions & 0 deletions app/views/contributions/show.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<section class="section">
<h1>Request for Contribution</h1>
<h2 class="title"><%= contribution.type %>: <%= contribution.all_tags_unique.map(&:humanize).join(", ") %></h2>
<br>
<div class="box">
<p><strong><%= contribution.person.anonymized_name_and_email %></strong></p>
<p class="subtitle"><%= contribution.description %></p>
<% if contribution.matched? %>
<i class="far fa-check-circle has-text-primary is-size-5 has-text-weight-semibold"><span class="ml-1">Claimed</span></i>
<% else %>
<%= button_to "Claim This #{contribution.type}", new_contribution_claim_path(contribution), class: "button is-primary", method: :get %>
<% end %>
</div>
<%= link_to "<i class=\"fas fa-arrow-left\"></i> View Contributions".html_safe, contributions_path %>
</section>
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<%= content_tag(:div, message) %>
3 changes: 2 additions & 1 deletion config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,14 @@
resources :contact_methods
get "/combined_form", to: "contributions#combined_form", as: "combined_form"
get "/thank_you", to: "contributions#thank_you", as: "contribution_thank_you"
resources :contributions, only: [:index] do
resources :contributions, only: [:index, :show] do
member do
get "/respond", to: "contributions#respond", as: "respond"
get "/triage", to: "contributions#triage", as: "triage"
patch "/triage", to: "contributions#triage_update"
post "/triage", to: "contributions#triage_update"
end
resources :claims, only: [:new, :create]
end
resources :custom_form_questions
resources :donations
Expand Down
1 change: 1 addition & 0 deletions spec/blueprints/contribution_blueprint_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
# "publish_until_humanized" => "this year",
"created_at" => (contribution.created_at.to_f * 1000), # Javascript wants miliseconds, not seconds
'respond_path' => nil,
'view_path' => nil,
'profile_path' => nil,
'match_path' => nil,
"service_area" => {"id" => contribution.service_area.id, "name" => expected_area_name},
Expand Down
12 changes: 12 additions & 0 deletions spec/requests/contributions_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,4 +64,16 @@
expect(response_ids).not_to include(no_tags_correct_area_listing.id)
end
end

describe "GET /contributions/:id" do
it "is successful" do
contribution = create(:listing)

get(
contribution_url(contribution),
)

expect(response).to be_successful
end
end
end