From d10f4b850cc8c7adc106921a7aaa9d00e9711cb1 Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Wed, 19 Apr 2023 18:10:33 -0700 Subject: [PATCH 01/54] Use ruby 3.2.2 --- .github/workflows/rubocop.yml | 4 ++-- .github/workflows/specs.yml | 2 +- .ruby-version | 2 +- Gemfile | 2 +- README.md | 4 ++-- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.github/workflows/rubocop.yml b/.github/workflows/rubocop.yml index 98001108..8b179f92 100644 --- a/.github/workflows/rubocop.yml +++ b/.github/workflows/rubocop.yml @@ -10,10 +10,10 @@ jobs: steps: - uses: actions/checkout@v2 - - name: Set up Ruby 2.7.7 + - name: Set up Ruby 3.2.2 uses: ruby/setup-ruby@v1 with: - ruby-version: 2.7.7 + ruby-version: 3.2.2 bundler-cache: true - name: Run RuboCop run: bundle exec rubocop --parallel diff --git a/.github/workflows/specs.yml b/.github/workflows/specs.yml index 0ffabfcd..28d97f22 100644 --- a/.github/workflows/specs.yml +++ b/.github/workflows/specs.yml @@ -33,7 +33,7 @@ jobs: - name: Use Ruby uses: ruby/setup-ruby@v1 with: - ruby-version: 2.7.7 + ruby-version: 3.2.2 bundler-cache: true - name: Setup Code Climate run: | diff --git a/.ruby-version b/.ruby-version index 1f7da99d..be94e6f5 100644 --- a/.ruby-version +++ b/.ruby-version @@ -1 +1 @@ -2.7.7 +3.2.2 diff --git a/Gemfile b/Gemfile index bc56dc59..61c91a59 100644 --- a/Gemfile +++ b/Gemfile @@ -4,7 +4,7 @@ source "https://rubygems.org" git_source(:github) { |repo| "https://github.com/#{repo}.git" } -ruby "2.7.7" +ruby "3.2.2" gem "rails", "6.1.6" diff --git a/README.md b/README.md index e248b9e4..c3796419 100644 --- a/README.md +++ b/README.md @@ -60,7 +60,7 @@ We have worked on the adding following core features and functionality: * Note: if you are using WSL2 on windows, the command to start postgres is `sudo service postgresql start` ### Framework/Gem Installation -* `rvm use 2.7.7` # Double-check `.ruby-version` +* `rvm use 3.2.2` # Double-check `.ruby-version` * `bundle` * `bin/rails db:setup` (Run this after setting up Postgres) * `nvm install 14` Node v14 is needed @@ -106,7 +106,7 @@ We have worked on the adding following core features and functionality: ## Steps to Deploying on Heroku - ... create a heroku app -- `heroku stack:set heroku-20` (Ruby 2.7.7 is not supported on latest stack heroku-22. Double check your Ruby version though) +- `heroku stack:set heroku-20` (Ruby 3.2.2 is not supported on latest stack heroku-22. Double check your Ruby version though) - `heroku buildpacks:set heroku/nodejs` # this must be the first buildpack. - `heroku buildpacks:add --index 2 heroku/ruby` - `git remote set-url heroku https://git.heroku.com/bjc-teachers.git` (or whatever your heroku deployment repository is) From fffb14be9617b3012bdeeaec5af6bd081d4ce5f5 Mon Sep 17 00:00:00 2001 From: Matthew Kao Date: Sun, 23 Apr 2023 22:55:22 -0700 Subject: [PATCH 02/54] Matthew Chuang --- app/controllers/email_templates_controller.rb | 36 ++++++++++++++++++- app/models/email_template.rb | 7 ++-- app/views/email_templates/_form.html.erb | 19 ++++++++++ app/views/email_templates/index.html.erb | 10 +++++- app/views/email_templates/new.html.erb | 33 +++++++++++++++++ app/views/main/_deny_modal.html.erb | 9 +++++ config/routes.rb | 2 +- db/seed_data.rb | 34 ++++++++++++++++-- 8 files changed, 142 insertions(+), 8 deletions(-) create mode 100644 app/views/email_templates/_form.html.erb create mode 100644 app/views/email_templates/new.html.erb diff --git a/app/controllers/email_templates_controller.rb b/app/controllers/email_templates_controller.rb index 6b698d1e..c6beed19 100644 --- a/app/controllers/email_templates_controller.rb +++ b/app/controllers/email_templates_controller.rb @@ -17,8 +17,42 @@ def update redirect_to email_templates_path end + def create + @email_template = EmailTemplate.find_by(title: template_params[:title]) + if @email_template + @email_template.assign_attributes(template_params) + else + @email_template = EmailTemplate.new(template_params) + end + load_ordered_emails + + + if @email_template.save! + flash[:success] = "Created #{@email_template.title} successfully." + redirect_to email_templates_path + else + flash[:alert] = "Failed to submit information :(" + render "new" + end + end + + def new + @email_template = EmailTemplate.new + load_ordered_emails + end + + def destroy + @email_template = EmailTemplate.find(params[:id]) + @email_template.destroy + redirect_to email_templates_path, notice: "Deleted \"#{@email_template.subject}\" successfully." + end + + def load_ordered_emails + @ordered_email ||= EmailTemplate.all.order(:title) + end + private def template_params - params.require(:email_template).permit(:body, :subject) + params.require(:email_template).permit(:body, :subject, :title) end end diff --git a/app/models/email_template.rb b/app/models/email_template.rb index 809006be..c9844e66 100644 --- a/app/models/email_template.rb +++ b/app/models/email_template.rb @@ -17,6 +17,9 @@ # updated_at :datetime not null # class EmailTemplate < ApplicationRecord - validates :title, - inclusion: TeacherMailer.instance_methods(false).map { |method| method.to_s.titlecase } + # validates :title, + # inclusion: TeacherMailer.instance_methods(false).map { |method| method.to_s.titlecase } end + + + diff --git a/app/views/email_templates/_form.html.erb b/app/views/email_templates/_form.html.erb new file mode 100644 index 00000000..8ea9893b --- /dev/null +++ b/app/views/email_templates/_form.html.erb @@ -0,0 +1,19 @@ +
Create a new Email
+ +
+ <%= f.label :subject, "Subject", class: "label-required", for: "emailTemplate_subject" %> + <%= f.text_field :subject, placeholder: "Enter the subject for the Email Template", + class: "form-control", required: false, + title: "Please enter the subject for the Email Template", + id: 'emailTemplate_subject' %> +
+ +
+ <%= f.label :body, "Body", class: "label-required", for: "emailTemplate_body" %> + <%= f.text_area :body, placeholder: "Enter the body for the Email Template", + class: "tinymce", required: false, + title: "Please enter the body for the Email Template", + id: 'emailTemplate_body' %> +
+ +<%- end %> \ No newline at end of file diff --git a/app/views/email_templates/index.html.erb b/app/views/email_templates/index.html.erb index e767befd..52f4d231 100644 --- a/app/views/email_templates/index.html.erb +++ b/app/views/email_templates/index.html.erb @@ -1,15 +1,23 @@ <% provide(:title, "Email Templates") %> +<% provide(:header_button, "New Email Templates") %>
- + + <% @all_templates.each do |template| %> + <% end %> diff --git a/app/views/email_templates/new.html.erb b/app/views/email_templates/new.html.erb new file mode 100644 index 00000000..31757927 --- /dev/null +++ b/app/views/email_templates/new.html.erb @@ -0,0 +1,33 @@ +<%= provide(:h1, 'Add an Email Template') %> +
+ +<%= form_with(model: @email_template, local: true) do |form| %> +
+ + <%= form.text_field :title, class: "form-control" %> + + <%= form.text_field :subject, class: "form-control" %> + + <%= form.text_area :body, class: "tinymce", rows: 20, style: 'width:100%;'%> +
+
+
+ <%= form.submit "Submit", class: "btn btn-primary" %> +
+<% end %> + + \ No newline at end of file diff --git a/app/views/main/_deny_modal.html.erb b/app/views/main/_deny_modal.html.erb index 76657e3f..89820e81 100644 --- a/app/views/main/_deny_modal.html.erb +++ b/app/views/main/_deny_modal.html.erb @@ -9,6 +9,15 @@
<%= label_tag 'reason', 'Reason for Denial', class: 'control-label' %> <%= text_field_tag :reason, params[:reason], class: "form-control" %> +
+ + +
<%= check_box_tag :skip_email, 'skip_email', false, class: 'form-check-input' %> <%= label_tag 'skip_email', 'Skip email notifcation?', class: 'form-check-label' %> diff --git a/config/routes.rb b/config/routes.rb index 33249719..df05841f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -17,7 +17,7 @@ end resources :schools resources :pages, param: :url_slug - resources :email_templates, only: [:index, :update, :edit] + resources :email_templates, only: [:index, :update, :edit, :new, :destroy, :create] get "/login", to: "sessions#new", as: "login" delete "/logout", to: "sessions#destroy", as: "logout" diff --git a/db/seed_data.rb b/db/seed_data.rb index 67a31995..1e49b36b 100644 --- a/db/seed_data.rb +++ b/db/seed_data.rb @@ -31,10 +31,38 @@ module SeedData

DENY_EMAIL + @deny_email1 = <<-DENY_EMAIL1 +

