From 7e9cf5d739c20384cfea048a1ae44d9aaeb14895 Mon Sep 17 00:00:00 2001 From: "Chris Baudouin, Jr" Date: Thu, 10 Dec 2020 23:25:35 -0500 Subject: [PATCH 01/10] feat: Adds Legal Agreements object for dynamic agreements --- app/assets/stylesheets/forms/_forms.sass | 3 + .../manage/agreements_controller.rb | 65 ++++++ app/controllers/manage/checkins_controller.rb | 1 + .../manage/questionnaires_controller.rb | 8 +- app/controllers/questionnaires_controller.rb | 10 +- app/controllers/rsvps_controller.rb | 2 +- app/models/agreement.rb | 13 ++ app/models/questionnaire.rb | 27 ++- .../_unaccepted_agreements_notice.html.haml | 10 + .../doorkeeper/applications/index.html.haml | 3 - .../layouts/manage/application.html.haml | 4 + app/views/manage/agreements/_form.html.haml | 10 + app/views/manage/agreements/edit.html.haml | 3 + app/views/manage/agreements/index.html.haml | 28 +++ app/views/manage/agreements/new.html.haml | 3 + .../_checkin_compliance_card.html.haml | 21 +- .../manage/questionnaires/_form.html.haml | 19 +- app/views/questionnaires/_form.html.haml | 29 +-- app/views/questionnaires/show.html.haml | 3 + app/views/rsvps/show.html.haml | 3 + config/locales/en.yml | 12 + config/routes.rb | 11 +- .../20201130220942_create_agreements.rb | 10 + ...2_create_agreements_questionnaires_join.rb | 9 + ...e_old_agreement_data_from_questionnaire.rb | 13 ++ db/schema.rb | 18 +- test/controllers/bus_lists_controller_test.rb | 2 +- .../manage/agreements_controller_test.rb | 219 ++++++++++++++++++ .../manage/bus_lists_controller_test.rb | 4 +- .../manage/dashboard_controller_test.rb | 32 +-- .../manage/questionnaires_controller_test.rb | 21 +- .../manage/schools_controller_test.rb | 6 +- .../manage/stats_controller_test.rb | 4 +- .../questionnaires_controller_test.rb | 14 +- test/controllers/rsvps_controller_test.rb | 11 +- test/factories/agreement.rb | 6 + test/factories/questionnaire.rb | 4 +- test/jobs/bulk_message_job_test.rb | 18 +- test/models/agreement_test.rb | 20 ++ test/models/bus_list_test.rb | 40 ++-- test/models/questionnaire_test.rb | 6 +- test/models/user_test.rb | 6 +- 42 files changed, 590 insertions(+), 161 deletions(-) create mode 100644 app/controllers/manage/agreements_controller.rb create mode 100644 app/models/agreement.rb create mode 100644 app/views/application/_unaccepted_agreements_notice.html.haml create mode 100644 app/views/manage/agreements/_form.html.haml create mode 100644 app/views/manage/agreements/edit.html.haml create mode 100644 app/views/manage/agreements/index.html.haml create mode 100644 app/views/manage/agreements/new.html.haml create mode 100644 db/migrate/20201130220942_create_agreements.rb create mode 100644 db/migrate/20201130221702_create_agreements_questionnaires_join.rb create mode 100644 db/migrate/20201209053827_remove_old_agreement_data_from_questionnaire.rb create mode 100644 test/controllers/manage/agreements_controller_test.rb create mode 100644 test/factories/agreement.rb create mode 100644 test/models/agreement_test.rb diff --git a/app/assets/stylesheets/forms/_forms.sass b/app/assets/stylesheets/forms/_forms.sass index ad14b353a..6d26169c0 100644 --- a/app/assets/stylesheets/forms/_forms.sass +++ b/app/assets/stylesheets/forms/_forms.sass @@ -18,6 +18,9 @@ $form-spacing-horizontal: 9px .section-title margin: 12px 0 5px +#disclaimer.alert + border: 2px solid var(--alert--border) + fieldset margin: 25px 0 0 border: 1px solid #aaa diff --git a/app/controllers/manage/agreements_controller.rb b/app/controllers/manage/agreements_controller.rb new file mode 100644 index 000000000..1799b4c82 --- /dev/null +++ b/app/controllers/manage/agreements_controller.rb @@ -0,0 +1,65 @@ +class Manage::AgreementsController < Manage::ApplicationController + before_action :require_director + before_action :set_agreement, only: [:show, :edit, :update, :destroy] + + respond_to :html, :json + + # GET /agreements + def index + @agreements = Agreement.all + end + + # GET /agreements/new + def new + @agreement = Agreement.new + end + + # GET /agreements/1/edit + def edit + end + + # POST /agreements + def create + if !agreement_params['agreement_url'].start_with?('http://', 'https://') + flash[:alert] = "Agreement URL must start with http:// or https://" + redirect_to new_manage_agreement_path + else + @agreement = Agreement.new(agreement_params) + @agreement.save + flash[:notice] = "#{@agreement.name} was successfully created." + redirect_to manage_agreements_path + end + end + + # PATCH/PUT /agreements/1 + def update + if !agreement_params['agreement_url'].start_with?('http://', 'https://') + flash[:alert] = "Agreement URL must start with http:// or https://" + render :edit + else + @agreement.update_attributes(agreement_params) + flash[:notice] = nil + redirect_to manage_agreements_path + end + end + + # DELETE /agreements/1 + def destroy + @agreement.destroy + flash[:notice] = 'Agreement was successfully destroyed.' + respond_with(:manage, @agreement, location: manage_agreements_path) + end + + private + # Use callbacks to share common setup or constraints between actions. + def set_agreement + @agreement = Agreement.find(params[:id]) + end + + # Only allow a trusted parameter "white list" through. + def agreement_params + params.require(:agreement).permit( + :name, :agreement_url + ) + end +end diff --git a/app/controllers/manage/checkins_controller.rb b/app/controllers/manage/checkins_controller.rb index 4da494cb8..0715f839d 100644 --- a/app/controllers/manage/checkins_controller.rb +++ b/app/controllers/manage/checkins_controller.rb @@ -12,6 +12,7 @@ def datatable end def show + @agreements = Agreement.all respond_with(:manage, @questionnaire) end diff --git a/app/controllers/manage/questionnaires_controller.rb b/app/controllers/manage/questionnaires_controller.rb index ce2ed1218..fdae4c654 100644 --- a/app/controllers/manage/questionnaires_controller.rb +++ b/app/controllers/manage/questionnaires_controller.rb @@ -14,6 +14,7 @@ def datatable end def show + @agreements = Agreement.all respond_with(:manage, @questionnaire) end @@ -70,7 +71,7 @@ def check_in index_redirect_path = redirect_to_checkins ? manage_checkins_path : manage_questionnaires_path if params[:check_in] == "true" if params[:questionnaire] - q_params = params.require(:questionnaire).permit(:agreement_accepted, :phone, :can_share_info, :email) + q_params = params.require(:questionnaire).permit(:phone, :can_share_info, :email) email = q_params.delete(:email) @questionnaire.update_attributes(q_params) @questionnaire.user.update_attributes(email: email) @@ -153,9 +154,8 @@ def questionnaire_params :email, :experience, :gender, :date_of_birth, :interest, :school_id, :school_name, :major, :level_of_study, :shirt_size, :dietary_restrictions, :special_needs, :international, - :portfolio_url, :vcs_url, :agreement_accepted, :bus_captain_interest, - :phone, :can_share_info, :code_of_conduct_accepted, - :travel_not_from_school, :travel_location, :data_sharing_accepted, + :portfolio_url, :vcs_url, :bus_captain_interest,:phone, :can_share_info, + :travel_not_from_school, :travel_location, :graduation_year, :race_ethnicity, :resume, :delete_resume, :why_attend, :bus_list_id, :is_bus_captain, :boarded_bus ) diff --git a/app/controllers/questionnaires_controller.rb b/app/controllers/questionnaires_controller.rb index 7a2e07ac5..566b845ac 100644 --- a/app/controllers/questionnaires_controller.rb +++ b/app/controllers/questionnaires_controller.rb @@ -24,6 +24,7 @@ def new return redirect_to questionnaires_path end @questionnaire = Questionnaire.new + @agreements = Agreement.all if session["devise.provider_data"] && session["devise.provider_data"]["info"] info = session["devise.provider_data"]["info"] @@ -50,6 +51,7 @@ def new # GET /apply/edit def edit + @agreements = Agreement.all end # POST /apply @@ -62,6 +64,7 @@ def create @questionnaire = Questionnaire.new(convert_school_name_to_id(questionnaire_params)) @questionnaire.user_id = current_user.id + @agreements = Agreement.all respond_to do |format| if @questionnaire.save @@ -126,10 +129,9 @@ def questionnaire_params :email, :experience, :gender, :date_of_birth, :interest, :school_id, :school_name, :major, :level_of_study, :shirt_size, :dietary_restrictions, :special_needs, :international, - :portfolio_url, :vcs_url, :agreement_accepted, :bus_captain_interest, - :phone, :can_share_info, :code_of_conduct_accepted, - :travel_not_from_school, :travel_location, :data_sharing_accepted, - :graduation_year, :race_ethnicity, :resume, :delete_resume, :why_attend + :portfolio_url, :vcs_url, :bus_captain_interest, + :phone, :can_share_info, :travel_not_from_school, :travel_location, + :graduation_year, :race_ethnicity, :resume, :delete_resume, :why_attend, :agreement_ids => [] ) end diff --git a/app/controllers/rsvps_controller.rb b/app/controllers/rsvps_controller.rb index 708b77c11..24c44c3ec 100644 --- a/app/controllers/rsvps_controller.rb +++ b/app/controllers/rsvps_controller.rb @@ -47,7 +47,7 @@ def update bus = @questionnaire.bus_list_id acc_status = @questionnaire.acc_status - unless @questionnaire.update_attributes(params.require(:questionnaire).permit(:agreement_accepted, :phone)) + unless @questionnaire.update_attributes(params.require(:questionnaire).permit(:phone)) flash[:alert] = @questionnaire.errors.full_messages.join(", ") redirect_to rsvp_path return diff --git a/app/models/agreement.rb b/app/models/agreement.rb new file mode 100644 index 000000000..abdd1256a --- /dev/null +++ b/app/models/agreement.rb @@ -0,0 +1,13 @@ +class Agreement < ApplicationRecord + include ActionView::Helpers::UrlHelper + validates_presence_of :name + validates_presence_of :agreement_url + + strip_attributes + + has_and_belongs_to_many :questionnaires + + def formatted_agreement + "I read and accept the #{link_to name, agreement_url, target: "_blank"} agreement.".html_safe + end +end diff --git a/app/models/questionnaire.rb b/app/models/questionnaire.rb index 310d6c6d1..4e35da532 100644 --- a/app/models/questionnaire.rb +++ b/app/models/questionnaire.rb @@ -18,19 +18,19 @@ class Questionnaire < ApplicationRecord belongs_to :user belongs_to :school belongs_to :bus_list, optional: true + has_and_belongs_to_many :agreements validates_uniqueness_of :user_id validates_presence_of :phone, :date_of_birth, :school_id, :experience, :shirt_size, :interest validates_presence_of :gender, :major, :level_of_study, :graduation_year, :race_ethnicity - validates_presence_of :agreement_accepted, message: "Please read & accept" - validates_presence_of :code_of_conduct_accepted, message: "Please read & accept" - validates_presence_of :data_sharing_accepted, message: "Please read & accept" DIETARY_SPECIAL_NEEDS_MAX_LENGTH = 500 validates_length_of :dietary_restrictions, maximum: DIETARY_SPECIAL_NEEDS_MAX_LENGTH validates_length_of :special_needs, maximum: DIETARY_SPECIAL_NEEDS_MAX_LENGTH + validate :agreements_present + # if HackathonManager.field_enabled?(:why_attend) # validates_presence_of :why_attend # end @@ -215,6 +215,26 @@ def verbal_status end end + def agreements_present + if (Agreement.all - self.agreements).any? + errors.add(:agreements, "must be accepted.") + end + end + + def all_agreements_accepted? + (Agreement.all - self.agreements).empty? + end + + def unaccepted_agreements + Agreement.all - self.agreements + end + + def as_json(options = {}) + result = super + result['all_agreements_accepted'] = all_agreements_accepted? + result + end + private def clean_for_non_rsvp @@ -277,4 +297,5 @@ def queue_triggered_email_rsvp_reminder end UserMailer.rsvp_reminder_email(user_id).deliver_later(wait_until: deliver_date) if deliver_date.present? end + end diff --git a/app/views/application/_unaccepted_agreements_notice.html.haml b/app/views/application/_unaccepted_agreements_notice.html.haml new file mode 100644 index 000000000..0d1dd83f8 --- /dev/null +++ b/app/views/application/_unaccepted_agreements_notice.html.haml @@ -0,0 +1,10 @@ +#disclaimer.alert + %h1.section-title + %span.emphasized.fa.fa-exclamation-circle + Missing + %span.emphasized Agreements + %p + You have unaccepted agreements. You will not be allowed at #{HackathonConfig['name']} until all agreements have been accepted. Please + %strong + #{link_to "edit your application", edit_questionnaires_path} + and review all agreements before attending. diff --git a/app/views/doorkeeper/applications/index.html.haml b/app/views/doorkeeper/applications/index.html.haml index d856aa59e..10d805773 100644 --- a/app/views/doorkeeper/applications/index.html.haml +++ b/app/views/doorkeeper/applications/index.html.haml @@ -10,8 +10,6 @@ = t(:name, scope: 'doorkeeper.applications.index') %th = t(:callback_url, scope: 'doorkeeper.applications.index') - %th - = t(:confidential, scope: 'doorkeeper.applications.index') %tbody @@ -26,4 +24,3 @@ = simple_format(application.redirect_uri) %td = application.confidential? ? t('doorkeeper.applications.index.confidentiality.yes') : t('doorkeeper.applications.index.confidentiality.no') - diff --git a/app/views/layouts/manage/application.html.haml b/app/views/layouts/manage/application.html.haml index 98b542311..318b9b9f0 100644 --- a/app/views/layouts/manage/application.html.haml +++ b/app/views/layouts/manage/application.html.haml @@ -99,6 +99,10 @@ = active_link_to manage_users_path, class: "nav-link" do .fa.fa-users.fa-fw.icon-space-r-half = t(:title, scope: 'pages.manage.users') + %li.nav-item + = active_link_to manage_agreements_path, class: "nav-link" do + .fa.fa-balance-scale.fa-fw.icon-space-r-half + = t(:title, scope: 'pages.manage.agreements') %li.nav-item = active_link_to manage_configs_path, class: "nav-link" do .fa.fa-wrench.fa-fw.icon-space-r-half diff --git a/app/views/manage/agreements/_form.html.haml b/app/views/manage/agreements/_form.html.haml new file mode 100644 index 000000000..beaf45816 --- /dev/null +++ b/app/views/manage/agreements/_form.html.haml @@ -0,0 +1,10 @@ +.form-container + = bs_horizontal_simple_form_for(@agreement, url: url_for(action: @agreement.new_record? ? "create" : "update", controller: "agreements")) do |f| + = f.error_notification + + .form-inputs + = f.input :name + = f.input :agreement_url, hint: "Should be a full https:// URL to a web page or .pdf file", label: t(:agreement_url, scope: 'pages.manage.agreements') + + .form-actions + = f.button :submit, class: 'btn-primary' diff --git a/app/views/manage/agreements/edit.html.haml b/app/views/manage/agreements/edit.html.haml new file mode 100644 index 000000000..e44f5b691 --- /dev/null +++ b/app/views/manage/agreements/edit.html.haml @@ -0,0 +1,3 @@ += render "layouts/manage/page_title", title: t(:edit, scope: 'pages.manage.agreements') + += render 'form' diff --git a/app/views/manage/agreements/index.html.haml b/app/views/manage/agreements/index.html.haml new file mode 100644 index 000000000..1907f6f36 --- /dev/null +++ b/app/views/manage/agreements/index.html.haml @@ -0,0 +1,28 @@ += render "layouts/manage/page_title", title: t(:title, scope: 'pages.manage.agreements') do + = link_to "New Agreement", new_manage_agreement_path, class: "btn btn-sm btn-outline-secondary" + +%p.text-muted + = t(:notice, scope: 'pages.manage.agreements', hackathon_name: HackathonConfig['name']) +.mb-4 + %table.table.table-striped.table-hover + %thead + %tr + %th + %th + %th + = t(:name, scope: 'pages.manage.agreements') + %th + = t(:agreement_url, scope: 'pages.manage.agreements') + + %tbody + - @agreements.each do |agreement| + %tr + %td + = link_to ''.html_safe, edit_manage_agreement_path(agreement) + %td + = link_to ''.html_safe, manage_agreement_path(agreement), method: :delete, data: { confirm: "Are you sure? The agreement will be permanently deleted. This action is irreversible." } + %td + %strong + = agreement.name + %td + = agreement.agreement_url diff --git a/app/views/manage/agreements/new.html.haml b/app/views/manage/agreements/new.html.haml new file mode 100644 index 000000000..c86f0bdc9 --- /dev/null +++ b/app/views/manage/agreements/new.html.haml @@ -0,0 +1,3 @@ += render "layouts/manage/page_title", title: t(:new, scope: 'pages.manage.agreements') + += render 'form' diff --git a/app/views/manage/questionnaires/_checkin_compliance_card.html.haml b/app/views/manage/questionnaires/_checkin_compliance_card.html.haml index 784a8376e..618a36a6b 100644 --- a/app/views/manage/questionnaires/_checkin_compliance_card.html.haml +++ b/app/views/manage/questionnaires/_checkin_compliance_card.html.haml @@ -33,12 +33,15 @@ %li.list-group-item.list-group-item-warning %span.fa.fa-exclamation-circle.fa-fw.icon-space-r 18 years or older - - if @questionnaire.agreement_accepted && @questionnaire.code_of_conduct_accepted && @questionnaire.data_sharing_accepted - %li.list-group-item - .text-success - %span.fa.fa-check.fa-fw.icon-space-r - Event agreement, MLH Code of Conduct, and MLH Data Sharing - - else - %li.list-group-item.list-group-item-danger - %span.fa.fa-close.fa-fw.icon-space-r - One or more of the following was not accepted: Event agreement, MLH Code of Conduct, and/or MLH Data Sharing + - if @agreements.any? + - if @questionnaire.all_agreements_accepted? + %li.list-group-item + .text-success + %span.fa.fa-check.fa-fw.icon-space-r + = @questionnaire.agreements.map(&:name).join(', ') + accepted. + - else + %li.list-group-item.list-group-item-danger + %span.fa.fa-close.fa-fw.icon-space-r + Unaccepted Agreements: + = @questionnaire.unaccepted_agreements.map(&:name).join(', ') diff --git a/app/views/manage/questionnaires/_form.html.haml b/app/views/manage/questionnaires/_form.html.haml index b52c22783..fe2588801 100644 --- a/app/views/manage/questionnaires/_form.html.haml +++ b/app/views/manage/questionnaires/_form.html.haml @@ -65,25 +65,8 @@ .card.mb-4 .card-header Agreements .card-body - .supporting-text - Please read the - = link_to asset_url(HackathonConfig['agreement_pdf_asset']), target: '_blank' do - #{HackathonConfig['name']} Agreement - %span.fa.fa-external-link.icon-space-l-half - = f.input :agreement_accepted, label: "I accept the #{HackathonConfig['name']} agreement.", input_html: { "data-validate" => "presence" } + = f.association :agreements, as: :check_boxes, label_method: :formatted_agreement, value_method: :id, label: "" - .supporting-text - Please read the - %a{ href:"http://static.mlh.io/docs/mlh-code-of-conduct.pdf", target: "_blank" } - MLH Code of Conduct - %span.fa.fa-external-link.icon-space-l-half - = f.input :code_of_conduct_accepted, label: "I accept the MLH Code of Conduct.", input_html: { "data-validate" => "presence" } - - .supporting-text - I agree to the terms of both the - MLH Contest Terms and Conditions and the - MLH Privacy Policy. Please note that you may receive pre and post-event informational e-mails and occasional messages about hackathons from MLH as per the MLH Privacy Policy. - = f.input :data_sharing_accepted, label: "I accept the MLH policies.", input_html: { "data-validate" => "presence" } .center.mb-4 = f.button :submit, value: ( @questionnaire.new_record? ? 'Create' : 'Save' ), class: 'btn-primary' diff --git a/app/views/questionnaires/_form.html.haml b/app/views/questionnaires/_form.html.haml index b895acebe..dc6fc4fa9 100644 --- a/app/views/questionnaires/_form.html.haml +++ b/app/views/questionnaires/_form.html.haml @@ -44,29 +44,12 @@ = f.input :dietary_restrictions, as: :text, label: "Health restrictions", wrapper_html: { class: 'input--half' }, maxlength: Questionnaire::DIETARY_SPECIAL_NEEDS_MAX_LENGTH = f.input :special_needs, as: :text, label: "Special needs", wrapper_html: { class: 'input--half' }, maxlength: Questionnaire::DIETARY_SPECIAL_NEEDS_MAX_LENGTH - %hr - - .form-inputs - .supporting-text - Please read the - = link_to asset_url(HackathonConfig['agreement_pdf_asset']), target: '_blank' do - #{HackathonConfig['name']} Agreement - %span.fa.fa-external-link.icon-space-l-half - = f.input :agreement_accepted, as: :formatted_boolean, label: "I read & accept the #{HackathonConfig['name']} agreement.", input_html: { "data-validate" => "presence" } - - .supporting-text - Please read the - %a{ href:"http://static.mlh.io/docs/mlh-code-of-conduct.pdf", target: "_blank" } - MLH Code of Conduct - %span.fa.fa-external-link.icon-space-l-half - = f.input :code_of_conduct_accepted, as: :formatted_boolean, label: "I read & accept the MLH Code of Conduct.", input_html: { "data-validate" => "presence" } - - .supporting-text - I authorize you to share certain application/registration information for event administration, ranking, MLH administration, pre and post-event informational e-mails, and occasional messages about hackathons in-line with the MLH Privacy Policy. I further I agree to the terms of both the - MLH Contest Terms and Conditions - and the - MLH Privacy Policy. - = f.input :data_sharing_accepted, as: :formatted_boolean, label: "I read & accept the MLH policies.", input_html: { "data-validate" => "presence" } + - if @agreements.any? + %hr + %strong Agreements + %p Please review the agreements and click the corresponding checkbox next to each agreement to agree. + .form-inputs + = f.association :agreements, as: :check_boxes, label_method: :formatted_agreement, value_method: :id, label: "" .right %button.button{ type: "button", "data-wizard" => "previous" } Previous diff --git a/app/views/questionnaires/show.html.haml b/app/views/questionnaires/show.html.haml index 3719d0d9e..fe10da604 100644 --- a/app/views/questionnaires/show.html.haml +++ b/app/views/questionnaires/show.html.haml @@ -1,4 +1,7 @@ - title "Application" +.form-container + - if @questionnaire.unaccepted_agreements.any? + = render partial: 'unaccepted_agreements_notice' .form-container #disclaimer %h1.section-title diff --git a/app/views/rsvps/show.html.haml b/app/views/rsvps/show.html.haml index c1348cf02..ae6de7927 100644 --- a/app/views/rsvps/show.html.haml +++ b/app/views/rsvps/show.html.haml @@ -1,4 +1,7 @@ - title "RSVP" +.form-container + - if @questionnaire.unaccepted_agreements.any? + = render partial: 'unaccepted_agreements_notice' = simple_form_for @questionnaire, url: url_for(controller: "rsvps", action: "update"), html: { "data-validate" => "form" } do |f| .form-container #disclaimer diff --git a/config/locales/en.yml b/config/locales/en.yml index 4e641763c..61fa43cd5 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -54,6 +54,8 @@ en: trigger: Sent automatically when a new or updated applicant matches this criteria. Does not send to anyone already matching this criteria. school: is_home: The "home" school is separated from all other schools on dashboard metrics. + agreement: + name: 'Agreements are displayed to applicants as: "I read & accept the *agreement_name* agreement."' hackathon_config: accepting_questionnaires: Specify and allow questionnaires to be accepted. last_day_to_apply: 'Last date to apply to your hackathon (format: YYYY-MM-DD)' @@ -187,6 +189,13 @@ en: yes: Yes no: No save: Save + agreements: + title: Legal Agreements + notice: "These are legal agreements that are required to be reviewed and agreed upon by all applicants of %{hackathon_name}." + name: Name + agreement_url: Agreement URL + new: New Agreement + edit: Edit Agreement settings: title: Hackathon Settings sidekiq: @@ -197,6 +206,9 @@ en: title: App Authentication data-exports: title: Data Exports + buttons: + edit: Edit + destroy: Delete layouts: manage: navigation: diff --git a/config/routes.rb b/config/routes.rb index d9c156a6c..55820e411 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -72,11 +72,6 @@ resources :checkins do post :datatable, on: :collection end - resources :users do - post :user_datatable, on: :collection - post :staff_datatable, on: :collection - patch :reset_password, on: :member - end resources :messages do get :preview, on: :member get :live_preview, on: :collection @@ -105,6 +100,12 @@ post :mlh_applied_datatable, on: :collection post :mlh_checked_in_datatable, on: :collection end + resources :users do + post :user_datatable, on: :collection + post :staff_datatable, on: :collection + patch :reset_password, on: :member + end + resources :agreements resources :configs do patch :update_only_css_variables, on: :member get :enter_theming_editor, on: :collection diff --git a/db/migrate/20201130220942_create_agreements.rb b/db/migrate/20201130220942_create_agreements.rb new file mode 100644 index 000000000..a864f573a --- /dev/null +++ b/db/migrate/20201130220942_create_agreements.rb @@ -0,0 +1,10 @@ +class CreateAgreements < ActiveRecord::Migration[5.2] + def change + create_table :agreements do |t| + t.string :name + + t.timestamps + end + add_column :agreements, :agreement_url, :text + end +end diff --git a/db/migrate/20201130221702_create_agreements_questionnaires_join.rb b/db/migrate/20201130221702_create_agreements_questionnaires_join.rb new file mode 100644 index 000000000..d3ed87ae3 --- /dev/null +++ b/db/migrate/20201130221702_create_agreements_questionnaires_join.rb @@ -0,0 +1,9 @@ +class CreateAgreementsQuestionnairesJoin < ActiveRecord::Migration[5.2] + def change + create_table :agreements_questionnaires, id: false do |t| + t.integer :agreement_id + t.integer :questionnaire_id + end + add_index :agreements_questionnaires, ["agreement_id", "questionnaire_id"], name: "index_agreements_questionnaires" + end +end diff --git a/db/migrate/20201209053827_remove_old_agreement_data_from_questionnaire.rb b/db/migrate/20201209053827_remove_old_agreement_data_from_questionnaire.rb new file mode 100644 index 000000000..0a44b9dec --- /dev/null +++ b/db/migrate/20201209053827_remove_old_agreement_data_from_questionnaire.rb @@ -0,0 +1,13 @@ +class RemoveOldAgreementDataFromQuestionnaire < ActiveRecord::Migration[5.2] + def self.up + remove_column :questionnaires, :agreement_accepted + remove_column :questionnaires, :code_of_conduct_accepted + remove_column :questionnaires, :data_sharing_accepted + end + + def self.down + add_column :questionnaires, :agreement_accepted, :bool, default: false + add_column :questionnaires, :code_of_conduct_accepted, :bool, default: false + add_column :questionnaires, :data_sharing_accepted, :bool, default: false + end +end diff --git a/db/schema.rb b/db/schema.rb index 7041720db..e6e0dc51f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2020_05_30_172450) do +ActiveRecord::Schema.define(version: 2020_12_09_053827) do create_table "active_storage_attachments", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8", force: :cascade do |t| t.string "name", null: false @@ -33,6 +33,19 @@ t.index ["key"], name: "index_active_storage_blobs_on_key", unique: true end + create_table "agreements", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8", force: :cascade do |t| + t.string "name" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.text "agreement_url" + end + + create_table "agreements_questionnaires", id: false, options: "ENGINE=InnoDB DEFAULT CHARSET=utf8", force: :cascade do |t| + t.integer "agreement_id" + t.integer "questionnaire_id" + t.index ["agreement_id", "questionnaire_id"], name: "index_agreements_questionnaires" + end + create_table "audits", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8", force: :cascade do |t| t.integer "auditable_id" t.string "auditable_type" @@ -204,7 +217,6 @@ t.string "portfolio_url" t.string "vcs_url" t.integer "user_id" - t.boolean "agreement_accepted", default: false t.string "acc_status", default: "pending" t.integer "acc_status_author_id" t.datetime "acc_status_date" @@ -214,13 +226,11 @@ t.datetime "checked_in_at" t.string "phone" t.boolean "can_share_info", default: false - t.boolean "code_of_conduct_accepted", default: false t.text "special_needs" t.string "gender" t.string "major" t.boolean "travel_not_from_school", default: false t.string "travel_location" - t.boolean "data_sharing_accepted" t.string "level_of_study" t.string "interest" t.text "why_attend" diff --git a/test/controllers/bus_lists_controller_test.rb b/test/controllers/bus_lists_controller_test.rb index e50ecd7fc..249bc49f4 100644 --- a/test/controllers/bus_lists_controller_test.rb +++ b/test/controllers/bus_lists_controller_test.rb @@ -99,7 +99,7 @@ class BusListsControllerTest < ActionController::TestCase should "not allow bus_list#boarded_bus for questionnaire not riding this bus" do bus_list = create(:bus_list, name: "A random bus list") - questionnaire = create(:questionnaire, acc_status: 'rsvp_confirmed', bus_list_id: bus_list.id) + questionnaire = create(:questionnaire, acc_status: 'rsvp_confirmed', bus_list_id: bus_list.id, agreements: Agreement.all) patch :boarded_bus, params: { questionnaire: { id: questionnaire.id, boarded_bus: true } } assert_response :bad_request assert_nil questionnaire.reload.boarded_bus_at diff --git a/test/controllers/manage/agreements_controller_test.rb b/test/controllers/manage/agreements_controller_test.rb new file mode 100644 index 000000000..39b0147a5 --- /dev/null +++ b/test/controllers/manage/agreements_controller_test.rb @@ -0,0 +1,219 @@ +require 'test_helper' + +class Manage::AgreementsControllerTest < ActionController::TestCase + + setup do + @agreement = create(:agreement) + end + + context "while not authenticated" do + should "redirect to sign in page on manage_agreements_#index" do + get :index + assert_response :redirect + assert_redirected_to new_user_session_path + end + + should "not allow access to manage_agreements#new" do + get :new + assert_response :redirect + assert_redirected_to new_user_session_path + end + + should "not allow access to manage_agreements#edit" do + get :edit, params: { id: @agreement } + assert_response :redirect + assert_redirected_to new_user_session_path + end + + should "not allow access to manage_agreements#create" do + post :create, params: { agreement: { name: "Fun Agreement", agreement_fun: "https://bar.edu" } } + assert_response :redirect + assert_redirected_to new_user_session_path + end + + should "not allow access to manage_agreements#update" do + patch :update, params: { id: @agreement, agreement: { name: "Not fun agreement" } } + assert_response :redirect + assert_redirected_to new_user_session_path + end + + should "not allow access to manage_agreements#destroy" do + patch :destroy, params: { id: @agreement } + assert_response :redirect + assert_redirected_to new_user_session_path + end + end + + context "while authenticated as a user" do + setup do + @user = create(:user) + @request.env["devise.mapping"] = Devise.mappings[:user] + sign_in @user + end + + should "not allow access to manage_agreements#index" do + get :index + assert_response :redirect + assert_redirected_to root_path + end + + should "not allow access to manage_agreements#new" do + get :new + assert_response :redirect + assert_redirected_to root_path + end + + should "not allow access to manage_agreements#edit" do + get :edit, params: { id: @agreement } + assert_response :redirect + assert_redirected_to root_path + end + + should "not allow access to manage_agreements#create" do + post :create, params: { agreement: { name: "Fun Agreement", agreement_fun: "https://bar.edu" } } + assert_response :redirect + assert_redirected_to root_path + end + + should "not allow access to manage_agreements#update" do + patch :update, params: { id: @agreement, agreement: { name: "Not fun agreement" } } + assert_response :redirect + assert_redirected_to root_path + end + + should "not allow access to manage_agreements#destroy" do + patch :destroy, params: { id: @agreement } + assert_response :redirect + assert_redirected_to root_path + end + end + + context "while authenticated as a volunteer" do + setup do + @user = create(:volunteer) + @request.env["devise.mapping"] = Devise.mappings[:user] + sign_in @user + end + + should "not allow access to manage_agreements#index" do + get :index + assert_response :redirect + end + + should "not allow access to manage_agreements#new" do + get :new + assert_response :redirect + assert_redirected_to manage_agreements_path + end + + should "not allow access to manage_agreements#edit" do + get :edit, params: { id: @agreement } + assert_response :redirect + assert_redirected_to manage_agreements_path + end + + should "not allow access to manage_agreements#create" do + post :create, params: { agreement: { name: "Fun Agreement", agreement_fun: "https://bar.edu" } } + assert_response :redirect + assert_redirected_to manage_agreements_path + end + + should "not allow access to manage_agreements#update" do + patch :update, params: { id: @agreement, agreement: { name: "Not fun agreement" } } + assert_response :redirect + assert_redirected_to manage_agreements_path + end + + should "not allow access to manage_agreements#destroy" do + patch :destroy, params: { id: @agreement } + assert_response :redirect + assert_redirected_to manage_agreements_path + end + end + + context "while authenticated as an organizer" do + setup do + @user = create(:organizer) + @request.env["devise.mapping"] = Devise.mappings[:user] + sign_in @user + end + + should "allow access to manage_agreements#index" do + get :index + assert_response :redirect + end + + should "not allow access to manage_agreements#new" do + get :new + assert_response :redirect + assert_redirected_to manage_agreements_path + end + + should "not allow access to manage_agreements#edit" do + get :edit, params: { id: @agreement } + assert_response :redirect + assert_redirected_to manage_agreements_path + end + + should "not allow access to manage_agreements#create" do + post :create, params: { agreement: { name: "Fun Agreement", agreement_fun: "https://bar.edu" } } + assert_response :redirect + assert_redirected_to manage_agreements_path + end + + should "not allow access to manage_agreements#update" do + patch :update, params: { id: @agreement, agreement: { name: "Not fun agreement" } } + assert_response :redirect + assert_redirected_to manage_agreements_path + end + + should "not allow access to manage_agreements#destroy" do + patch :destroy, params: { id: @agreement } + assert_response :redirect + assert_redirected_to manage_agreements_path + end + end + + context "while authenticated as a director" do + setup do + @user = create(:director) + @request.env["devise.mapping"] = Devise.mappings[:user] + sign_in @user + end + + should "allow access to manage_agreements#index" do + get :index + assert_response :success + end + + should "allow access to manage_agreements#new" do + get :new + assert_response :success + end + + should "create a new agreement" do + post :create, params: { agreement: { name: "Fun Agreement" } } + assert_response :success + end + + should "allow access to manage_agreements#edit" do + get :edit, params: { id: @agreement } + assert_response :success + end + + should "update agreement" do + patch :update, params: { id: @agreement, agreement: { name: "New agreement Name" } } + assert_response :redirect + assert_redirected_to manage_agreements_path + end + + context "#destroy" do + should "destroy agreement" do + assert_difference("Agreement.count", -1) do + patch :destroy, params: { id: @agreement } + end + assert_redirected_to manage_agreements_path + end + end + end +end diff --git a/test/controllers/manage/bus_lists_controller_test.rb b/test/controllers/manage/bus_lists_controller_test.rb index 1d12d9363..483569a4a 100644 --- a/test/controllers/manage/bus_lists_controller_test.rb +++ b/test/controllers/manage/bus_lists_controller_test.rb @@ -366,8 +366,8 @@ class Manage::BusListsControllerTest < ActionController::TestCase end should "reset everyone's bus_list_id" do - questionnaire = create(:questionnaire, bus_list_id: @bus_list.id) - questionnaire2 = create(:questionnaire, bus_list_id: @bus_list.id) + questionnaire = create(:questionnaire, bus_list_id: @bus_list.id, agreements: Agreement.all) + questionnaire2 = create(:questionnaire, bus_list_id: @bus_list.id, agreements: Agreement.all) patch :destroy, params: { id: @bus_list } assert_equal false, questionnaire.reload.bus_list_id? assert_equal false, questionnaire2.reload.bus_list_id? diff --git a/test/controllers/manage/dashboard_controller_test.rb b/test/controllers/manage/dashboard_controller_test.rb index f473968e6..fd6e7f817 100644 --- a/test/controllers/manage/dashboard_controller_test.rb +++ b/test/controllers/manage/dashboard_controller_test.rb @@ -25,11 +25,11 @@ class Manage::DashboardControllerTest < ActionController::TestCase should "not allow access to all data endpoints" do school1 = FactoryBot.create(:school) school2 = FactoryBot.create(:school) - FactoryBot.create_list(:questionnaire, 20, school_id: school1.id, acc_status: "pending") - FactoryBot.create_list(:questionnaire, 20, school_id: school1.id, acc_status: "accepted") - FactoryBot.create_list(:questionnaire, 10, school_id: school2.id, acc_status: "accepted") + FactoryBot.create_list(:questionnaire, 20, school_id: school1.id, acc_status: "pending", agreements: Agreement.all) + FactoryBot.create_list(:questionnaire, 20, school_id: school1.id, acc_status: "accepted", agreements: Agreement.all) + FactoryBot.create_list(:questionnaire, 10, school_id: school2.id, acc_status: "accepted", agreements: Agreement.all) Questionnaire::POSSIBLE_ACC_STATUS.each do |status, _name| - FactoryBot.create_list(:questionnaire, 1, school_id: school2.id, acc_status: status) + FactoryBot.create_list(:questionnaire, 1, school_id: school2.id, acc_status: status, agreements: Agreement.all) end stub_request(:get, "https://geocoding.geo.census.gov/geocoder/locations/address?street=123+Fake+Street&city=Rochester&state=NY&benchmark=Public_AR_Current&format=json") @@ -75,11 +75,11 @@ class Manage::DashboardControllerTest < ActionController::TestCase should "not allow access to all data endpoints" do school1 = FactoryBot.create(:school) school2 = FactoryBot.create(:school) - FactoryBot.create_list(:questionnaire, 20, school_id: school1.id, acc_status: "pending") - FactoryBot.create_list(:questionnaire, 20, school_id: school1.id, acc_status: "accepted") - FactoryBot.create_list(:questionnaire, 10, school_id: school2.id, acc_status: "accepted") + FactoryBot.create_list(:questionnaire, 20, school_id: school1.id, acc_status: "pending", agreements: Agreement.all) + FactoryBot.create_list(:questionnaire, 20, school_id: school1.id, acc_status: "accepted", agreements: Agreement.all) + FactoryBot.create_list(:questionnaire, 10, school_id: school2.id, acc_status: "accepted", agreements: Agreement.all) Questionnaire::POSSIBLE_ACC_STATUS.each do |status, _name| - FactoryBot.create_list(:questionnaire, 1, school_id: school2.id, acc_status: status) + FactoryBot.create_list(:questionnaire, 1, school_id: school2.id, acc_status: status, agreements: Agreement.all) end stub_request(:get, "https://geocoding.geo.census.gov/geocoder/locations/address?street=123+Fake+Street&city=Rochester&state=NY&benchmark=Public_AR_Current&format=json") @@ -124,11 +124,11 @@ class Manage::DashboardControllerTest < ActionController::TestCase should "allow access to all data endpoints" do school1 = FactoryBot.create(:school) school2 = FactoryBot.create(:school) - FactoryBot.create_list(:questionnaire, 20, school_id: school1.id, acc_status: "pending") - FactoryBot.create_list(:questionnaire, 20, school_id: school1.id, acc_status: "accepted") - FactoryBot.create_list(:questionnaire, 10, school_id: school2.id, acc_status: "accepted") + FactoryBot.create_list(:questionnaire, 20, school_id: school1.id, acc_status: "pending", agreements: Agreement.all) + FactoryBot.create_list(:questionnaire, 20, school_id: school1.id, acc_status: "accepted", agreements: Agreement.all) + FactoryBot.create_list(:questionnaire, 10, school_id: school2.id, acc_status: "accepted", agreements: Agreement.all) Questionnaire::POSSIBLE_ACC_STATUS.each do |status, _name| - FactoryBot.create_list(:questionnaire, 1, school_id: school2.id, acc_status: status) + FactoryBot.create_list(:questionnaire, 1, school_id: school2.id, acc_status: status, agreements: Agreement.all) end stub_request(:get, "https://geocoding.geo.census.gov/geocoder/locations/address?street=123+Fake+Street&city=Rochester&state=NY&benchmark=Public_AR_Current&format=json") @@ -173,11 +173,11 @@ class Manage::DashboardControllerTest < ActionController::TestCase should "allow access to all data endpoints" do school1 = FactoryBot.create(:school) school2 = FactoryBot.create(:school) - FactoryBot.create_list(:questionnaire, 20, school_id: school1.id, acc_status: "pending") - FactoryBot.create_list(:questionnaire, 20, school_id: school1.id, acc_status: "accepted") - FactoryBot.create_list(:questionnaire, 10, school_id: school2.id, acc_status: "accepted") + FactoryBot.create_list(:questionnaire, 20, school_id: school1.id, acc_status: "pending", agreements: Agreement.all) + FactoryBot.create_list(:questionnaire, 20, school_id: school1.id, acc_status: "accepted", agreements: Agreement.all) + FactoryBot.create_list(:questionnaire, 10, school_id: school2.id, acc_status: "accepted", agreements: Agreement.all) Questionnaire::POSSIBLE_ACC_STATUS.each do |status, _name| - FactoryBot.create_list(:questionnaire, 1, school_id: school2.id, acc_status: status) + FactoryBot.create_list(:questionnaire, 1, school_id: school2.id, acc_status: status, agreements: Agreement.all) end stub_request(:get, "https://geocoding.geo.census.gov/geocoder/locations/address?street=123+Fake+Street&city=Rochester&state=NY&benchmark=Public_AR_Current&format=json") diff --git a/test/controllers/manage/questionnaires_controller_test.rb b/test/controllers/manage/questionnaires_controller_test.rb index ab7f84023..338924367 100644 --- a/test/controllers/manage/questionnaires_controller_test.rb +++ b/test/controllers/manage/questionnaires_controller_test.rb @@ -293,7 +293,7 @@ class Manage::QuestionnairesControllerTest < ActionController::TestCase should "not create a duplicate questionnaire for a user" do user = create(:user, email: "existing@example.com") - create(:questionnaire, user_id: user.id) + create(:questionnaire, user_id: user.id, agreements: Agreement.all) assert_difference("User.count", 0) do assert_difference("Questionnaire.count", 0) do post :create, params: { @@ -308,9 +308,6 @@ class Manage::QuestionnairesControllerTest < ActionController::TestCase shirt_size: @questionnaire.shirt_size, school_id: @questionnaire.school_id, email: "existing@example.com", - agreement_accepted: "1", - code_of_conduct_accepted: "1", - data_sharing_accepted: "1", gender: @questionnaire.gender, major: @questionnaire.major, why_attend: @questionnaire.why_attend, @@ -372,14 +369,12 @@ class Manage::QuestionnairesControllerTest < ActionController::TestCase end should "check in the questionnaire and update information" do - @questionnaire.update_attribute(:agreement_accepted, false) @questionnaire.update_attribute(:can_share_info, false) @questionnaire.update_attribute(:phone, "") patch :check_in, params: { id: @questionnaire, check_in: "true", questionnaire: { - agreement_accepted: 1, can_share_info: 1, phone: "(123) 333-3333", email: "new_email@example.com" @@ -388,7 +383,7 @@ class Manage::QuestionnairesControllerTest < ActionController::TestCase @questionnaire.reload assert 1.minute.ago < @questionnaire.checked_in_at assert_equal @user.id, @questionnaire.checked_in_by_id - assert_equal true, @questionnaire.agreement_accepted + assert_equal true, @questionnaire.all_agreements_accepted? assert_equal true, @questionnaire.can_share_info assert_equal "1233333333", @questionnaire.phone assert_equal "new_email@example.com", @questionnaire.email @@ -398,7 +393,7 @@ class Manage::QuestionnairesControllerTest < ActionController::TestCase end should "require a new action to check in" do - @questionnaire.update_attribute(:agreement_accepted, false) + @questionnaire.update_attribute(:agreements, []) @questionnaire.update_attribute(:can_share_info, false) @questionnaire.update_attribute(:phone, "") @questionnaire.user.update_attribute(:email, "old_email@example.com") @@ -417,7 +412,7 @@ class Manage::QuestionnairesControllerTest < ActionController::TestCase @questionnaire.reload assert_nil @questionnaire.checked_in_at assert_nil @questionnaire.checked_in_by_id - assert_equal false, @questionnaire.agreement_accepted + assert_equal false, @questionnaire.all_agreements_accepted? assert_equal false, @questionnaire.can_share_info assert_equal "", @questionnaire.phone assert_equal "old_email@example.com", @questionnaire.email @@ -426,8 +421,8 @@ class Manage::QuestionnairesControllerTest < ActionController::TestCase assert_redirected_to manage_questionnaire_path(@questionnaire) end - should "require agreement_accepted to check in" do - @questionnaire.update_attribute(:agreement_accepted, false) + should "require all agreements to be accepted to check in" do + @questionnaire.update_attribute(:agreements, []) patch :check_in, params: { id: @questionnaire, check_in: "true" } assert_nil @questionnaire.reload.checked_in_at assert_nil @questionnaire.reload.checked_in_by_id @@ -435,8 +430,8 @@ class Manage::QuestionnairesControllerTest < ActionController::TestCase assert_redirected_to manage_questionnaire_path(@questionnaire) end - should "accept agreement and check in" do - @questionnaire.update_attribute(:agreement_accepted, false) + should "accept all agreements and check in" do + @questionnaire.update_attribute(:agreements, Agreement.all) patch :check_in, params: { id: @questionnaire, check_in: "true", questionnaire: { agreement_accepted: 1 } } assert 1.minute.ago < @questionnaire.reload.checked_in_at assert_equal @user.id, @questionnaire.reload.checked_in_by_id diff --git a/test/controllers/manage/schools_controller_test.rb b/test/controllers/manage/schools_controller_test.rb index a19f9406d..e64c64f50 100644 --- a/test/controllers/manage/schools_controller_test.rb +++ b/test/controllers/manage/schools_controller_test.rb @@ -326,9 +326,9 @@ class Manage::SchoolsControllerTest < ActionController::TestCase should "merge schools" do new_school = create(:school, name: "Unique School") @school.update_attribute(:name, "Unique Sch00l") - create(:questionnaire, school_id: @school.id) - create(:questionnaire, school_id: @school.id) - create(:questionnaire, school_id: new_school.id) + create(:questionnaire, school_id: @school.id, agreements: Agreement.all) + create(:questionnaire, school_id: @school.id, agreements: Agreement.all) + create(:questionnaire, school_id: new_school.id, agreements: Agreement.all) assert_difference('SchoolNameDuplicate.count', 1) do assert_difference('School.count', -1) do assert_difference('new_school.reload.questionnaire_count', 2) do diff --git a/test/controllers/manage/stats_controller_test.rb b/test/controllers/manage/stats_controller_test.rb index 79aa2c9cb..2c48aa112 100644 --- a/test/controllers/manage/stats_controller_test.rb +++ b/test/controllers/manage/stats_controller_test.rb @@ -76,7 +76,7 @@ class Manage::StatsControllerTest < ActionController::TestCase should "allow access to all data endpoints" do school = create(:school) Questionnaire::POSSIBLE_ACC_STATUS.each do |status, _name| - create_list(:questionnaire, 5, school_id: school.id, acc_status: status, dietary_restrictions: "Vegetarian", special_needs: "Something") + create_list(:questionnaire, 5, school_id: school.id, acc_status: status, dietary_restrictions: "Vegetarian", special_needs: "Something", agreements: Agreement.all) end paths.each do |path| @@ -101,7 +101,7 @@ class Manage::StatsControllerTest < ActionController::TestCase should "allow access to all data endpoints" do school = create(:school) Questionnaire::POSSIBLE_ACC_STATUS.each do |status, _name| - create_list(:questionnaire, 5, school_id: school.id, acc_status: status, dietary_restrictions: "Vegetarian", special_needs: "Something") + create_list(:questionnaire, 5, school_id: school.id, acc_status: status, dietary_restrictions: "Vegetarian", special_needs: "Something", agreements: Agreement.all) end paths.each do |path| diff --git a/test/controllers/questionnaires_controller_test.rb b/test/controllers/questionnaires_controller_test.rb index a3f14b404..490d07d2d 100644 --- a/test/controllers/questionnaires_controller_test.rb +++ b/test/controllers/questionnaires_controller_test.rb @@ -47,7 +47,7 @@ class QuestionnairesControllerTest < ActionController::TestCase should "create questionnaire" do assert_difference('Questionnaire.count', 1) do - post :create, params: { questionnaire: { experience: @questionnaire.experience, interest: @questionnaire.interest, phone: @questionnaire.phone, level_of_study: @questionnaire.level_of_study, date_of_birth: @questionnaire.date_of_birth, shirt_size: @questionnaire.shirt_size, school_id: @school.id, agreement_accepted: "1", code_of_conduct_accepted: "1", data_sharing_accepted: "1", major: @questionnaire.major, gender: @questionnaire.gender, why_attend: @questionnaire.why_attend, graduation_year: @questionnaire.graduation_year, race_ethnicity: @questionnaire.race_ethnicity } } + post :create, params: { questionnaire: { experience: @questionnaire.experience, interest: @questionnaire.interest, phone: @questionnaire.phone, level_of_study: @questionnaire.level_of_study, date_of_birth: @questionnaire.date_of_birth, shirt_size: @questionnaire.shirt_size, school_id: @school.id, major: @questionnaire.major, gender: @questionnaire.gender, why_attend: @questionnaire.why_attend, graduation_year: @questionnaire.graduation_year, race_ethnicity: @questionnaire.race_ethnicity, agreement_ids: @questionnaire.agreements.map(&:id) } } end assert_redirected_to questionnaires_path @@ -60,8 +60,8 @@ class QuestionnairesControllerTest < ActionController::TestCase should "not allow multiple questionnaires" do assert_difference('Questionnaire.count', 1) do - post :create, params: { questionnaire: { experience: @questionnaire.experience, interest: @questionnaire.interest, phone: @questionnaire.phone, level_of_study: @questionnaire.level_of_study, date_of_birth: @questionnaire.date_of_birth, shirt_size: @questionnaire.shirt_size, school_id: @school.id, agreement_accepted: "1", code_of_conduct_accepted: "1", data_sharing_accepted: "1", major: @questionnaire.major, gender: @questionnaire.gender, why_attend: @questionnaire.why_attend, graduation_year: @questionnaire.graduation_year, race_ethnicity: @questionnaire.race_ethnicity } } - post :create, params: { questionnaire: { experience: @questionnaire.experience, interest: @questionnaire.interest, phone: @questionnaire.phone, level_of_study: @questionnaire.level_of_study, date_of_birth: @questionnaire.date_of_birth, shirt_size: @questionnaire.shirt_size, school_id: @school.id, agreement_accepted: "1", code_of_conduct_accepted: "1", data_sharing_accepted: "1", major: @questionnaire.major, gender: @questionnaire.gender, why_attend: @questionnaire.why_attend, graduation_year: @questionnaire.graduation_year, race_ethnicity: @questionnaire.race_ethnicity } } + post :create, params: { questionnaire: { experience: @questionnaire.experience, interest: @questionnaire.interest, phone: @questionnaire.phone, level_of_study: @questionnaire.level_of_study, date_of_birth: @questionnaire.date_of_birth, shirt_size: @questionnaire.shirt_size, school_id: @school.id, major: @questionnaire.major, gender: @questionnaire.gender, why_attend: @questionnaire.why_attend, graduation_year: @questionnaire.graduation_year, race_ethnicity: @questionnaire.race_ethnicity, agreement_ids: @questionnaire.agreements.map(&:id) } } + post :create, params: { questionnaire: { experience: @questionnaire.experience, interest: @questionnaire.interest, phone: @questionnaire.phone, level_of_study: @questionnaire.level_of_study, date_of_birth: @questionnaire.date_of_birth, shirt_size: @questionnaire.shirt_size, school_id: @school.id, major: @questionnaire.major, gender: @questionnaire.gender, why_attend: @questionnaire.why_attend, graduation_year: @questionnaire.graduation_year, race_ethnicity: @questionnaire.race_ethnicity, agreement_ids: @questionnaire.agreements.map(&:id) } } end assert_redirected_to questionnaires_path @@ -80,7 +80,7 @@ class QuestionnairesControllerTest < ActionController::TestCase should "not allow creation" do HackathonConfig['accepting_questionnaires'] = false assert_difference('Questionnaire.count', 0) do - post :create, params: { questionnaire: { experience: @questionnaire.experience, interest: @questionnaire.interest, phone: @questionnaire.phone, level_of_study: @questionnaire.level_of_study, date_of_birth: @questionnaire.date_of_birth, shirt_size: @questionnaire.shirt_size, school_id: @school.id, agreement_accepted: "1", code_of_conduct_accepted: "1", data_sharing_accepted: "1", major: @questionnaire.major, gender: @questionnaire.gender, why_attend: @questionnaire.why_attend, graduation_year: @questionnaire.graduation_year, race_ethnicity: @questionnaire.race_ethnicity } } + post :create, params: { questionnaire: { experience: @questionnaire.experience, interest: @questionnaire.interest, phone: @questionnaire.phone, level_of_study: @questionnaire.level_of_study, date_of_birth: @questionnaire.date_of_birth, shirt_size: @questionnaire.shirt_size, school_id: @school.id, major: @questionnaire.major, gender: @questionnaire.gender, why_attend: @questionnaire.why_attend, graduation_year: @questionnaire.graduation_year, race_ethnicity: @questionnaire.race_ethnicity, agreement_ids: @questionnaire.agreements.map(&:id) } } end end end @@ -88,13 +88,13 @@ class QuestionnairesControllerTest < ActionController::TestCase context "#school_name" do context "on create" do should "save existing school name" do - post :create, params: { questionnaire: { experience: @questionnaire.experience, interest: @questionnaire.interest, phone: @questionnaire.phone, level_of_study: @questionnaire.level_of_study, date_of_birth: @questionnaire.date_of_birth, shirt_size: @questionnaire.shirt_size, school_name: @school.name, agreement_accepted: "1", code_of_conduct_accepted: "1", data_sharing_accepted: "1", major: @questionnaire.major, gender: @questionnaire.gender, why_attend: @questionnaire.why_attend, graduation_year: @questionnaire.graduation_year, race_ethnicity: @questionnaire.race_ethnicity } } + post :create, params: { questionnaire: { experience: @questionnaire.experience, interest: @questionnaire.interest, phone: @questionnaire.phone, level_of_study: @questionnaire.level_of_study, date_of_birth: @questionnaire.date_of_birth, shirt_size: @questionnaire.shirt_size, school_name: @school.name, major: @questionnaire.major, gender: @questionnaire.gender, why_attend: @questionnaire.why_attend, graduation_year: @questionnaire.graduation_year, race_ethnicity: @questionnaire.race_ethnicity, agreement_ids: @questionnaire.agreements.map(&:id) } } assert_redirected_to questionnaires_path assert_equal 1, School.all.count end should "create a new school when unknown" do - post :create, params: { questionnaire: { experience: @questionnaire.experience, interest: @questionnaire.interest, phone: @questionnaire.phone, level_of_study: @questionnaire.level_of_study, date_of_birth: @questionnaire.date_of_birth, shirt_size: @questionnaire.shirt_size, school_name: "New School", agreement_accepted: "1", code_of_conduct_accepted: "1", data_sharing_accepted: "1", major: @questionnaire.major, gender: @questionnaire.gender, why_attend: @questionnaire.why_attend, graduation_year: @questionnaire.graduation_year, race_ethnicity: @questionnaire.race_ethnicity } } + post :create, params: { questionnaire: { experience: @questionnaire.experience, interest: @questionnaire.interest, phone: @questionnaire.phone, level_of_study: @questionnaire.level_of_study, date_of_birth: @questionnaire.date_of_birth, shirt_size: @questionnaire.shirt_size, school_name: "New School", major: @questionnaire.major, gender: @questionnaire.gender, why_attend: @questionnaire.why_attend, graduation_year: @questionnaire.graduation_year, race_ethnicity: @questionnaire.race_ethnicity, agreement_ids: @questionnaire.agreements.map(&:id) } } assert_redirected_to questionnaires_path assert_equal 2, School.all.count end @@ -103,7 +103,7 @@ class QuestionnairesControllerTest < ActionController::TestCase message = create(:message, type: 'automated', trigger: "questionnaire.pending") assert_difference 'enqueued_jobs.size', 1 do assert_difference 'Questionnaire.count', 1 do - post :create, params: { questionnaire: { experience: @questionnaire.experience, interest: @questionnaire.interest, phone: @questionnaire.phone, level_of_study: @questionnaire.level_of_study, date_of_birth: @questionnaire.date_of_birth, shirt_size: @questionnaire.shirt_size, school_name: @school.name, agreement_accepted: "1", code_of_conduct_accepted: "1", data_sharing_accepted: "1", major: @questionnaire.major, gender: @questionnaire.gender, why_attend: @questionnaire.why_attend, graduation_year: @questionnaire.graduation_year, race_ethnicity: @questionnaire.race_ethnicity } } + post :create, params: { questionnaire: { experience: @questionnaire.experience, interest: @questionnaire.interest, phone: @questionnaire.phone, level_of_study: @questionnaire.level_of_study, date_of_birth: @questionnaire.date_of_birth, shirt_size: @questionnaire.shirt_size, school_name: @school.name, major: @questionnaire.major, gender: @questionnaire.gender, why_attend: @questionnaire.why_attend, graduation_year: @questionnaire.graduation_year, race_ethnicity: @questionnaire.race_ethnicity, agreement_ids: @questionnaire.agreements.map(&:id) } } end end questionnaire = Questionnaire.last diff --git a/test/controllers/rsvps_controller_test.rb b/test/controllers/rsvps_controller_test.rb index 7a61408a5..565e81701 100644 --- a/test/controllers/rsvps_controller_test.rb +++ b/test/controllers/rsvps_controller_test.rb @@ -5,7 +5,10 @@ class RsvpsControllerTest < ActionController::TestCase setup do @school = create(:school, name: "Another School") - @questionnaire = create(:questionnaire, school_id: @school.id) + @questionnaire = build(:questionnaire, school_id: @school.id) + @agreement = create(:agreement) + @questionnaire.agreements << @agreement + @questionnaire.save end context "while not authenticated" do @@ -102,8 +105,8 @@ class RsvpsControllerTest < ActionController::TestCase context "not update status for invalid questionnaire" do setup do - @questionnaire.update_attribute(:agreement_accepted, false) @questionnaire.update_attribute(:acc_status, "accepted") + @questionnaire.update_attribute(:agreements, []) end [:accept, :deny].each do |status| @@ -221,7 +224,7 @@ class RsvpsControllerTest < ActionController::TestCase should "not allow invalid updates to questionnaire via rsvp page" do @questionnaire.update_attribute(:phone, "1111111111") - @questionnaire.update_attribute(:agreement_accepted, false) + @questionnaire.update_attribute(:agreements, []) patch :update, params: { questionnaire: { phone: "1234567890" } } assert_not_nil flash[:alert] assert_equal "1111111111", @questionnaire.reload.phone @@ -230,7 +233,7 @@ class RsvpsControllerTest < ActionController::TestCase should "not allow updates to invalid questionnaire via rsvp page" do @questionnaire.update_attribute(:phone, "1111111111") - @questionnaire.update_attribute(:agreement_accepted, false) + @questionnaire.update_attribute(:agreements, []) patch :update, params: { questionnaire: { phone: "1234567890" } } assert_not_nil flash[:alert] assert_equal "1111111111", @questionnaire.reload.phone diff --git a/test/factories/agreement.rb b/test/factories/agreement.rb new file mode 100644 index 000000000..85caaddcf --- /dev/null +++ b/test/factories/agreement.rb @@ -0,0 +1,6 @@ +FactoryBot.define do + factory :agreement do + name {"HackFoo Agreement"} + agreement_url {"https://www.foo.com"} + end +end diff --git a/test/factories/questionnaire.rb b/test/factories/questionnaire.rb index f732b8992..de734683b 100644 --- a/test/factories/questionnaire.rb +++ b/test/factories/questionnaire.rb @@ -9,9 +9,6 @@ shirt_size { "Unisex - M" } dietary_restrictions { "" } special_needs { "" } - agreement_accepted { true } - code_of_conduct_accepted { true } - data_sharing_accepted { true } can_share_info { true } gender { "Male" } major { "Computer Science" } @@ -19,6 +16,7 @@ graduation_year { Date.today.year } race_ethnicity { "Other" } why_attend { "This sounds cool" } + agreements { [create(:agreement)] } association :user end diff --git a/test/jobs/bulk_message_job_test.rb b/test/jobs/bulk_message_job_test.rb index 953530b2d..14eba314a 100644 --- a/test/jobs/bulk_message_job_test.rb +++ b/test/jobs/bulk_message_job_test.rb @@ -10,7 +10,7 @@ class BulkMessageJobTest < ActiveJob::TestCase end should "queue a mailer per recipient" do - create_list(:questionnaire, 5) + create_list(:questionnaire, 5, agreements: Agreement.all) message = create(:message, recipients: ["all"], queued_at: Time.now) assert_difference "enqueued_jobs.size", 5 do worker = BulkMessageJob.new @@ -20,9 +20,9 @@ class BulkMessageJobTest < ActiveJob::TestCase should "support query recipients and simple recipients" do create(:school, name: "My University", id: 492) - create_list(:questionnaire, 4, school_id: 492, acc_status: "accepted") - create_list(:questionnaire, 4, school_id: 492, acc_status: "rsvp_confirmed") - create_list(:questionnaire, 4, school_id: 492, acc_status: "denied") # Decoy + create_list(:questionnaire, 4, school_id: 492, acc_status: "accepted", agreements: Agreement.all) + create_list(:questionnaire, 4, school_id: 492, acc_status: "rsvp_confirmed", agreements: Agreement.all) + create_list(:questionnaire, 4, school_id: 492, acc_status: "denied", agreements: Agreement.all) # Decoy create_list(:user, 4) message = create(:message, recipients: ["incomplete", "school::492"], queued_at: Time.now) assert_difference "enqueued_jobs.size", 12 do @@ -42,11 +42,11 @@ class BulkMessageJobTest < ActiveJob::TestCase context "recipient queries" do setup do bus_list = create(:bus_list, name: "Bus Foo", id: 186) - create(:questionnaire, acc_status: "pending") - create(:questionnaire, acc_status: "waitlist") - create(:questionnaire, acc_status: "accepted") - create(:questionnaire, acc_status: "rsvp_confirmed", bus_list_id: bus_list.id) - create(:questionnaire, acc_status: "rsvp_denied") + create(:questionnaire, acc_status: "pending", agreements: Agreement.all) + create(:questionnaire, acc_status: "waitlist", agreements: Agreement.all) + create(:questionnaire, acc_status: "accepted", agreements: Agreement.all) + create(:questionnaire, acc_status: "rsvp_confirmed", bus_list_id: bus_list.id, agreements: Agreement.all) + create(:questionnaire, acc_status: "rsvp_denied", agreements: Agreement.all) create_list(:user, 4) end diff --git a/test/models/agreement_test.rb b/test/models/agreement_test.rb new file mode 100644 index 000000000..2ffddab95 --- /dev/null +++ b/test/models/agreement_test.rb @@ -0,0 +1,20 @@ +require 'test_helper' + +class AgreementTest < ActiveSupport::TestCase + should have_and_belong_to_many :questionnaires + + should strip_attribute :name + should strip_attribute :agreement_url + + should validate_presence_of :name + should validate_presence_of :agreement_url + + should "not allow questionnaires to accept agreements for others" do + @questionnaire1 = create(:questionnaire) + @agreement = create(:agreement) + @questionnaire2 = create(:questionnaire, agreements: Agreement.all) + + assert_equal false, @questionnaire1.all_agreements_accepted? + assert_equal true, @questionnaire2.all_agreements_accepted? + end +end diff --git a/test/models/bus_list_test.rb b/test/models/bus_list_test.rb index 67b204b4d..cb7693b20 100644 --- a/test/models/bus_list_test.rb +++ b/test/models/bus_list_test.rb @@ -16,8 +16,8 @@ class BusListTest < ActiveSupport::TestCase end should "return all passengers" do - questionnaire1 = create(:questionnaire, bus_list_id: @bus_list.id, acc_status: "rsvp_confirmed") - questionnaire2 = create(:questionnaire, bus_list_id: @bus_list.id, acc_status: "rsvp_confirmed") + questionnaire1 = create(:questionnaire, bus_list_id: @bus_list.id, acc_status: "rsvp_confirmed", agreements: Agreement.all) + questionnaire2 = create(:questionnaire, bus_list_id: @bus_list.id, acc_status: "rsvp_confirmed", agreements: Agreement.all) assert_equal 2, @bus_list.passengers.count allowed_ids = [questionnaire1, questionnaire2].map(&:id) assert allowed_ids.include? @bus_list.passengers[0].id @@ -25,19 +25,19 @@ class BusListTest < ActiveSupport::TestCase end should "only return passengers who have RSVP'd" do - questionnaire1 = create(:questionnaire, acc_status: "rsvp_confirmed", bus_list_id: @bus_list.id) + questionnaire1 = create(:questionnaire, acc_status: "rsvp_confirmed", bus_list_id: @bus_list.id, agreements: Agreement.all) Questionnaire::POSSIBLE_ACC_STATUS.each do |status, _title| next if status == "rsvp_confirmed" - create(:questionnaire, acc_status: status, bus_list_id: @bus_list.id) + create(:questionnaire, acc_status: status, bus_list_id: @bus_list.id, agreements: Agreement.all) end assert_equal 1, @bus_list.passengers.count assert_equal questionnaire1.id, @bus_list.passengers[0].id end should "not return passengers from another bus" do - questionnaire1 = create(:questionnaire, acc_status: "rsvp_confirmed", bus_list_id: @bus_list.id) + questionnaire1 = create(:questionnaire, acc_status: "rsvp_confirmed", bus_list_id: @bus_list.id, agreements: Agreement.all) bus_list2 = create(:bus_list) - create(:questionnaire, acc_status: "rsvp_confirmed", bus_list_id: bus_list2.id) + create(:questionnaire, acc_status: "rsvp_confirmed", bus_list_id: bus_list2.id, agreements: Agreement.all) assert_equal 1, @bus_list.passengers.count assert_equal questionnaire1.id, @bus_list.passengers[0].id end @@ -49,9 +49,9 @@ class BusListTest < ActiveSupport::TestCase end should "return all boarded passengers" do - questionnaire1 = create(:questionnaire, bus_list_id: @bus_list.id, acc_status: "rsvp_confirmed", boarded_bus_at: nil) - questionnaire2 = create(:questionnaire, bus_list_id: @bus_list.id, acc_status: "rsvp_confirmed", boarded_bus_at: Time.now) - questionnaire3 = create(:questionnaire, bus_list_id: @bus_list.id, acc_status: "rsvp_confirmed", boarded_bus_at: Time.now) + questionnaire1 = create(:questionnaire, bus_list_id: @bus_list.id, acc_status: "rsvp_confirmed", boarded_bus_at: nil, agreements: Agreement.all) + questionnaire2 = create(:questionnaire, bus_list_id: @bus_list.id, acc_status: "rsvp_confirmed", boarded_bus_at: Time.now, agreements: Agreement.all) + questionnaire3 = create(:questionnaire, bus_list_id: @bus_list.id, acc_status: "rsvp_confirmed", boarded_bus_at: Time.now, agreements: Agreement.all) assert_equal 2, @bus_list.boarded_passengers.count allowed_ids = [questionnaire2, questionnaire3].map(&:id) assert allowed_ids.include? @bus_list.boarded_passengers[0].id @@ -69,8 +69,8 @@ class BusListTest < ActiveSupport::TestCase end should "only return passengers who have checked in" do - create(:questionnaire, acc_status: "rsvp_confirmed", bus_list_id: @bus_list.id, checked_in_at: 2.minutes.ago) - create(:questionnaire, acc_status: "rsvp_confirmed", bus_list_id: @bus_list.id, checked_in_at: nil) + create(:questionnaire, acc_status: "rsvp_confirmed", bus_list_id: @bus_list.id, checked_in_at: 2.minutes.ago, agreements: Agreement.all) + create(:questionnaire, acc_status: "rsvp_confirmed", bus_list_id: @bus_list.id, checked_in_at: nil, agreements: Agreement.all) assert_equal 2, @bus_list.passengers.count assert_equal 1, @bus_list.checked_in_passengers.count end @@ -82,11 +82,11 @@ class BusListTest < ActiveSupport::TestCase end should "return all captains" do - create(:questionnaire, acc_status: "rsvp_confirmed", bus_list_id: @bus_list.id) - create(:questionnaire, acc_status: "rsvp_confirmed", bus_list_id: @bus_list.id) - questionnaire1 = create(:questionnaire, acc_status: "rsvp_confirmed", bus_list_id: @bus_list.id, is_bus_captain: true) - questionnaire2 = create(:questionnaire, acc_status: "rsvp_confirmed", bus_list_id: @bus_list.id, is_bus_captain: true) - create(:questionnaire, acc_status: "rsvp_confirmed") + create(:questionnaire, acc_status: "rsvp_confirmed", bus_list_id: @bus_list.id, agreements: Agreement.all) + create(:questionnaire, acc_status: "rsvp_confirmed", bus_list_id: @bus_list.id, agreements: Agreement.all) + questionnaire1 = create(:questionnaire, acc_status: "rsvp_confirmed", bus_list_id: @bus_list.id, is_bus_captain: true, agreements: Agreement.all) + questionnaire2 = create(:questionnaire, acc_status: "rsvp_confirmed", bus_list_id: @bus_list.id, is_bus_captain: true, agreements: Agreement.all) + create(:questionnaire, acc_status: "rsvp_confirmed", agreements: Agreement.all) assert_equal 2, @bus_list.captains.count allowed_ids = [questionnaire1, questionnaire2].map(&:id) assert allowed_ids.include? @bus_list.captains[0].id @@ -94,19 +94,19 @@ class BusListTest < ActiveSupport::TestCase end should "only return captains who have RSVP'd" do - questionnaire1 = create(:questionnaire, acc_status: "rsvp_confirmed", bus_list_id: @bus_list.id, is_bus_captain: true) + questionnaire1 = create(:questionnaire, acc_status: "rsvp_confirmed", bus_list_id: @bus_list.id, is_bus_captain: true, agreements: Agreement.all) Questionnaire::POSSIBLE_ACC_STATUS.each do |status, _title| next if status == "rsvp_confirmed" - create(:questionnaire, acc_status: status, bus_list_id: @bus_list.id, is_bus_captain: true) + create(:questionnaire, acc_status: status, bus_list_id: @bus_list.id, is_bus_captain: true, agreements: Agreement.all) end assert_equal 1, @bus_list.captains.count assert_equal questionnaire1.id, @bus_list.captains[0].id end should "not return captains from another bus" do - questionnaire1 = create(:questionnaire, acc_status: "rsvp_confirmed", bus_list_id: @bus_list.id, is_bus_captain: true) + questionnaire1 = create(:questionnaire, acc_status: "rsvp_confirmed", bus_list_id: @bus_list.id, is_bus_captain: true, agreements: Agreement.all) bus_list2 = create(:bus_list) - create(:questionnaire, acc_status: "rsvp_confirmed", bus_list_id: bus_list2.id, is_bus_captain: true) + create(:questionnaire, acc_status: "rsvp_confirmed", bus_list_id: bus_list2.id, is_bus_captain: true, agreements: Agreement.all) assert_equal 1, @bus_list.captains.count assert_equal questionnaire1.id, @bus_list.captains[0].id end diff --git a/test/models/questionnaire_test.rb b/test/models/questionnaire_test.rb index 78ef68293..bfd141b00 100644 --- a/test/models/questionnaire_test.rb +++ b/test/models/questionnaire_test.rb @@ -6,6 +6,7 @@ class QuestionnaireTest < ActiveSupport::TestCase should belong_to :user should belong_to :school should belong_to :bus_list + should have_and_belong_to_many :agreements should validate_uniqueness_of :user_id @@ -68,10 +69,7 @@ class QuestionnaireTest < ActiveSupport::TestCase should_not allow_value("M").for(:shirt_size) should_not allow_value("foo").for(:shirt_size) - should allow_value(true).for(:agreement_accepted) - should_not allow_value(false).for(:agreement_accepted) - should allow_value(true).for(:code_of_conduct_accepted) - should_not allow_value(false).for(:code_of_conduct_accepted) + should allow_value(Agreement.all).for(:agreements) should allow_value("pending").for(:acc_status) should allow_value("accepted").for(:acc_status) diff --git a/test/models/user_test.rb b/test/models/user_test.rb index 2bac1093b..1b288f72f 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -51,15 +51,15 @@ class UserTest < ActiveSupport::TestCase context "without_questionnaire" do should "not return users with a questionnaire" do - create_list(:questionnaire, 3) + create_list(:questionnaire, 3, agreements: Agreement.all) assert_equal 3, User.count assert_equal 0, User.without_questionnaire.count end should "return users without questionnaire" do - create_list(:questionnaire, 1) + create_list(:questionnaire, 1, agreements: Agreement.all) create_list(:user, 2) - create_list(:questionnaire, 3) + create_list(:questionnaire, 3, agreements: Agreement.all) assert_equal 6, User.count assert_equal 2, User.without_questionnaire.count end From c5e97bfa6df7e5db5c66931d023e3f16b4eda173 Mon Sep 17 00:00:00 2001 From: "Chris Baudouin, Jr" Date: Fri, 11 Dec 2020 00:10:28 -0500 Subject: [PATCH 02/10] fix: Fixes several bugs with agreement error checking --- app/controllers/manage/agreements_controller.rb | 2 +- app/controllers/questionnaires_controller.rb | 2 +- test/controllers/manage/agreements_controller_test.rb | 9 +++++++-- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/app/controllers/manage/agreements_controller.rb b/app/controllers/manage/agreements_controller.rb index 1799b4c82..337bfa71d 100644 --- a/app/controllers/manage/agreements_controller.rb +++ b/app/controllers/manage/agreements_controller.rb @@ -33,7 +33,7 @@ def create # PATCH/PUT /agreements/1 def update - if !agreement_params['agreement_url'].start_with?('http://', 'https://') + if !agreement_params['agreement_url'].nil? and agreement_params['agreement_url'].start_with?('http://', 'https://') flash[:alert] = "Agreement URL must start with http:// or https://" render :edit else diff --git a/app/controllers/questionnaires_controller.rb b/app/controllers/questionnaires_controller.rb index 566b845ac..6db18a7d9 100644 --- a/app/controllers/questionnaires_controller.rb +++ b/app/controllers/questionnaires_controller.rb @@ -89,7 +89,7 @@ def update format.html { redirect_to questionnaires_path, notice: 'Application was successfully updated.' } format.json { head :no_content } else - format.html { render action: "edit" } + format.html { redirect_to edit_questionnaires_url } format.json { render json: @questionnaire.errors, status: :unprocessable_entity } end end diff --git a/test/controllers/manage/agreements_controller_test.rb b/test/controllers/manage/agreements_controller_test.rb index 39b0147a5..23b376fb4 100644 --- a/test/controllers/manage/agreements_controller_test.rb +++ b/test/controllers/manage/agreements_controller_test.rb @@ -192,8 +192,8 @@ class Manage::AgreementsControllerTest < ActionController::TestCase end should "create a new agreement" do - post :create, params: { agreement: { name: "Fun Agreement" } } - assert_response :success + post :create, params: { agreement: { name: "Fun Agreement", agreement_url: "https://foo.com" } } + assert_response :redirect end should "allow access to manage_agreements#edit" do @@ -207,6 +207,11 @@ class Manage::AgreementsControllerTest < ActionController::TestCase assert_redirected_to manage_agreements_path end + should "enforce agreement_url to be a link" do + patch :update, params: { id: @agreement, agreement: { name: "New agreement Name", agreement_url: "hello" } } + assert_response :redirect + end + context "#destroy" do should "destroy agreement" do assert_difference("Agreement.count", -1) do From e9667dcd72bba81214fe99328d85d21a7e3ebbd5 Mon Sep 17 00:00:00 2001 From: "Chris Baudouin, Jr" Date: Fri, 11 Dec 2020 00:46:26 -0500 Subject: [PATCH 03/10] fix: Fixes several Hound issues --- .../manage/agreements_controller.rb | 23 ++++++++++--------- .../manage/questionnaires_controller.rb | 2 +- app/controllers/questionnaires_controller.rb | 2 +- app/models/agreement.rb | 16 ++++++------- app/models/questionnaire.rb | 7 +++--- .../_unaccepted_agreements_notice.html.haml | 4 ++-- .../_checkin_compliance_card.html.haml | 2 +- .../manage/agreements_controller_test.rb | 1 - test/factories/agreement.rb | 6 ++--- test/models/bus_list_test.rb | 2 +- 10 files changed, 32 insertions(+), 33 deletions(-) diff --git a/app/controllers/manage/agreements_controller.rb b/app/controllers/manage/agreements_controller.rb index 337bfa71d..cd6459b58 100644 --- a/app/controllers/manage/agreements_controller.rb +++ b/app/controllers/manage/agreements_controller.rb @@ -33,7 +33,7 @@ def create # PATCH/PUT /agreements/1 def update - if !agreement_params['agreement_url'].nil? and agreement_params['agreement_url'].start_with?('http://', 'https://') + if !agreement_params['agreement_url'].nil? && agreement_params['agreement_url'].start_with?('http://', 'https://') flash[:alert] = "Agreement URL must start with http:// or https://" render :edit else @@ -51,15 +51,16 @@ def destroy end private - # Use callbacks to share common setup or constraints between actions. - def set_agreement - @agreement = Agreement.find(params[:id]) - end + + # Use callbacks to share common setup or constraints between actions. + def set_agreement + @agreement = Agreement.find(params[:id]) + end - # Only allow a trusted parameter "white list" through. - def agreement_params - params.require(:agreement).permit( - :name, :agreement_url - ) - end + # Only allow a trusted parameter "white list" through. + def agreement_params + params.require(:agreement).permit( + :name, :agreement_url + ) + end end diff --git a/app/controllers/manage/questionnaires_controller.rb b/app/controllers/manage/questionnaires_controller.rb index fdae4c654..f920cd68d 100644 --- a/app/controllers/manage/questionnaires_controller.rb +++ b/app/controllers/manage/questionnaires_controller.rb @@ -154,7 +154,7 @@ def questionnaire_params :email, :experience, :gender, :date_of_birth, :interest, :school_id, :school_name, :major, :level_of_study, :shirt_size, :dietary_restrictions, :special_needs, :international, - :portfolio_url, :vcs_url, :bus_captain_interest,:phone, :can_share_info, + :portfolio_url, :vcs_url, :bus_captain_interest, :phone, :can_share_info, :travel_not_from_school, :travel_location, :graduation_year, :race_ethnicity, :resume, :delete_resume, :why_attend, :bus_list_id, :is_bus_captain, :boarded_bus diff --git a/app/controllers/questionnaires_controller.rb b/app/controllers/questionnaires_controller.rb index 6db18a7d9..0aed9e76f 100644 --- a/app/controllers/questionnaires_controller.rb +++ b/app/controllers/questionnaires_controller.rb @@ -131,7 +131,7 @@ def questionnaire_params :shirt_size, :dietary_restrictions, :special_needs, :international, :portfolio_url, :vcs_url, :bus_captain_interest, :phone, :can_share_info, :travel_not_from_school, :travel_location, - :graduation_year, :race_ethnicity, :resume, :delete_resume, :why_attend, :agreement_ids => [] + :graduation_year, :race_ethnicity, :resume, :delete_resume, :why_attend, agreement_ids: [] ) end diff --git a/app/models/agreement.rb b/app/models/agreement.rb index abdd1256a..b9bcf711e 100644 --- a/app/models/agreement.rb +++ b/app/models/agreement.rb @@ -1,13 +1,13 @@ class Agreement < ApplicationRecord - include ActionView::Helpers::UrlHelper - validates_presence_of :name - validates_presence_of :agreement_url + include ActionView::Helpers::UrlHelper + validates_presence_of :name + validates_presence_of :agreement_url - strip_attributes + strip_attributes - has_and_belongs_to_many :questionnaires + has_and_belongs_to_many :questionnaires - def formatted_agreement - "I read and accept the #{link_to name, agreement_url, target: "_blank"} agreement.".html_safe - end + def formatted_agreement + "I read and accept the #{link_to name, agreement_url, target: '_blank'} agreement.".html_safe + end end diff --git a/app/models/questionnaire.rb b/app/models/questionnaire.rb index 4e35da532..52ed77a6c 100644 --- a/app/models/questionnaire.rb +++ b/app/models/questionnaire.rb @@ -216,17 +216,17 @@ def verbal_status end def agreements_present - if (Agreement.all - self.agreements).any? + if (Agreement.all - agreements).any? errors.add(:agreements, "must be accepted.") end end def all_agreements_accepted? - (Agreement.all - self.agreements).empty? + (Agreement.all - agreements).empty? end def unaccepted_agreements - Agreement.all - self.agreements + Agreement.all - agreements end def as_json(options = {}) @@ -297,5 +297,4 @@ def queue_triggered_email_rsvp_reminder end UserMailer.rsvp_reminder_email(user_id).deliver_later(wait_until: deliver_date) if deliver_date.present? end - end diff --git a/app/views/application/_unaccepted_agreements_notice.html.haml b/app/views/application/_unaccepted_agreements_notice.html.haml index 0d1dd83f8..416ed3806 100644 --- a/app/views/application/_unaccepted_agreements_notice.html.haml +++ b/app/views/application/_unaccepted_agreements_notice.html.haml @@ -4,7 +4,7 @@ Missing %span.emphasized Agreements %p - You have unaccepted agreements. You will not be allowed at #{HackathonConfig['name']} until all agreements have been accepted. Please + You have unaccepted agreements. You will not be allowed at #{HackathonConfig['name']} until all agreements have been accepted. Please %strong - #{link_to "edit your application", edit_questionnaires_path} + #{link_to "edit your application", edit_questionnaires_path} and review all agreements before attending. diff --git a/app/views/manage/questionnaires/_checkin_compliance_card.html.haml b/app/views/manage/questionnaires/_checkin_compliance_card.html.haml index 618a36a6b..6c22efd6d 100644 --- a/app/views/manage/questionnaires/_checkin_compliance_card.html.haml +++ b/app/views/manage/questionnaires/_checkin_compliance_card.html.haml @@ -43,5 +43,5 @@ - else %li.list-group-item.list-group-item-danger %span.fa.fa-close.fa-fw.icon-space-r - Unaccepted Agreements: + Unaccepted Agreements: = @questionnaire.unaccepted_agreements.map(&:name).join(', ') diff --git a/test/controllers/manage/agreements_controller_test.rb b/test/controllers/manage/agreements_controller_test.rb index 23b376fb4..9cc4d441b 100644 --- a/test/controllers/manage/agreements_controller_test.rb +++ b/test/controllers/manage/agreements_controller_test.rb @@ -1,7 +1,6 @@ require 'test_helper' class Manage::AgreementsControllerTest < ActionController::TestCase - setup do @agreement = create(:agreement) end diff --git a/test/factories/agreement.rb b/test/factories/agreement.rb index 85caaddcf..0f893250a 100644 --- a/test/factories/agreement.rb +++ b/test/factories/agreement.rb @@ -1,6 +1,6 @@ FactoryBot.define do - factory :agreement do - name {"HackFoo Agreement"} - agreement_url {"https://www.foo.com"} + factory :agreement do + name { "HackFoo Agreement" } + agreement_url { "https://www.foo.com" } end end diff --git a/test/models/bus_list_test.rb b/test/models/bus_list_test.rb index cb7693b20..fd13a5a06 100644 --- a/test/models/bus_list_test.rb +++ b/test/models/bus_list_test.rb @@ -49,7 +49,7 @@ class BusListTest < ActiveSupport::TestCase end should "return all boarded passengers" do - questionnaire1 = create(:questionnaire, bus_list_id: @bus_list.id, acc_status: "rsvp_confirmed", boarded_bus_at: nil, agreements: Agreement.all) + create(:questionnaire, bus_list_id: @bus_list.id, acc_status: "rsvp_confirmed", boarded_bus_at: nil, agreements: Agreement.all) questionnaire2 = create(:questionnaire, bus_list_id: @bus_list.id, acc_status: "rsvp_confirmed", boarded_bus_at: Time.now, agreements: Agreement.all) questionnaire3 = create(:questionnaire, bus_list_id: @bus_list.id, acc_status: "rsvp_confirmed", boarded_bus_at: Time.now, agreements: Agreement.all) assert_equal 2, @bus_list.boarded_passengers.count From 9f6687cf93a96086f1e32999e8e55fbd2645e7d0 Mon Sep 17 00:00:00 2001 From: "Chris Baudouin, Jr" Date: Fri, 11 Dec 2020 00:50:51 -0500 Subject: [PATCH 04/10] fix: Fixes some more Hound issues --- app/controllers/manage/agreements_controller.rb | 2 +- test/factories/agreement.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/manage/agreements_controller.rb b/app/controllers/manage/agreements_controller.rb index cd6459b58..41d4302b9 100644 --- a/app/controllers/manage/agreements_controller.rb +++ b/app/controllers/manage/agreements_controller.rb @@ -51,7 +51,7 @@ def destroy end private - + # Use callbacks to share common setup or constraints between actions. def set_agreement @agreement = Agreement.find(params[:id]) diff --git a/test/factories/agreement.rb b/test/factories/agreement.rb index 0f893250a..ad1d8909c 100644 --- a/test/factories/agreement.rb +++ b/test/factories/agreement.rb @@ -1,6 +1,6 @@ FactoryBot.define do factory :agreement do name { "HackFoo Agreement" } - agreement_url { "https://www.foo.com" } - end + agreement_url { "https://www.foo.com" } + end end From 04203db3e28e06f40858507e62127c516ddf4a13 Mon Sep 17 00:00:00 2001 From: "Chris Baudouin, Jr" Date: Fri, 11 Dec 2020 00:52:22 -0500 Subject: [PATCH 05/10] Update app/views/application/_unaccepted_agreements_notice.html.haml Co-authored-by: Peter Kos --- app/views/application/_unaccepted_agreements_notice.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/application/_unaccepted_agreements_notice.html.haml b/app/views/application/_unaccepted_agreements_notice.html.haml index 416ed3806..6a431aaf5 100644 --- a/app/views/application/_unaccepted_agreements_notice.html.haml +++ b/app/views/application/_unaccepted_agreements_notice.html.haml @@ -1,4 +1,4 @@ -#disclaimer.alert +.alert#disclaimer %h1.section-title %span.emphasized.fa.fa-exclamation-circle Missing From 2882c29c0ae83bd935024a2b75570d3384a9ef50 Mon Sep 17 00:00:00 2001 From: "Chris Baudouin, Jr" Date: Fri, 11 Dec 2020 01:58:57 -0500 Subject: [PATCH 06/10] fix: Fixes Peter's requests --- .gitignore | 3 +++ app/controllers/manage/agreements_controller.rb | 4 ++-- config/locales/en.yml | 2 +- db/migrate/20201130220942_create_agreements.rb | 3 +-- db/schema.rb | 2 +- 5 files changed, 8 insertions(+), 6 deletions(-) diff --git a/.gitignore b/.gitignore index 1013cc1ca..15effefdf 100644 --- a/.gitignore +++ b/.gitignore @@ -38,3 +38,6 @@ # macOS .DS_Store + +# Redis +dump.rdb diff --git a/app/controllers/manage/agreements_controller.rb b/app/controllers/manage/agreements_controller.rb index 41d4302b9..1eb815535 100644 --- a/app/controllers/manage/agreements_controller.rb +++ b/app/controllers/manage/agreements_controller.rb @@ -33,9 +33,9 @@ def create # PATCH/PUT /agreements/1 def update - if !agreement_params['agreement_url'].nil? && agreement_params['agreement_url'].start_with?('http://', 'https://') + if !agreement_params['agreement_url'].nil? && !agreement_params['agreement_url'].start_with?('http://', 'https://') flash[:alert] = "Agreement URL must start with http:// or https://" - render :edit + redirect_to edit_manage_agreement_url else @agreement.update_attributes(agreement_params) flash[:notice] = nil diff --git a/config/locales/en.yml b/config/locales/en.yml index c29e751f8..303dd5d48 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -55,7 +55,7 @@ en: school: is_home: The "home" school is separated from all other schools on dashboard metrics. agreement: - name: 'Agreements are displayed to applicants as: "I read & accept the *agreement_name* agreement."' + name: 'Agreements are displayed to applicants as: "I read & accept the [agreement_name] agreement"' hackathon_config: accepting_questionnaires: Specify and allow questionnaires to be accepted. digital_hackathon: Optimize HackathonManager for a digital hackathon. (Removes travel, dietary restrictions, etc.) diff --git a/db/migrate/20201130220942_create_agreements.rb b/db/migrate/20201130220942_create_agreements.rb index a864f573a..df59f3d59 100644 --- a/db/migrate/20201130220942_create_agreements.rb +++ b/db/migrate/20201130220942_create_agreements.rb @@ -2,9 +2,8 @@ class CreateAgreements < ActiveRecord::Migration[5.2] def change create_table :agreements do |t| t.string :name - + t.string :agreement_url t.timestamps end - add_column :agreements, :agreement_url, :text end end diff --git a/db/schema.rb b/db/schema.rb index e6e0dc51f..5d5d5831b 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -35,9 +35,9 @@ create_table "agreements", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8", force: :cascade do |t| t.string "name" + t.string "agreement_url" t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.text "agreement_url" end create_table "agreements_questionnaires", id: false, options: "ENGINE=InnoDB DEFAULT CHARSET=utf8", force: :cascade do |t| From cb13563d394332967ff00bdd280e82e2a625a540 Mon Sep 17 00:00:00 2001 From: Peter Kos Date: Fri, 11 Dec 2020 02:29:07 -0500 Subject: [PATCH 07/10] Fix agreement checkbox wrap with small names Signed-off-by: Peter Kos --- app/views/questionnaires/_form.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/questionnaires/_form.html.haml b/app/views/questionnaires/_form.html.haml index a7bb9dc1f..5002e7c5b 100644 --- a/app/views/questionnaires/_form.html.haml +++ b/app/views/questionnaires/_form.html.haml @@ -52,7 +52,7 @@ %strong Agreements %p Please review the agreements and click the corresponding checkbox next to each agreement to agree. .form-inputs - = f.association :agreements, as: :check_boxes, label_method: :formatted_agreement, value_method: :id, label: "" + = f.association :agreements, as: :check_boxes, label_method: :formatted_agreement, value_method: :id, label: "", wrapper_html: { style: 'display: block;' } .right %button.button{ type: "button", "data-wizard" => "previous" } Previous From 334124511aa31024826dc53427d5c8135fe31e8a Mon Sep 17 00:00:00 2001 From: Peter Kos Date: Fri, 11 Dec 2020 02:35:55 -0500 Subject: [PATCH 08/10] Fix straggling merge conflict error Signed-off-by: Peter Kos --- .../manage/questionnaires/_form.html.haml | 32 +++---------------- 1 file changed, 4 insertions(+), 28 deletions(-) diff --git a/app/views/manage/questionnaires/_form.html.haml b/app/views/manage/questionnaires/_form.html.haml index 46b7b9799..3206d3ab8 100644 --- a/app/views/manage/questionnaires/_form.html.haml +++ b/app/views/manage/questionnaires/_form.html.haml @@ -34,29 +34,11 @@ = f.association :bus_list, label: "Bus list", include_blank: 'Not travelling on a sponsored bus' = f.input :boarded_bus, as: :boolean, label: "Boarded bus", input_html: { checked: @questionnaire.boarded_bus_at.present? } = f.input :is_bus_captain, label: "Is Bus Captain" - - else - .card.mb-4 - .card-header Agreements - .card-body - .supporting-text - Please read the - = link_to asset_url(HackathonConfig['agreement_pdf_asset']), target: '_blank' do - #{HackathonConfig['name']} Agreement - %span.fa.fa-external-link.icon-space-l-half - = f.input :agreement_accepted, label: "I accept the #{HackathonConfig['name']} agreement.", input_html: { "data-validate" => "presence" } - .supporting-text - Please read the - %a{ href:"http://static.mlh.io/docs/mlh-code-of-conduct.pdf", target: "_blank" } - MLH Code of Conduct - %span.fa.fa-external-link.icon-space-l-half - = f.input :code_of_conduct_accepted, label: "I accept the MLH Code of Conduct.", input_html: { "data-validate" => "presence" } - - .supporting-text - I agree to the terms of both the - MLH Contest Terms and Conditions and the - MLH Privacy Policy. Please note that you may receive pre and post-event informational e-mails and occasional messages about hackathons from MLH as per the MLH Privacy Policy. - = f.input :data_sharing_accepted, label: "I accept the MLH policies.", input_html: { "data-validate" => "presence" } + .card.mb-4 + .card-header Agreements + .card-body + = f.association :agreements, as: :check_boxes, label_method: :formatted_agreement, value_method: :id, label: "" .col-xl-6 .card.mb-4 @@ -85,11 +67,5 @@ = f.input :can_share_info, label: "Share resume with employers?" - .col-xl-6 - .card.mb-4 - .card-header Agreements - .card-body - = f.association :agreements, as: :check_boxes, label_method: :formatted_agreement, value_method: :id, label: "" - .center.mb-4 = f.button :submit, value: ( @questionnaire.new_record? ? 'Create' : 'Save' ), class: 'btn-primary' From 22249c57da4d5e1c0f8305d081ba4177163482df Mon Sep 17 00:00:00 2001 From: Peter Kos Date: Fri, 11 Dec 2020 02:38:23 -0500 Subject: [PATCH 09/10] Removed agreements card Signed-off-by: Peter Kos --- app/views/manage/questionnaires/_form.html.haml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/app/views/manage/questionnaires/_form.html.haml b/app/views/manage/questionnaires/_form.html.haml index 3206d3ab8..8e5f23220 100644 --- a/app/views/manage/questionnaires/_form.html.haml +++ b/app/views/manage/questionnaires/_form.html.haml @@ -35,11 +35,6 @@ = f.input :boarded_bus, as: :boolean, label: "Boarded bus", input_html: { checked: @questionnaire.boarded_bus_at.present? } = f.input :is_bus_captain, label: "Is Bus Captain" - .card.mb-4 - .card-header Agreements - .card-body - = f.association :agreements, as: :check_boxes, label_method: :formatted_agreement, value_method: :id, label: "" - .col-xl-6 .card.mb-4 .card-header Special notices From 1a50f1f8024827c1658436b859780db538642dc5 Mon Sep 17 00:00:00 2001 From: Peter Kos Date: Fri, 11 Dec 2020 02:53:16 -0500 Subject: [PATCH 10/10] Force validation for agreement on questionnaire pg FRONTEND Signed-off-by: Peter Kos --- app/views/questionnaires/_form.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/questionnaires/_form.html.haml b/app/views/questionnaires/_form.html.haml index 5002e7c5b..2b2fe540a 100644 --- a/app/views/questionnaires/_form.html.haml +++ b/app/views/questionnaires/_form.html.haml @@ -52,7 +52,7 @@ %strong Agreements %p Please review the agreements and click the corresponding checkbox next to each agreement to agree. .form-inputs - = f.association :agreements, as: :check_boxes, label_method: :formatted_agreement, value_method: :id, label: "", wrapper_html: { style: 'display: block;' } + = f.association :agreements, as: :check_boxes, label_method: :formatted_agreement, value_method: :id, label: "", wrapper_html: { style: 'display: block' }, input_html: { "data-validate" => "presence" } .right %button.button{ type: "button", "data-wizard" => "previous" } Previous