Skip to content

Commit

Permalink
Surveys for unregistered users (decidim#4996) (#847)
Browse files Browse the repository at this point in the history
* Surveys for unregistered users (decidim#4996)

* Add allow_unregistered option to surveys component

* locales for allow_unregistered settings action

* relax permissions to allow answers withou users

* Add session token to answer for grouping anonymous answers

* check if an unregistered user has already answered the questionnaire

* add unregistered users test

* rubocop happiness

* change test to match new locale messages

* add changelog entry [ci skip]

* unregisterd users settings warning

* add ip hash to unregistered surveys

* add invisible_captcha to forms

* Add user status to answers export

* remove unused key

* avoid NULL column

* use current instead of now

* refactor questionnaire validation for unregistered users

* simplify questionnaire validation and fix meetings reuse

* fix meetings tests

* Fix missing translations [ci skip]
  • Loading branch information
armandfardeau authored Nov 14, 2019
1 parent 1411c88 commit c99eac6
Show file tree
Hide file tree
Showing 22 changed files with 310 additions and 19 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ end
- **decidim-system**: Add custom SMTP settings for multitenant [#4698](https://github.com/decidim/decidim/pull/4698)
- **decidim-proposals**: Add proposal answers to the proposal export [#5139](https://github.com/decidim/decidim/pull/5139)
- **decidim-core**: Provide "good" password rules [#5106](https://github.com/decidim/decidim/pull/5106)
- **decidim-surveys**: Added a setting to surveys to allow unregistered (aka: anonymous) users to answer a survey. [\#4996](https://github.com/decidim/decidim/pull/4996)

**Changed**:

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,20 @@ def call
broadcast(:ok)
end

attr_reader :form

private

def answer_questionnaire
Answer.transaction do
@form.answers.each do |form_answer|
form.answers.each do |form_answer|
answer = Answer.new(
user: @current_user,
questionnaire: @questionnaire,
question: form_answer.question,
body: form_answer.body
body: form_answer.body,
session_token: form.context.session_token,
ip_hash: form.context.ip_hash
)

form_answer.selected_choices.each do |choice|
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ module HasQuestionnaire
helper Decidim::Forms::ApplicationHelper
include FormFactory

helper_method :questionnaire_for, :questionnaire, :allow_answers?, :update_url
helper_method :questionnaire_for, :questionnaire, :allow_answers?, :visitor_can_answer?, :visitor_already_answered?, :update_url

invisible_captcha on_spam: :spam_detected

def show
@form = form(Decidim::Forms::QuestionnaireForm).from_model(questionnaire)
Expand All @@ -26,7 +28,7 @@ def show
def answer
enforce_permission_to :answer, :questionnaire

@form = form(Decidim::Forms::QuestionnaireForm).from_params(params)
@form = form(Decidim::Forms::QuestionnaireForm).from_params(params, session_token: session_token)

Decidim::Forms::AnswerQuestionnaire.call(@form, current_user, questionnaire) do
on(:ok) do
Expand All @@ -49,6 +51,22 @@ def allow_answers?
raise "#{self.class.name} is expected to implement #allow_answers?"
end

# Public: Method to be implemented at the controller if needed. You need to
# return true if the questionnaire can receive answers by unregistered users
def allow_unregistered?
false
end

# Public: return true if the current user (if logged) can answer the questionnaire
def visitor_can_answer?
current_user || allow_unregistered?
end

# Public: return true if the current user (or session visitor) can answer the questionnaire
def visitor_already_answered?
questionnaire.answered_by?(current_user || tokenize(session[:session_id]))
end

# Public: Returns a String or Object that will be passed to `redirect_to` after
# answering the questionnaire. By default it redirects to the questionnaire_for.
#
Expand Down Expand Up @@ -78,6 +96,35 @@ def i18n_flashes_scope
def questionnaire
@questionnaire ||= Questionnaire.includes(questions: :answer_options).find_by(questionnaire_for: questionnaire_for)
end

def spam_detected
enforce_permission_to :answer, :questionnaire

@form = form(Decidim::Forms::QuestionnaireForm).from_params(params)

flash.now[:alert] = I18n.t("answer.spam_detected", scope: i18n_flashes_scope)
render template: "decidim/forms/questionnaires/show"
end

def ip_hash
return nil unless request&.remote_ip

@ip_hash ||= tokenize(request&.remote_ip)
end

# token is used as a substitute of user_id if unregistered
def session_token
id = current_user&.id
session_id = request.session[:session_id] if request&.session

return nil unless id || session_id

@session_token ||= tokenize(id || session_id)
end

def tokenize(id)
Digest::MD5.hexdigest("#{id}-#{Rails.application.secrets.secret_key_base}")
end
end
end
end
Expand Down
8 changes: 8 additions & 0 deletions decidim-forms/app/forms/decidim/forms/questionnaire_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ class QuestionnaireForm < Decidim::Form
attribute :user_group_id, Integer

attribute :tos_agreement, Boolean

validates :tos_agreement, allow_nil: false, acceptance: true
validate :session_token_in_context

# Private: Create the answers from the questionnaire questions
#
Expand All @@ -18,6 +20,12 @@ def map_model(model)
AnswerForm.from_model(Decidim::Forms::Answer.new(question: question))
end
end

def session_token_in_context
return if context&.session_token

errors.add(:tos_agreement, I18n.t("activemodel.errors.models.questionnaire.request_invalid"))
end
end
end
end
4 changes: 2 additions & 2 deletions decidim-forms/app/models/decidim/forms/answer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ class Answer < Forms::ApplicationRecord
include Decidim::DataPortability
include Decidim::NewsletterParticipant

belongs_to :user, class_name: "Decidim::User", foreign_key: "decidim_user_id"
belongs_to :user, class_name: "Decidim::User", foreign_key: "decidim_user_id", optional: true
belongs_to :questionnaire, class_name: "Questionnaire", foreign_key: "decidim_questionnaire_id"
belongs_to :question, class_name: "Question", foreign_key: "decidim_question_id"

Expand Down Expand Up @@ -45,7 +45,7 @@ def self.newsletter_participant_ids(component)
private

def user_questionnaire_same_organization
return if user&.organization == questionnaire.questionnaire_for&.organization
return if user.nil? || user&.organization == questionnaire.questionnaire_for&.organization

errors.add(:user, :invalid)
end
Expand Down
3 changes: 2 additions & 1 deletion decidim-forms/app/models/decidim/forms/questionnaire.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ def questions_editable?

# Public: returns whether the questionnaire is answered by the user or not.
def answered_by?(user)
answers.where(user: user).any? if questions.present?
query = user.is_a?(String) ? { session_token: user } : { user: user }
answers.where(query).any? if questions.present?
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def initialize(questionnaire)
# Finds and group answers by user for each questionnaire's question.
def query
answers = Answer.where(questionnaire: @questionnaire)
answers.sort_by { |answer| answer.question.position }.group_by(&:user).values
answers.sort_by { |answer| answer.question.position }.group_by { |a| a.user || a.session_token }.values
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
<div class="card">
<div class="card__content">
<% if allow_answers? %>
<% if current_user %>
<% if questionnaire.answered_by?(current_user) %>
<% if visitor_can_answer? %>
<% if visitor_already_answered? %>
<div class="section">
<div class="callout success">
<h5><%= t(".questionnaire_answered.title") %></h5>
Expand All @@ -39,6 +39,7 @@
<% end %>

<%= decidim_form_for(@form, url: update_url, method: :post, html: { class: "form answer-questionnaire" }) do |form| %>
<%= invisible_captcha %>
<% @form.answers.each_with_index do |answer, answer_idx| %>
<div class="row column">
<%= fields_for "questionnaire[answers][#{answer_idx}]", answer do |answer_form| %>
Expand Down
6 changes: 6 additions & 0 deletions decidim-forms/config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ en:
choices:
missing: are not complete
too_many: are too many
questionnaire:
request_invalid: There's been an error handling the request. Please try again
decidim:
forms:
admin:
Expand Down Expand Up @@ -81,3 +83,7 @@ en:
user_answers_serializer:
created_at: Answered on
id: Answer ID
ip_hash: Ip Hash
registered: Registered
unregistered: Unregistered
user_status: User status
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# frozen_string_literal: true

class AddSessionTokenToDecidimFormsAnswers < ActiveRecord::Migration[5.2]
class Answer < ApplicationRecord
self.table_name = :decidim_forms_answers
end

def change
add_column :decidim_forms_answers, :session_token, :string, null: false, default: ""
add_index :decidim_forms_answers, :session_token

Answer.find_each do |answer|
answer.session_token = Digest::MD5.hexdigest("#{answer.decidim_user_id}-#{Rails.application.secrets.secret_key_base}")
answer.save!
end
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# frozen_string_literal: true

class AddIpHashToDecidimFormAnswers < ActiveRecord::Migration[5.2]
class Answer < ApplicationRecord
self.table_name = :decidim_forms_answers
end

def change
add_column :decidim_forms_answers, :ip_hash, :string
add_index :decidim_forms_answers, :ip_hash
end
end
2 changes: 2 additions & 0 deletions decidim-forms/lib/decidim/forms/user_answers_serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ def serialize
serialized.update(
answer_translated_attribute_name(:id) => answer.id,
answer_translated_attribute_name(:created_at) => answer.created_at.to_s(:db),
answer_translated_attribute_name(:ip_hash) => answer.ip_hash,
answer_translated_attribute_name(:user_status) => answer_translated_attribute_name(answer.decidim_user_id.present? ? "registered" : "unregistered"),
"#{idx + 1}. #{translated_attribute(answer.question.body)}" => normalize_body(answer)
)
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,23 @@
module Decidim
module Forms
describe AnswerQuestionnaire do
def tokenize(id)
Digest::MD5.hexdigest("#{id}-#{Rails.application.secrets.secret_key_base}")
end

let(:current_organization) { create(:organization) }
let(:current_user) { create(:user, organization: current_organization) }
let(:session_id) { "session-string" }
let(:session_token) { tokenize(current_user&.id || session_id) }
let(:remote_ip) { "1.1.1.1" }
let(:ip_hash) { tokenize(remote_ip) }
let(:request) do
double(
session: { session_id: session_id },
remote_ip: remote_ip
)
end

let(:participatory_process) { create(:participatory_process, organization: current_organization) }
let(:questionnaire) { create(:questionnaire, questionnaire_for: participatory_process) }
let(:question_1) { create(:questionnaire_question, questionnaire: questionnaire) }
Expand Down Expand Up @@ -44,7 +59,9 @@ module Forms
QuestionnaireForm.from_params(
form_params
).with_context(
current_organization: current_organization
current_organization: current_organization,
session_token: session_token,
ip_hash: ip_hash
)
end
let(:command) { described_class.new(form, current_user, questionnaire) }
Expand Down Expand Up @@ -84,6 +101,61 @@ module Forms
expect(Answer.second.choices.pluck(:body)).to eq(%w(My second answer))
expect(Answer.third.choices.pluck(:body, :position)).to eq([["Third", 0], ["answer", 1]])
end

# This is to ensure that always exists a uniq identifier per-user
context "and user is registered" do
let(:ip_hash) { nil }

it "broadcasts ok" do
expect { command.call }.to broadcast(:ok)
end

it "have answers with session_token information" do
command.call

expect(Answer.first.session_token).to eq(tokenize(current_user.id))
expect(Answer.first.ip_hash).to eq(nil)
expect(Answer.second.session_token).to eq(tokenize(current_user.id))
expect(Answer.second.ip_hash).to eq(nil)
expect(Answer.third.session_token).to eq(tokenize(current_user.id))
expect(Answer.third.ip_hash).to eq(nil)
end
end
end

describe "when the user is unregistered" do
let(:current_user) { nil }

context "and session exists" do
it "broadcasts ok" do
expect { command.call }.to broadcast(:ok)
end

it "have answers session/ip information" do
command.call

expect(Answer.first.session_token).to eq(tokenize(session_id))
expect(Answer.first.ip_hash).to eq(ip_hash)
expect(Answer.second.session_token).to eq(tokenize(session_id))
expect(Answer.second.ip_hash).to eq(ip_hash)
expect(Answer.third.session_token).to eq(tokenize(session_id))
expect(Answer.third.ip_hash).to eq(ip_hash)
end
end

context "and session is missing" do
let(:session_token) { nil }

it "broadcasts invalid" do
expect { command.call }.to broadcast(:invalid)
end

it "doesn't create questionnaire answers" do
expect do
command.call
end.not_to change(Answer, :count)
end
end
end
end
end
Expand Down
23 changes: 20 additions & 3 deletions decidim-forms/spec/forms/decidim/forms/questionnaire_form_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,18 @@ module Decidim
module Forms
describe QuestionnaireForm do
subject do
described_class.from_model(questionnaire)
described_class.from_model(questionnaire).with_context(context)
end

let!(:questionnaire) { create(:questionnaire) }
let!(:question) { create(:questionnaire_question, questionnaire: questionnaire) }
let(:current_user) { create(:user) }
let(:session_token) { "some-token" }
let(:context) do
{
session_token: session_token
}
end

it "builds empty answers for each question" do
expect(subject.answers.length).to eq(1)
Expand All @@ -20,12 +27,22 @@ module Forms
it { is_expected.not_to be_valid }
end

context "when tos_agreement is not accepted" do
context "when tos_agreement is accepted" do
before do
subject.tos_agreement = true
end

it { is_expected.to be_valid }
context "and no token is present" do
let(:session_token) { nil }

it { is_expected.not_to be_valid }
end

context "and token is present" do
let(:ip_hash) { nil }

it { is_expected.to be_valid }
end
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class RegistrationsController < Decidim::Meetings::ApplicationController
def answer
enforce_permission_to :join, :meeting, meeting: meeting

@form = form(Decidim::Forms::QuestionnaireForm).from_params(params)
@form = form(Decidim::Forms::QuestionnaireForm).from_params(params, session_token: session_token)

JoinMeeting.call(meeting, current_user, @form) do
on(:ok) do
Expand Down
Loading

0 comments on commit c99eac6

Please sign in to comment.