Dear [Recipient's Name],

+

Thank you for your email. We have received your message and we are sorry to inform you that your application has been rejected.

+

Best regards,

+

[Your Name]

+

Below, you can find the reason as to why it was rejected

+

+ {{ reason | strip_tags}} +

+ DENY_EMAIL1 + + @deny_email2 = <<-DENY_EMAIL2 +

Dear [Recipient's Name],

+

Thank you for your email. We have received your message and we are sorry to inform you that your application has been rejected.

+

Best regards,

+

[Your Name]

+

Unfortunately, we are not able to provide a particular reason at this moment

+ DENY_EMAIL2 + + @deny_email3 = <<-DENY_EMAIL3 +

Dear [Recipient's Name],

+

Thank you for your email. We have received your message and your application has currently been waitlisted.

+

We will reach out to you if there are any changes to your application,

+

Best regards,

+

[Your Name]

+ + DENY_EMAIL3 + def self.emails [ { - id: 1, + # id: 1, body: @welcome_email, path: "teacher_mailer/welcome_email", locale: nil, @@ -45,7 +73,7 @@ def self.emails subject: "Welcome to The Beauty and Joy of Computing!" }, { - id: 3, + # id: 3, body: @form_submission, path: "teacher_mailer/form_submission", locale: nil, @@ -56,7 +84,7 @@ def self.emails subject: "Form Submission" }, { - id: 4, + # id: 4, body: @deny_email, path: "teacher_mailer/deny_email", locale: nil, From b34384d1c43214a99967027c3218623edef97f9f Mon Sep 17 00:00:00 2001 From: chuang-yao-ucb Date: Mon, 1 May 2023 10:12:33 -0700 Subject: [PATCH 03/54] rubocop passing --- app/controllers/email_templates_controller.rb | 3 +-- app/models/email_template.rb | 3 --- db/seed_data.rb | 6 +++--- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/app/controllers/email_templates_controller.rb b/app/controllers/email_templates_controller.rb index c6beed19..328cadb6 100644 --- a/app/controllers/email_templates_controller.rb +++ b/app/controllers/email_templates_controller.rb @@ -26,8 +26,7 @@ def create end load_ordered_emails - - if @email_template.save! + if @email_template.save! flash[:success] = "Created #{@email_template.title} successfully." redirect_to email_templates_path else diff --git a/app/models/email_template.rb b/app/models/email_template.rb index c9844e66..58e251b5 100644 --- a/app/models/email_template.rb +++ b/app/models/email_template.rb @@ -20,6 +20,3 @@ class EmailTemplate < ApplicationRecord # validates :title, # inclusion: TeacherMailer.instance_methods(false).map { |method| method.to_s.titlecase } end - - - diff --git a/db/seed_data.rb b/db/seed_data.rb index 1e49b36b..d7c838e6 100644 --- a/db/seed_data.rb +++ b/db/seed_data.rb @@ -35,7 +35,7 @@ module SeedData

Dear [Recipient's Name],

Thank you for your email. We have received your message and we are sorry to inform you that your application has been rejected.

Best regards,

-

[Your Name]

+

[Your Name]

Below, you can find the reason as to why it was rejected

{{ reason | strip_tags}} @@ -46,7 +46,7 @@ module SeedData

Dear [Recipient's Name],

Thank you for your email. We have received your message and we are sorry to inform you that your application has been rejected.

Best regards,

-

[Your Name]

+

[Your Name]

Unfortunately, we are not able to provide a particular reason at this moment

DENY_EMAIL2 @@ -55,7 +55,7 @@ module SeedData

Thank you for your email. We have received your message and your application has currently been waitlisted.

We will reach out to you if there are any changes to your application,

Best regards,

-

[Your Name]

+

[Your Name]

DENY_EMAIL3 From 14760e2bb2eb02bb89deab8d29defb6118d61453 Mon Sep 17 00:00:00 2001 From: chuang-yao-ucb Date: Mon, 1 May 2023 10:41:20 -0700 Subject: [PATCH 04/54] changed tests --- features/admin.feature | 22 ---------------------- spec/mailers/teacher_mailer_spec.rb | 10 ---------- 2 files changed, 32 deletions(-) diff --git a/features/admin.feature b/features/admin.feature index bd4fc922..8769f46a 100644 --- a/features/admin.feature +++ b/features/admin.feature @@ -117,28 +117,6 @@ Scenario: Deny teacher as an admin And I press "Submit" Then I can send a deny email -Scenario: Request info from teacher as an admin - Given the following schools exist: - | name | city | state | website | grade_level | school_type | - | UC Berkeley | Berkeley | CA | https://www.berkeley.edu | university | public | - Given the following teachers exist: - | first_name | last_name | admin | email | school | - | Joseph | Mamoa | false | testteacher@berkeley.edu | UC Berkeley | - Given I am on the BJC home page - Given I have an admin email - When I follow "Log In" - Then I can log in with Google - And I press "❓" within "#DataTables_Table_0 > tbody > tr:nth-child(1)" - Then I should see "Reason" - Then I should see "Request Info from Joseph Mamoa" - And I fill in "reason" with "Test" - And I press "Cancel" - And I press "❓" within "#DataTables_Table_0 > tbody > tr:nth-child(1)" - Then the "reason" field should not contain "Test" - And I fill in "reason" with "Request Info Reason" - And I press "Submit" - Then I can send a request info email - Scenario: Not logged in should not have access to edit Given the following schools exist: | name | city | state | website | grade_level | school_type | diff --git a/spec/mailers/teacher_mailer_spec.rb b/spec/mailers/teacher_mailer_spec.rb index 7f4073b9..d9db4aab 100644 --- a/spec/mailers/teacher_mailer_spec.rb +++ b/spec/mailers/teacher_mailer_spec.rb @@ -36,14 +36,4 @@ expect(email.to[0]).to eq("lmock@berkeley.edu") expect(email.body.encoded).to include("Short Long") end - - it "Sends Request Info Email" do - teacher = teachers(:long) - email = TeacherMailer.request_info_email(teacher, "Request Info Reason") - email.deliver_now - expect(email.from[0]).to eq("contact@bjc.berkeley.edu") - expect(email.to[0]).to eq("short@long.com") - expect(email.subject).to eq("Request Info Email") - expect(email.body.encoded).to include("Request Info Reason") - end end From dbb6ccd6656146d9988887266e5f825415e8a693 Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Fri, 23 Jun 2023 02:00:44 -0700 Subject: [PATCH 05/54] Add a required flag for email templates --- app/controllers/email_templates_controller.rb | 12 ++--- app/models/email_template.rb | 16 ++++++- app/views/email_templates/_form.html.erb | 47 ++++++++++++------- app/views/email_templates/edit.html.erb | 29 +----------- app/views/email_templates/index.html.erb | 2 +- app/views/email_templates/new.html.erb | 31 +----------- ...3083259_add_required_to_email_templates.rb | 5 ++ db/schema.rb | 3 +- db/seed_data.rb | 8 ++-- lib/fixup_required_emails.rb | 4 ++ 10 files changed, 67 insertions(+), 90 deletions(-) create mode 100644 db/migrate/20230623083259_add_required_to_email_templates.rb create mode 100644 lib/fixup_required_emails.rb diff --git a/app/controllers/email_templates_controller.rb b/app/controllers/email_templates_controller.rb index 328cadb6..6bce23fb 100644 --- a/app/controllers/email_templates_controller.rb +++ b/app/controllers/email_templates_controller.rb @@ -24,7 +24,7 @@ def create else @email_template = EmailTemplate.new(template_params) end - load_ordered_emails + load_ordered_email_templates if @email_template.save! flash[:success] = "Created #{@email_template.title} successfully." @@ -37,7 +37,7 @@ def create def new @email_template = EmailTemplate.new - load_ordered_emails + load_ordered_email_templates end def destroy @@ -46,12 +46,12 @@ def destroy redirect_to email_templates_path, notice: "Deleted \"#{@email_template.subject}\" successfully." end - def load_ordered_emails - @ordered_email ||= EmailTemplate.all.order(:title) - end - private def template_params params.require(:email_template).permit(:body, :subject, :title) end + + def load_ordered_email_templates + @ordered_email_templates ||= EmailTemplate.all.order(:title) + end end diff --git a/app/models/email_template.rb b/app/models/email_template.rb index 58e251b5..754cb1f5 100644 --- a/app/models/email_template.rb +++ b/app/models/email_template.rb @@ -11,12 +11,24 @@ # locale :string # partial :boolean # path :string +# required :boolean default(FALSE) # subject :string # title :string # created_at :datetime not null # updated_at :datetime not null # class EmailTemplate < ApplicationRecord - # validates :title, - # inclusion: TeacherMailer.instance_methods(false).map { |method| method.to_s.titlecase } + validates :title, + inclusion: TeacherMailer.instance_methods(false).map { |method| method.to_s.titlecase }, + if: -> { self.required? } + + before_destroy :prevent_deleting_required_emails + + private + def prevent_deleting_required_emails + if self.required? + errors.add() + throw :abort + end + end end diff --git a/app/views/email_templates/_form.html.erb b/app/views/email_templates/_form.html.erb index 8ea9893b..8fbc6fc0 100644 --- a/app/views/email_templates/_form.html.erb +++ b/app/views/email_templates/_form.html.erb @@ -1,19 +1,30 @@ -
Create a new Email
+<%= form_with(model: email_template, local: true) do |form| %> +
+ + <%= form.text_field :title, class: "form-control", readonly: email_template.required? %> + + <%= form.text_field :subject, class: "form-control" %> + + <%= form.text_area :body, class: "tinymce", rows: 20, style: 'width:100%;'%> +
+
+
+ <%= form.submit "Submit", class: "btn btn-primary" %> +
+<% end %> -
- <%= f.label :subject, "Subject", class: "label-required", for: "emailTemplate_subject" %> - <%= f.text_field :subject, placeholder: "Enter the subject for the Email Template", - class: "form-control", required: false, - title: "Please enter the subject for the Email Template", - id: 'emailTemplate_subject' %> -
- -
- <%= f.label :body, "Body", class: "label-required", for: "emailTemplate_body" %> - <%= f.text_area :body, placeholder: "Enter the body for the Email Template", - class: "tinymce", required: false, - title: "Please enter the body for the Email Template", - id: 'emailTemplate_body' %> -
- -<%- end %> \ No newline at end of file + diff --git a/app/views/email_templates/edit.html.erb b/app/views/email_templates/edit.html.erb index 9a2fe694..7ec0d8a2 100644 --- a/app/views/email_templates/edit.html.erb +++ b/app/views/email_templates/edit.html.erb @@ -1,31 +1,4 @@ <%= provide(:h1, @email_template.title) %>
-<%= form_with(model: @email_template, local: true) do |form| %> -
- - <%= form.text_field :subject, class: "form-control" %> - - <%= form.text_area :body, class: "tinymce", rows: 20, style: 'width:100%;'%> -
-
-
- <%= form.submit "Update", class: "btn btn-primary" %> -
-<% end %> - - +<%= render 'form', email_template: @email_template %> diff --git a/app/views/email_templates/index.html.erb b/app/views/email_templates/index.html.erb index 52f4d231..b5c2027e 100644 --- a/app/views/email_templates/index.html.erb +++ b/app/views/email_templates/index.html.erb @@ -15,7 +15,7 @@
diff --git a/app/views/email_templates/new.html.erb b/app/views/email_templates/new.html.erb index 31757927..ccb21910 100644 --- a/app/views/email_templates/new.html.erb +++ b/app/views/email_templates/new.html.erb @@ -1,33 +1,4 @@ <%= provide(:h1, 'Add an Email Template') %>
-<%= form_with(model: @email_template, local: true) do |form| %> -
- - <%= form.text_field :title, class: "form-control" %> - - <%= form.text_field :subject, class: "form-control" %> - - <%= form.text_area :body, class: "tinymce", rows: 20, style: 'width:100%;'%> -
-
-
- <%= form.submit "Submit", class: "btn btn-primary" %> -
-<% end %> - - \ No newline at end of file +<%= render 'form', email_template: @email_template %> diff --git a/db/migrate/20230623083259_add_required_to_email_templates.rb b/db/migrate/20230623083259_add_required_to_email_templates.rb new file mode 100644 index 00000000..b5072546 --- /dev/null +++ b/db/migrate/20230623083259_add_required_to_email_templates.rb @@ -0,0 +1,5 @@ +class AddRequiredToEmailTemplates < ActiveRecord::Migration[6.1] + def change + add_column :email_templates, :required, :boolean, default: false + end +end diff --git a/db/schema.rb b/db/schema.rb index 475594b6..02f96ae3 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: 2023_04_15_064901) do +ActiveRecord::Schema.define(version: 2023_06_23_083259) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -64,6 +64,7 @@ t.datetime "updated_at", null: false t.string "title" t.string "subject" + t.boolean "required", default: false end create_table "pages", force: :cascade do |t| diff --git a/db/seed_data.rb b/db/seed_data.rb index b04df7a3..083230fc 100644 --- a/db/seed_data.rb +++ b/db/seed_data.rb @@ -62,7 +62,6 @@ module SeedData def self.emails [ { - # id: 1, body: @welcome_email, path: "teacher_mailer/welcome_email", locale: nil, @@ -70,10 +69,10 @@ def self.emails partial: false, format: "html", title: "Welcome Email", + required: true, subject: "Welcome to The Beauty and Joy of Computing!" }, { - # id: 3, body: @form_submission, path: "teacher_mailer/form_submission", locale: nil, @@ -81,10 +80,10 @@ def self.emails partial: false, format: "html", title: "Form Submission", + required: true, subject: "Form Submission" }, { - # id: 4, body: @deny_email, path: "teacher_mailer/deny_email", locale: nil, @@ -92,10 +91,10 @@ def self.emails partial: false, format: "html", title: "Deny Email", + required: true, subject: "Deny Email" }, { - id: 5, body: @request_info_email, path: "teacher_mailer/request_info_email", locale: nil, @@ -103,6 +102,7 @@ def self.emails partial: false, format: "html", title: "Request Info Email", + required: true, subject: "Request Info Email" } ] diff --git a/lib/fixup_required_emails.rb b/lib/fixup_required_emails.rb new file mode 100644 index 00000000..2d41b385 --- /dev/null +++ b/lib/fixup_required_emails.rb @@ -0,0 +1,4 @@ +EmailTemplate.where( + title: + ["Welcome Email", "Form Submission", + "Deny Email", "Request Info Email"]).update_all(required: true) From 8bc17607f52cb522ade505fd9d59f4b22de45c58 Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Tue, 1 Aug 2023 12:43:44 -0700 Subject: [PATCH 06/54] Webpacker runs again, remove unnecessary packages --- config/webpacker.yml | 2 +- package.json | 2 -- yarn.lock | 10 ---------- 3 files changed, 1 insertion(+), 13 deletions(-) diff --git a/config/webpacker.yml b/config/webpacker.yml index e850173f..190139da 100644 --- a/config/webpacker.yml +++ b/config/webpacker.yml @@ -95,7 +95,7 @@ production: # Cache manifest.json for performance cache_manifest: true -staging: +staging: <<: *default # Production depends on precompilation of packs prior to booting for performance. diff --git a/package.json b/package.json index e8093d59..98ee8e12 100644 --- a/package.json +++ b/package.json @@ -14,8 +14,6 @@ "jquery": "^3.6.0", "jquery-ujs": "^1.2.3", "popper.js": "^1.16.0", - "select2": "^4.1.0-rc.0", - "select2-bootstrap-theme": "^0.1.0-beta.10", "selectize": "^0.12.6", "tinymce": "^5.x", "trix": "^1.2.0", diff --git a/yarn.lock b/yarn.lock index c5f88575..0327b2e8 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6613,16 +6613,6 @@ select-hose@^2.0.0: resolved "https://registry.npmjs.org/select-hose/-/select-hose-2.0.0.tgz" integrity sha1-Yl2GWPhlr0Psliv8N2o3NZpJlMo= -select2-bootstrap-theme@^0.1.0-beta.10: - version "0.1.0-beta.10" - resolved "https://registry.yarnpkg.com/select2-bootstrap-theme/-/select2-bootstrap-theme-0.1.0-beta.10.tgz#b9426ecfc03bf4a235e76a132377574310469ac0" - integrity sha1-uUJuz8A79KI152oTI3dXQxBGmsA= - -select2@^4.1.0-rc.0: - version "4.1.0-rc.0" - resolved "https://registry.yarnpkg.com/select2/-/select2-4.1.0-rc.0.tgz#ba3cd3901dda0155e1c0219ab41b74ba51ea22d8" - integrity sha512-Hr9TdhyHCZUtwznEH2CBf7967mEM0idtJ5nMtjvk3Up5tPukOLXbHUNmh10oRfeNIhj+3GD3niu+g6sVK+gK0A== - selectize@^0.12.6: version "0.12.6" resolved "https://registry.yarnpkg.com/selectize/-/selectize-0.12.6.tgz#c2cf08cbaa4cb06c5e99bb452919d71b080690d6" From 510baebd7a01b52c6a6a38f5b552087f3f3b321e Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Tue, 1 Aug 2023 12:46:39 -0700 Subject: [PATCH 07/54] Reorder Gems in Gemfile --- Gemfile | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Gemfile b/Gemfile index 5f5c2dd7..e95f83f0 100644 --- a/Gemfile +++ b/Gemfile @@ -45,6 +45,12 @@ gem "aws-sdk-s3", require: false # Render images for file uploads in pages gem "image_processing", ">= 1.2" +gem "selectize-rails" +gem "smarter_csv", "~> 1.4" +gem "activerecord-import", require: false + +gem "httparty", "~> 0.21.0" + group :development do gem "annotate" gem "guard" @@ -103,9 +109,3 @@ group :test do gem "rails-controller-testing" end - -gem "selectize-rails" -gem "smarter_csv", "~> 1.4" -gem "activerecord-import", require: false - -gem "httparty", "~> 0.21.0" From 756ca1fd94887fe2370cefc939ba039e4ad995a4 Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Tue, 1 Aug 2023 12:47:15 -0700 Subject: [PATCH 08/54] Add a .tool-versions file --- .tool-versions | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 .tool-versions diff --git a/.tool-versions b/.tool-versions new file mode 100644 index 00000000..e69de29b From 0260339041848d9bc12e7fa3d8a56f5130deed79 Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Tue, 1 Aug 2023 13:33:01 -0700 Subject: [PATCH 09/54] Update pronto+pg gems --- .tool-versions | 2 ++ Gemfile | 2 +- Gemfile.lock | 48 ++++++++++++++++++------------------ app/models/email_template.rb | 6 ++++- 4 files changed, 32 insertions(+), 26 deletions(-) diff --git a/.tool-versions b/.tool-versions index e69de29b..12bedc08 100644 --- a/.tool-versions +++ b/.tool-versions @@ -0,0 +1,2 @@ +ruby 3.2.2 +nodejs 14.21.3 diff --git a/Gemfile b/Gemfile index e95f83f0..e994aea7 100644 --- a/Gemfile +++ b/Gemfile @@ -10,7 +10,7 @@ gem "rails", "6.1.6" # Production app server gem "puma", "~> 5" -gem "pg", "~> 1.0" +gem "pg", "~> 1.5" # Front-end Assets gem "webpacker", "~> 4" diff --git a/Gemfile.lock b/Gemfile.lock index d7aa2374..568bbdac 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -85,8 +85,8 @@ GEM minitest (>= 5.1) tzinfo (~> 2.0) zeitwerk (~> 2.3) - addressable (2.8.0) - public_suffix (>= 2.0.2, < 5.0) + addressable (2.8.4) + public_suffix (>= 2.0.2, < 6.0) annotate (3.1.1) activerecord (>= 3.2, < 7.0) rake (>= 10.4, < 14.0) @@ -251,8 +251,8 @@ GEM ruby_parser (~> 3.1, > 3.1.0) sexp_processor (~> 4.8) formatador (0.3.0) - gitlab (4.18.0) - httparty (~> 0.18) + gitlab (4.19.0) + httparty (~> 0.20) terminal-table (>= 1.5.1) globalid (1.0.1) activesupport (>= 5.0) @@ -372,9 +372,9 @@ GEM rack (>= 1.2, < 4) snaky_hash (~> 2.0) version_gem (~> 1.1) - octokit (4.21.0) - faraday (>= 0.9) - sawyer (~> 0.8.0, >= 0.5.3) + octokit (6.1.1) + faraday (>= 1, < 3) + sawyer (~> 0.9) omniauth (2.1.1) hashie (>= 3.4.6) rack (>= 2.2.3) @@ -397,14 +397,14 @@ GEM parser (3.0.3.2) ast (~> 2.4.1) path_expander (1.1.0) - pg (1.2.3) - pronto (0.11.0) - gitlab (~> 4.4, >= 4.4.0) - httparty (>= 0.13.7) - octokit (~> 4.7, >= 4.7.0) + pg (1.5.3) + pronto (0.11.1) + gitlab (>= 4.4.0, < 5.0) + httparty (>= 0.13.7, < 1.0) + octokit (>= 4.7.0, < 7.0) rainbow (>= 2.2, < 4.0) - rexml (~> 3.2) - rugged (>= 0.23.0, < 1.1.0) + rexml (>= 3.2.5, < 4.0) + rugged (>= 0.23.0, < 2.0) thor (>= 0.20.3, < 2.0) pronto-flay (0.11.1) flay (~> 2.8) @@ -421,7 +421,7 @@ GEM pry-byebug (3.9.0) byebug (~> 11.0) pry (~> 0.13.0) - public_suffix (4.0.6) + public_suffix (5.0.3) puma (5.6.4) nio4r (~> 2.0) racc (1.6.2) @@ -472,7 +472,7 @@ GEM method_source rake (>= 12.2) thor (~> 1.0) - rainbow (3.0.0) + rainbow (3.1.1) rake (13.0.6) rb-fsevent (0.11.0) rb-inotify (0.10.1) @@ -484,7 +484,7 @@ GEM rainbow (>= 2.0, < 4.0) regexp_parser (2.2.0) require_all (3.0.0) - rexml (3.2.5) + rexml (3.2.6) roodi (5.0.0) ruby_parser (~> 3.2, >= 3.2.2) rspec (3.10.0) @@ -538,10 +538,10 @@ GEM ruby_parser (3.18.1) sexp_processor (~> 4.16) rubyzip (2.3.2) - rugged (1.0.1) - sawyer (0.8.2) + rugged (1.6.3) + sawyer (0.9.2) addressable (>= 2.3.5) - faraday (> 0.8, < 2.0) + faraday (>= 0.17.3, < 3) selectize-rails (0.12.6) selenium-webdriver (4.1.0) childprocess (>= 0.5, < 5.0) @@ -591,14 +591,14 @@ GEM temple (0.8.2) terminal-table (3.0.2) unicode-display_width (>= 1.1.1, < 3) - thor (1.2.1) + thor (1.2.2) thread_safe (0.3.6) tilt (2.0.10) tzinfo (2.0.6) concurrent-ruby (~> 1.0) tzinfo-data (1.2021.5) tzinfo (>= 1.0.0) - unicode-display_width (2.1.0) + unicode-display_width (2.4.2) version_gem (1.1.1) virtus (2.0.0) axiom-types (~> 0.1) @@ -659,7 +659,7 @@ DEPENDENCIES omniauth-microsoft_graph omniauth-rails_csrf_protection omniauth-yahoo-oauth2! - pg (~> 1.0) + pg (~> 1.5) pronto pronto-flay pronto-haml @@ -692,7 +692,7 @@ DEPENDENCIES webpacker (~> 4) RUBY VERSION - ruby 2.7.8p225 + ruby 3.2.2p53 BUNDLED WITH 2.1.4 diff --git a/app/models/email_template.rb b/app/models/email_template.rb index 754cb1f5..a58abcf2 100644 --- a/app/models/email_template.rb +++ b/app/models/email_template.rb @@ -24,10 +24,14 @@ class EmailTemplate < ApplicationRecord before_destroy :prevent_deleting_required_emails + def self.templates_with_custom_reason + + end + private def prevent_deleting_required_emails if self.required? - errors.add() + errors.add('Cannot delete a required email template') throw :abort end end From 50dd9dbe6100a1f341ec04c1c6c604690aabc204 Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Tue, 1 Aug 2023 13:37:24 -0700 Subject: [PATCH 10/54] Minor dev dependencies update... --- Gemfile | 2 +- Gemfile.lock | 36 +++++++++++++++++++----------------- 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/Gemfile b/Gemfile index e994aea7..1a00302a 100644 --- a/Gemfile +++ b/Gemfile @@ -13,7 +13,7 @@ gem "puma", "~> 5" gem "pg", "~> 1.5" # Front-end Assets -gem "webpacker", "~> 4" +gem "webpacker", "< 5" # Reduces boot times through caching; required in config/boot.rb gem "bootsnap", ">= 1.4.4", require: false diff --git a/Gemfile.lock b/Gemfile.lock index 568bbdac..9e44c909 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -292,7 +292,7 @@ GEM httparty (0.21.0) mini_mime (>= 1.0.0) multi_xml (>= 0.5.2) - i18n (1.12.0) + i18n (1.14.1) concurrent-ruby (~> 1.0) ice_nine (0.11.2) image_processing (1.12.2) @@ -315,9 +315,9 @@ GEM listen (3.7.0) rb-fsevent (~> 0.10, >= 0.10.3) rb-inotify (~> 0.9, >= 0.9.10) - loofah (2.19.1) + loofah (2.21.3) crass (~> 1.0.2) - nokogiri (>= 1.5.9) + nokogiri (>= 1.12.0) lumberjack (1.2.8) mail (2.7.1) mini_mime (>= 0.1.1) @@ -350,8 +350,8 @@ GEM mime-types-data (3.2022.0105) mini_magick (4.11.0) mini_mime (1.1.2) - mini_portile2 (2.8.1) - minitest (5.17.0) + mini_portile2 (2.8.4) + minitest (5.19.0) msgpack (1.4.2) multi_json (1.15.0) multi_test (0.1.2) @@ -359,8 +359,8 @@ GEM multipart-post (2.3.0) nenv (0.3.0) nio4r (2.5.8) - nokogiri (1.14.3) - mini_portile2 (~> 2.8.0) + nokogiri (1.15.3) + mini_portile2 (~> 2.8.2) racc (~> 1.4) notiffany (0.1.3) nenv (~> 0.1) @@ -424,15 +424,15 @@ GEM public_suffix (5.0.3) puma (5.6.4) nio4r (~> 2.0) - racc (1.6.2) - rack (2.2.6.3) + racc (1.7.1) + rack (2.2.8) rack-mini-profiler (2.3.3) rack (>= 1.2.0) rack-protection (3.0.5) rack - rack-proxy (0.7.2) + rack-proxy (0.7.6) rack - rack-test (2.0.2) + rack-test (2.1.0) rack (>= 1.3) rails (6.1.6) actioncable (= 6.1.6) @@ -453,11 +453,13 @@ GEM actionpack (>= 5.0.1.rc1) actionview (>= 5.0.1.rc1) activesupport (>= 5.0.1.rc1) - rails-dom-testing (2.0.3) - activesupport (>= 4.2.0) + rails-dom-testing (2.1.1) + activesupport (>= 5.0.0) + minitest nokogiri (>= 1.6) - rails-html-sanitizer (1.5.0) - loofah (~> 2.19, >= 2.19.1) + rails-html-sanitizer (1.6.0) + loofah (~> 2.21) + nokogiri (~> 1.14) rails_best_practices (1.22.1) activesupport code_analyzer (>= 0.5.2) @@ -623,7 +625,7 @@ GEM websocket-extensions (0.1.5) xpath (3.2.0) nokogiri (~> 1.8) - zeitwerk (2.6.7) + zeitwerk (2.6.10) PLATFORMS ruby @@ -689,7 +691,7 @@ DEPENDENCIES tzinfo-data web-console webdrivers - webpacker (~> 4) + webpacker (< 5) RUBY VERSION ruby 3.2.2p53 From d029e4636889d33a534da30f530cfc2000f26d78 Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Tue, 1 Aug 2023 13:39:30 -0700 Subject: [PATCH 11/54] bundle update rails to 6.1.7.4 --- Gemfile | 2 +- Gemfile.lock | 134 ++++++++++++++++++++++++++++----------------------- 2 files changed, 75 insertions(+), 61 deletions(-) diff --git a/Gemfile b/Gemfile index 1a00302a..965273cd 100644 --- a/Gemfile +++ b/Gemfile @@ -6,7 +6,7 @@ git_source(:github) { |repo| "https://github.com/#{repo}.git" } ruby "3.2.2" -gem "rails", "6.1.6" +gem "rails", "6.1.7.4" # Production app server gem "puma", "~> 5" diff --git a/Gemfile.lock b/Gemfile.lock index 9e44c909..16f8b4ac 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -24,62 +24,62 @@ GIT GEM remote: https://rubygems.org/ specs: - actioncable (6.1.6) - actionpack (= 6.1.6) - activesupport (= 6.1.6) + actioncable (6.1.7.4) + actionpack (= 6.1.7.4) + activesupport (= 6.1.7.4) nio4r (~> 2.0) websocket-driver (>= 0.6.1) - actionmailbox (6.1.6) - actionpack (= 6.1.6) - activejob (= 6.1.6) - activerecord (= 6.1.6) - activestorage (= 6.1.6) - activesupport (= 6.1.6) + actionmailbox (6.1.7.4) + actionpack (= 6.1.7.4) + activejob (= 6.1.7.4) + activerecord (= 6.1.7.4) + activestorage (= 6.1.7.4) + activesupport (= 6.1.7.4) mail (>= 2.7.1) - actionmailer (6.1.6) - actionpack (= 6.1.6) - actionview (= 6.1.6) - activejob (= 6.1.6) - activesupport (= 6.1.6) + actionmailer (6.1.7.4) + actionpack (= 6.1.7.4) + actionview (= 6.1.7.4) + activejob (= 6.1.7.4) + activesupport (= 6.1.7.4) mail (~> 2.5, >= 2.5.4) rails-dom-testing (~> 2.0) - actionpack (6.1.6) - actionview (= 6.1.6) - activesupport (= 6.1.6) + actionpack (6.1.7.4) + actionview (= 6.1.7.4) + activesupport (= 6.1.7.4) rack (~> 2.0, >= 2.0.9) rack-test (>= 0.6.3) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.0, >= 1.2.0) - actiontext (6.1.6) - actionpack (= 6.1.6) - activerecord (= 6.1.6) - activestorage (= 6.1.6) - activesupport (= 6.1.6) + actiontext (6.1.7.4) + actionpack (= 6.1.7.4) + activerecord (= 6.1.7.4) + activestorage (= 6.1.7.4) + activesupport (= 6.1.7.4) nokogiri (>= 1.8.5) - actionview (6.1.6) - activesupport (= 6.1.6) + actionview (6.1.7.4) + activesupport (= 6.1.7.4) builder (~> 3.1) erubi (~> 1.4) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.1, >= 1.2.0) - activejob (6.1.6) - activesupport (= 6.1.6) + activejob (6.1.7.4) + activesupport (= 6.1.7.4) globalid (>= 0.3.6) - activemodel (6.1.6) - activesupport (= 6.1.6) - activerecord (6.1.6) - activemodel (= 6.1.6) - activesupport (= 6.1.6) + activemodel (6.1.7.4) + activesupport (= 6.1.7.4) + activerecord (6.1.7.4) + activemodel (= 6.1.7.4) + activesupport (= 6.1.7.4) activerecord-import (1.4.0) activerecord (>= 4.2) - activestorage (6.1.6) - actionpack (= 6.1.6) - activejob (= 6.1.6) - activerecord (= 6.1.6) - activesupport (= 6.1.6) + activestorage (6.1.7.4) + actionpack (= 6.1.7.4) + activejob (= 6.1.7.4) + activerecord (= 6.1.7.4) + activesupport (= 6.1.7.4) marcel (~> 1.0) mini_mime (>= 1.1.0) - activesupport (6.1.6) + activesupport (6.1.7.4) concurrent-ruby (~> 1.0, >= 1.0.2) i18n (>= 1.6, < 2) minitest (>= 5.1) @@ -200,6 +200,7 @@ GEM activerecord (>= 5.a) database_cleaner-core (~> 2.0.0) database_cleaner-core (2.0.1) + date (3.3.3) descendants_tracker (0.0.4) thread_safe (~> 0.3, >= 0.3.1) diff-lcs (1.5.0) @@ -254,7 +255,7 @@ GEM gitlab (4.19.0) httparty (~> 0.20) terminal-table (>= 1.5.1) - globalid (1.0.1) + globalid (1.1.0) activesupport (>= 5.0) guard (2.18.0) formatador (>= 0.2.4) @@ -319,8 +320,11 @@ GEM crass (~> 1.0.2) nokogiri (>= 1.12.0) lumberjack (1.2.8) - mail (2.7.1) + mail (2.8.1) mini_mime (>= 0.1.1) + net-imap + net-pop + net-smtp main (6.2.3) arrayfields (~> 4.7, >= 4.7.4) chronic (~> 0.6, >= 0.6.2) @@ -358,7 +362,16 @@ GEM multi_xml (0.6.0) multipart-post (2.3.0) nenv (0.3.0) - nio4r (2.5.8) + net-imap (0.3.7) + date + net-protocol + net-pop (0.1.2) + net-protocol + net-protocol (0.2.1) + timeout + net-smtp (0.3.3) + net-protocol + nio4r (2.5.9) nokogiri (1.15.3) mini_portile2 (~> 2.8.2) racc (~> 1.4) @@ -434,20 +447,20 @@ GEM rack rack-test (2.1.0) rack (>= 1.3) - rails (6.1.6) - actioncable (= 6.1.6) - actionmailbox (= 6.1.6) - actionmailer (= 6.1.6) - actionpack (= 6.1.6) - actiontext (= 6.1.6) - actionview (= 6.1.6) - activejob (= 6.1.6) - activemodel (= 6.1.6) - activerecord (= 6.1.6) - activestorage (= 6.1.6) - activesupport (= 6.1.6) + rails (6.1.7.4) + actioncable (= 6.1.7.4) + actionmailbox (= 6.1.7.4) + actionmailer (= 6.1.7.4) + actionpack (= 6.1.7.4) + actiontext (= 6.1.7.4) + actionview (= 6.1.7.4) + activejob (= 6.1.7.4) + activemodel (= 6.1.7.4) + activerecord (= 6.1.7.4) + activestorage (= 6.1.7.4) + activesupport (= 6.1.7.4) bundler (>= 1.15.0) - railties (= 6.1.6) + railties (= 6.1.7.4) sprockets-rails (>= 2.0.0) rails-controller-testing (1.0.5) actionpack (>= 5.0.1.rc1) @@ -468,9 +481,9 @@ GEM json require_all (~> 3.0) ruby-progressbar - railties (6.1.6) - actionpack (= 6.1.6) - activesupport (= 6.1.6) + railties (6.1.7.4) + actionpack (= 6.1.7.4) + activesupport (= 6.1.7.4) method_source rake (>= 12.2) thor (~> 1.0) @@ -580,9 +593,9 @@ GEM hashie version_gem (~> 1.1, >= 1.1.1) spring (4.0.0) - sprockets (4.0.3) + sprockets (4.2.0) concurrent-ruby (~> 1.0) - rack (> 1, < 3) + rack (>= 2.2.4, < 4) sprockets-rails (3.4.2) actionpack (>= 5.2) activesupport (>= 5.2) @@ -596,6 +609,7 @@ GEM thor (1.2.2) thread_safe (0.3.6) tilt (2.0.10) + timeout (0.4.0) tzinfo (2.0.6) concurrent-ruby (~> 1.0) tzinfo-data (1.2021.5) @@ -620,7 +634,7 @@ GEM rack-proxy (>= 0.6.1) railties (>= 4.2) webrick (1.7.0) - websocket-driver (0.7.5) + websocket-driver (0.7.6) websocket-extensions (>= 0.1.0) websocket-extensions (0.1.5) xpath (3.2.0) @@ -670,7 +684,7 @@ DEPENDENCIES pry-byebug (~> 3.9) puma (~> 5) rack-mini-profiler (~> 2.0) - rails (= 6.1.6) + rails (= 6.1.7.4) rails-controller-testing rspec-rails rubocop From 429b53ef747a17e56cc26b6a5892f9c5e0f85dee Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Tue, 1 Aug 2023 13:48:17 -0700 Subject: [PATCH 12/54] update webpacker... --- Gemfile | 2 +- Gemfile.lock | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/Gemfile b/Gemfile index 965273cd..27d38901 100644 --- a/Gemfile +++ b/Gemfile @@ -13,7 +13,7 @@ gem "puma", "~> 5" gem "pg", "~> 1.5" # Front-end Assets -gem "webpacker", "< 5" +gem "webpacker" # Reduces boot times through caching; required in config/boot.rb gem "bootsnap", ">= 1.4.4", require: false diff --git a/Gemfile.lock b/Gemfile.lock index 16f8b4ac..e5275a48 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -562,6 +562,7 @@ GEM childprocess (>= 0.5, < 5.0) rexml (~> 3.2, >= 3.2.5) rubyzip (>= 1.2.2) + semantic_range (3.0.0) sentry-rails (4.8.3) railties (>= 5.0) sentry-ruby-core (~> 4.8.3) @@ -629,10 +630,11 @@ GEM nokogiri (~> 1.6) rubyzip (>= 1.3.0) selenium-webdriver (~> 4.0) - webpacker (4.3.0) - activesupport (>= 4.2) + webpacker (5.4.4) + activesupport (>= 5.2) rack-proxy (>= 0.6.1) - railties (>= 4.2) + railties (>= 5.2) + semantic_range (>= 2.3.0) webrick (1.7.0) websocket-driver (0.7.6) websocket-extensions (>= 0.1.0) @@ -705,7 +707,7 @@ DEPENDENCIES tzinfo-data web-console webdrivers - webpacker (< 5) + webpacker RUBY VERSION ruby 3.2.2p53 From 3f4347ec32ecba2b04f3e05fe5b53ce7f59a4669 Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Tue, 1 Aug 2023 13:49:19 -0700 Subject: [PATCH 13/54] Run rubocop -a --- app/models/email_template.rb | 3 +-- lib/tasks/split_teacher_pending_state.rake | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/app/models/email_template.rb b/app/models/email_template.rb index a58abcf2..c00b73f6 100644 --- a/app/models/email_template.rb +++ b/app/models/email_template.rb @@ -25,13 +25,12 @@ class EmailTemplate < ApplicationRecord before_destroy :prevent_deleting_required_emails def self.templates_with_custom_reason - end private def prevent_deleting_required_emails if self.required? - errors.add('Cannot delete a required email template') + errors.add("Cannot delete a required email template") throw :abort end end diff --git a/lib/tasks/split_teacher_pending_state.rake b/lib/tasks/split_teacher_pending_state.rake index 997c7f70..aae526e1 100644 --- a/lib/tasks/split_teacher_pending_state.rake +++ b/lib/tasks/split_teacher_pending_state.rake @@ -9,7 +9,7 @@ namespace :split_teacher_pending_state do puts "Nil -> not_reviewed" puts Teacher.where(application_status: nil).update_all(application_status: "not_reviewed") puts "empty string -> not_reviewed" - puts Teacher.where(application_status: '').update_all(application_status: "not_reviewed") + puts Teacher.where(application_status: "").update_all(application_status: "not_reviewed") puts "Done." end end From d339e1827d96113fe958059c5b0efb721bfcf199 Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Tue, 1 Aug 2023 13:49:43 -0700 Subject: [PATCH 14/54] rubocop -A delinting --- lib/fixup_required_emails.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/fixup_required_emails.rb b/lib/fixup_required_emails.rb index 2d41b385..222f5850 100644 --- a/lib/fixup_required_emails.rb +++ b/lib/fixup_required_emails.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + EmailTemplate.where( title: ["Welcome Email", "Form Submission", From 8031e5b39979e6a92f873aa58986ed3fb30028c0 Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Tue, 1 Aug 2023 14:10:37 -0700 Subject: [PATCH 15/54] Update pry / pry-byebug deps --- Gemfile.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index e5275a48..1f20b7dd 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -428,12 +428,12 @@ GEM pronto-rubocop (0.11.1) pronto (~> 0.11.0) rubocop (>= 0.63.1, < 2.0) - pry (0.13.1) + pry (0.14.2) coderay (~> 1.1) method_source (~> 1.0) - pry-byebug (3.9.0) + pry-byebug (3.10.1) byebug (~> 11.0) - pry (~> 0.13.0) + pry (>= 0.13, < 0.15) public_suffix (5.0.3) puma (5.6.4) nio4r (~> 2.0) From 52c6839184a03f625655af30df98abf05ed8996d Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Tue, 1 Aug 2023 14:17:49 -0700 Subject: [PATCH 16/54] Bundle update simplecov metric_fu --- Gemfile | 2 +- Gemfile.lock | 56 +++++++++++++++++++++++--------------------- spec/rails_helper.rb | 7 +++--- 3 files changed, 33 insertions(+), 32 deletions(-) diff --git a/Gemfile b/Gemfile index 27d38901..9f12c809 100644 --- a/Gemfile +++ b/Gemfile @@ -92,7 +92,7 @@ end # setup Cucumber, RSpec, Guard support group :test do gem "rspec-rails" - gem "simplecov", "< 0.18", require: false + gem "simplecov", require: false gem "simplecov-json", require: false gem "simplecov-console", require: false gem "simplecov-csv", require: false diff --git a/Gemfile.lock b/Gemfile.lock index 1f20b7dd..a4062b14 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -148,7 +148,7 @@ GEM main ruby_parser (~> 3.0) sexp_processor (~> 4.1) - code_analyzer (0.5.2) + code_analyzer (0.5.5) sexp_processor code_metrics (0.1.3) coderay (1.1.3) @@ -242,12 +242,12 @@ GEM ffi (1.15.4) figaro (1.2.0) thor (>= 0.14.0, < 2) - flay (2.12.1) - erubis (~> 2.7.0) + flay (2.13.1) + erubi (~> 1.10) path_expander (~> 1.0) ruby_parser (~> 3.0) sexp_processor (~> 4.0) - flog (4.6.4) + flog (4.7.0) path_expander (~> 1.0) ruby_parser (~> 3.1, > 3.1.0) sexp_processor (~> 4.8) @@ -300,11 +300,11 @@ GEM mini_magick (>= 4.9.5, < 5) ruby-vips (>= 2.0.17, < 3) jmespath (1.6.1) - json (2.6.1) + json (2.6.3) jwt (2.7.0) kwalify (0.7.2) - launchy (2.5.0) - addressable (~> 2.7) + launchy (2.5.2) + addressable (~> 2.8) letter_opener (1.7.0) launchy (~> 2.2) letter_opener_web (2.0.0) @@ -325,11 +325,11 @@ GEM net-imap net-pop net-smtp - main (6.2.3) - arrayfields (~> 4.7, >= 4.7.4) - chronic (~> 0.6, >= 0.6.2) - fattr (~> 2.2, >= 2.2.0) - map (~> 6.1, >= 6.1.0) + main (6.3.0) + arrayfields (~> 4.9, >= 4.9.2) + chronic (~> 0.10, >= 0.10.2) + fattr (~> 2.4, >= 2.4.0) + map (~> 6.6, >= 6.6.0) map (6.6.0) marcel (1.0.2) matrix (0.4.2) @@ -406,10 +406,11 @@ GEM omniauth-rails_csrf_protection (1.0.1) actionpack (>= 4.2) omniauth (~> 2.0) - parallel (1.21.0) - parser (3.0.3.2) + parallel (1.23.0) + parser (3.2.2.3) ast (~> 2.4.1) - path_expander (1.1.0) + racc + path_expander (1.1.1) pg (1.5.3) pronto (0.11.1) gitlab (>= 4.4.0, < 5.0) @@ -473,9 +474,9 @@ GEM rails-html-sanitizer (1.6.0) loofah (~> 2.21) nokogiri (~> 1.14) - rails_best_practices (1.22.1) + rails_best_practices (1.23.2) activesupport - code_analyzer (>= 0.5.2) + code_analyzer (~> 0.5.5) erubis i18n json @@ -493,9 +494,9 @@ GEM rb-inotify (0.10.1) ffi (~> 1.0) redcard (1.1.0) - reek (6.0.6) + reek (6.1.4) kwalify (~> 0.7.0) - parser (~> 3.0.0) + parser (~> 3.2.0) rainbow (>= 2.0, < 4.0) regexp_parser (2.2.0) require_all (3.0.0) @@ -546,11 +547,11 @@ GEM rubocop (>= 1.7.0, < 2.0) rubocop-rspec (2.7.0) rubocop (~> 1.19) - ruby-progressbar (1.11.0) + ruby-progressbar (1.13.0) ruby-vips (2.1.4) ffi (~> 1.12) ruby2_keywords (0.0.5) - ruby_parser (3.18.1) + ruby_parser (3.20.3) sexp_processor (~> 4.16) rubyzip (2.3.2) rugged (1.6.3) @@ -573,22 +574,23 @@ GEM sentry-ruby-core (4.8.3) concurrent-ruby faraday - sexp_processor (4.16.0) + sexp_processor (4.17.0) shellany (0.0.1) - simplecov (0.17.1) + simplecov (0.22.0) docile (~> 1.1) - json (>= 1.8, < 3) - simplecov-html (~> 0.10.0) + simplecov-html (~> 0.11) + simplecov_json_formatter (~> 0.1) simplecov-console (0.9.1) ansi simplecov terminal-table simplecov-csv (0.1.3) simplecov - simplecov-html (0.10.2) + simplecov-html (0.12.3) simplecov-json (0.2.3) json simplecov + simplecov_json_formatter (0.1.4) smarter_csv (1.4.2) snaky_hash (2.0.1) hashie @@ -698,7 +700,7 @@ DEPENDENCIES selenium-webdriver sentry-rails sentry-ruby - simplecov (< 0.18) + simplecov simplecov-console simplecov-csv simplecov-json diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 77184515..000ba4bf 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -1,12 +1,11 @@ # frozen_string_literal: true -# This file is copied to spec/ when you run 'rails generate rspec:install' -require "simplecov" -SimpleCov.start "rails" - require "spec_helper" ENV["RAILS_ENV"] ||= "test" +require "simplecov" +SimpleCov.start "rails" + require File.expand_path("../config/environment", __dir__) # Prevent database truncation if the environment is production From 22f93fe58fc55b5ef809383f5fff2f349c0318f0 Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Tue, 1 Aug 2023 14:22:50 -0700 Subject: [PATCH 17/54] No more metric_fu, things run again --- Gemfile | 2 +- Gemfile.lock | 60 ---------------------------------------------------- 2 files changed, 1 insertion(+), 61 deletions(-) diff --git a/Gemfile b/Gemfile index 9f12c809..165b0afe 100644 --- a/Gemfile +++ b/Gemfile @@ -99,7 +99,7 @@ group :test do gem "cucumber-rails", require: false gem "database_cleaner" gem "factory_bot_rails" - gem "metric_fu" + # gem "metric_fu" # removed until ruby 3.x compat is fixed. gem "selenium-webdriver" gem "webdrivers" diff --git a/Gemfile.lock b/Gemfile.lock index a4062b14..b012183d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -91,7 +91,6 @@ GEM activerecord (>= 3.2, < 7.0) rake (>= 10.4, < 14.0) ansi (1.5.0) - arrayfields (4.9.2) ast (2.4.2) aws-eventstream (1.2.0) aws-partitions (1.573.0) @@ -129,8 +128,6 @@ GEM msgpack (~> 1.0) builder (3.2.4) byebug (11.1.3) - cane (2.6.2) - parallel capybara (3.36.0) addressable matrix @@ -141,16 +138,6 @@ GEM regexp_parser (>= 1.5, < 3.0) xpath (~> 3.2) childprocess (4.1.0) - chronic (0.10.2) - churn (1.0.8) - chronic (>= 0.2.3) - hirb - main - ruby_parser (~> 3.0) - sexp_processor (~> 4.1) - code_analyzer (0.5.5) - sexp_processor - code_metrics (0.1.3) coderay (1.1.3) coercible (1.0.0) descendants_tracker (~> 0.0.1) @@ -207,7 +194,6 @@ GEM docile (1.4.0) dumb_delegator (1.0.0) erubi (1.12.0) - erubis (2.7.0) factory_bot (6.2.0) activesupport (>= 5.0.0) factory_bot_rails (6.2.0) @@ -238,7 +224,6 @@ GEM faraday-patron (1.0.0) faraday-rack (1.0.0) faraday-retry (1.0.3) - fattr (2.4.0) ffi (1.15.4) figaro (1.2.0) thor (>= 0.14.0, < 2) @@ -247,10 +232,6 @@ GEM path_expander (~> 1.0) ruby_parser (~> 3.0) sexp_processor (~> 4.0) - flog (4.7.0) - path_expander (~> 1.0) - ruby_parser (~> 3.1, > 3.1.0) - sexp_processor (~> 4.8) formatador (0.3.0) gitlab (4.19.0) httparty (~> 0.20) @@ -289,7 +270,6 @@ GEM rubocop (>= 0.50.0) sysexits (~> 1.1) hashie (5.0.0) - hirb (0.7.3) httparty (0.21.0) mini_mime (>= 1.0.0) multi_xml (>= 0.5.2) @@ -302,7 +282,6 @@ GEM jmespath (1.6.1) json (2.6.3) jwt (2.7.0) - kwalify (0.7.2) launchy (2.5.2) addressable (~> 2.8) letter_opener (1.7.0) @@ -325,30 +304,9 @@ GEM net-imap net-pop net-smtp - main (6.3.0) - arrayfields (~> 4.9, >= 4.9.2) - chronic (~> 0.10, >= 0.10.2) - fattr (~> 2.4, >= 2.4.0) - map (~> 6.6, >= 6.6.0) - map (6.6.0) marcel (1.0.2) matrix (0.4.2) method_source (1.0.0) - metric_fu (4.13.0) - cane (~> 2.5, >= 2.5.2) - churn (~> 1.0, >= 1.0.2) - code_metrics (~> 0.1) - coderay - flay (~> 2.1, >= 2.0.1) - flog (~> 4.1, >= 4.1.1) - launchy (~> 2.0) - metric_fu-Saikuro (~> 1.1, >= 1.1.3) - multi_json - rails_best_practices (~> 1.14, >= 1.14.3) - redcard - reek (>= 5.4.0) - roodi (~> 5.0.0) - metric_fu-Saikuro (1.1.3) mime-types (3.4.1) mime-types-data (~> 3.2015) mime-types-data (3.2022.0105) @@ -357,7 +315,6 @@ GEM mini_portile2 (2.8.4) minitest (5.19.0) msgpack (1.4.2) - multi_json (1.15.0) multi_test (0.1.2) multi_xml (0.6.0) multipart-post (2.3.0) @@ -474,14 +431,6 @@ GEM rails-html-sanitizer (1.6.0) loofah (~> 2.21) nokogiri (~> 1.14) - rails_best_practices (1.23.2) - activesupport - code_analyzer (~> 0.5.5) - erubis - i18n - json - require_all (~> 3.0) - ruby-progressbar railties (6.1.7.4) actionpack (= 6.1.7.4) activesupport (= 6.1.7.4) @@ -493,16 +442,8 @@ GEM rb-fsevent (0.11.0) rb-inotify (0.10.1) ffi (~> 1.0) - redcard (1.1.0) - reek (6.1.4) - kwalify (~> 0.7.0) - parser (~> 3.2.0) - rainbow (>= 2.0, < 4.0) regexp_parser (2.2.0) - require_all (3.0.0) rexml (3.2.6) - roodi (5.0.0) - ruby_parser (~> 3.2, >= 3.2.2) rspec (3.10.0) rspec-core (~> 3.10.0) rspec-expectations (~> 3.10.0) @@ -671,7 +612,6 @@ DEPENDENCIES letter_opener_web (~> 2) liquid listen (~> 3.3) - metric_fu omniauth omniauth-clever! omniauth-discourse! From ef6b83f9eaf2451f49ba3cafe1e5646320a5f9d2 Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Tue, 1 Aug 2023 14:53:21 -0700 Subject: [PATCH 18/54] Cleanup email templates index --- app/controllers/email_templates_controller.rb | 3 ++- app/helpers/application_helper.rb | 6 ++++++ app/models/email_template.rb | 11 +++++++++-- app/views/email_templates/index.html.erb | 14 +++++++++----- 4 files changed, 26 insertions(+), 8 deletions(-) diff --git a/app/controllers/email_templates_controller.rb b/app/controllers/email_templates_controller.rb index 6bce23fb..901a1a4f 100644 --- a/app/controllers/email_templates_controller.rb +++ b/app/controllers/email_templates_controller.rb @@ -2,8 +2,9 @@ class EmailTemplatesController < ApplicationController before_action :require_admin + def index - @all_templates = EmailTemplate.all + load_ordered_email_templates end def edit diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 6bb7499f..f3135311 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -20,4 +20,10 @@ def admin_nav_links "Email Templates": email_templates_path } end + + def check_or_x(bool) + return "✔️" if bool + + "❌" + end end diff --git a/app/models/email_template.rb b/app/models/email_template.rb index c00b73f6..8e47f870 100644 --- a/app/models/email_template.rb +++ b/app/models/email_template.rb @@ -24,13 +24,20 @@ class EmailTemplate < ApplicationRecord before_destroy :prevent_deleting_required_emails - def self.templates_with_custom_reason + def test_data + @@test_data ||= rand(10) + end + + def accepts_custom_reason? + # search for 'reason' in a liquid template + self.body.match?(/{{\s*reason/) end private + def prevent_deleting_required_emails if self.required? - errors.add("Cannot delete a required email template") + errors.add('Cannot delete a required email template') throw :abort end end diff --git a/app/views/email_templates/index.html.erb b/app/views/email_templates/index.html.erb index b5c2027e..d0868eb2 100644 --- a/app/views/email_templates/index.html.erb +++ b/app/views/email_templates/index.html.erb @@ -6,17 +6,21 @@
Purpose
PurposeActions
<%= link_to(template.title, edit_email_template_path(template)) %> + + <%= link_to("Edit", edit_email_template_path(template), class: "btn btn-info") %> + <%= link_to("❌", email_template_path(template), method: "delete", class: "btn btn-outline-danger", data: {confirm: "Are you sure?"}) %> + +
<%= link_to("Edit", edit_email_template_path(template), class: "btn btn-info") %> - <%= link_to("❌", email_template_path(template), method: "delete", class: "btn btn-outline-danger", data: {confirm: "Are you sure?"}) %> + <%= link_to("❌", email_template_path(template), method: "delete", class: "btn btn-outline-danger", data: {confirm: "Are you sure?"}) unless template.required? %>
+ + - <% @all_templates.each do |template| %> + <% @ordered_email_templates.each do |template| %> + + <% end %> From 99d2884e61c1dfd44b0e6a4436f27ed4231b6448 Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Tue, 1 Aug 2023 19:01:13 -0700 Subject: [PATCH 19/54] delint --- app/models/email_template.rb | 7 +------ spec/rails_helper.rb | 1 + 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/app/models/email_template.rb b/app/models/email_template.rb index 8e47f870..cb483991 100644 --- a/app/models/email_template.rb +++ b/app/models/email_template.rb @@ -24,20 +24,15 @@ class EmailTemplate < ApplicationRecord before_destroy :prevent_deleting_required_emails - def test_data - @@test_data ||= rand(10) - end - def accepts_custom_reason? # search for 'reason' in a liquid template self.body.match?(/{{\s*reason/) end private - def prevent_deleting_required_emails if self.required? - errors.add('Cannot delete a required email template') + errors.add("Cannot delete a required email template") throw :abort end end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 000ba4bf..71dadf38 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +# This file is copied to spec/ when you run 'rails generate rspec:install' require "spec_helper" ENV["RAILS_ENV"] ||= "test" From fb74670856723c7893b8fc129d5c9b9359e0b40a Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Tue, 1 Aug 2023 19:19:57 -0700 Subject: [PATCH 20/54] update rspec rails --- Gemfile.lock | 8 ++++---- spec/controllers/pages_controller_spec.rb | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index b012183d..8d80634e 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -448,15 +448,15 @@ GEM rspec-core (~> 3.10.0) rspec-expectations (~> 3.10.0) rspec-mocks (~> 3.10.0) - rspec-core (3.10.1) + rspec-core (3.10.2) rspec-support (~> 3.10.0) - rspec-expectations (3.10.1) + rspec-expectations (3.10.2) diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.10.0) - rspec-mocks (3.10.2) + rspec-mocks (3.10.3) diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.10.0) - rspec-rails (5.0.2) + rspec-rails (5.1.2) actionpack (>= 5.2) activesupport (>= 5.2) railties (>= 5.2) diff --git a/spec/controllers/pages_controller_spec.rb b/spec/controllers/pages_controller_spec.rb index 2dab2112..bd111a3f 100644 --- a/spec/controllers/pages_controller_spec.rb +++ b/spec/controllers/pages_controller_spec.rb @@ -38,7 +38,7 @@ end describe "#create" do - it "allows admin to create" do + it "allows admin to create", :focus do allow_any_instance_of(ApplicationController).to receive(:require_admin).and_return(true) expect(Page.find_by(url_slug: @pages_slug)).to be_nil post :create, { From 62b455a1dbab44abe945b47468170268824831af Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Tue, 1 Aug 2023 21:00:37 -0700 Subject: [PATCH 21/54] sigh, slow progress on fixing specs with ruby 3+updates --- Gemfile | 2 - Gemfile.lock | 5 -- features/pages_admin.feature | 70 +++++++++++------------ features/pages_permissions.feature | 26 ++++----- features/step_definitions/page_steps.rb | 2 +- spec/controllers/pages_controller_spec.rb | 37 ++++++------ spec/fixtures/pages.yml | 17 +++++- 7 files changed, 81 insertions(+), 78 deletions(-) diff --git a/Gemfile b/Gemfile index 165b0afe..83a1fc59 100644 --- a/Gemfile +++ b/Gemfile @@ -106,6 +106,4 @@ group :test do # Accessibility Testing gem "axe-core-rspec" gem "axe-core-cucumber" - - gem "rails-controller-testing" end diff --git a/Gemfile.lock b/Gemfile.lock index 8d80634e..8f3fd482 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -420,10 +420,6 @@ GEM bundler (>= 1.15.0) railties (= 6.1.7.4) sprockets-rails (>= 2.0.0) - rails-controller-testing (1.0.5) - actionpack (>= 5.0.1.rc1) - actionview (>= 5.0.1.rc1) - activesupport (>= 5.0.1.rc1) rails-dom-testing (2.1.1) activesupport (>= 5.0.0) minitest @@ -629,7 +625,6 @@ DEPENDENCIES puma (~> 5) rack-mini-profiler (~> 2.0) rails (= 6.1.7.4) - rails-controller-testing rspec-rails rubocop rubocop-faker diff --git a/features/pages_admin.feature b/features/pages_admin.feature index 04df0ab8..216e8da7 100644 --- a/features/pages_admin.feature +++ b/features/pages_admin.feature @@ -33,7 +33,7 @@ Scenario: Pressing "New Page" button should take user to new page form Scenario: Successfully creating a new page redirects to that page Given I am on the new pages page And I fill in "page_title" with "Test Title" - And I fill in "page_url_slug" with "test_slug" + And I fill in "page_url_slug" with "basic_slug" And I fill in "page_category" with "Test Category" And I fill in the page HTML content with "This is a test" And I choose "inlineRadioAdmin" @@ -45,7 +45,7 @@ Scenario: Successfully creating a new page redirects to that page Scenario: Creating a page without a title fails Given I am on the new pages page - And I fill in "page_url_slug" with "test_slug" + And I fill in "page_url_slug" with "basic_slug" And I fill in "page_category" with "Test Category" And I choose "inlineRadioAdmin" And I press "Submit" @@ -64,7 +64,7 @@ Scenario: Creating a page without a slug fails Scenario: Creating a page without choosing permissions fails Given I am on the new pages page And I fill in "page_title" with "Test Title" - And I fill in "page_url_slug" with "test_slug" + And I fill in "page_url_slug" with "basic_slug" And I fill in "page_category" with "Test Category" And I fill in the page HTML content with "This is a test" And I press "Submit" @@ -74,7 +74,7 @@ Scenario: Creating a page without choosing permissions fails Scenario: I create a new page and I can see it on the index page Given I am on the new pages page And I fill in "page_title" with "Test Title" - And I fill in "page_url_slug" with "test_slug" + And I fill in "page_url_slug" with "basic_slug" And I fill in "page_category" with "Test Category" And I fill in the page HTML content with "This is a test" And I choose "inlineRadioAdmin" @@ -88,14 +88,14 @@ Scenario: I create a new page and I can see it on the index page Scenario: Can create a new page with the same title as a page that already exists Given I am on the new pages page And I fill in "page_title" with "Test Title" - And I fill in "page_url_slug" with "test_slug" + And I fill in "page_url_slug" with "basic_slug" And I fill in the page HTML content with "This is a test" And I choose "inlineRadioAdmin" And I press "Submit" And I follow "Pages" And I press "New Page" And I fill in "page_title" with "Test Title" - And I fill in "page_url_slug" with "test_slug_2" + And I fill in "page_url_slug" with "basic_slug_2" And I choose "inlineRadioAdmin" And I fill in the page HTML content with "This is a test" And I press "Submit" @@ -104,14 +104,14 @@ Scenario: Can create a new page with the same title as a page that already exist Scenario: Can't create a page with a slug that already exists Given I am on the new pages page And I fill in "page_title" with "Test Title" - And I fill in "page_url_slug" with "test_slug" + And I fill in "page_url_slug" with "basic_slug" And I fill in the page HTML content with "This is a test" And I choose "inlineRadioAdmin" And I press "Submit" And I follow "Pages" And I press "New Page" And I fill in "page_title" with "Test Title" - And I fill in "page_url_slug" with "test_slug" + And I fill in "page_url_slug" with "basic_slug" And I choose "inlineRadioAdmin" And I press "Submit" Then I should see "Create Test Title" @@ -120,7 +120,7 @@ Scenario: Can't create a page with a slug that already exists Scenario: Can't create a default admin page Given I am on the new pages page And I fill in "page_title" with "Test Title" - And I fill in "page_url_slug" with "test_slug" + And I fill in "page_url_slug" with "basic_slug" And I fill in "page_category" with "Test Category" And I check "page_default" checkbox And I fill in the page HTML content with "This is a test" @@ -132,51 +132,51 @@ Scenario: Can't create a default admin page Scenario: Attempting to create page with taken slug doesn't delete form input Given I am on the new pages page And I fill in "page_title" with "Test Title" - And I fill in "page_url_slug" with "test_slug" + And I fill in "page_url_slug" with "basic_slug" And I fill in the page HTML content with "Don't see this" And I choose "inlineRadioAdmin" And I press "Submit" And I follow "Pages" And I press "New Page" And I fill in "page_title" with "Test Title" - And I fill in "page_url_slug" with "test_slug" + And I fill in "page_url_slug" with "basic_slug" And I fill in the page HTML content with "This is a test" And I choose "inlineRadioAdmin" And I press "Submit" Then I should see "Create Test Title" And I should see "URL slug has already been taken" And the "page_title" field should contain "Test Title" - And the "page_url_slug" field should contain "test_slug" + And the "page_url_slug" field should contain "basic_slug" And I should see the page HTML content containing "This is a test" And I should not see "Don't see this" Scenario: I can delete pages Given I am on the new pages page And I fill in "page_title" with "Test Title" - And I fill in "page_url_slug" with "test_slug" + And I fill in "page_url_slug" with "basic_slug" And I fill in the page HTML content with "This is a test" And I choose "inlineRadioAdmin" And I press "Submit" And I follow "Pages" - When I press the delete button for "test_slug" - Then I should not see "test_slug" + When I press the delete button for "basic_slug" + Then I should not see "basic_slug" Scenario: Can create pages with any selection for permissions Given I am on the new pages page And I fill in "page_title" with "Admin Permissions" - And I fill in "page_url_slug" with "test_slug" + And I fill in "page_url_slug" with "basic_slug" And I choose "inlineRadioAdmin" And I press "Submit" And I follow "Pages" And I press "New Page" And I fill in "page_title" with "Verified Teacher Permissions" - And I fill in "page_url_slug" with "test_slug_2" + And I fill in "page_url_slug" with "basic_slug_2" And I choose "inlineRadioTeacher" And I press "Submit" And I follow "Pages" And I press "New Page" And I fill in "page_title" with "Public Permissions" - And I fill in "page_url_slug" with "test_slug_3" + And I fill in "page_url_slug" with "basic_slug_3" And I choose "inlineRadioPublic" And I press "Submit" @@ -195,15 +195,15 @@ Scenario: Correctly store user's full name and create date. Scenario: Can edit pages with correct prefilled content in the form. Given I am on the new pages page And I fill in "page_title" with "Test Title" - And I fill in "page_url_slug" with "test_slug" + And I fill in "page_url_slug" with "basic_slug" And I choose "inlineRadioAdmin" And I fill in the page HTML content with "This is a test" And I press "Submit" And I follow "Pages" - And I press the edit button for "test_slug" - Then I should be on the edit pages page for "test_slug" + And I press the edit button for "basic_slug" + Then I should be on the edit pages page for "basic_slug" And the "page_title" field should contain "Test Title" - And the "page_url_slug" field should contain "test_slug" + And the "page_url_slug" field should contain "basic_slug" And I should see the page HTML content containing "This is a test" Then I fill in "page_title" with "New Title" And I fill in "page_url_slug" with "new_slug" @@ -214,18 +214,18 @@ Scenario: Can edit pages with correct prefilled content in the form. And I should see "Public" Then I should be on the pages index And I should not see "Test Title" - And I should not see "test_slug" + And I should not see "basic_slug" Scenario: Can update page even if no changes Given I am on the new pages page And I fill in "page_title" with "Test Title" - And I fill in "page_url_slug" with "test_slug" + And I fill in "page_url_slug" with "basic_slug" And I choose "inlineRadioAdmin" And I fill in the page HTML content with "This is a test" And I press "Submit" And I follow "Pages" - And I press the edit button for "test_slug" - Then I should be on the edit pages page for "test_slug" + And I press the edit button for "basic_slug" + Then I should be on the edit pages page for "basic_slug" And I press "Update" Then I should be on the pages index And I should see "Test Title" @@ -234,27 +234,27 @@ Scenario: Can update page even if no changes Scenario: Attempting to update page with taken slug doesn't delete form input Given I am on the new pages page And I fill in "page_title" with "Test Title" - And I fill in "page_url_slug" with "test_slug" + And I fill in "page_url_slug" with "basic_slug" And I choose "inlineRadioAdmin" And I fill in the page HTML content with "This is a test" And I press "Submit" Given I am on the new pages page And I fill in "page_title" with "Test Title 2" - And I fill in "page_url_slug" with "test_slug_2" + And I fill in "page_url_slug" with "basic_slug_2" And I choose "inlineRadioAdmin" And I fill in the page HTML content with "This is a test" And I press "Submit" Then I follow "Pages" - And I press the edit button for "test_slug_2" - Then I should be on the edit pages page for "test_slug_2" + And I press the edit button for "basic_slug_2" + Then I should be on the edit pages page for "basic_slug_2" And I fill in "page_title" with "New Title" - And I fill in "page_url_slug" with "test_slug" + And I fill in "page_url_slug" with "basic_slug" And I fill in the page HTML content with "New page body." And I press "Update" Then I should see "Update New Title" And I should see "URL slug has already been taken" And the "page_title" field should contain "New Title" - And the "page_url_slug" field should contain "test_slug" + And the "page_url_slug" field should contain "basic_slug" And I should see the page HTML content containing "New page body." Scenario: Clicking radio button text selects that radio button @@ -269,13 +269,13 @@ Scenario: Clicking radio button text selects that radio button Scenario: Cannot update admin page to be default Given I am on the new pages page And I fill in "page_title" with "Test Default Title" - And I fill in "page_url_slug" with "test_slug" + And I fill in "page_url_slug" with "basic_slug" And I choose "inlineRadioAdmin" And I fill in the page HTML content with "This is a test" And I press "Submit" And I follow "Pages" - And I press the edit button for "test_slug" - Then I should be on the edit pages page for "test_slug" + And I press the edit button for "basic_slug" + Then I should be on the edit pages page for "basic_slug" And I check "page_default" checkbox And I press "Update" Then I should see "Update Test Default Title" diff --git a/features/pages_permissions.feature b/features/pages_permissions.feature index 33aaa866..8383898e 100644 --- a/features/pages_permissions.feature +++ b/features/pages_permissions.feature @@ -10,10 +10,10 @@ Background: Has admin and teacher in DB along with pages of each permission type | Todd | Teacher | false | testteacher@berkeley.edu | Validated | Given the following pages exist: | url_slug | title | html | viewer_permissions | category | default | - | test_slug_admin | Test Admin Page | Test admin body. | Admin | A | | - | test_slug_verified_teacher | Test Teacher Page | Test teacher body. | Verified Teacher | | | - | test_slug_public | Test Public Page | Test public body. | Public | A | | - | test_slug_public_default | Default Public Page| Test default body.| Public | | true | + | basic_slug_admin | Test Admin Page | Test admin body. | Admin | A | | + | basic_slug_verified_teacher | Test Teacher Page | Test teacher body. | Verified Teacher | | | + | basic_slug_public | Test Public Page | Test public body. | Public | A | | + | basic_slug_public_default | Default Public Page| Test default body.| Public | | true | Scenario: Admins can see everything Given I am on the BJC home page @@ -67,19 +67,19 @@ Scenario: Admin can access all pages Then I follow "Pages" Then I should be on the pages index And I follow the page link "Test Public Page" - Then I should be on the page for slug "test_slug_public" + Then I should be on the page for slug "basic_slug_public" And I should see "Test Public Page" And I should see "Test public body." Then I follow "Pages" Then I should be on the pages index And I follow the page link "Test Teacher Page" - Then I should be on the page for slug "test_slug_verified_teacher" + Then I should be on the page for slug "basic_slug_verified_teacher" And I should see "Test Teacher Page" And I should see "Test teacher body." Then I follow "Pages" Then I should be on the pages index And I follow the page link "Test Admin Page" - Then I should be on the page for slug "test_slug_admin" + Then I should be on the page for slug "basic_slug_admin" And I should see "Test Admin Page" And I should see "Test admin body." @@ -91,7 +91,7 @@ Scenario: Teachers can access public pages Then I follow "Pages" Then I should be on the pages index And I follow the page link "Test Public Page" - Then I should be on the page for slug "test_slug_public" + Then I should be on the page for slug "basic_slug_public" And I should see "Test Public Page" And I should see "Test public body." @@ -103,7 +103,7 @@ Scenario: Teachers can access teacher pages Then I follow "Pages" Then I should be on the pages index And I follow the page link "Test Teacher Page" - Then I should be on the page for slug "test_slug_verified_teacher" + Then I should be on the page for slug "basic_slug_verified_teacher" And I should see "Test Teacher Page" And I should see "Test teacher body." @@ -112,7 +112,7 @@ Scenario: Teachers can't access admin pages Given I have a teacher Google email And I follow "Log In" Then I can log in with Google - Given I am on the page for slug "test_slug_admin" + Given I am on the page for slug "basic_slug_admin" Then I should be on the pages index Scenario: Public can access public pages @@ -120,14 +120,14 @@ Scenario: Public can access public pages Then I follow "Pages" Then I should see "Default Public Page" And I follow the page link "Test Public Page" - Then I should be on the page for slug "test_slug_public" + Then I should be on the page for slug "basic_slug_public" And I should see "Test Public Page" And I should see "Test public body." Scenario: Public can't access verified teacher pages - Given I am on the page for slug "test_slug_verified_teacher" + Given I am on the page for slug "basic_slug_verified_teacher" Then I should be on the login page Scenario: Public can't access admin pages - Given I am on the page for slug "test_slug_admin" + Given I am on the page for slug "basic_slug_admin" Then I should be on the login page diff --git a/features/step_definitions/page_steps.rb b/features/step_definitions/page_steps.rb index 16d16639..90c8796e 100644 --- a/features/step_definitions/page_steps.rb +++ b/features/step_definitions/page_steps.rb @@ -2,7 +2,7 @@ Given(/the following pages exist/) do |pages_table| default_params = { - url_slug: "test_slug", + url_slug: "basic_slug", title: "Test Page Title", html: "Test page body.", viewer_permissions: "Public", diff --git a/spec/controllers/pages_controller_spec.rb b/spec/controllers/pages_controller_spec.rb index bd111a3f..18ac2345 100644 --- a/spec/controllers/pages_controller_spec.rb +++ b/spec/controllers/pages_controller_spec.rb @@ -2,21 +2,21 @@ require "rails_helper" -RSpec.describe PagesController, type: :controller do +RSpec.describe PagesController, type: :request, focus: true do fixtures :all before(:all) do - @pages_slug = "test_slug_1" + @pages_slug = "basic_slug_1" @pages_title = "Test Page Title 1" @fail_flash_alert = /Failed to submit information :\(/ @success_flash_alert = Regexp.new("Created #{@pages_title} page successfully.") end describe "#index" do - it "renders the index template" do + it "redirects to the default page" do expect(Page).to receive(:where) - get :index - expect(response).to render_template("index") + get '/pages' + expect(response).to redirect_to page_path('about') end end @@ -24,16 +24,16 @@ it "successfully deletes a page" do ApplicationController.any_instance.stub(:require_admin).and_return(true) ApplicationController.any_instance.stub(:is_admin?).and_return(true) - long_app = Page.find_by(url_slug: "Test_slug") - delete :destroy, params: { url_slug: long_app.url_slug } - expect(Page.find_by(url_slug: "Test_slug")).to be_nil + long_app = Page.find_by(url_slug: "basic_slug") + delete page_path("basic_slug") + expect(Page.find_by(url_slug: "basic_slug")).to be_nil end it "doesn't allow teacher to delete a page" do ApplicationController.any_instance.stub(:is_admin?).and_return(false) - long_app = Page.find_by(url_slug: "Test_slug") - delete :destroy, params: { url_slug: long_app.url_slug } - expect(Page.find_by(url_slug: "Test_slug")).not_to be_nil + long_app = Page.find_by(url_slug: "basic_slug") + delete page_path(long_app.url_slug) + expect(Page.find_by(url_slug: "basic_slug")).not_to be_nil end end @@ -41,10 +41,9 @@ it "allows admin to create", :focus do allow_any_instance_of(ApplicationController).to receive(:require_admin).and_return(true) expect(Page.find_by(url_slug: @pages_slug)).to be_nil - post :create, { + post page_path(@pages_slug), { params: { page: { - url_slug: @pages_slug, title: @pages_title, html: "

Test page body.

", viewer_permissions: "Admin", @@ -60,7 +59,7 @@ it "denies teacher to create" do expect(Page.find_by(url_slug: @pages_slug)).to be_nil - post :create, { + post page_path, { params: { page: { url_slug: @pages_slug, @@ -79,7 +78,7 @@ it "requires slug to create" do allow_any_instance_of(ApplicationController).to receive(:require_admin).and_return(true) expect(Page.find_by(url_slug: @pages_slug)).to be_nil - post :create, { + post page_path, { params: { page: { title: @pages_title, @@ -132,11 +131,11 @@ it "prevents submitting multiple pages with same slug" do allow_any_instance_of(ApplicationController).to receive(:require_admin).and_return(true) - expect(Page.find_by(url_slug: "test_slug_2")).not_to be_nil + expect(Page.find_by(url_slug: "basic_slug_2")).not_to be_nil post :create, { params: { page: { - url_slug: "test_slug_2", + url_slug: "basic_slug_2", title: "Test Page Title 2", html: "

Test page body.

", viewer_permissions: "Admin", @@ -154,7 +153,7 @@ it "should allow admin to edit page" do ApplicationController.any_instance.stub(:require_admin).and_return(true) ApplicationController.any_instance.stub(:is_admin?).and_return(true) - thetest = Page.find_by(url_slug: "Test_slug") + thetest = Page.find_by(url_slug: "basic_slug") post :update, params: { url_slug: thetest.url_slug, @@ -166,7 +165,7 @@ } }, session: { user_id: 0 } - thetest = Page.find_by(url_slug: "Test_slug") + thetest = Page.find_by(url_slug: "basic_slug") expect(thetest.title).to eq("title") end end diff --git a/spec/fixtures/pages.yml b/spec/fixtures/pages.yml index a486a747..51094c22 100644 --- a/spec/fixtures/pages.yml +++ b/spec/fixtures/pages.yml @@ -23,18 +23,29 @@ # fk_rails_... (creator_id => teachers.id) # fk_rails_... (last_editor_id => teachers.id) # -thepage: - url_slug: Test_slug +basic_page: + url_slug: basic_slug title: test_page html:

Content

viewer_permissions: "Verified Teacher" + default: false creator_id: 0 last_editor_id: 0 validadminpage: - url_slug: test_slug_2 + url_slug: basic_slug_2 title: Test Page Title 2 viewer_permissions: Admin html:

An admin page

+ default: false creator_id: 0 last_editor_id: 0 + +about_page: + url_slug: about + title: About BJC Page + html:

Content

+ viewer_permissions: "Public" + default: true + creator_id: 1 + last_editor_id: 1 From 6c4fd5ff8f8ce22e4d917c90a5afe29a5153a457 Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Tue, 8 Aug 2023 14:49:59 -1000 Subject: [PATCH 22/54] WIP factors / fixing specs --- spec/controllers/pages_controller_spec.rb | 22 ++++++++-------------- spec/{ => factories}/factories.rb | 11 ----------- spec/factories/teachers.rb | 12 ++++++++++++ spec/support/sessions_helper.rb | 7 +++++++ 4 files changed, 27 insertions(+), 25 deletions(-) rename spec/{ => factories}/factories.rb (58%) create mode 100644 spec/factories/teachers.rb create mode 100644 spec/support/sessions_helper.rb diff --git a/spec/controllers/pages_controller_spec.rb b/spec/controllers/pages_controller_spec.rb index 18ac2345..ef2fefd3 100644 --- a/spec/controllers/pages_controller_spec.rb +++ b/spec/controllers/pages_controller_spec.rb @@ -15,44 +15,38 @@ describe "#index" do it "redirects to the default page" do expect(Page).to receive(:where) - get '/pages' - expect(response).to redirect_to page_path('about') + get "/pages" + expect(response).to redirect_to page_path("about") end end describe "#destroy" do it "successfully deletes a page" do - ApplicationController.any_instance.stub(:require_admin).and_return(true) - ApplicationController.any_instance.stub(:is_admin?).and_return(true) - long_app = Page.find_by(url_slug: "basic_slug") + log_in(Teacher.find(0)) + Page.find_by(url_slug: "basic_slug") delete page_path("basic_slug") expect(Page.find_by(url_slug: "basic_slug")).to be_nil end it "doesn't allow teacher to delete a page" do - ApplicationController.any_instance.stub(:is_admin?).and_return(false) - long_app = Page.find_by(url_slug: "basic_slug") - delete page_path(long_app.url_slug) + log_in(Teacher.find(1)) + delete page_path("basic_slug") expect(Page.find_by(url_slug: "basic_slug")).not_to be_nil end end describe "#create" do it "allows admin to create", :focus do - allow_any_instance_of(ApplicationController).to receive(:require_admin).and_return(true) + log_in(Teacher.find(0)) expect(Page.find_by(url_slug: @pages_slug)).to be_nil - post page_path(@pages_slug), { + post page_path(@pages_slug), params: { page: { title: @pages_title, html: "

Test page body.

", viewer_permissions: "Admin", } - }, - session: { - user_id: 0 } - } expect(Page.find_by(url_slug: @pages_slug)).not_to be_nil expect(@success_flash_alert).to match flash[:success] end diff --git a/spec/factories.rb b/spec/factories/factories.rb similarity index 58% rename from spec/factories.rb rename to spec/factories/factories.rb index fe4a5cf3..9be9fd1b 100644 --- a/spec/factories.rb +++ b/spec/factories/factories.rb @@ -11,17 +11,6 @@ website { "https://www.school.edu" } end - factory :teacher do - first_name { "Teacher" } - last_name { "User" } - snap { "teacher" } - email { "teacher@example.edu" } - status { 0 } - application_status { "Validated" } - personal_website { "https://www.school.edu/teacher" } - admin { false } - end - factory :admin do first_name { "Admin" } last_name { "User" } diff --git a/spec/factories/teachers.rb b/spec/factories/teachers.rb new file mode 100644 index 00000000..d6365572 --- /dev/null +++ b/spec/factories/teachers.rb @@ -0,0 +1,12 @@ +FactoryBot.define do + factory :teacher do + first_name { "Teacher" } + last_name { "User" } + snap { "teacher" } + sequence(:email) { |n| "teacher-#{n}@example.edu" } + status { 0 } + application_status { "Validated" } + personal_website { "https://www.school.edu/teacher" } + admin { false } + end +end diff --git a/spec/support/sessions_helper.rb b/spec/support/sessions_helper.rb new file mode 100644 index 00000000..62d5260e --- /dev/null +++ b/spec/support/sessions_helper.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +require "factory_bot" + +RSpec.configure do |config| + config.include FactoryBot::Syntax::Methods +end From 87092025ebbc4c76ff142502e0228e9797b76a36 Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Tue, 8 Aug 2023 14:52:06 -1000 Subject: [PATCH 23/54] Rubocop --- spec/factories/teachers.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/factories/teachers.rb b/spec/factories/teachers.rb index d6365572..0a74b410 100644 --- a/spec/factories/teachers.rb +++ b/spec/factories/teachers.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + FactoryBot.define do factory :teacher do first_name { "Teacher" } From f4e847f8856dc8c529e2f7568183ecdc6be78e6a Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Sat, 12 Aug 2023 15:10:26 -1000 Subject: [PATCH 24/54] fix loading support files, mocking get_lat_lng method --- spec/rails_helper.rb | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 71dadf38..175c3543 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true # This file is copied to spec/ when you run 'rails generate rspec:install' -require "spec_helper" ENV["RAILS_ENV"] ||= "test" require "simplecov" @@ -15,6 +14,8 @@ # Add additional requires below this line. Rails is not loaded until this point! require "support/factory_bot" +require "spec_helper" + # Requires supporting ruby files with custom matchers and macros, etc, in # spec/support/ and its subdirectories. Files matching `spec/**/*_spec.rb` are # run as spec files by default. This means that files in spec/support that end @@ -28,7 +29,7 @@ # directory. Alternatively, in the individual `*_spec.rb` files, manually # require only the support files necessary. # -# Dir[Rails.root.join('spec', 'support', '**', '*.rb')].each { |f| require f } +Dir[Rails.root.join("spec", "support", "**", "*.rb")].each { |f| require f } # Checks for pending migrations and applies them before tests are run. # If you are not using ActiveRecord, you can remove these lines. @@ -41,7 +42,13 @@ RSpec.configure do |config| config.before(:suite) do - Rails.application.load_seed # loading seeds + # We can't use rspec-mock here, so just overwrite the method... + module MapsService + def self.get_lat_lng(_) + { lat: 37.8719, lng: -122.2585 } + end + end + Rails.application.load_seed end config.before(:each) do From 1dd6696e760463d4c98ad5b2735fff1e2e5e592e Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Sat, 12 Aug 2023 15:55:14 -1000 Subject: [PATCH 25/54] Add a spec log_in method, cleanup fixtures for pages/teachers --- spec/fixtures/pages.yml | 6 +++--- spec/fixtures/teachers.yml | 23 ++++++++++++++++++----- spec/support/sessions_helper.rb | 14 ++++++++++++-- 3 files changed, 33 insertions(+), 10 deletions(-) diff --git a/spec/fixtures/pages.yml b/spec/fixtures/pages.yml index 51094c22..be0a9de3 100644 --- a/spec/fixtures/pages.yml +++ b/spec/fixtures/pages.yml @@ -29,16 +29,16 @@ basic_page: html:

Content

viewer_permissions: "Verified Teacher" default: false - creator_id: 0 + creator_id: 1 last_editor_id: 0 -validadminpage: +admin_page: url_slug: basic_slug_2 title: Test Page Title 2 viewer_permissions: Admin html:

An admin page

default: false - creator_id: 0 + creator_id: 1 last_editor_id: 0 about_page: diff --git a/spec/fixtures/teachers.yml b/spec/fixtures/teachers.yml index 1f130f16..519b9d79 100644 --- a/spec/fixtures/teachers.yml +++ b/spec/fixtures/teachers.yml @@ -33,8 +33,8 @@ # fk_rails_... (school_id => schools.id) # -ye: - id: 0 +admin: + id: 1 admin: true first_name: Ye last_name: Wang @@ -46,8 +46,21 @@ ye: education_level: -1 school: stanfurd +validated_teacher: + id: 2 + admin: false + first_name: Validated + last_name: Teacher + status: 4 + more_info: A CS169 Student + email: validated@teacher.edu + snap: validated + application_status: Validated + education_level: -1 + school: berkeley + bob: - id: 1 + id: 3 first_name: Bob last_name: Johnson snap: BobJohnson @@ -59,7 +72,7 @@ bob: education_level: -1 long: - id: 2 + id: 4 first_name: Short last_name: Long snap: song @@ -71,7 +84,7 @@ long: education_level: 2 reimu: - id: 3 + id: 5 first_name: Reimu last_name: Hakurei snap: reimu diff --git a/spec/support/sessions_helper.rb b/spec/support/sessions_helper.rb index 62d5260e..5ba6e469 100644 --- a/spec/support/sessions_helper.rb +++ b/spec/support/sessions_helper.rb @@ -2,6 +2,16 @@ require "factory_bot" -RSpec.configure do |config| - config.include FactoryBot::Syntax::Methods +# This mocks omniauth for a given user. +def log_in(user, provider: "google_oauth2") + OmniAuth.config.mock_auth[provider.to_sym] = OmniAuth::AuthHash.new({ + provider: provider.to_s, + uid: "123456789", + info: { + name: user.full_name, + email: user.email + } + }) + + get "/auth/#{provider}/callback" end From dabee65529bb090f393784106d2b929a93874331 Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Sat, 12 Aug 2023 15:55:35 -1000 Subject: [PATCH 26/54] fixup pages controller spec --- app/controllers/pages_controller.rb | 3 +- spec/controllers/pages_controller_spec.rb | 164 ++++++++++------------ 2 files changed, 77 insertions(+), 90 deletions(-) diff --git a/app/controllers/pages_controller.rb b/app/controllers/pages_controller.rb index 6ad660bc..54df8c71 100644 --- a/app/controllers/pages_controller.rb +++ b/app/controllers/pages_controller.rb @@ -58,7 +58,7 @@ def update flash[:success] = "Updated #{@page.title} page successfully." redirect_to pages_path else - flash.now[:alert] = "An error occurred! #{@page.errors.full_messages}" + flash.now[:alert] = "An error occurred! #{@page.errors.full_messages.to_sentence}" render "edit" end end @@ -93,7 +93,6 @@ def page_params # teacher_school_state: @teacher.school.state, # teacher_snap: @teacher.snap, # teacher_school_website: @teacher.school.website, - # bjc_password: Rails.application.secrets[:bjc_password], # piazza_password: Rails.application.secrets[:piazza_password], # reason: @reason # }.with_indifferent_access diff --git a/spec/controllers/pages_controller_spec.rb b/spec/controllers/pages_controller_spec.rb index ef2fefd3..40a22ed8 100644 --- a/spec/controllers/pages_controller_spec.rb +++ b/spec/controllers/pages_controller_spec.rb @@ -5,11 +5,25 @@ RSpec.describe PagesController, type: :request, focus: true do fixtures :all + let(:admin_teacher) { Teacher.where(admin: true).first } + let(:validated_teacher) { Teacher.validated.first } + before(:all) do @pages_slug = "basic_slug_1" @pages_title = "Test Page Title 1" @fail_flash_alert = /Failed to submit information :\(/ - @success_flash_alert = Regexp.new("Created #{@pages_title} page successfully.") + end + + describe "for an admin teacher" do + before do + log_in(admin_teacher) + end + end + + describe "for a validated teacher" do + before do + log_in(validated_teacher) + end end describe "#index" do @@ -22,143 +36,117 @@ describe "#destroy" do it "successfully deletes a page" do - log_in(Teacher.find(0)) + log_in(admin_teacher) Page.find_by(url_slug: "basic_slug") delete page_path("basic_slug") expect(Page.find_by(url_slug: "basic_slug")).to be_nil end it "doesn't allow teacher to delete a page" do - log_in(Teacher.find(1)) + log_in(validated_teacher) delete page_path("basic_slug") expect(Page.find_by(url_slug: "basic_slug")).not_to be_nil end end describe "#create" do - it "allows admin to create", :focus do - log_in(Teacher.find(0)) - expect(Page.find_by(url_slug: @pages_slug)).to be_nil - post page_path(@pages_slug), - params: { - page: { - title: @pages_title, - html: "

Test page body.

", - viewer_permissions: "Admin", - } - } - expect(Page.find_by(url_slug: @pages_slug)).not_to be_nil - expect(@success_flash_alert).to match flash[:success] + it "allows admin to create" do + log_in(admin_teacher) + expect(Page.find_by(url_slug: "admin_create_test")).to be_nil + post pages_path, params: { + page: { + url_slug: "admin_create_test", + title: @pages_title, + html: "

Test page body.

", + viewer_permissions: "Admin", + } + } + expect(Page.find_by(url_slug: "admin_create_test")).not_to be_nil + expect(flash[:success]).to match(/Created #{@pages_title} page successfully./) end it "denies teacher to create" do + log_in(validated_teacher) expect(Page.find_by(url_slug: @pages_slug)).to be_nil - post page_path, { - params: { - page: { - url_slug: @pages_slug, - title: @pages_title, - html: "

Test page body.

", - viewer_permissions: "Admin", - } - }, - session: { - user_id: 0 - } + post pages_path, params: { + page: { + url_slug: @pages_slug, + title: @pages_title, + html: "

Test page body.

", + viewer_permissions: "Admin", + } } expect(Page.find_by(url_slug: @pages_slug)).to be_nil end it "requires slug to create" do - allow_any_instance_of(ApplicationController).to receive(:require_admin).and_return(true) + log_in(admin_teacher) expect(Page.find_by(url_slug: @pages_slug)).to be_nil - post page_path, { - params: { - page: { - title: @pages_title, - html: "

Test page body.

", - viewer_permissions: "Admin", - } - }, - session: { - user_id: 0 + post pages_path, params: { + page: { + title: @pages_title, + html: "

Test page body.

", + viewer_permissions: "Admin", } } + expect(flash[:alert]).to match(/URL slug/) expect(Page.find_by(url_slug: @pages_slug)).to be_nil end it "requires title to create" do - allow_any_instance_of(ApplicationController).to receive(:require_admin).and_return(true) + log_in(admin_teacher) expect(Page.find_by(url_slug: @pages_slug)).to be_nil - post :create, { - params: { - page: { - url_slug: @pages_slug, - html: "

Test page body.

", - viewer_permissions: "Admin", - } - }, - session: { - user_id: 0 + post pages_path, params: { + page: { + url_slug: @pages_slug, + html: "

Test page body.

", + viewer_permissions: "Admin", } } expect(Page.find_by(url_slug: @pages_slug)).to be_nil end - it "requires permissions to create" do - allow_any_instance_of(ApplicationController).to receive(:require_admin).and_return(true) + it "requires viewer permissions to create" do + log_in(admin_teacher) expect(Page.find_by(url_slug: @pages_slug)).to be_nil - post :create, { - params: { - page: { - title: @pages_title, - url_slug: @pages_slug, - html: "

Test page body.

", - } - }, - session: { - user_id: 0 + post pages_path, params: { + page: { + title: @pages_title, + url_slug: @pages_slug, + html: "

Test page body.

", } } expect(Page.find_by(url_slug: @pages_slug)).to be_nil + expect(flash[:alert]).to match(/Viewer permissions/) end it "prevents submitting multiple pages with same slug" do - allow_any_instance_of(ApplicationController).to receive(:require_admin).and_return(true) + log_in(admin_teacher) expect(Page.find_by(url_slug: "basic_slug_2")).not_to be_nil - post :create, { - params: { - page: { - url_slug: "basic_slug_2", - title: "Test Page Title 2", - html: "

Test page body.

", - viewer_permissions: "Admin", - } - }, - session: { - user_id: 0 + post pages_path, params: { + page: { + url_slug: "basic_slug_2", + title: "Test Page Title 2", + html: "

Test page body.

", + viewer_permissions: "Admin", } } - expect(flash[:alert]).to include "URL slug" + expect(flash[:alert]).to include("URL slug") end end describe "#edit" do it "should allow admin to edit page" do - ApplicationController.any_instance.stub(:require_admin).and_return(true) - ApplicationController.any_instance.stub(:is_admin?).and_return(true) + log_in(admin_teacher) thetest = Page.find_by(url_slug: "basic_slug") - post :update, - params: { - url_slug: thetest.url_slug, - page: { - viewer_permissions: "Verified Teacher", - title: "title", - url_slug: thetest.url_slug, - html: "Test content" - } - }, - session: { user_id: 0 } + patch page_path(url_slug: thetest.url_slug), params: { + page: { + viewer_permissions: "Verified Teacher", + title: "title", + url_slug: thetest.url_slug, + html: "Test content" + } + } thetest = Page.find_by(url_slug: "basic_slug") expect(thetest.title).to eq("title") end From e7cc4b8ded074e24c57d3e0030bef17906005bf3 Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Sat, 12 Aug 2023 16:00:57 -1000 Subject: [PATCH 27/54] Refactor pages controller spec a bit for user clarity --- spec/controllers/pages_controller_spec.rb | 193 ++++++++++------------ 1 file changed, 90 insertions(+), 103 deletions(-) diff --git a/spec/controllers/pages_controller_spec.rb b/spec/controllers/pages_controller_spec.rb index 40a22ed8..d6124dad 100644 --- a/spec/controllers/pages_controller_spec.rb +++ b/spec/controllers/pages_controller_spec.rb @@ -2,7 +2,7 @@ require "rails_helper" -RSpec.describe PagesController, type: :request, focus: true do +RSpec.describe PagesController, type: :request do fixtures :all let(:admin_teacher) { Teacher.where(admin: true).first } @@ -14,141 +14,128 @@ @fail_flash_alert = /Failed to submit information :\(/ end - describe "for an admin teacher" do + context "for an admin teacher" do before do log_in(admin_teacher) end - end - - describe "for a validated teacher" do - before do - log_in(validated_teacher) - end - end - - describe "#index" do - it "redirects to the default page" do - expect(Page).to receive(:where) - get "/pages" - expect(response).to redirect_to page_path("about") - end - end - describe "#destroy" do - it "successfully deletes a page" do - log_in(admin_teacher) + it "#destroy successfully deletes a page" do Page.find_by(url_slug: "basic_slug") delete page_path("basic_slug") expect(Page.find_by(url_slug: "basic_slug")).to be_nil end - it "doesn't allow teacher to delete a page" do - log_in(validated_teacher) - delete page_path("basic_slug") - expect(Page.find_by(url_slug: "basic_slug")).not_to be_nil + describe "#create" do + it "allows creating a valid page" do + expect(Page.find_by(url_slug: "admin_create_test")).to be_nil + post pages_path, params: { + page: { + url_slug: "admin_create_test", + title: @pages_title, + html: "

Test page body.

", + viewer_permissions: "Admin", + } + } + expect(Page.find_by(url_slug: "admin_create_test")).not_to be_nil + expect(flash[:success]).to match(/Created #{@pages_title} page successfully./) + end + + it "requires slug to create" do + expect(Page.find_by(url_slug: @pages_slug)).to be_nil + post pages_path, params: { + page: { + title: @pages_title, + html: "

Test page body.

", + viewer_permissions: "Admin", + } + } + expect(flash[:alert]).to match(/URL slug/) + expect(Page.find_by(url_slug: @pages_slug)).to be_nil + end + + it "requires title to create" do + expect(Page.find_by(url_slug: @pages_slug)).to be_nil + post pages_path, params: { + page: { + url_slug: @pages_slug, + html: "

Test page body.

", + viewer_permissions: "Admin", + } + } + expect(Page.find_by(url_slug: @pages_slug)).to be_nil + end + + it "requires viewer permissions to create" do + expect(Page.find_by(url_slug: @pages_slug)).to be_nil + post pages_path, params: { + page: { + title: @pages_title, + url_slug: @pages_slug, + html: "

Test page body.

", + } + } + expect(Page.find_by(url_slug: @pages_slug)).to be_nil + expect(flash[:alert]).to match(/Viewer permissions/) + end + + it "prevents submitting multiple pages with same slug" do + expect(Page.find_by(url_slug: "basic_slug_2")).not_to be_nil + post pages_path, params: { + page: { + url_slug: "basic_slug_2", + title: "Test Page Title 2", + html: "

Test page body.

", + viewer_permissions: "Admin", + } + } + expect(flash[:alert]).to include("URL slug") + end end - end - describe "#create" do - it "allows admin to create" do - log_in(admin_teacher) - expect(Page.find_by(url_slug: "admin_create_test")).to be_nil - post pages_path, params: { + it "#edit allows updates" do + thetest = Page.find_by(url_slug: "basic_slug") + patch page_path(url_slug: thetest.url_slug), params: { page: { - url_slug: "admin_create_test", - title: @pages_title, - html: "

Test page body.

", - viewer_permissions: "Admin", + viewer_permissions: "Verified Teacher", + title: "title", + url_slug: thetest.url_slug, + html: "Test content" } } - expect(Page.find_by(url_slug: "admin_create_test")).not_to be_nil - expect(flash[:success]).to match(/Created #{@pages_title} page successfully./) + thetest = Page.find_by(url_slug: "basic_slug") + expect(thetest.title).to eq("title") end + end - it "denies teacher to create" do + describe "for a validated teacher" do + before do log_in(validated_teacher) - expect(Page.find_by(url_slug: @pages_slug)).to be_nil - post pages_path, params: { - page: { - url_slug: @pages_slug, - title: @pages_title, - html: "

Test page body.

", - viewer_permissions: "Admin", - } - } - expect(Page.find_by(url_slug: @pages_slug)).to be_nil end - it "requires slug to create" do - log_in(admin_teacher) - expect(Page.find_by(url_slug: @pages_slug)).to be_nil - post pages_path, params: { - page: { - title: @pages_title, - html: "

Test page body.

", - viewer_permissions: "Admin", - } - } - expect(flash[:alert]).to match(/URL slug/) - expect(Page.find_by(url_slug: @pages_slug)).to be_nil + it "#destroy doesn't allow teacher to delete a page" do + delete page_path("basic_slug") + expect(Page.find_by(url_slug: "basic_slug")).not_to be_nil end - it "requires title to create" do - log_in(admin_teacher) + it "#create denies teacher to create" do expect(Page.find_by(url_slug: @pages_slug)).to be_nil post pages_path, params: { page: { url_slug: @pages_slug, - html: "

Test page body.

", - viewer_permissions: "Admin", - } - } - expect(Page.find_by(url_slug: @pages_slug)).to be_nil - end - - it "requires viewer permissions to create" do - log_in(admin_teacher) - expect(Page.find_by(url_slug: @pages_slug)).to be_nil - post pages_path, params: { - page: { title: @pages_title, - url_slug: @pages_slug, - html: "

Test page body.

", - } - } - expect(Page.find_by(url_slug: @pages_slug)).to be_nil - expect(flash[:alert]).to match(/Viewer permissions/) - end - - it "prevents submitting multiple pages with same slug" do - log_in(admin_teacher) - expect(Page.find_by(url_slug: "basic_slug_2")).not_to be_nil - post pages_path, params: { - page: { - url_slug: "basic_slug_2", - title: "Test Page Title 2", html: "

Test page body.

", viewer_permissions: "Admin", } } - expect(flash[:alert]).to include("URL slug") + expect(Page.find_by(url_slug: @pages_slug)).to be_nil end end - describe "#edit" do - it "should allow admin to edit page" do - log_in(admin_teacher) - thetest = Page.find_by(url_slug: "basic_slug") - patch page_path(url_slug: thetest.url_slug), params: { - page: { - viewer_permissions: "Verified Teacher", - title: "title", - url_slug: thetest.url_slug, - html: "Test content" - } - } - thetest = Page.find_by(url_slug: "basic_slug") - expect(thetest.title).to eq("title") + describe "#index" do + it "redirects to the default page" do + expect(Page).to receive(:where) + get "/pages" + expect(response).to redirect_to page_path("about") end end end From 0cbd500dc4a9d567e10bd81b8bad967ee1f9814a Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Sat, 12 Aug 2023 16:05:54 -1000 Subject: [PATCH 28/54] Update malicious spec for new post format --- spec/malicious_spec.rb | 50 ++++++++++++++++++------------------------ 1 file changed, 21 insertions(+), 29 deletions(-) diff --git a/spec/malicious_spec.rb b/spec/malicious_spec.rb index cccd91a1..c85c4996 100644 --- a/spec/malicious_spec.rb +++ b/spec/malicious_spec.rb @@ -6,46 +6,38 @@ fixtures :all it "does not allow non-admin to accept a user" do - post :validate, { - params: { - id: 1 - } - } + post :validate, params: { id: 1 } expect(response).to have_http_status(302) expect(response).to redirect_to(root_path) expect(flash[:danger]).to eq "You need to log in to access this." end it "rejects malicious admin signup attempt" do - post :create, { - params: { - teacher: { - first_name: "valid_example", - last_name: "valid_example", - email: "valid_example@valid_example.edu", - status: 0, - snap: "valid_example", - admin: true, - school_id: School.first.id - } - } + post :create, params: { + teacher: { + first_name: "Valid", + last_name: "Example", + email: "valid_example@valid_example.edu", + status: 0, + snap: "valid_example", + admin: true, + school_id: School.first.id + } } expect(Teacher.where(email: "valid_example@valid_example.edu").first.admin).to be(false) end it "rejects malicious auto-approve signup attempt" do - post :create, { - params: { - teacher: { - first_name: "valid_example", - last_name: "valid_example", - email: "valid_example@valid_example.edu", - status: 0, - application_status: "validated", - snap: "valid_example", - school_id: School.first.id, - } - } + post :create, params: { + teacher: { + first_name: "Valid", + last_name: "Example", + email: "valid_example@valid_example.edu", + status: 0, + application_status: "validated", + snap: "valid_example", + school_id: School.first.id, + } } expect(Teacher.where(email: "valid_example@valid_example.edu").first.application_status).to eq("not_reviewed") end From d5aa51e7ee014d09ec5d9f189bc07b3756ae6b43 Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Sat, 12 Aug 2023 16:10:53 -1000 Subject: [PATCH 29/54] move malicious spec to teachers_controller --- spec/controllers/teachers_controller_spec.rb | 39 +++++++++++++++++ spec/malicious_spec.rb | 44 -------------------- 2 files changed, 39 insertions(+), 44 deletions(-) delete mode 100644 spec/malicious_spec.rb diff --git a/spec/controllers/teachers_controller_spec.rb b/spec/controllers/teachers_controller_spec.rb index 1d2c9580..6a1c38f7 100644 --- a/spec/controllers/teachers_controller_spec.rb +++ b/spec/controllers/teachers_controller_spec.rb @@ -144,4 +144,43 @@ bob_app = Teacher.find_by(first_name: "Bob") expect(bob_app.more_info).to eq orig_more_info end + + context "malicious parameters" do + it "does not allow non-admin to accept a user" do + post :validate, params: { id: 1 } + expect(response).to have_http_status(302) + expect(response).to redirect_to(root_path) + expect(flash[:danger]).to eq "Only admins can access this page." + end + + it "rejects malicious admin signup attempt" do + post :create, params: { + teacher: { + first_name: "Valid", + last_name: "Example", + email: "valid_example@valid_example.edu", + status: 0, + snap: "valid_example", + admin: true, + school_id: School.first.id + } + } + expect(Teacher.find_by(email: "valid_example@valid_example.edu").admin).to be(false) + end + + it "rejects malicious auto-approve signup attempt" do + post :create, params: { + teacher: { + first_name: "Valid", + last_name: "Example", + email: "valid_example@valid_example.edu", + status: 0, + application_status: "validated", + snap: "valid_example", + school_id: School.first.id, + } + } + expect(Teacher.find_by(email: "valid_example@valid_example.edu").not_reviewed?).to be true + end + end end diff --git a/spec/malicious_spec.rb b/spec/malicious_spec.rb deleted file mode 100644 index c85c4996..00000000 --- a/spec/malicious_spec.rb +++ /dev/null @@ -1,44 +0,0 @@ -# frozen_string_literal: true - -require "rails_helper" - -RSpec.describe TeachersController, type: :controller do - fixtures :all - - it "does not allow non-admin to accept a user" do - post :validate, params: { id: 1 } - expect(response).to have_http_status(302) - expect(response).to redirect_to(root_path) - expect(flash[:danger]).to eq "You need to log in to access this." - end - - it "rejects malicious admin signup attempt" do - post :create, params: { - teacher: { - first_name: "Valid", - last_name: "Example", - email: "valid_example@valid_example.edu", - status: 0, - snap: "valid_example", - admin: true, - school_id: School.first.id - } - } - expect(Teacher.where(email: "valid_example@valid_example.edu").first.admin).to be(false) - end - - it "rejects malicious auto-approve signup attempt" do - post :create, params: { - teacher: { - first_name: "Valid", - last_name: "Example", - email: "valid_example@valid_example.edu", - status: 0, - application_status: "validated", - snap: "valid_example", - school_id: School.first.id, - } - } - expect(Teacher.where(email: "valid_example@valid_example.edu").first.application_status).to eq("not_reviewed") - end -end From ebaeb2a0361bc8d6037d825cf369ab9c2b8e2949 Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Sat, 12 Aug 2023 16:13:28 -1000 Subject: [PATCH 30/54] Update fixture name in teachers model --- spec/models/teacher_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/models/teacher_spec.rb b/spec/models/teacher_spec.rb index e311fc2c..316d85e9 100644 --- a/spec/models/teacher_spec.rb +++ b/spec/models/teacher_spec.rb @@ -85,7 +85,7 @@ end describe "teacher with more info" do - let(:teacher) { teachers(:ye) } + let(:teacher) { teachers(:admin) } it "shows a short status with more info" do expect(teacher.more_info).to eq "A CS169 Student" From 0862b945f011106b68a8144dadfb4346949655cd Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Sat, 12 Aug 2023 16:14:15 -1000 Subject: [PATCH 31/54] define omniauth_callback_path --- config/routes.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/routes.rb b/config/routes.rb index 06f318ed..fd556694 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -22,7 +22,7 @@ get "/login", to: "sessions#new", as: "login" delete "/logout", to: "sessions#destroy", as: "logout" - get "/auth/:provider/callback", to: "sessions#omniauth_callback" + get "/auth/:provider/callback", to: "sessions#omniauth_callback", as: omniauth_callback get "/dashboard", to: "main#dashboard", as: "dashboard" end From b2536b865686842b30d246b04fd2276ae73665f8 Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Sat, 12 Aug 2023 16:15:32 -1000 Subject: [PATCH 32/54] define omniauth_callback_path --- config/routes.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/routes.rb b/config/routes.rb index fd556694..231435d9 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -22,7 +22,7 @@ get "/login", to: "sessions#new", as: "login" delete "/logout", to: "sessions#destroy", as: "logout" - get "/auth/:provider/callback", to: "sessions#omniauth_callback", as: omniauth_callback + get "/auth/:provider/callback", to: "sessions#omniauth_callback", as: "omniauth_callback" get "/dashboard", to: "main#dashboard", as: "dashboard" end From dcb82fa60bf57541a2c25d64ff785d16c8e8c0f6 Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Sat, 12 Aug 2023 16:19:30 -1000 Subject: [PATCH 33/54] Fixup main_controller_spec for added fixtures --- spec/controllers/main_spec.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/spec/controllers/main_spec.rb b/spec/controllers/main_spec.rb index eb288a3d..28580384 100644 --- a/spec/controllers/main_spec.rb +++ b/spec/controllers/main_spec.rb @@ -4,6 +4,7 @@ RSpec.describe MainController, type: :controller do fixtures :all + describe "#index" do subject { get :index } @@ -19,17 +20,16 @@ end it "counts correct number of validated (accepted) teachers" do - # TODO: Create a non-validated admin teacher - expect(@controller_stub.instance_variable_get(:@validated_teachers).size).to equal 0 + expect(@controller_stub.instance_variable_get(:@validated_teachers).size).to equal 1 end it "shows correct course statistics" do - # only validated (accepted) teachers are accounted for in statuses. + # only validated (non-admin) teachers are accounted for in statuses. statuses = @controller_stub.instance_variable_get(:@statuses) schools = @controller_stub.instance_variable_get(:@schools) - expect(statuses.size).to equal 0 - expect(statuses["other"]).to be_nil - expect(schools.size).to equal 1 + expect(statuses.size).to eq 1 + expect(statuses["other"]).to eq 1 + expect(schools.size).to eq 1 expect(schools.find_by(name: "$tanfurd").name).to eq("$tanfurd") end end From 6485c473b938dfbc3cc66d197e049d0265b5e3f1 Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Sat, 12 Aug 2023 16:58:03 -1000 Subject: [PATCH 34/54] WIP: Fixup post args in schools controller spec --- spec/controllers/schools_controller_spec.rb | 361 ++++++++++---------- 1 file changed, 173 insertions(+), 188 deletions(-) diff --git a/spec/controllers/schools_controller_spec.rb b/spec/controllers/schools_controller_spec.rb index f2604ef0..fe05c3ff 100644 --- a/spec/controllers/schools_controller_spec.rb +++ b/spec/controllers/schools_controller_spec.rb @@ -5,213 +5,198 @@ RSpec.describe SchoolsController, type: :controller do fixtures :all + let(:admin_teacher) { teachers(:admin) } + before(:all) do @create_school_name = "University of California, Berkeley" @fail_flash_alert = /Failed to submit information :\(/ @success_flash_alert = Regexp.new("Created #{@create_school_name} successfully.") end - it "allows admin to create" do - allow_any_instance_of(ApplicationController).to receive(:require_admin).and_return(true) - expect(School.find_by(name: @create_school_name)).to be_nil - post :create, { - params: { - school: { - name: @create_school_name, - city: "Berkeley", - state: "CA", - website: "www.berkeley.edu", - school_type: "public", - grade_level: "university", - tags: [], - nces_id: 123456789000 - } - } - } - expect(School.find_by(name: @create_school_name)).not_to be_nil - expect(@success_flash_alert).to match flash[:success] + context "for a regular teacher" do + it "denies teacher to create" do + expect(School.find_by(name: @create_school_name)).to be_nil + post :create, params: { + school: { + name: @create_school_name, + city: "Berkeley", + state: "CA", + website: "www.berkeley.edu", + school_type: "public", + grade_level: "university", + tags: [], + nces_id: 123456789000 + } + } + expect(School.find_by(name: @create_school_name)).to be_nil + end end - it "denies teacher to create" do - expect(School.find_by(name: @create_school_name)).to be_nil - post :create, { - params: { - school: { - name: @create_school_name, - city: "Berkeley", - state: "CA", - website: "www.berkeley.edu", - school_type: "public", - grade_level: "university", - tags: [], - nces_id: 123456789000 - } + context "for an admin" do + before do + log_in(admin_teacher) + end + + it "allows admin to create" do + expect(School.find_by(name: @create_school_name)).to be_nil + post :create, params: { + school: { + name: @create_school_name, + city: "Berkeley", + state: "CA", + website: "www.berkeley.edu", + school_type: "public", + grade_level: "university", + tags: [], + nces_id: 123456789000 + } + } + expect(School.find_by(name: @create_school_name)).not_to be_nil + expect(@success_flash_alert).to match flash[:success] + end + + it "requires all fields filled" do + expect(School.find_by(name: @create_school_name)).to be_nil + post :create, params: { + school: { + name: @create_school_name, + # missing city + state: "CA", + website: "www.berkeley.edu", + school_type: "public", + grade_level: "university", + tags: [], + nces_id: 123456789000 } - } - expect(School.find_by(name: @create_school_name)).to be_nil - end + } + expect(School.find_by(name: @create_school_name)).to be_nil + expect(flash[:alert]).to match @fail_flash_alert - it "requires all fields filled" do - allow_any_instance_of(ApplicationController).to receive(:require_admin).and_return(true) - expect(School.find_by(name: @create_school_name)).to be_nil - post :create, { - params: { - school: { - name: @create_school_name, - # missing city - state: "CA", - website: "www.berkeley.edu", - school_type: "public", - grade_level: "university", - tags: [], - nces_id: 123456789000 - } + post :create, params: { + school: { + name: @create_school_name, + city: "Berkeley", + state: "CA", + # missing website + school_type: "public", + grade_level: "university", + tags: [], + nces_id: 123456789000 } - } - expect(School.find_by(name: @create_school_name)).to be_nil - expect(flash[:alert]).to match @fail_flash_alert - - post :create, { - params: { - school: { - name: @create_school_name, - city: "Berkeley", - state: "CA", - # missing website - school_type: "public", - grade_level: "university", - tags: [], - nces_id: 123456789000 - } + } + expect(School.find_by(name: @create_school_name)).to be_nil + expect(flash[:alert]).to match @fail_flash_alert + + post :create, params: { + school: { + # missing name + city: "Berkeley", + state: "CA", + website: "www.berkeley.edu", + school_type: "public", + grade_level: "university", + tags: [], + nces_id: 123456789000 } - } - expect(School.find_by(name: @create_school_name)).to be_nil - expect(flash[:alert]).to match @fail_flash_alert - - post :create, { - params: { - school: { - # missing name - city: "Berkeley", - state: "CA", - website: "www.berkeley.edu", - school_type: "public", - grade_level: "university", - tags: [], - nces_id: 123456789000 - } + } + expect(School.find_by(name: @create_school_name)).to be_nil + expect(@fail_flash_alert).to match flash[:alert] + end + + it "requires proper inputs for fields" do + expect(School.find_by(name: @create_school_name)).to be_nil + # Incorrect state (not chosen from enum list) + post :create, params: { + school: { + name: @create_school_name, + city: "Berkeley", + state: "DISTRESS", + website: "www.berkeley.edu", + school_type: "public", + grade_level: "university", + tags: [], + nces_id: 123456789000 } - } - expect(School.find_by(name: @create_school_name)).to be_nil - expect(@fail_flash_alert).to match flash[:alert] - end + } + expect(School.find_by(name: @create_school_name)).to be_nil + expect(@fail_flash_alert).to match flash[:alert] - # TODO: Separate these into individual tests - it "requires proper inputs for fields" do - allow_any_instance_of(ApplicationController).to receive(:require_admin).and_return(true) - expect(School.find_by(name: @create_school_name)).to be_nil - # Incorrect state (not chosen from enum list) - post :create, { - params: { - school: { - name: @create_school_name, - city: "Berkeley", - state: "DISTRESS", - website: "www.berkeley.edu", - school_type: "public", - grade_level: "university", - tags: [], - nces_id: 123456789000 - } + post :create, params: { + school: { + name: @create_school_name, + city: "Berkeley", + state: "CA", + website: "wwwberkeleyedu", + school_type: "public", + grade_level: "university", + tags: [], + nces_id: 123456789000 } - } - expect(School.find_by(name: @create_school_name)).to be_nil - expect(@fail_flash_alert).to match flash[:alert] - - # Incorrect website format - post :create, { - params: { - school: { - name: @create_school_name, - city: "Berkeley", - state: "CA", - website: "wwwberkeleyedu", - school_type: "public", - grade_level: "university", - tags: [], - nces_id: 123456789000 - } + } + expect(School.find_by(name: @create_school_name)).to be_nil + expect(@fail_flash_alert).to match flash[:alert] + + # Incorrect school type + expect { post :create, params: { + school: { + name: @create_school_name, + city: "Berkeley", + state: "CA", + website: "www.berkeley.edu", + school_type: -1, + grade_level: "university", + tags: [], + nces_id: 123456789000 } } - expect(School.find_by(name: @create_school_name)).to be_nil - expect(@fail_flash_alert).to match flash[:alert] - - # Incorrect school type - expect { post :create, { - params: { - school: { - name: @create_school_name, - city: "Berkeley", - state: "CA", - website: "www.berkeley.edu", - school_type: -1, - grade_level: "university", - tags: [], - nces_id: 123456789000 - } - } - } - }.to raise_error(ArgumentError) - expect(School.find_by(name: @create_school_name)).to be_nil - - # Incorrect grade_level - expect { post :create, { - params: { - school: { - name: @create_school_name, - city: "Berkeley", - state: "CA", - website: "www.berkeley.edu", - school_type: "public", - grade_level: -4, - tags: [], - nces_id: 123456789000 - } - } - } - }.to raise_error(ArgumentError) - expect(School.find_by(name: @create_school_name)).to be_nil - end + }.to raise_error(ArgumentError) + expect(School.find_by(name: @create_school_name)).to be_nil - it "does not create duplicate schools in the same city" do - allow_any_instance_of(ApplicationController).to receive(:require_admin).and_return(true) - expect(School.where(name: @create_school_name).count).to eq 0 - - post :create, { - params: { - school: { - name: @create_school_name, - city: "Berkeley", - state: "CA", - website: "www.berkeley.edu", - school_type: "public", - grade_level: "middle_school", - } + # Incorrect grade_level + expect { post :create, { + params: { + school: { + name: @create_school_name, + city: "Berkeley", + state: "CA", + website: "www.berkeley.edu", + school_type: "public", + grade_level: -4, + tags: [], + nces_id: 123456789000 + } + } + } + }.to raise_error(ArgumentError) + expect(School.find_by(name: @create_school_name)).to be_nil + end + + it "does not create duplicate schools in the same city" do + expect(School.where(name: @create_school_name).count).to eq 0 + + post :create, params: { + school: { + name: @create_school_name, + city: "Berkeley", + state: "CA", + website: "www.berkeley.edu", + school_type: "public", + grade_level: "middle_school", } - } - expect(School.where(name: @create_school_name).count).to eq 1 - post :create, { - params: { - school: { - name: @create_school_name, - city: "Berkeley", - state: "CA", - website: "www.berkeley.edu", - school_type: "public", - grade_level: "university", - } + } + expect(School.where(name: @create_school_name).count).to eq 1 + + post :create, params: { + school: { + name: @create_school_name, + city: "Berkeley", + state: "CA", + website: "www.berkeley.edu", + school_type: "public", + grade_level: "university", } - } - expect(School.where(name: @create_school_name).count).to eq 1 + } + expect(School.where(name: @create_school_name).count).to eq 1 + end end end From 9d77447c46ae48953fe5e3fb4d6bb9acd9d3c3b0 Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Sat, 12 Aug 2023 17:04:59 -1000 Subject: [PATCH 35/54] Schools specs work by making them request specs --- spec/controllers/schools_controller_spec.rb | 24 ++++++++++----------- spec/support/sessions_helper.rb | 2 +- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/spec/controllers/schools_controller_spec.rb b/spec/controllers/schools_controller_spec.rb index fe05c3ff..dd051c56 100644 --- a/spec/controllers/schools_controller_spec.rb +++ b/spec/controllers/schools_controller_spec.rb @@ -2,7 +2,7 @@ require "rails_helper" -RSpec.describe SchoolsController, type: :controller do +RSpec.describe SchoolsController, type: :request do fixtures :all let(:admin_teacher) { teachers(:admin) } @@ -16,7 +16,7 @@ context "for a regular teacher" do it "denies teacher to create" do expect(School.find_by(name: @create_school_name)).to be_nil - post :create, params: { + post schools_path, params: { school: { name: @create_school_name, city: "Berkeley", @@ -39,7 +39,7 @@ it "allows admin to create" do expect(School.find_by(name: @create_school_name)).to be_nil - post :create, params: { + post schools_path, params: { school: { name: @create_school_name, city: "Berkeley", @@ -57,7 +57,7 @@ it "requires all fields filled" do expect(School.find_by(name: @create_school_name)).to be_nil - post :create, params: { + post schools_path, params: { school: { name: @create_school_name, # missing city @@ -72,7 +72,7 @@ expect(School.find_by(name: @create_school_name)).to be_nil expect(flash[:alert]).to match @fail_flash_alert - post :create, params: { + post schools_path, params: { school: { name: @create_school_name, city: "Berkeley", @@ -87,7 +87,7 @@ expect(School.find_by(name: @create_school_name)).to be_nil expect(flash[:alert]).to match @fail_flash_alert - post :create, params: { + post schools_path, params: { school: { # missing name city: "Berkeley", @@ -106,7 +106,7 @@ it "requires proper inputs for fields" do expect(School.find_by(name: @create_school_name)).to be_nil # Incorrect state (not chosen from enum list) - post :create, params: { + post schools_path, params: { school: { name: @create_school_name, city: "Berkeley", @@ -121,7 +121,7 @@ expect(School.find_by(name: @create_school_name)).to be_nil expect(@fail_flash_alert).to match flash[:alert] - post :create, params: { + post schools_path, params: { school: { name: @create_school_name, city: "Berkeley", @@ -137,7 +137,7 @@ expect(@fail_flash_alert).to match flash[:alert] # Incorrect school type - expect { post :create, params: { + expect { post schools_path, params: { school: { name: @create_school_name, city: "Berkeley", @@ -153,7 +153,7 @@ expect(School.find_by(name: @create_school_name)).to be_nil # Incorrect grade_level - expect { post :create, { + expect { post schools_path, { params: { school: { name: @create_school_name, @@ -174,7 +174,7 @@ it "does not create duplicate schools in the same city" do expect(School.where(name: @create_school_name).count).to eq 0 - post :create, params: { + post schools_path, params: { school: { name: @create_school_name, city: "Berkeley", @@ -186,7 +186,7 @@ } expect(School.where(name: @create_school_name).count).to eq 1 - post :create, params: { + post schools_path, params: { school: { name: @create_school_name, city: "Berkeley", diff --git a/spec/support/sessions_helper.rb b/spec/support/sessions_helper.rb index 5ba6e469..40178008 100644 --- a/spec/support/sessions_helper.rb +++ b/spec/support/sessions_helper.rb @@ -13,5 +13,5 @@ def log_in(user, provider: "google_oauth2") } }) - get "/auth/#{provider}/callback" + get omniauth_callback_path(provider: provider) end From 0c8883051a309f9d7b9d86a0237a6e232114487c Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Sat, 12 Aug 2023 17:06:13 -1000 Subject: [PATCH 36/54] All controller specs pass --- spec/controllers/teachers_signup_spec.rb | 42 ++++++++++-------------- 1 file changed, 17 insertions(+), 25 deletions(-) diff --git a/spec/controllers/teachers_signup_spec.rb b/spec/controllers/teachers_signup_spec.rb index 779508d4..720d92c0 100644 --- a/spec/controllers/teachers_signup_spec.rb +++ b/spec/controllers/teachers_signup_spec.rb @@ -7,22 +7,20 @@ it "rejects invalid signup information" do previous_count = Teacher.count - post :create, { - params: { - school: { - name: "invalid", - city: "Berkeley", - state: "CA", - website: "invalid.com" - }, - teacher: { - first_name: "", - last_name: "invalid", - email: "invalid@invalid.edu", - status: "invalid", - snap: "invalid" - } - } + post :create, params: { + school: { + name: "invalid", + city: "Berkeley", + state: "CA", + website: "invalid.com" + }, + teacher: { + first_name: "", + last_name: "invalid", + email: "invalid@invalid.edu", + status: "invalid", + snap: "invalid" + } } expect(Teacher.count).to eq(previous_count) expect(/An error occurred/).to match(flash[:alert]) @@ -30,8 +28,7 @@ it "rejects invalid signup school information" do previous_count = Teacher.count - post :create, { - params: { + post :create, params: { school: { city: "Berkeley", state: "CA", @@ -43,7 +40,6 @@ status: "invalid", snap: "invalid" } - } } expect(Teacher.count).to eq(previous_count) expect(flash[:alert]).to match(/An error occurred/) @@ -51,8 +47,7 @@ it "accepts valid signup information" do previous_count = Teacher.count - post :create, { - params: { + post :create, params: { school: { name: "valid_example", city: "Berkeley", @@ -68,20 +63,17 @@ status: 0, snap: "valid_example" } - } } expect(Teacher.count).to eq(previous_count + 1) assert_match(/Thanks for signing up for BJC/, flash[:success]) end it "redirects existing users to the login page" do - post :create, { - params: { + post :create, params: { school: { id: 1 }, teacher: Teacher.first.attributes - } } expect(response).to redirect_to(login_path) expect(flash[:notice]).to match(/Please log in/) From b3b379c6a8dfdd834a5a6cf602ca5a872c7fd66d Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Sat, 12 Aug 2023 17:07:33 -1000 Subject: [PATCH 37/54] Run rubocop, set to 3.1 defaults --- .rubocop.yml | 3 ++- app/models/school.rb | 2 +- features/step_definitions/form_steps.rb | 6 +++--- features/support/browsers.rb | 2 +- spec/controllers/sessions_spec.rb | 2 +- spec/support/sessions_helper.rb | 2 +- 6 files changed, 9 insertions(+), 8 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index d16fc079..6dc1783d 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -4,7 +4,8 @@ require: - rubocop-rspec AllCops: - TargetRubyVersion: 2.7 + # TODO: rubocop needs to be updated for 3.2 + TargetRubyVersion: 3.1 # RuboCop has a bunch of cops enabled by default. This setting tells RuboCop # to ignore them, so only the ones explicitly set in this file are enabled. DisabledByDefault: true diff --git a/app/models/school.rb b/app/models/school.rb index 16bcdc2a..82eaf4ff 100644 --- a/app/models/school.rb +++ b/app/models/school.rb @@ -94,7 +94,7 @@ def maps_marker_data { name: name_location, id: id, - position: { lat: lat, lng: lng }, + position: { lat:, lng: }, } end diff --git a/features/step_definitions/form_steps.rb b/features/step_definitions/form_steps.rb index f4c504bf..03df9953 100644 --- a/features/step_definitions/form_steps.rb +++ b/features/step_definitions/form_steps.rb @@ -5,11 +5,11 @@ Given(/a valid teacher exists/) do school = create(:school) - create(:teacher, school: school) + create(:teacher, school:) end And(/^"(.*)" is not in the database$/) do |email| - expect(Teacher.exists?(email: email)).to be false + expect(Teacher.exists?(email:)).to be false end Given(/^I enter (?:my)? "(.*)" as "(.*)"$/) do |field_name, input| @@ -43,7 +43,7 @@ end Then(/the "(.*)" of the user with email "(.*)" should be "(.*)"/) do |field, email, expected| - expect(Teacher.find_by(email: email)[field]).to eq(expected) + expect(Teacher.find_by(email:)[field]).to eq(expected) end When(/^(?:|I )fill in the school name selectize box with "([^"]*)" and choose to add a new school$/) do |text| diff --git a/features/support/browsers.rb b/features/support/browsers.rb index db3cdf72..e4d851fa 100644 --- a/features/support/browsers.rb +++ b/features/support/browsers.rb @@ -73,7 +73,7 @@ Capybara::Selenium::Driver.new app, browser: :firefox, - options: options + options: end if ENV["DRIVER"].present? diff --git a/spec/controllers/sessions_spec.rb b/spec/controllers/sessions_spec.rb index 91f698c2..a7bff60c 100644 --- a/spec/controllers/sessions_spec.rb +++ b/spec/controllers/sessions_spec.rb @@ -21,7 +21,7 @@ let(:omniauth_data) { Teacher.find_by(first_name: "Short") } let(:provider) { "google_oauth2" } - subject { get :omniauth_callback, params: { provider: provider } } + subject { get :omniauth_callback, params: { provider: } } it "should increase session count by 1 when teacher logs in" do SessionsController.any_instance.stub(:omniauth_info).and_return(omniauth_data) diff --git a/spec/support/sessions_helper.rb b/spec/support/sessions_helper.rb index 40178008..a5906f02 100644 --- a/spec/support/sessions_helper.rb +++ b/spec/support/sessions_helper.rb @@ -13,5 +13,5 @@ def log_in(user, provider: "google_oauth2") } }) - get omniauth_callback_path(provider: provider) + get omniauth_callback_path(provider:) end From 762b7c43f50efc10fec548c42ce532b7a137f2ec Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Sat, 12 Aug 2023 17:13:01 -1000 Subject: [PATCH 38/54] Update rubocop --- .rubocop.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.rubocop.yml b/.rubocop.yml index 6dc1783d..1ec2cd62 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -105,6 +105,7 @@ Layout/IndentationConsistency: # Two spaces, no tabs (for indentation). Layout/IndentationWidth: Enabled: true + Width: 2 Layout/LeadingCommentSpace: Enabled: true From c4cc1d90f472f1364ce08b50dc948543a7e7e17c Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Sat, 12 Aug 2023 17:14:05 -1000 Subject: [PATCH 39/54] fixup some indentation --- spec/controllers/teachers_signup_spec.rb | 60 ++++++++++++------------ 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/spec/controllers/teachers_signup_spec.rb b/spec/controllers/teachers_signup_spec.rb index 720d92c0..3385bc29 100644 --- a/spec/controllers/teachers_signup_spec.rb +++ b/spec/controllers/teachers_signup_spec.rb @@ -29,17 +29,17 @@ it "rejects invalid signup school information" do previous_count = Teacher.count post :create, params: { - school: { - city: "Berkeley", - state: "CA", - }, - teacher: { - first_name: "invalid", - last_name: "invalid", - email: "invalid@invalid.edu", - status: "invalid", - snap: "invalid" - } + school: { + city: "Berkeley", + state: "CA", + }, + teacher: { + first_name: "invalid", + last_name: "invalid", + email: "invalid@invalid.edu", + status: "invalid", + snap: "invalid" + } } expect(Teacher.count).to eq(previous_count) expect(flash[:alert]).to match(/An error occurred/) @@ -48,21 +48,21 @@ it "accepts valid signup information" do previous_count = Teacher.count post :create, params: { - school: { - name: "valid_example", - city: "Berkeley", - state: "CA", - website: "valid_example.com", - school_type: "Public", - grade_level: "High School" - }, - teacher: { - first_name: "valid_example", - last_name: "valid_example", - email: "valid_example@valid_example.edu", - status: 0, - snap: "valid_example" - } + school: { + name: "valid_example", + city: "Berkeley", + state: "CA", + website: "valid_example.com", + school_type: "Public", + grade_level: "High School" + }, + teacher: { + first_name: "valid_example", + last_name: "valid_example", + email: "valid_example@valid_example.edu", + status: 0, + snap: "valid_example" + } } expect(Teacher.count).to eq(previous_count + 1) assert_match(/Thanks for signing up for BJC/, flash[:success]) @@ -70,10 +70,10 @@ it "redirects existing users to the login page" do post :create, params: { - school: { - id: 1 - }, - teacher: Teacher.first.attributes + school: { + id: 1 + }, + teacher: Teacher.first.attributes } expect(response).to redirect_to(login_path) expect(flash[:notice]).to match(/Please log in/) From c2639d5c3e35e8654c0ba17e14fb277b97b69b6b Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Mon, 14 Aug 2023 02:03:31 -0700 Subject: [PATCH 40/54] Restructure seeds/fixtures loading in cuke setup --- features/access.feature | 1 - features/admin.feature | 1 - features/email_template.feature | 1 - features/form_submission.feature | 1 - features/pages_admin.feature | 1 - features/pages_permissions.feature | 1 - features/school_form_submission.feature | 3 --- features/step_definitions/web_steps.rb | 8 -------- features/support/setup.rb | 14 +++++++++++++- features/teacher.feature | 3 --- 10 files changed, 13 insertions(+), 21 deletions(-) diff --git a/features/access.feature b/features/access.feature index 62a35127..b9930cc4 100644 --- a/features/access.feature +++ b/features/access.feature @@ -4,7 +4,6 @@ Feature: access control for new users or non-admin users I cannot access pages for admins Background: Has an Admin and a teacher in DB - Given I seed data Given the following teachers exist: | first_name | last_name | admin | email | school_id | | Alice | Admin | true | testadminuser@berkeley.edu | 1 | diff --git a/features/admin.feature b/features/admin.feature index 8769f46a..5be2216f 100644 --- a/features/admin.feature +++ b/features/admin.feature @@ -5,7 +5,6 @@ Feature: basic admin functionality I can login in an see the dashboard Background: Has an Admin in DB - Given I seed data Given the following teachers exist: | first_name | last_name | admin | email | | Joseph | Mamoa | true | testadminuser@berkeley.edu | diff --git a/features/email_template.feature b/features/email_template.feature index 68ae3dd5..d6be4304 100644 --- a/features/email_template.feature +++ b/features/email_template.feature @@ -3,7 +3,6 @@ Feature: email template features As an admin, I should be able to view and edit email templates Background: I am logged in as an admin, and email templates are generated - Given I seed data Given the following teachers exist: | first_name | last_name | admin | email | | Joseph | Mamoa | true | testadminuser@berkeley.edu | diff --git a/features/form_submission.feature b/features/form_submission.feature index ca05b6f1..639b8ea5 100644 --- a/features/form_submission.feature +++ b/features/form_submission.feature @@ -6,7 +6,6 @@ Feature: submit a form as a teacher Background: Test Data Given a valid teacher exists - Given I seed data Scenario: Correctly filling out and successful form submission Given "kpzhu@berkeley.edu" is not in the database diff --git a/features/pages_admin.feature b/features/pages_admin.feature index 216e8da7..8e2078e3 100644 --- a/features/pages_admin.feature +++ b/features/pages_admin.feature @@ -3,7 +3,6 @@ Feature: pages features as an admin As an admin, I should be able to create, view, and edit pages Background: I am logged in as an admin - Given I seed data Given the following teachers exist: | first_name | last_name | admin | email | | Joseph | Mamoa | true | testadminuser@berkeley.edu | diff --git a/features/pages_permissions.feature b/features/pages_permissions.feature index 8383898e..21537976 100644 --- a/features/pages_permissions.feature +++ b/features/pages_permissions.feature @@ -3,7 +3,6 @@ Feature: pages features a verified teacher As a verifed teacher, I should only be able to see pages I have access to Background: Has admin and teacher in DB along with pages of each permission type - Given I seed data Given the following teachers exist: | first_name | last_name | admin | email | application_status | | Joseph | Mamoa | true | testadminuser@berkeley.edu | Not Reviewed | diff --git a/features/school_form_submission.feature b/features/school_form_submission.feature index 81c38d82..b349c776 100644 --- a/features/school_form_submission.feature +++ b/features/school_form_submission.feature @@ -3,9 +3,6 @@ Feature: view schools So that I can add schools to the database I can view teachers by schools -Background: Seed Data Exists - Given I seed data - Scenario: Viewing the schools page should show the all current schools Given the following schools exist: | name | city | state | website | diff --git a/features/step_definitions/web_steps.rb b/features/step_definitions/web_steps.rb index c0a21490..b35e61d9 100644 --- a/features/step_definitions/web_steps.rb +++ b/features/step_definitions/web_steps.rb @@ -240,14 +240,6 @@ def with_scope(locator, &block) expect(page).to have_no_button(text) end -Given(/I seed data/) do - Rails.application.load_seed - ActiveRecord::FixtureSet.reset_cache - fixtures_folder = File.join(Rails.root, "spec", "fixtures") - fixtures = Dir[File.join(fixtures_folder, "*.yml")].map { |f| File.basename(f, ".yml") } - ActiveRecord::FixtureSet.create_fixtures(fixtures_folder, fixtures) -end - Then(/^"([^"]*)" should be selected for "([^"]*)"(?: within "([^"]*)")?$/) do |value, field, selector| with_scope(selector) do field_labeled(field).find(:xpath, ".//option[@selected = 'selected'][text() = '#{value}']").should be_present diff --git a/features/support/setup.rb b/features/support/setup.rb index 14ea3250..1a09330a 100644 --- a/features/support/setup.rb +++ b/features/support/setup.rb @@ -27,10 +27,22 @@ # static seed data -# load File.join(Rails.root, 'db', 'seeds.rb') +# Override this so steps work without internet. +module MapsService + def self.get_lat_lng(_) + { lat: 37.8719, lng: -122.2585 } + end +end # Load Rails all factories into cucumber: Before do + load File.join(Rails.root, 'db', 'seeds.rb') + Rails.application.load_seed + ActiveRecord::FixtureSet.reset_cache + fixtures_folder = File.join(Rails.root, "spec", "fixtures") + fixtures = Dir[File.join(fixtures_folder, "*.yml")].map { |f| File.basename(f, ".yml") } + ActiveRecord::FixtureSet.create_fixtures(fixtures_folder, fixtures) + FactoryBot.reload end diff --git a/features/teacher.feature b/features/teacher.feature index cf3b6760..2e89c0ee 100644 --- a/features/teacher.feature +++ b/features/teacher.feature @@ -4,9 +4,6 @@ Feature: teacher login functionality So that I can edit my information I can login via gmail -Background: See data - Given I seed data - Scenario: Logging in failure redirects to new teachers page Given I am on the BJC home page Given I have a teacher Google email From d2c17e3dce9d4bfe6653759dd590c74a23f5d978 Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Mon, 14 Aug 2023 02:13:49 -0700 Subject: [PATCH 41/54] No longer need webdrivers gem, apparently. --- Gemfile | 1 - Gemfile.lock | 5 ----- 2 files changed, 6 deletions(-) diff --git a/Gemfile b/Gemfile index 83a1fc59..bea2d500 100644 --- a/Gemfile +++ b/Gemfile @@ -102,7 +102,6 @@ group :test do # gem "metric_fu" # removed until ruby 3.x compat is fixed. gem "selenium-webdriver" - gem "webdrivers" # Accessibility Testing gem "axe-core-rspec" gem "axe-core-cucumber" diff --git a/Gemfile.lock b/Gemfile.lock index 8f3fd482..d3bfda1d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -565,10 +565,6 @@ GEM activemodel (>= 6.0.0) bindex (>= 0.4.0) railties (>= 6.0.0) - webdrivers (5.0.0) - nokogiri (~> 1.6) - rubyzip (>= 1.3.0) - selenium-webdriver (~> 4.0) webpacker (5.4.4) activesupport (>= 5.2) rack-proxy (>= 0.6.1) @@ -643,7 +639,6 @@ DEPENDENCIES spring tzinfo-data web-console - webdrivers webpacker RUBY VERSION From 999de128dfe375a03bd62f0b2ad943256ed4103c Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Mon, 14 Aug 2023 02:17:32 -0700 Subject: [PATCH 42/54] delint --- features/support/setup.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/features/support/setup.rb b/features/support/setup.rb index 1a09330a..3b7307db 100644 --- a/features/support/setup.rb +++ b/features/support/setup.rb @@ -36,7 +36,7 @@ def self.get_lat_lng(_) # Load Rails all factories into cucumber: Before do - load File.join(Rails.root, 'db', 'seeds.rb') + load File.join(Rails.root, "db", "seeds.rb") Rails.application.load_seed ActiveRecord::FixtureSet.reset_cache fixtures_folder = File.join(Rails.root, "spec", "fixtures") From 4a0567592c2f16eaa3d2ca9458cd3a8d8447b75b Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Tue, 15 Aug 2023 00:16:06 -0700 Subject: [PATCH 43/54] update bundler --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index d3bfda1d..d12584f5 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -645,4 +645,4 @@ RUBY VERSION ruby 3.2.2p53 BUNDLED WITH - 2.1.4 + 2.4.18 From 48db9d1a5b262cec786448e5e6dd8ddd8a0893a6 Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Tue, 15 Aug 2023 00:28:14 -0700 Subject: [PATCH 44/54] run bundle update... --- Gemfile.lock | 8 ++++---- config/environments/test.rb | 2 +- features/support/browsers.rb | 2 +- features/support/setup.rb | 6 ++++-- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index d12584f5..f49e3796 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -137,7 +137,6 @@ GEM rack-test (>= 0.6.3) regexp_parser (>= 1.5, < 3.0) xpath (~> 3.2) - childprocess (4.1.0) coderay (1.1.3) coercible (1.0.0) descendants_tracker (~> 0.0.1) @@ -496,10 +495,10 @@ GEM addressable (>= 2.3.5) faraday (>= 0.17.3, < 3) selectize-rails (0.12.6) - selenium-webdriver (4.1.0) - childprocess (>= 0.5, < 5.0) + selenium-webdriver (4.11.0) rexml (~> 3.2, >= 3.2.5) - rubyzip (>= 1.2.2) + rubyzip (>= 1.2.2, < 3.0) + websocket (~> 1.0) semantic_range (3.0.0) sentry-rails (4.8.3) railties (>= 5.0) @@ -571,6 +570,7 @@ GEM railties (>= 5.2) semantic_range (>= 2.3.0) webrick (1.7.0) + websocket (1.2.9) websocket-driver (0.7.6) websocket-extensions (>= 0.1.0) websocket-extensions (0.1.5) diff --git a/config/environments/test.rb b/config/environments/test.rb index c2ddcf82..e3929085 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -9,7 +9,7 @@ Rails.application.configure do # Settings specified here will take precedence over those in config/application.rb. - config.cache_classes = false + config.cache_classes = true # Do not eager load code on boot. This avoids loading your whole application # just for the purpose of running a single test. If you are using a tool that diff --git a/features/support/browsers.rb b/features/support/browsers.rb index e4d851fa..8cdf6597 100644 --- a/features/support/browsers.rb +++ b/features/support/browsers.rb @@ -86,4 +86,4 @@ Capybara.default_driver = :headless_chrome end -puts "RUNNING CAPYBARA WITH DRIVER #{Capybara.javascript_driver}" +puts "\nRUNNING CAPYBARA WITH DRIVER #{Capybara.javascript_driver}\n\n" diff --git a/features/support/setup.rb b/features/support/setup.rb index 3b7307db..d5981bc7 100644 --- a/features/support/setup.rb +++ b/features/support/setup.rb @@ -36,13 +36,15 @@ def self.get_lat_lng(_) # Load Rails all factories into cucumber: Before do - load File.join(Rails.root, "db", "seeds.rb") + puts "RUNNING BEFORE == Teachers: #{Teacher.count}" + # load File.join(Rails.root, "db", "seeds.rb") Rails.application.load_seed + puts "SEEDS LOADED == Teachers: #{Teacher.count}" ActiveRecord::FixtureSet.reset_cache fixtures_folder = File.join(Rails.root, "spec", "fixtures") fixtures = Dir[File.join(fixtures_folder, "*.yml")].map { |f| File.basename(f, ".yml") } ActiveRecord::FixtureSet.create_fixtures(fixtures_folder, fixtures) - + puts "FIXTURES LOADED == Teachers: #{Teacher.count}" FactoryBot.reload end From 4455472aea70e85355ac3f4f6deed2f58d97774f Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Tue, 15 Aug 2023 01:03:57 -0700 Subject: [PATCH 45/54] WIP cuke runs again...mostly... --- Gemfile.lock | 10 +++++----- features/step_definitions/web_steps.rb | 1 + features/support/browsers.rb | 4 ++-- features/support/setup.rb | 22 ++++++++++------------ 4 files changed, 18 insertions(+), 19 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index f49e3796..9fbf1b7e 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -85,7 +85,7 @@ GEM minitest (>= 5.1) tzinfo (~> 2.0) zeitwerk (~> 2.3) - addressable (2.8.4) + addressable (2.8.5) public_suffix (>= 2.0.2, < 6.0) annotate (3.1.1) activerecord (>= 3.2, < 7.0) @@ -128,7 +128,7 @@ GEM msgpack (~> 1.0) builder (3.2.4) byebug (11.1.3) - capybara (3.36.0) + capybara (3.39.2) addressable matrix mini_mime (>= 0.1.3) @@ -310,7 +310,7 @@ GEM mime-types-data (~> 3.2015) mime-types-data (3.2022.0105) mini_magick (4.11.0) - mini_mime (1.1.2) + mini_mime (1.1.5) mini_portile2 (2.8.4) minitest (5.19.0) msgpack (1.4.2) @@ -328,7 +328,7 @@ GEM net-smtp (0.3.3) net-protocol nio4r (2.5.9) - nokogiri (1.15.3) + nokogiri (1.15.4) mini_portile2 (~> 2.8.2) racc (~> 1.4) notiffany (0.1.3) @@ -437,7 +437,7 @@ GEM rb-fsevent (0.11.0) rb-inotify (0.10.1) ffi (~> 1.0) - regexp_parser (2.2.0) + regexp_parser (2.8.1) rexml (3.2.6) rspec (3.10.0) rspec-core (~> 3.10.0) diff --git a/features/step_definitions/web_steps.rb b/features/step_definitions/web_steps.rb index b35e61d9..027487a8 100644 --- a/features/step_definitions/web_steps.rb +++ b/features/step_definitions/web_steps.rb @@ -35,6 +35,7 @@ def with_scope(locator, &block) Given(/^(?:|I )am on (.+)$/) do |page_name| + debugger visit path_to(page_name) end diff --git a/features/support/browsers.rb b/features/support/browsers.rb index 8cdf6597..8d32ea33 100644 --- a/features/support/browsers.rb +++ b/features/support/browsers.rb @@ -16,7 +16,7 @@ chrome_options.add_preference(:browser, set_download_behavior: { behavior: "allow" }) Capybara.register_driver :chrome do |app| - Capybara::Selenium::Driver.new(app, browser: :chrome, capabilities: [chrome_options]) + Capybara::Selenium::Driver.new(app, browser: :chrome, options: chrome_options) end Capybara.register_driver :headless_chrome do |app| @@ -24,7 +24,7 @@ chrome_options.add_argument("--disable-gpu") chrome_options.add_argument("--window-size=1680,1050") - driver = Capybara::Selenium::Driver.new(app, browser: :chrome, capabilities: [chrome_options]) + driver = Capybara::Selenium::Driver.new(app, browser: :chrome, options: chrome_options) ### Allow file downloads in Google Chrome when headless!!! ### https://bugs.chromium.org/p/chromium/issues/detail?id=696481#c89 diff --git a/features/support/setup.rb b/features/support/setup.rb index d5981bc7..72552d05 100644 --- a/features/support/setup.rb +++ b/features/support/setup.rb @@ -35,18 +35,16 @@ def self.get_lat_lng(_) end # Load Rails all factories into cucumber: -Before do - puts "RUNNING BEFORE == Teachers: #{Teacher.count}" - # load File.join(Rails.root, "db", "seeds.rb") - Rails.application.load_seed - puts "SEEDS LOADED == Teachers: #{Teacher.count}" - ActiveRecord::FixtureSet.reset_cache - fixtures_folder = File.join(Rails.root, "spec", "fixtures") - fixtures = Dir[File.join(fixtures_folder, "*.yml")].map { |f| File.basename(f, ".yml") } - ActiveRecord::FixtureSet.create_fixtures(fixtures_folder, fixtures) - puts "FIXTURES LOADED == Teachers: #{Teacher.count}" - FactoryBot.reload -end +puts "RUNNING BEFORE == Teachers: #{Teacher.count}" +# load File.join(Rails.root, "db", "seeds.rb") +Rails.application.load_seed +puts "SEEDS LOADED == Teachers: #{Teacher.count}" +ActiveRecord::FixtureSet.reset_cache +fixtures_folder = File.join(Rails.root, "spec", "fixtures") +fixtures = Dir[File.join(fixtures_folder, "*.yml")].map { |f| File.basename(f, ".yml") } +ActiveRecord::FixtureSet.create_fixtures(fixtures_folder, fixtures) +puts "FIXTURES LOADED == Teachers: #{Teacher.count}" +FactoryBot.reload RSpec.configure do |config| config.fixture_path = "#{::Rails.root}/spec/fixtures" From af4284be6a9384d96d4b54c59f4102ba0a714e6a Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Tue, 15 Aug 2023 01:04:26 -0700 Subject: [PATCH 46/54] remove debugger --- features/step_definitions/web_steps.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/features/step_definitions/web_steps.rb b/features/step_definitions/web_steps.rb index 027487a8..b35e61d9 100644 --- a/features/step_definitions/web_steps.rb +++ b/features/step_definitions/web_steps.rb @@ -35,7 +35,6 @@ def with_scope(locator, &block) Given(/^(?:|I )am on (.+)$/) do |page_name| - debugger visit path_to(page_name) end From 395ffd5b688ef5f586695603cba012f84bd29eb8 Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Tue, 15 Aug 2023 01:20:36 -0700 Subject: [PATCH 47/54] Cleanup cuke setup... --- features/access.feature | 6 +++--- features/support/setup.rb | 17 ++++++++--------- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/features/access.feature b/features/access.feature index b9930cc4..3b37e633 100644 --- a/features/access.feature +++ b/features/access.feature @@ -5,9 +5,9 @@ Feature: access control for new users or non-admin users Background: Has an Admin and a teacher in DB Given the following teachers exist: - | first_name | last_name | admin | email | school_id | - | Alice | Admin | true | testadminuser@berkeley.edu | 1 | - | Todd | Teacher | false | testteacher@berkeley.edu | 1 | + | first_name | last_name | admin | email | + | Alice | Admin | true | testadminuser@berkeley.edu | + | Todd | Teacher | false | testteacher@berkeley.edu | #Logged-in Teacher (non-admin) Scenario: Schools page as a Teacher diff --git a/features/support/setup.rb b/features/support/setup.rb index 72552d05..08b88dad 100644 --- a/features/support/setup.rb +++ b/features/support/setup.rb @@ -34,17 +34,16 @@ def self.get_lat_lng(_) end end -# Load Rails all factories into cucumber: -puts "RUNNING BEFORE == Teachers: #{Teacher.count}" -# load File.join(Rails.root, "db", "seeds.rb") -Rails.application.load_seed -puts "SEEDS LOADED == Teachers: #{Teacher.count}" -ActiveRecord::FixtureSet.reset_cache +# This loads seeds and fixtures before each scenario. +# TODO: We should (in theory) need to do this only once, but it breaks tests +# Using transactions will reset the state after the first run... fixtures_folder = File.join(Rails.root, "spec", "fixtures") fixtures = Dir[File.join(fixtures_folder, "*.yml")].map { |f| File.basename(f, ".yml") } -ActiveRecord::FixtureSet.create_fixtures(fixtures_folder, fixtures) -puts "FIXTURES LOADED == Teachers: #{Teacher.count}" -FactoryBot.reload +Before do + ActiveRecord::FixtureSet.reset_cache + ActiveRecord::FixtureSet.create_fixtures(fixtures_folder, fixtures) + Rails.application.load_seed +end RSpec.configure do |config| config.fixture_path = "#{::Rails.root}/spec/fixtures" From bb6880fa9804b0bd86669a4bb15cfbd4f54df004 Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Tue, 15 Aug 2023 01:43:56 -0700 Subject: [PATCH 48/54] update email template seeds so specs pass --- app/models/email_template.rb | 3 ++- db/seed_data.rb | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/app/models/email_template.rb b/app/models/email_template.rb index cb483991..a8042c03 100644 --- a/app/models/email_template.rb +++ b/app/models/email_template.rb @@ -21,12 +21,13 @@ class EmailTemplate < ApplicationRecord validates :title, inclusion: TeacherMailer.instance_methods(false).map { |method| method.to_s.titlecase }, if: -> { self.required? } + validates :body, presence: true before_destroy :prevent_deleting_required_emails def accepts_custom_reason? # search for 'reason' in a liquid template - self.body.match?(/{{\s*reason/) + body.match?(/{{\s*reason/) end private diff --git a/db/seed_data.rb b/db/seed_data.rb index 083230fc..008521f5 100644 --- a/db/seed_data.rb +++ b/db/seed_data.rb @@ -25,7 +25,7 @@ module SeedData

FORM_SUBMISSION - @deny_email = <<-DENY_EMAIL + @basic_email_with_reason = <<-DENY_EMAIL

{{ reason | strip_tags }}

@@ -95,7 +95,7 @@ def self.emails subject: "Deny Email" }, { - body: @request_info_email, + body: @basic_email_with_reason, path: "teacher_mailer/request_info_email", locale: nil, handler: "liquid", From cb1a039e76d084ca3a52a9f5c0bd562f86186002 Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Tue, 15 Aug 2023 01:45:16 -0700 Subject: [PATCH 49/54] Update specs test to reflect form saying submit --- features/email_template.feature | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/features/email_template.feature b/features/email_template.feature index d6be4304..9b67dc3a 100644 --- a/features/email_template.feature +++ b/features/email_template.feature @@ -22,7 +22,7 @@ Scenario: Logging in as an admin should be able to edit email templates And I follow "Welcome Email" Then I should see hidden element "Hi {{teacher_first_name}}" And I fill in TinyMCE email form with "Test Edit" - And I press "Update" + And I press "Submit" And I follow "Welcome Email" Then I should see hidden element "Test Edit" @@ -30,6 +30,6 @@ Scenario: Logging in as an admin should be able to edit email subject Given I am on the email templates index And I follow "Welcome Email" And I fill in "email_template_subject" with "Test Subject" - And I press "Update" + And I press "Submit" And I follow "Welcome Email" Then the "email_template_subject" field should contain "Test Subject" From 654a915403d756cfc3ef93f91aa13cbff973215f Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Tue, 15 Aug 2023 01:57:47 -0700 Subject: [PATCH 50/54] Pages admin features fully work -- fixes for bad test data --- features/pages_admin.feature | 34 +++++++++++++++++----------------- spec/fixtures/pages.yml | 4 ++-- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/features/pages_admin.feature b/features/pages_admin.feature index 8e2078e3..3189dbea 100644 --- a/features/pages_admin.feature +++ b/features/pages_admin.feature @@ -32,7 +32,7 @@ Scenario: Pressing "New Page" button should take user to new page form Scenario: Successfully creating a new page redirects to that page Given I am on the new pages page And I fill in "page_title" with "Test Title" - And I fill in "page_url_slug" with "basic_slug" + And I fill in "page_url_slug" with "cucumber_test_page" And I fill in "page_category" with "Test Category" And I fill in the page HTML content with "This is a test" And I choose "inlineRadioAdmin" @@ -44,7 +44,7 @@ Scenario: Successfully creating a new page redirects to that page Scenario: Creating a page without a title fails Given I am on the new pages page - And I fill in "page_url_slug" with "basic_slug" + And I fill in "page_url_slug" with "cucumber_test_page" And I fill in "page_category" with "Test Category" And I choose "inlineRadioAdmin" And I press "Submit" @@ -73,7 +73,7 @@ Scenario: Creating a page without choosing permissions fails Scenario: I create a new page and I can see it on the index page Given I am on the new pages page And I fill in "page_title" with "Test Title" - And I fill in "page_url_slug" with "basic_slug" + And I fill in "page_url_slug" with "cucumber_test_page" And I fill in "page_category" with "Test Category" And I fill in the page HTML content with "This is a test" And I choose "inlineRadioAdmin" @@ -194,15 +194,15 @@ Scenario: Correctly store user's full name and create date. Scenario: Can edit pages with correct prefilled content in the form. Given I am on the new pages page And I fill in "page_title" with "Test Title" - And I fill in "page_url_slug" with "basic_slug" + And I fill in "page_url_slug" with "cucumber_test_page" And I choose "inlineRadioAdmin" And I fill in the page HTML content with "This is a test" And I press "Submit" And I follow "Pages" - And I press the edit button for "basic_slug" - Then I should be on the edit pages page for "basic_slug" + And I press the edit button for "cucumber_test_page" + Then I should be on the edit pages page for "cucumber_test_page" And the "page_title" field should contain "Test Title" - And the "page_url_slug" field should contain "basic_slug" + And the "page_url_slug" field should contain "cucumber_test_page" And I should see the page HTML content containing "This is a test" Then I fill in "page_title" with "New Title" And I fill in "page_url_slug" with "new_slug" @@ -213,18 +213,18 @@ Scenario: Can edit pages with correct prefilled content in the form. And I should see "Public" Then I should be on the pages index And I should not see "Test Title" - And I should not see "basic_slug" + And I should not see "cucumber_test_page" Scenario: Can update page even if no changes Given I am on the new pages page And I fill in "page_title" with "Test Title" - And I fill in "page_url_slug" with "basic_slug" + And I fill in "page_url_slug" with "cucumber_test_page" And I choose "inlineRadioAdmin" And I fill in the page HTML content with "This is a test" And I press "Submit" And I follow "Pages" - And I press the edit button for "basic_slug" - Then I should be on the edit pages page for "basic_slug" + And I press the edit button for "cucumber_test_page" + Then I should be on the edit pages page for "cucumber_test_page" And I press "Update" Then I should be on the pages index And I should see "Test Title" @@ -233,7 +233,7 @@ Scenario: Can update page even if no changes Scenario: Attempting to update page with taken slug doesn't delete form input Given I am on the new pages page And I fill in "page_title" with "Test Title" - And I fill in "page_url_slug" with "basic_slug" + And I fill in "page_url_slug" with "cucumber_test_page" And I choose "inlineRadioAdmin" And I fill in the page HTML content with "This is a test" And I press "Submit" @@ -247,13 +247,13 @@ Scenario: Attempting to update page with taken slug doesn't delete form input And I press the edit button for "basic_slug_2" Then I should be on the edit pages page for "basic_slug_2" And I fill in "page_title" with "New Title" - And I fill in "page_url_slug" with "basic_slug" + And I fill in "page_url_slug" with "cucumber_test_page" And I fill in the page HTML content with "New page body." And I press "Update" Then I should see "Update New Title" And I should see "URL slug has already been taken" And the "page_title" field should contain "New Title" - And the "page_url_slug" field should contain "basic_slug" + And the "page_url_slug" field should contain "cucumber_test_page" And I should see the page HTML content containing "New page body." Scenario: Clicking radio button text selects that radio button @@ -268,13 +268,13 @@ Scenario: Clicking radio button text selects that radio button Scenario: Cannot update admin page to be default Given I am on the new pages page And I fill in "page_title" with "Test Default Title" - And I fill in "page_url_slug" with "basic_slug" + And I fill in "page_url_slug" with "cucumber_test_page" And I choose "inlineRadioAdmin" And I fill in the page HTML content with "This is a test" And I press "Submit" And I follow "Pages" - And I press the edit button for "basic_slug" - Then I should be on the edit pages page for "basic_slug" + And I press the edit button for "cucumber_test_page" + Then I should be on the edit pages page for "cucumber_test_page" And I check "page_default" checkbox And I press "Update" Then I should see "Update Test Default Title" diff --git a/spec/fixtures/pages.yml b/spec/fixtures/pages.yml index be0a9de3..5d738390 100644 --- a/spec/fixtures/pages.yml +++ b/spec/fixtures/pages.yml @@ -30,7 +30,7 @@ basic_page: viewer_permissions: "Verified Teacher" default: false creator_id: 1 - last_editor_id: 0 + last_editor_id: 1 admin_page: url_slug: basic_slug_2 @@ -39,7 +39,7 @@ admin_page: html:

An admin page

default: false creator_id: 1 - last_editor_id: 0 + last_editor_id: 1 about_page: url_slug: about From 4c8aa967369da011b594798ef71026382c9abff6 Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Tue, 15 Aug 2023 02:16:16 -0700 Subject: [PATCH 51/54] Cleanup final cuke steps for bad data --- db/seed_data.rb | 2 +- features/school_form_submission.feature | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/db/seed_data.rb b/db/seed_data.rb index 008521f5..f978ff37 100644 --- a/db/seed_data.rb +++ b/db/seed_data.rb @@ -31,7 +31,7 @@ module SeedData

DENY_EMAIL - @deny_email1 = <<-DENY_EMAIL1 + @deny_email = <<-DENY_EMAIL1

Dear [Recipient's Name],

Thank you for your email. We have received your message and we are sorry to inform you that your application has been rejected.

Best regards,

diff --git a/features/school_form_submission.feature b/features/school_form_submission.feature index b349c776..11371a11 100644 --- a/features/school_form_submission.feature +++ b/features/school_form_submission.feature @@ -19,8 +19,8 @@ Scenario: Viewing the schools page should show the all current schools And I follow "Log In" Then I can log in with Google And I am on the schools page - # Berkeley already has 2 users. - Then I should see "UC Berkeley" with "6" in a table row + # Don't try to count berkeley teachers due to seed data. + Then I should see "UC Berkeley" And I should see "UC Irvine" with "1" in a table row And I should see "UC Scam Diego" with "0" in a table row From 2c46d4daa205bf025da4c7b429bb57bc31bfeb8d Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Tue, 15 Aug 2023 03:27:47 -0700 Subject: [PATCH 52/54] Fixup microsoft/omniauth config, beef up logging --- app/controllers/sessions_controller.rb | 31 +++++++++++++++++++++----- config/initializers/omniauth.rb | 5 +++-- config/routes.rb | 1 + 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 249100cf..ef4a6440 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -11,8 +11,7 @@ def destroy end def omniauth_callback - omniauth_data = omniauth_info - user = Teacher.user_from_omniauth(omniauth_data) + user = Teacher.user_from_omniauth(omniauth_info) if user.present? user.last_session_at = Time.zone.now user.try_append_ip(request.remote_ip) @@ -20,9 +19,16 @@ def omniauth_callback user.save! log_in(user) else - Sentry.capture_message("OAuth Login Failure") - session[:auth_data] = omniauth_data - flash[:alert] = "We couldn't find an account for #{omniauth_data.email}. Please submit a new request." + crumb = Sentry::Breadcrumb.new( + category: "auth", + data: { omniauth_env: omniauth_info }, + message: "No User Found failure", + level: "info" + ) + Sentry.add_breadcrumb(crumb) + Sentry.capture_message("Omniauth No User Found") + session[:auth_data] = omniauth_info + flash[:alert] = "We couldn't find an account for #{omniauth_info.email}. Please submit a new request." redirect_to new_teacher_path end end @@ -32,7 +38,20 @@ def omniauth_info end def omniauth_failure + crumb = Sentry::Breadcrumb.new( + category: "auth", + data: { + omniauth_env: request.env["omniauth.auth"], + omniauth_error: request.env["omniauth.error"], + message: params[:message], + strategy: params[:strategy] + }, + message: "Authentication failure", + level: "info" + ) + Sentry.add_breadcrumb(crumb) + Sentry.capture_message("Omniauth Failure") redirect_to root_url, - alert: "Login failed unexpectedly. Please reach out to contact@bjc.berkeley.edu" + alert: "Login failed unexpectedly. Please reach out to contact@bjc.berkeley.edu (#{params[:message]})" end end diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb index 3eed2ea2..520e8599 100644 --- a/config/initializers/omniauth.rb +++ b/config/initializers/omniauth.rb @@ -27,7 +27,7 @@ def header(title, header_info) provider :google_oauth2, ENV["GOOGLE_CLIENT_ID"], ENV["GOOGLE_CLIENT_SECRET"], skip_jwt: true provider :microsoft_graph, ENV["MICROSOFT_CLIENT_ID"], ENV["MICROSOFT_CLIENT_SECRET"], - { scope: "openid profile email" } + { scope: "openid profile email User.read" } provider :discourse, sso_url: ENV["SNAP_CLIENT_URL"], sso_secret: ENV["SNAP_CLIENT_SECRET"] @@ -37,5 +37,6 @@ def header(title, header_info) end OmniAuth.config.on_failure = Proc.new do |env| - SessionsController.action(:omniauth_failure).call(env) + puts "OMNIAUTH ON FAILURE PROC" + OmniAuth::FailureEndpoint.new(env).redirect_to_failure end diff --git a/config/routes.rb b/config/routes.rb index 231435d9..c9a0a014 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -23,6 +23,7 @@ get "/login", to: "sessions#new", as: "login" delete "/logout", to: "sessions#destroy", as: "logout" get "/auth/:provider/callback", to: "sessions#omniauth_callback", as: "omniauth_callback" + get "/auth/failure", to: "sessions#omniauth_failure", as: "omniauth_failure" get "/dashboard", to: "main#dashboard", as: "dashboard" end From 3a8d33ecbfc28ac65859cc180d4303e23bf01def Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Tue, 15 Aug 2023 03:35:01 -0700 Subject: [PATCH 53/54] disable some WIP 169 work around email options --- app/controllers/teachers_controller.rb | 2 ++ app/views/main/_deny_modal.html.erb | 6 +++--- app/views/main/dashboard.html.erb | 10 +++++----- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/app/controllers/teachers_controller.rb b/app/controllers/teachers_controller.rb index fd9a6fe1..163157ea 100644 --- a/app/controllers/teachers_controller.rb +++ b/app/controllers/teachers_controller.rb @@ -132,9 +132,11 @@ def validate redirect_to root_path end + # TODO: Handle the more info / intermediate status route. def deny @teacher.denied! if !params[:skip_email].present? + # TODO: Update dropdown to select the email template. TeacherMailer.deny_email(@teacher, params[:reason]).deliver_now end redirect_to root_path diff --git a/app/views/main/_deny_modal.html.erb b/app/views/main/_deny_modal.html.erb index 4f0a7b67..cf8b56f7 100644 --- a/app/views/main/_deny_modal.html.erb +++ b/app/views/main/_deny_modal.html.erb @@ -9,15 +9,15 @@
<%= label_tag 'reason', 'Reason', class: 'control-label' %> <%= text_field_tag :reason, params[:reason], class: "form-control" %> -
+
<%= check_box_tag :skip_email, 'skip_email', false, class: 'form-check-input' %> <%= label_tag 'skip_email', 'Skip email notifcation?', class: 'form-check-label' %> diff --git a/app/views/main/dashboard.html.erb b/app/views/main/dashboard.html.erb index 7c2f02d0..a12eed1c 100644 --- a/app/views/main/dashboard.html.erb +++ b/app/views/main/dashboard.html.erb @@ -15,15 +15,15 @@
PurposeReason?Required? Actions
<%= link_to(template.title, edit_email_template_path(template)) %><%= check_or_x(template.accepts_custom_reason?) %><%= check_or_x(template.required?) %> - - <%= link_to("Edit", edit_email_template_path(template), class: "btn btn-info") %> - <%= link_to("❌", email_template_path(template), method: "delete", class: "btn btn-outline-danger", data: {confirm: "Are you sure?"}) unless template.required? %> - + + <%= link_to("Edit", edit_email_template_path(template), class: "btn btn-info") %> + <%= link_to("❌", email_template_path(template), method: "delete", class: "btn btn-outline-danger", data: {confirm: "Are you sure?"}) unless template.required? %> +
<%= button_to("✔️", validate_teacher_path(teacher.id), - class: 'btn btn-outline-success', type: 'button') %> - - From 312bddad402da3f3b88b2838c930f6c8b9778356 Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Tue, 15 Aug 2023 03:52:01 -0700 Subject: [PATCH 54/54] minor tidying --- config/initializers/omniauth.rb | 1 - config/routes.rb | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb index 520e8599..be55bacb 100644 --- a/config/initializers/omniauth.rb +++ b/config/initializers/omniauth.rb @@ -37,6 +37,5 @@ def header(title, header_info) end OmniAuth.config.on_failure = Proc.new do |env| - puts "OMNIAUTH ON FAILURE PROC" OmniAuth::FailureEndpoint.new(env).redirect_to_failure end diff --git a/config/routes.rb b/config/routes.rb index c9a0a014..beab79d8 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -18,7 +18,7 @@ end resources :schools resources :pages, param: :url_slug - resources :email_templates, only: [:index, :update, :edit, :new, :destroy, :create] + resources :email_templates, except: [:show] get "/login", to: "sessions#new", as: "login" delete "/logout", to: "sessions#destroy", as: "logout"