@@ -158,6 +159,17 @@ status ONLY IF the person viewing this page is an admin. %>
diff --git a/app/views/workshops/edit.html.erb b/app/views/workshops/edit.html.erb
index bed91225..82ee5a36 100644
--- a/app/views/workshops/edit.html.erb
+++ b/app/views/workshops/edit.html.erb
@@ -1,2 +1,13 @@
-
From 8a3415953537ea22c0cb5753d1d1ab3c10cf6319 Mon Sep 17 00:00:00 2001
From: Arush Chhatrapati <65872167+ArushC@users.noreply.github.com>
Date: Thu, 21 Mar 2024 16:48:56 -0700
Subject: [PATCH 14/88] minor delete unnecessary comment
---
features/step_definitions/form_steps.rb | 1 -
1 file changed, 1 deletion(-)
diff --git a/features/step_definitions/form_steps.rb b/features/step_definitions/form_steps.rb
index ec901e82..58e5039c 100644
--- a/features/step_definitions/form_steps.rb
+++ b/features/step_definitions/form_steps.rb
@@ -82,7 +82,6 @@
# Necessary for the Admin School create page
page.execute_script('$("#submit_button").show()')
fill_in("School Name", with: text)
- # page.find(".label-required", text: "School Name").click
end
Then(/^"([^"]*)" click and fill option for "([^"]*)"(?: within "([^"]*)")?$/) do |value|
From ce2c40a38a655f550fb730d216fcffa7e2d256c4 Mon Sep 17 00:00:00 2001
From: Arush Chhatrapati <65872167+ArushC@users.noreply.github.com>
Date: Thu, 21 Mar 2024 17:14:51 -0700
Subject: [PATCH 15/88] minor comment change
---
app/views/schools/_form.html.erb | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/app/views/schools/_form.html.erb b/app/views/schools/_form.html.erb
index b0b3a37e..33316ce8 100644
--- a/app/views/schools/_form.html.erb
+++ b/app/views/schools/_form.html.erb
@@ -111,7 +111,7 @@
function handleCountryChange() {
if (countrySelected.val() === 'US') {
stateSelectContainer.show();
- stateSelect.attr('required', '') //make state select required
+ stateSelect.attr('required', '') //make state select required if country USA
stateTextfieldContainer.hide();
stateTextfield.removeAttr('name');
@@ -119,7 +119,7 @@
} else {
stateTextfieldContainer.show();
- stateSelect.removeAttr('required')
+ stateSelect.removeAttr('required') //else make state select not required
stateSelectContainer.hide();
From 291715ce7a7aa3b713fc69e775c398552caa39c2 Mon Sep 17 00:00:00 2001
From: Arush Chhatrapati <65872167+ArushC@users.noreply.github.com>
Date: Fri, 22 Mar 2024 10:55:53 -0700
Subject: [PATCH 16/88] added relevant cucumber step and reverted back to older
version of code
---
app/models/teacher.rb | 7 +++----
features/step_definitions/form_steps.rb | 5 +++++
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/app/models/teacher.rb b/app/models/teacher.rb
index d3affd0a..effcf411 100644
--- a/app/models/teacher.rb
+++ b/app/models/teacher.rb
@@ -38,8 +38,7 @@
# fk_rails_... (school_id => schools.id)
#
class Teacher < ApplicationRecord
-
- WORLD_LANGUAGES = [ 'Afrikaans', 'Albanian', 'Arabic', 'Armenian', 'Basque', 'Bengali', 'Bulgarian', 'Catalan', 'Cambodian', 'Chinese (Mandarin)', 'Croatian', 'Czech', 'Danish', 'Dutch', 'English', 'Estonian', 'Fiji', 'Finnish', 'French', 'Georgian', 'German', 'Greek', 'Gujarati', 'Hebrew', 'Hindi', 'Hungarian', 'Icelandic', 'Indonesian', 'Irish', 'Italian', 'Japanese', 'Javanese', 'Korean', 'Latin', 'Latvian', 'Lithuanian', 'Macedonian', 'Malay', 'Malayalam', 'Maltese', 'Maori', 'Marathi', 'Mongolian', 'Nepali', 'Norwegian', 'Persian', 'Polish', 'Portuguese', 'Punjabi', 'Quechua', 'Romanian', 'Russian', 'Samoan', 'Serbian', 'Slovak', 'Slovenian', 'Spanish', 'Swahili', 'Swedish ', 'Tamil', 'Tatar', 'Telugu', 'Thai', 'Tibetan', 'Tonga', 'Turkish', 'Ukrainian', 'Urdu', 'Uzbek', 'Vietnamese', 'Welsh', 'Xhosa' ].freeze
+ WORLD_LANGUAGES = [ "Afrikaans", "Albanian", "Arabic", "Armenian", "Basque", "Bengali", "Bulgarian", "Catalan", "Cambodian", "Chinese (Mandarin)", "Croatian", "Czech", "Danish", "Dutch", "English", "Estonian", "Fiji", "Finnish", "French", "Georgian", "German", "Greek", "Gujarati", "Hebrew", "Hindi", "Hungarian", "Icelandic", "Indonesian", "Irish", "Italian", "Japanese", "Javanese", "Korean", "Latin", "Latvian", "Lithuanian", "Macedonian", "Malay", "Malayalam", "Maltese", "Maori", "Marathi", "Mongolian", "Nepali", "Norwegian", "Persian", "Polish", "Portuguese", "Punjabi", "Quechua", "Romanian", "Russian", "Samoan", "Serbian", "Slovak", "Slovenian", "Spanish", "Swahili", "Swedish ", "Tamil", "Tatar", "Telugu", "Thai", "Tibetan", "Tonga", "Turkish", "Ukrainian", "Urdu", "Uzbek", "Vietnamese", "Welsh", "Xhosa" ].freeze
validates :first_name, :last_name, :email, :status, presence: true
validates :email, uniqueness: true
@@ -169,11 +168,11 @@ def self.education_level_options
def self.language_options
WORLD_LANGUAGES
- #language_codes = ISO3166::Country.all.map { |country| country.languages}.flatten.uniq
+ # language_codes = ISO3166::Country.all.map { |country| country.languages}.flatten.uniq
end
def display_languages
- languages.select { |value| WORLD_LANGUAGES.include?(value)}.join(', ')
+ languages.select { |value| WORLD_LANGUAGES.include?(value) }.join(", ")
end
def display_education_level
diff --git a/features/step_definitions/form_steps.rb b/features/step_definitions/form_steps.rb
index 567706c6..fd42469b 100644
--- a/features/step_definitions/form_steps.rb
+++ b/features/step_definitions/form_steps.rb
@@ -24,6 +24,11 @@
select(input, from: "application_status_select_value")
end
+When(/^I select "(.*?)" from the languages dropdown$/) do |option|
+ first(".selectize-input").click # Click on the dropdown to open it
+ find(".selectize-dropdown-content .option", text: option).click # Click on the desired option
+end
+
Given(/^I set my request reason as "(.*)"$/) do |input|
fill_in("request_reason", with: input)
end
From ddce70120d6b773b88a279678fb0df139c768691 Mon Sep 17 00:00:00 2001
From: Arush Chhatrapati <65872167+ArushC@users.noreply.github.com>
Date: Fri, 22 Mar 2024 12:52:46 -0700
Subject: [PATCH 17/88] finished testing for languages feature and added
backend validations
---
app/models/teacher.rb | 14 ++++++++-
features/admin.feature | 35 ++++++++++++++++++++++
features/step_definitions/form_steps.rb | 18 ++++++++++-
features/step_definitions/teacher_steps.rb | 12 ++++++--
features/teacher.feature | 2 ++
5 files changed, 77 insertions(+), 4 deletions(-)
diff --git a/app/models/teacher.rb b/app/models/teacher.rb
index effcf411..65bce8a2 100644
--- a/app/models/teacher.rb
+++ b/app/models/teacher.rb
@@ -44,6 +44,8 @@ class Teacher < ApplicationRecord
validates :email, uniqueness: true
validates :personal_email, uniqueness: true, if: -> { personal_email.present? }
validate :ensure_unique_personal_email, if: -> { email_changed? || personal_email_changed? }
+ validate :valid_languages
+ before_validation :sort_and_clean_languages
enum application_status: {
validated: "Validated",
@@ -172,7 +174,17 @@ def self.language_options
end
def display_languages
- languages.select { |value| WORLD_LANGUAGES.include?(value) }.join(", ")
+ languages.join(", ")
+ end
+
+ def valid_languages
+ languages.all? { |value| WORLD_LANGUAGES.include?(value) }
+ end
+
+ def sort_and_clean_languages
+ # a weird selectize bug results in the empty string sometimes included in languages
+ # so remove all occurences of empty string
+ languages.sort!.reject!(&:blank?)
end
def display_education_level
diff --git a/features/admin.feature b/features/admin.feature
index 23882731..3d1ba0e4 100644
--- a/features/admin.feature
+++ b/features/admin.feature
@@ -94,6 +94,41 @@ Feature: basic admin functionality
And I press "Update"
Then I see a confirmation "Saved"
+ Scenario: Teacher info displays alphabetically ordered comma separated classroom languages
+ Given the following schools exist:
+ | name | country | city | state | website |
+ | UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu |
+ Given the following teachers exist:
+ | first_name | last_name | admin | email | school | languages |
+ | Joseph | Mamoa | false | testteacher@berkeley.edu | UC Berkeley | - English\n- French\n- Spanish |
+ Given I am on the BJC home page
+ And I have an admin email
+ And I follow "Log In"
+ Then I can log in with Google
+ When I go to the show page for Joseph Mamoa
+ Then I should see "English, French, Spanish"
+
+ Scenario: Adding/removing classroom languages in arbitrary order still displays alphabetically sorted
+ Given the following schools exist:
+ | name | country | city | state | website |
+ | UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu |
+ Given the following teachers exist:
+ | first_name | last_name | admin | email | school | languages |
+ | Joseph | Mamoa | false | testteacher@berkeley.edu | UC Berkeley | - English\n- Hindi |
+ Given I am on the BJC home page
+ And I have an admin email
+ And I follow "Log In"
+ Then I can log in with Google
+ When I go to the edit page for Joseph Mamoa
+ And I select "Spanish" from the languages dropdown
+ And I select "French" from the languages dropdown
+ And I select "German" from the languages dropdown
+ And I remove "English" from the languages dropdown
+ And I remove "Hindi" from the languages dropdown
+ And I press "Update"
+ And I go to the show page for Joseph Mamoa
+ Then I should see "French, German, Spanish"
+
Scenario: Changing application status as admin sends emails
Given the following schools exist:
| name | country | city | state | website | grade_level | school_type |
diff --git a/features/step_definitions/form_steps.rb b/features/step_definitions/form_steps.rb
index fd42469b..f5d4621c 100644
--- a/features/step_definitions/form_steps.rb
+++ b/features/step_definitions/form_steps.rb
@@ -24,11 +24,28 @@
select(input, from: "application_status_select_value")
end
+# assumes that languages dropdown is the FIRST selectize menu to appear on the page
When(/^I select "(.*?)" from the languages dropdown$/) do |option|
first(".selectize-input").click # Click on the dropdown to open it
find(".selectize-dropdown-content .option", text: option).click # Click on the desired option
end
+# also assumes that languages dropdown is the FIRST selectize menu to appear on the page
+When(/^I remove "(.*?)" from the languages dropdown$/) do |item|
+ first(".selectize-input .item", text: item).find(".remove").click
+end
+
+Then(/^the languages dropdown should have the option "(.*?)" selected$/) do |selected_option|
+ # Find the Selectize dropdown by its CSS class
+ selectize_dropdown = first(".selectize-input")
+
+ # Find the selected option within the dropdown
+ selected_option_element = selectize_dropdown.find(".item", text: selected_option)
+
+ # Assert that the selected option exists
+ expect(selected_option_element).to be_visible
+end
+
Given(/^I set my request reason as "(.*)"$/) do |input|
fill_in("request_reason", with: input)
end
@@ -87,7 +104,6 @@
# Necessary for the Admin School create page
page.execute_script('$("#submit_button").show()')
fill_in("School Name", with: text)
- # page.find(".label-required", text: "School Name").click
end
Then(/^"([^"]*)" click and fill option for "([^"]*)"(?: within "([^"]*)")?$/) do |value|
diff --git a/features/step_definitions/teacher_steps.rb b/features/step_definitions/teacher_steps.rb
index fd34c692..07798cee 100644
--- a/features/step_definitions/teacher_steps.rb
+++ b/features/step_definitions/teacher_steps.rb
@@ -52,12 +52,20 @@
more_info: "I'm teaching a college course",
admin: false,
personal_website: "https://snap.berkeley.edu",
- application_status: "Not Reviewed"
+ application_status: "Not Reviewed",
+ languages: ["English"]
}
teachers_table.symbolic_hashes.each do |teacher|
teachers_default.each do |key, value|
- teacher[key] = teacher[key].presence || value
+ # Parse the 'languages' field as an array of strings using YAML.safe_load
+ if key == :languages
+ languages = if teacher[key].present? then YAML.safe_load(teacher[key]) else nil end
+ teacher[key] = languages.presence || value
+ else
+ # Handle other fields as usual
+ teacher[key] = teacher[key].presence || value
+ end
end
school_name = teacher.delete(:school)
diff --git a/features/teacher.feature b/features/teacher.feature
index 9242618a..8edbb36c 100644
--- a/features/teacher.feature
+++ b/features/teacher.feature
@@ -43,9 +43,11 @@ Scenario: Logging in as a teacher with Google account should be able to edit the
And I enter my "City" as "Cupertino"
And I select "CA" from "State" dropdown
And I enter my "School Website" as "https://chs.fuhsd.org"
+ And I select "Spanish" from the languages dropdown
And I press "Update"
Then I see a confirmation "Successfully updated your information"
Then the "First Name" field should contain "Joe"
+ And the languages dropdown should have the option "Spanish" selected
Scenario: Logging in as a teacher with Microsoft account should be able to edit their info
Given the following schools exist:
From edfedf57ad778adae8b275884ebc9e593b75f317 Mon Sep 17 00:00:00 2001
From: JacksonXu33
Date: Fri, 22 Mar 2024 18:28:56 -0700
Subject: [PATCH 18/88] crud, but no schema yet
---
.../pd_registrations_controller.rb | 41 ++++++++++++++++---
.../professional_developments_controller.rb | 41 +++++++++++++------
app/models/pd_registration.rb | 6 +++
app/models/professional_development.rb | 5 +++
app/models/teacher.rb | 2 +
5 files changed, 76 insertions(+), 19 deletions(-)
diff --git a/app/controllers/pd_registrations_controller.rb b/app/controllers/pd_registrations_controller.rb
index e92ea50f..e51c8ccf 100644
--- a/app/controllers/pd_registrations_controller.rb
+++ b/app/controllers/pd_registrations_controller.rb
@@ -1,18 +1,47 @@
# frozen_string_literal: true
+# Not sure how the ids are going to work with professional development.
+# With a belongs to relationship, depending on customer implementation,
+# might have 2 ids in params that we need to distinguish between.
class PdRegistrationsController < ApplicationController
+ def index
+ set_pds
+ end
+
+ def show
+ @pd_registration = PdRegistration.find(params[:id])
+ end
+
+ def new
+ @pd_registration = PdRegistration.new
+ end
+
+ def edit
+ @pd_registration = PdRegistration.find(params[:id])
+ end
+
def create
- flash[:danger] = "Create feature is not yet implemented."
- redirect_to professional_development_path(params[:professional_development_id])
+ @pd_registration = PdRegistration.new(pd_registration_params)
+ if !@pd_registration.save
+ flash.now[:alert] = "Failed to save registration: #{@pd_registration.errors.full_messages.join(", ")}"
+ render :new
+ end
+ redirect_to pd_registrations_path, success: "PD registration created successfully."
end
def update
- flash[:danger] = "Update feature is not yet implemented."
- redirect_to professional_development_path(params[:professional_development_id])
+ @pd_registration = PdRegistration.find(params[:id])
+ if !@pd_registration.update(pd_registration_params)
+ flash.now[:alert] = "Failed to save registration: #{@pd_registration.errors.full_messages.join(", ")}"
+ render "edit"
+ end
+ redirect_to pd_registrations_path, success: "Saved the PD registration."
end
def destroy
- flash[:danger] = "Destroy feature is not yet implemented."
- redirect_to professional_development_path(params[:professional_development_id])
+ if !@pd_registration.destroy
+ redirect_back(fallback_location: pd_registrations_path, alert: "Failed to delete PD registration.")
+ end
+ redirect_to pd_registrations_path, success: "Deleted PD registration successfully."
end
end
diff --git a/app/controllers/professional_developments_controller.rb b/app/controllers/professional_developments_controller.rb
index bc91219a..3a195a7f 100644
--- a/app/controllers/professional_developments_controller.rb
+++ b/app/controllers/professional_developments_controller.rb
@@ -3,37 +3,56 @@
class ProfessionalDevelopmentsController < ApplicationController
# TODO: revise any method using `set_pds` to use `MockProfessionalDevelopments.all` instead. It's currently used for mocking data.
before_action :set_pds, only: [:show, :edit, :update, :destroy]
+ before_action :require_login
+ before_action :require_admin
def index
set_pds
end
def show
- @professional_development = @professional_developments.find { |pd| pd.id == params[:id].to_i }
+ @professional_development = ProfessionalDevelopment.find(params[:id])
+ @pd_registrations = @professional_development.pd_registrations
end
def new
@professional_development = ProfessionalDevelopment.new
- load_ordered_pds
end
def edit
- @professional_development = @professional_developments.find { |pd| pd.id == params[:id].to_i }
+ @professional_development = ProfessionalDevelopment.find(params[:id])
+ @pd_registrations = @professional_development.pd_registrations
end
def create
- flash[:danger] = "Create is not yet implemented."
- redirect_to new_professional_development_path
+ @professional_development = ProfessionalDevelopment.find_by(name: professional_development_params[:name])
+ if @professional_development
+ flash.now[:alert] = "A professional development with the name #{@professional_development.name} already exists."
+ render :new
+ end
+ @professional_development = ProfessionalDevelopment.new(professional_development_params)
+ if !@professional_development.save
+ flash.now[:alert] = "Failed to save #{@professional_development.name}: #{@professional_development.errors.full_messages.join(", ")}"
+ render :new
+ end
+ redirect_to professional_developments_path, success: "Professional development created successfully."
end
def update
- flash[:danger] = "Update feature is not yet implemented."
- redirect_to edit_professional_development_path
+ @professional_development = ProfessionalDevelopment.find(params[:id])
+ @pd_registrations = @professional_development.pd_registrations
+ if !@professional_development.update(professional_development_params)
+ flash.now[:alert] = "Failed to save #{@professional_development.name}: #{@professional_development.errors.full_messages.join(", ")}"
+ render "edit"
+ end
+ redirect_to professional_developments_path, success: "Saved #{@professional_development.name}"
end
def destroy
- flash[:danger] = "Destroy feature is not yet implemented."
- redirect_to professional_developments_path
+ if !@professional_development.destroy
+ redirect_back(fallback_location: professional_developments_path, alert: "Failed to delete #{@professional_development.name}")
+ end
+ redirect_to professional_developments_path, success: "Deleted #{@professional_development.name} successfully."
end
def set_pds
@@ -85,8 +104,4 @@ def set_pds
)
]
end
-
- def load_ordered_pds
- # not yet implemented
- end
end
diff --git a/app/models/pd_registration.rb b/app/models/pd_registration.rb
index c86744c8..4585f327 100644
--- a/app/models/pd_registration.rb
+++ b/app/models/pd_registration.rb
@@ -13,6 +13,12 @@ class PdRegistration
attribute :role, :string
attribute :teacher_name, :string # Adding this for convenience in mocking
+ # probably don't need the 3 id attributes (id, teacherid, pdid) if adding this relationship status into model
+ belongs_to :professional_development
+ belongs_to :teacher
+
+ validates :professional_development_id, uniqueness: { scope: :teacher_id, message: "Teacher already has a registration for this PD" }
+
def initialize(attributes = {})
super(attributes)
end
diff --git a/app/models/professional_development.rb b/app/models/professional_development.rb
index 2954844c..ce2b835a 100644
--- a/app/models/professional_development.rb
+++ b/app/models/professional_development.rb
@@ -41,6 +41,8 @@ class ProfessionalDevelopment
attribute :registration_open, :boolean
attribute :pd_registrations, default: []
+ validates :name, presence: { message: "can't be blank" }, uniqueness: { message: "must be unique" }
+
GRADE_LEVELS = {
elementary: 0,
middle_school: 1,
@@ -49,6 +51,9 @@ class ProfessionalDevelopment
university: 4
}.freeze
+ has_many :pd_registrations
+ has_many :teachers, through: :pd_registrations
+
def initialize(attributes = {})
super(attributes)
# Now ActiveModel handles attributes, no need to manually set defaults for attributes defined above
diff --git a/app/models/teacher.rb b/app/models/teacher.rb
index 76fa7b34..a6ab386d 100644
--- a/app/models/teacher.rb
+++ b/app/models/teacher.rb
@@ -51,6 +51,8 @@ class Teacher < ApplicationRecord
validates_inclusion_of :application_status, in: application_statuses.keys
belongs_to :school, counter_cache: true
+ has_many :professional_development_registrations
+ has_many :professional_developments, through: :professional_development_registrations
# Non-admin teachers whose application has neither been accepted nor denied
# It might or might not have been reviewed.
From 1a6c703c406ffa36b0286f1e694b3a3cf2b2f0d6 Mon Sep 17 00:00:00 2001
From: JacksonXu33
Date: Sat, 30 Mar 2024 16:20:01 -0700
Subject: [PATCH 19/88] add comments
---
.../pd_registrations_controller.rb | 32 ++++++++++++++++++-
app/models/pd_registration.rb | 3 +-
app/models/professional_development.rb | 3 +-
3 files changed, 35 insertions(+), 3 deletions(-)
diff --git a/app/controllers/pd_registrations_controller.rb b/app/controllers/pd_registrations_controller.rb
index e51c8ccf..7b63d770 100644
--- a/app/controllers/pd_registrations_controller.rb
+++ b/app/controllers/pd_registrations_controller.rb
@@ -1,9 +1,11 @@
# frozen_string_literal: true
# Not sure how the ids are going to work with professional development.
-# With a belongs to relationship, depending on customer implementation,
+# With a belongs to relationship, depending on the implementation,
# might have 2 ids in params that we need to distinguish between.
class PdRegistrationsController < ApplicationController
+ before_action :require_login
+
def index
set_pds
end
@@ -20,8 +22,16 @@ def edit
@pd_registration = PdRegistration.find(params[:id])
end
+ # Create and update do NOT support admin overriding user functionality yet, only normal user functionality
def create
+ @professional_development = pd_name_to_pd()
+ if (!@professional_development)
+ flash.now[:alert] = "Failed to save registration: No PD with that name exists.}"
+ render :new
+ end
@pd_registration = PdRegistration.new(pd_registration_params)
+ @pd_registration.professional_development = @professional_development
+ @pd_registration.teacher = current_user
if !@pd_registration.save
flash.now[:alert] = "Failed to save registration: #{@pd_registration.errors.full_messages.join(", ")}"
render :new
@@ -30,7 +40,14 @@ def create
end
def update
+ @professional_development = pd_name_to_pd()
+ if (!@professional_development)
+ flash.now[:alert] = "Failed to save registration: No PD with that name exists.}"
+ render :new
+ end
@pd_registration = PdRegistration.find(params[:id])
+ @pd_registration.professional_development = @professional_development
+ @pd_registration.teacher = current_user
if !@pd_registration.update(pd_registration_params)
flash.now[:alert] = "Failed to save registration: #{@pd_registration.errors.full_messages.join(", ")}"
render "edit"
@@ -44,4 +61,17 @@ def destroy
end
redirect_to pd_registrations_path, success: "Deleted PD registration successfully."
end
+
+ private
+ # This method will take in a that will be the professional_development name,
+ # and return the pd; nil if doesn't exist or pd is not open
+ # It assumes the pd_registrations form will have a place for the user to input the name of a pd to register for
+ # But feel free to edit this if this is not the planned implementation
+ def pd_name_to_pd
+ professional_development = ProfessionalDevelopment.find(pd_registration_params[:name])
+ if professional_development.nil? || !professional_development.registration_open
+ return nil
+ end
+ return professional_development
+ end
end
diff --git a/app/models/pd_registration.rb b/app/models/pd_registration.rb
index 4585f327..4cb6b641 100644
--- a/app/models/pd_registration.rb
+++ b/app/models/pd_registration.rb
@@ -2,7 +2,7 @@
# This class is a mock representation of the PdRegistration model.
# In the final application, Professional Developments and Teachers are associated through PdRegistrations.
-class PdRegistration
+class PdRegistration < ApplicationRecord
include ActiveModel::Model
include ActiveModel::Attributes
@@ -17,6 +17,7 @@ class PdRegistration
belongs_to :professional_development
belongs_to :teacher
+ # if teacher should only have one registration per pd
validates :professional_development_id, uniqueness: { scope: :teacher_id, message: "Teacher already has a registration for this PD" }
def initialize(attributes = {})
diff --git a/app/models/professional_development.rb b/app/models/professional_development.rb
index ce2b835a..de718379 100644
--- a/app/models/professional_development.rb
+++ b/app/models/professional_development.rb
@@ -24,7 +24,7 @@
# This class is a mock representation of the ProfessionalDevelopment model.
# In the final application, Professional Developments and Teachers are associated through PdRegistrations.
# This mock setup uses arrays of mock PdRegistration objects to simulate many-to-many relationships.
-class ProfessionalDevelopment
+class ProfessionalDevelopment < ApplicationRecord
include ActiveModel::Model
include ActiveModel::Attributes # Make sure this is included
@@ -41,6 +41,7 @@ class ProfessionalDevelopment
attribute :registration_open, :boolean
attribute :pd_registrations, default: []
+ # Assuming that the names will have to be unique, to make it possible for teachers to link registration to pd
validates :name, presence: { message: "can't be blank" }, uniqueness: { message: "must be unique" }
GRADE_LEVELS = {
From 08273f2e97ba6f5636309beb3e3ca070bf7c7119 Mon Sep 17 00:00:00 2001
From: JacksonXu33
Date: Sat, 30 Mar 2024 16:22:19 -0700
Subject: [PATCH 20/88] link registrations and pd and teacher
---
app/controllers/pd_registrations_controller.rb | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/app/controllers/pd_registrations_controller.rb b/app/controllers/pd_registrations_controller.rb
index 7b63d770..695fa82b 100644
--- a/app/controllers/pd_registrations_controller.rb
+++ b/app/controllers/pd_registrations_controller.rb
@@ -25,7 +25,7 @@ def edit
# Create and update do NOT support admin overriding user functionality yet, only normal user functionality
def create
@professional_development = pd_name_to_pd()
- if (!@professional_development)
+ if !@professional_development
flash.now[:alert] = "Failed to save registration: No PD with that name exists.}"
render :new
end
@@ -41,7 +41,7 @@ def create
def update
@professional_development = pd_name_to_pd()
- if (!@professional_development)
+ if !@professional_development
flash.now[:alert] = "Failed to save registration: No PD with that name exists.}"
render :new
end
@@ -72,6 +72,6 @@ def pd_name_to_pd
if professional_development.nil? || !professional_development.registration_open
return nil
end
- return professional_development
+ professional_development
end
end
From 99623bac009fbbf7b5876cf1a92a11ecbe9d5ac2 Mon Sep 17 00:00:00 2001
From: Arush Chhatrapati <65872167+ArushC@users.noreply.github.com>
Date: Mon, 1 Apr 2024 17:53:01 -0700
Subject: [PATCH 21/88] uncommented out procfile
---
Procfile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Procfile b/Procfile
index 7cbe22f8..cb90e342 100644
--- a/Procfile
+++ b/Procfile
@@ -1,2 +1,2 @@
web: bundle exec puma -C config/puma.rb -p $PORT
-release: bin/rails db:prepare
+#release: bin/rails db:prepare
From 6f33d635e15de10417fa1194328ccc186e3c15ef Mon Sep 17 00:00:00 2001
From: Arush Chhatrapati <65872167+ArushC@users.noreply.github.com>
Date: Mon, 1 Apr 2024 17:53:48 -0700
Subject: [PATCH 22/88] ok actually uncommented out this time
---
Procfile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Procfile b/Procfile
index cb90e342..7cbe22f8 100644
--- a/Procfile
+++ b/Procfile
@@ -1,2 +1,2 @@
web: bundle exec puma -C config/puma.rb -p $PORT
-#release: bin/rails db:prepare
+release: bin/rails db:prepare
From 5853ab1d35d972b32eaa5b2a56f880393a406534 Mon Sep 17 00:00:00 2001
From: Arush Chhatrapati <65872167+ArushC@users.noreply.github.com>
Date: Mon, 1 Apr 2024 17:59:13 -0700
Subject: [PATCH 23/88] clean up comment
---
app/models/teacher.rb | 1 -
1 file changed, 1 deletion(-)
diff --git a/app/models/teacher.rb b/app/models/teacher.rb
index 65bce8a2..6cb92a3f 100644
--- a/app/models/teacher.rb
+++ b/app/models/teacher.rb
@@ -170,7 +170,6 @@ def self.education_level_options
def self.language_options
WORLD_LANGUAGES
- # language_codes = ISO3166::Country.all.map { |country| country.languages}.flatten.uniq
end
def display_languages
From 227842a10d35742d923ebfaab95514a9dae37d6b Mon Sep 17 00:00:00 2001
From: Jingchao Zhong <92573736+perryzjc@users.noreply.github.com>
Date: Tue, 2 Apr 2024 11:10:15 -0700
Subject: [PATCH 24/88] Add test for teacher edit teacher without mandatory
field
---
features/admin.feature | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/features/admin.feature b/features/admin.feature
index 5b5486be..7d0f0d2a 100644
--- a/features/admin.feature
+++ b/features/admin.feature
@@ -328,6 +328,24 @@ Feature: basic admin functionality
Then I send a request info email
+ Scenario: Admin update info without mandatory field shows error
+ Given the following schools exist:
+ | name | country | city | state | website | grade_level | school_type |
+ | UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | university | public |
+ And the following teachers exist:
+ | first_name | last_name | admin | email | school |
+ | Jane | Doe | false | janedoe@berkeley.edu | UC Berkeley |
+ Given I am on the BJC home page
+ And I have an admin email
+ And I follow "Log In"
+ Then I can log in with Google
+ When I go to the teachers page
+ And I go to the edit page for Jane Doe
+ And I fill in "teacher_email" with ""
+ And I press "Update"
+ Then I should be on the edit page for Jane Doe
+
+
# Scenario: Admin can import csv file. The loader should filter invalid record and create associate school.
# Given the following schools exist:
# | name | country | city | state | website |
From b1a128aefc8e4cc535e4f74fe676b08ffeec2da8 Mon Sep 17 00:00:00 2001
From: Jingchao Zhong <92573736+perryzjc@users.noreply.github.com>
Date: Tue, 2 Apr 2024 11:26:55 -0700
Subject: [PATCH 25/88] Remove debuger from production code
---
app/controllers/schools_controller.rb | 1 -
1 file changed, 1 deletion(-)
diff --git a/app/controllers/schools_controller.rb b/app/controllers/schools_controller.rb
index 127fb8ca..500f6ca0 100644
--- a/app/controllers/schools_controller.rb
+++ b/app/controllers/schools_controller.rb
@@ -44,7 +44,6 @@ def edit
def update
@school = School.find(params[:id])
@school.assign_attributes(school_params)
- debugger
if @school.save
flash[:success] = "Updated #{@school.name} successfully."
redirect_to school_path(@school)
From 6f6f7b5638dbdbafda703ec285955d35b89ef51f Mon Sep 17 00:00:00 2001
From: Jingchao Zhong <92573736+perryzjc@users.noreply.github.com>
Date: Tue, 2 Apr 2024 11:27:21 -0700
Subject: [PATCH 26/88] Remove redundant comment
---
app/views/schools/_form.html.erb | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/app/views/schools/_form.html.erb b/app/views/schools/_form.html.erb
index b0b3a37e..aa6797c3 100644
--- a/app/views/schools/_form.html.erb
+++ b/app/views/schools/_form.html.erb
@@ -111,7 +111,7 @@
function handleCountryChange() {
if (countrySelected.val() === 'US') {
stateSelectContainer.show();
- stateSelect.attr('required', '') //make state select required
+ stateSelect.attr('required', '')
stateTextfieldContainer.hide();
stateTextfield.removeAttr('name');
From 4e2b40c9d12e2b3e7db748fc4cbd97143d6284a7 Mon Sep 17 00:00:00 2001
From: Arush Chhatrapati <65872167+ArushC@users.noreply.github.com>
Date: Wed, 3 Apr 2024 21:48:07 -0700
Subject: [PATCH 27/88] make comment more formal
---
app/models/teacher.rb | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/app/models/teacher.rb b/app/models/teacher.rb
index 6cb92a3f..1bef4e66 100644
--- a/app/models/teacher.rb
+++ b/app/models/teacher.rb
@@ -181,8 +181,8 @@ def valid_languages
end
def sort_and_clean_languages
- # a weird selectize bug results in the empty string sometimes included in languages
- # so remove all occurences of empty string
+ # Due to an identified bug in the Selectize plugin, an empty string is occasionally appended to the 'languages' list.
+ # To ensure data integrity, the following code removes any occurrences of empty strings from the list.
languages.sort!.reject!(&:blank?)
end
From 3b1625fd6cae99c65825eee9fea73ea34092c740 Mon Sep 17 00:00:00 2001
From: Jingchao Zhong <92573736+perryzjc@users.noreply.github.com>
Date: Thu, 4 Apr 2024 14:54:28 -0700
Subject: [PATCH 28/88] Implement backend model & migration
---
app/models/pd_registration.rb | 57 +++++++++----
app/models/professional_development.rb | 84 ++++++++-----------
...190834_create_professional_developments.rb | 17 ++++
.../20240404191433_create_pd_registrations.rb | 17 ++++
db/schema.rb | 27 +++++-
5 files changed, 135 insertions(+), 67 deletions(-)
create mode 100644 db/migrate/20240404190834_create_professional_developments.rb
create mode 100644 db/migrate/20240404191433_create_pd_registrations.rb
diff --git a/app/models/pd_registration.rb b/app/models/pd_registration.rb
index 4cb6b641..e786ce25 100644
--- a/app/models/pd_registration.rb
+++ b/app/models/pd_registration.rb
@@ -1,26 +1,49 @@
# frozen_string_literal: true
-# This class is a mock representation of the PdRegistration model.
-# In the final application, Professional Developments and Teachers are associated through PdRegistrations.
+# == Schema Information
+#
+# Table name: pd_registrations
+#
+# id :bigint not null, primary key
+# attended :boolean default(FALSE)
+# role :string not null
+# created_at :datetime not null
+# updated_at :datetime not null
+# professional_development_id :integer not null
+# teacher_id :integer not null
+#
+# Indexes
+#
+# index_pd_reg_on_teacher_id_and_pd_id (teacher_id,professional_development_id) UNIQUE
+#
+# Foreign Keys
+#
+# fk_rails_... (professional_development_id => professional_developments.id)
+# fk_rails_... (teacher_id => teachers.id)
+#
class PdRegistration < ApplicationRecord
- include ActiveModel::Model
- include ActiveModel::Attributes
+ belongs_to :teacher
+ belongs_to :professional_development
- attribute :id, :integer
- attribute :teacher_id, :integer
- attribute :pd_id, :integer
- attribute :attended, :boolean
- attribute :role, :string
- attribute :teacher_name, :string # Adding this for convenience in mocking
+ validates :teacher_id, uniqueness: { scope: :professional_development_id, message: "already has a registration for this PD" }
+ validates :role, inclusion: { in: %w[leader attendee], message: "%{value} is not a valid role" }
+ validates :attended, inclusion: { in: [true, false] }
- # probably don't need the 3 id attributes (id, teacherid, pdid) if adding this relationship status into model
- belongs_to :professional_development
- belongs_to :teacher
+ validate :professional_development_dates_passed, on: :create
+
+ def teacher_name
+ teacher = Teacher.find_by(id: teacher_id)
+ if teacher.present?
+ "#{teacher.first_name} #{teacher.last_name}"
+ else
+ "Teacher not found"
+ end
+ end
- # if teacher should only have one registration per pd
- validates :professional_development_id, uniqueness: { scope: :teacher_id, message: "Teacher already has a registration for this PD" }
+ private
+ def professional_development_dates_passed
+ return unless professional_development&.end_date&.past?
- def initialize(attributes = {})
- super(attributes)
+ errors.add(:professional_development_id, "can't register for past events")
end
end
diff --git a/app/models/professional_development.rb b/app/models/professional_development.rb
index de718379..8962e17f 100644
--- a/app/models/professional_development.rb
+++ b/app/models/professional_development.rb
@@ -4,75 +4,61 @@
#
# Table name: professional_developments
#
-# id :integer not null, primary key
-# city :string
-# country :string
-# grade_level :integer
-# lat :float
-# lng :float
-# name :string
-# state :string
-# teachers_count :integer default(0)
-# website :string TODO: Confirm is it necessary field
-# created_at :datetime
-# updated_at :datetime
+# id :bigint not null, primary key
+# city :string not null
+# country :string not null
+# end_date :date not null
+# grade_level :integer not null
+# name :string not null
+# start_date :date not null
+# state :string
+# created_at :datetime not null
+# updated_at :datetime not null
#
# Indexes
#
-# TODO: Define indexes
-
-# This class is a mock representation of the ProfessionalDevelopment model.
-# In the final application, Professional Developments and Teachers are associated through PdRegistrations.
-# This mock setup uses arrays of mock PdRegistration objects to simulate many-to-many relationships.
+# index_professional_developments_on_name_and_start_date (name,start_date) UNIQUE
+#
class ProfessionalDevelopment < ApplicationRecord
- include ActiveModel::Model
- include ActiveModel::Attributes # Make sure this is included
-
- # Define attributes
- attribute :id, :integer
- attribute :name, :string
- attribute :city, :string
- attribute :state, :string
- attribute :country, :string
- attribute :start_date, :date
- attribute :end_date, :date
- attribute :grade_level, :integer, default: -1
- attribute :teachers_count, :integer, default: 0
- attribute :registration_open, :boolean
- attribute :pd_registrations, default: []
+ VALID_STATES = %w[AL AK AS AZ AR CA CO CT DE DC FM FL GA GU HI ID IL IN IA KS KY LA ME MH MD MA MI MN MS MO MT NE NV
+ NH NJ NM NY NC ND MP OH OK OR PW PA PR RI SC SD TN TX UT VT VI VA WA WV WI WY].freeze
- # Assuming that the names will have to be unique, to make it possible for teachers to link registration to pd
- validates :name, presence: { message: "can't be blank" }, uniqueness: { message: "must be unique" }
+ validates :name, :city, :country, :start_date, :end_date, presence: true
+ validates :name, uniqueness: { scope: :start_date, message: "should be unique per start date" }
+ validates :state, presence: true, if: -> { country == "US" }
+ validates :state, inclusion: { in: VALID_STATES, message: "%{value} is not a valid state" },
+ if: -> { country == "US" }
+ validate :end_date_after_start_date
- GRADE_LEVELS = {
+ enum grade_level: {
elementary: 0,
middle_school: 1,
high_school: 2,
community_college: 3,
university: 4
- }.freeze
+ }
- has_many :pd_registrations
+ has_many :pd_registrations, dependent: :destroy
has_many :teachers, through: :pd_registrations
- def initialize(attributes = {})
- super(attributes)
- # Now ActiveModel handles attributes, no need to manually set defaults for attributes defined above
+ def location
+ "#{city}, #{state}, #{country}"
end
- def persisted?
- id.present?
+ def display_grade_level
+ return "Unknown" if grade_level_before_type_cast.to_i == -1
+
+ grade_level.to_s.titlecase
end
- def location
- "#{city}, #{state}, #{country}"
+ def registration_open
+ @professional_development.registration_open ? "Yes" : "No"
end
- def display_grade_level
- # Directly access the grade_level attribute
- grade_level_value = self.grade_level
- return "Unknown" if grade_level_value == -1
+ private
+ def end_date_after_start_date
+ return if end_date.blank? || start_date.blank?
- GRADE_LEVELS.key(grade_level_value).to_s.titlecase
+ errors.add(:end_date, "must be after the start date") if end_date < start_date
end
end
diff --git a/db/migrate/20240404190834_create_professional_developments.rb b/db/migrate/20240404190834_create_professional_developments.rb
new file mode 100644
index 00000000..61ba5f50
--- /dev/null
+++ b/db/migrate/20240404190834_create_professional_developments.rb
@@ -0,0 +1,17 @@
+class CreateProfessionalDevelopments < ActiveRecord::Migration[6.1]
+ def change
+ create_table :professional_developments do |t|
+ t.string :name, null: false
+ t.string :city, null: false
+ t.string :state
+ t.string :country, null: false
+ t.date :start_date, null: false
+ t.date :end_date, null: false
+ t.integer :grade_level, null: false
+
+ t.timestamps
+ end
+
+ add_index :professional_developments, [:name, :start_date], unique: true
+ end
+end
diff --git a/db/migrate/20240404191433_create_pd_registrations.rb b/db/migrate/20240404191433_create_pd_registrations.rb
new file mode 100644
index 00000000..e81ddd0e
--- /dev/null
+++ b/db/migrate/20240404191433_create_pd_registrations.rb
@@ -0,0 +1,17 @@
+class CreatePdRegistrations < ActiveRecord::Migration[6.1]
+ def change
+ create_table :pd_registrations do |t|
+ t.integer :teacher_id, null: false
+ t.integer :professional_development_id, null: false
+ t.boolean :attended, default: false
+ t.string :role, null: false
+
+ t.timestamps
+ end
+
+ add_index :pd_registrations, [:teacher_id, :professional_development_id], unique: true,
+ name: 'index_pd_reg_on_teacher_id_and_pd_id'
+ add_foreign_key :pd_registrations, :teachers
+ add_foreign_key :pd_registrations, :professional_developments
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index a9927666..44f6d7f9 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: 2024_03_07_225738) do
+ActiveRecord::Schema.define(version: 2024_04_04_191433) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
@@ -82,6 +82,29 @@
t.index ["url_slug"], name: "index_pages_on_url_slug", unique: true
end
+ create_table "pd_registrations", force: :cascade do |t|
+ t.integer "teacher_id", null: false
+ t.integer "professional_development_id", null: false
+ t.boolean "attended", default: false
+ t.string "role", null: false
+ t.datetime "created_at", precision: 6, null: false
+ t.datetime "updated_at", precision: 6, null: false
+ t.index ["teacher_id", "professional_development_id"], name: "index_pd_reg_on_teacher_id_and_pd_id", unique: true
+ end
+
+ create_table "professional_developments", force: :cascade do |t|
+ t.string "name", null: false
+ t.string "city", null: false
+ t.string "state"
+ t.string "country", null: false
+ t.date "start_date", null: false
+ t.date "end_date", null: false
+ t.integer "grade_level", null: false
+ t.datetime "created_at", precision: 6, null: false
+ t.datetime "updated_at", precision: 6, null: false
+ t.index ["name", "start_date"], name: "index_professional_developments_on_name_and_start_date", unique: true
+ end
+
create_table "schools", id: :serial, force: :cascade do |t|
t.string "name"
t.string "city"
@@ -130,5 +153,7 @@
add_foreign_key "active_storage_variant_records", "active_storage_blobs", column: "blob_id"
add_foreign_key "pages", "teachers", column: "creator_id"
add_foreign_key "pages", "teachers", column: "last_editor_id"
+ add_foreign_key "pd_registrations", "professional_developments"
+ add_foreign_key "pd_registrations", "teachers"
add_foreign_key "teachers", "schools"
end
From 67dfaf5f9ca0e2ea005a0bc2bf92c2739e3a5a03 Mon Sep 17 00:00:00 2001
From: Jingchao Zhong <92573736+perryzjc@users.noreply.github.com>
Date: Thu, 4 Apr 2024 14:54:56 -0700
Subject: [PATCH 29/88] Adjust frontend view to align with backend
implementation
---
.../professional_developments/_form.html.erb | 6 ++--
.../professional_developments/index.html.erb | 4 +--
.../professional_developments/show.html.erb | 30 +++++++++++--------
3 files changed, 23 insertions(+), 17 deletions(-)
diff --git a/app/views/professional_developments/_form.html.erb b/app/views/professional_developments/_form.html.erb
index 1d4244f6..f9748bea 100644
--- a/app/views/professional_developments/_form.html.erb
+++ b/app/views/professional_developments/_form.html.erb
@@ -25,7 +25,7 @@
From 27351f135162d2587d5effce692f3fc5c96156e2 Mon Sep 17 00:00:00 2001
From: Jingchao Zhong <92573736+perryzjc@users.noreply.github.com>
Date: Fri, 5 Apr 2024 16:55:10 -0700
Subject: [PATCH 37/88] Add cucumber tests
---
features/professional_development.feature | 190 ++++++++++++++++++
features/step_definitions/page_steps.rb | 13 ++
.../professional_development_steps.rb | 28 +++
3 files changed, 231 insertions(+)
create mode 100644 features/professional_development.feature
create mode 100644 features/step_definitions/professional_development_steps.rb
diff --git a/features/professional_development.feature b/features/professional_development.feature
new file mode 100644
index 00000000..3dbc6558
--- /dev/null
+++ b/features/professional_development.feature
@@ -0,0 +1,190 @@
+Feature: Professional Development and Registration Management
+
+ As an admin or a teacher
+ I want to manage professional development events and registrations
+ So that I can organize and track participation in PD activities
+
+ Background: Admin and Teacher exist
+ Given the following teachers exist:
+ | first_name | last_name | admin | email | id |
+ | Perry | Zhong | true | testadminuser@berkeley.edu | 100 |
+ | Joseph | Mamoa | false | testteacher@berkeley.edu | 101 |
+ And the following professional developments exist:
+ | id | name | city | state | country | start_date | end_date | grade_level | created_at | updated_at |
+ | 10 | Classroom Management | San Francisco | CA | United States| 2023-02-01 | 2023-02-05 | university | 2023-02-01 00:00:00 | 2023-02-01 00:00:00 |
+ | 11 | Teaching Strategies | San Francisco | CA | United States| 2023-01-01 | 2023-01-05 | university | 2023-01-01 00:00:00 | 2023-01-01 00:00:00 |
+
+# Basic CRUD operations for Professional Developments
+ Scenario: Admin creates a new Professional Development
+ Given I am on the BJC home page
+ And I have an admin email
+ And I follow "Log In"
+ Then I can log in with Google
+ Then I should see "New Requests"
+ Then I go to the new professional development page
+ Then I should see "Create a new Professional Development"
+ And I fill in "Professional Development Name" with "Innovative Teaching Techniques"
+ And I fill in "City" with "San Francisco"
+ And I select "CA" from "State" dropdown
+ And I select "United States" from "Country"
+ And I fill in "Start Date" with "2023-12-01"
+ And I fill in "End Date" with "2023-12-05"
+ And I select "University" from "Grade Level"
+ And I press "Submit"
+ Then I should see a "info" flash message "Professional development created successfully."
+ And I should arrive at the professional development show page titled "Innovative Teaching Techniques"
+
+ Scenario: Admin updates a Professional Development
+ Given I am on the BJC home page
+ And I have an admin email
+ And I follow "Log In"
+ Then I can log in with Google
+ When I go to the professional developments page
+# first "Edit" link is for the Classroom Management, due to alphabetical order in the table
+ And I follow the first "Edit" link
+ And I fill in "Professional Development Name" with "Classroom Management 2.0"
+ And I press "Submit"
+ Then I should see a "info" flash message "Professional development updated successfully."
+ And I should arrive at the professional development show page titled "Classroom Management 2.0"
+
+ Scenario: Admin deletes a Professional Development with confirmation
+ Given I am on the BJC home page
+ And I have an admin email
+ And I follow "Log In"
+ Then I can log in with Google
+ When I go to the professional developments page
+ And I should see "Classroom Management"
+ Then I follow the first "❌" link
+ And I confirm the action
+ Then I should see a "info" flash message "Professional development deleted successfully."
+ And I should be on the professional developments page
+ And I should not see "Classroom Management"
+
+ Scenario: Admin deletes a Professional Development without confirmation
+ Given I am on the BJC home page
+ And I have an admin email
+ And I follow "Log In"
+ Then I can log in with Google
+ When I go to the professional developments page
+ And I should see "Classroom Management"
+ Then I follow the first "❌" link
+ And I dismiss the action
+ And I should be on the professional developments page
+ And I should see "Classroom Management"
+
+ Scenario: Admin registers a teacher for a Professional Development
+ Given I am on the BJC home page
+ And I have an admin email
+ And I follow "Log In"
+ Then I can log in with Google
+ Then I go to the professional developments page
+ And I should see "Classroom Management"
+ And I follow the first "Classroom Management" link
+ Then I should see "Classroom Management"
+ Then I press "Add Registration"
+ Then I should see "Add Teacher to PD Session"
+ And I fill in "Teacher ID" with "100"
+ And I select "Attendee" from "Role"
+ And I select "Yes" from "Attended"
+ And I press "Add"
+ Then I should see a "info" flash message "Registration for professional development was successfully created."
+ And I should see "Total Registered Teachers: 1"
+ And I should see "Perry Zhong"
+
+# Basic CRUD operations for Professional Development Registrations
+ Scenario: Admin updates a teacher's registration for a Professional Development
+ Given the following professional developments registrations exist
+ | id | professional_development_id | teacher_id | role | attended | created_at | updated_at |
+ | 1 | 10 | 100 | attendee | yes | 2023-02-01 00:00:00 | 2023-02-01 00:00:00 |
+ Given I am on the BJC home page
+ And I have an admin email
+ And I follow "Log In"
+ Then I can log in with Google
+ Then I go to the professional developments page
+ And I should see "Classroom Management"
+ And I follow the first "Classroom Management" link
+ Then I should see "Classroom Management"
+ And I should see "Total Registered Teachers: 1"
+ And I should see "Perry Zhong"
+ And I should see "Yes"
+ And I follow "Update"
+ Then I should see "Edit Teacher in PD Session"
+ Then I select "No" from "Attended"
+ And I press "Add"
+ Then I should see a "info" flash message "Registration was successfully updated."
+ And I should see "Perry Zhong"
+ And I should see "No"
+ And I should not see "Yes"
+
+ Scenario: Admin cancels a teacher's registration for a Professional Development
+ Given the following professional developments registrations exist
+ | id | professional_development_id | teacher_id | role | attended | created_at | updated_at |
+ | 1 | 10 | 100 | attendee | yes | 2023-02-01 00:00:00 | 2023-02-01 00:00:00 |
+ Given I am on the BJC home page
+ And I have an admin email
+ And I follow "Log In"
+ Then I can log in with Google
+ Then I go to the professional developments page
+ And I should see "Classroom Management"
+ And I follow the first "Classroom Management" link
+ Then I should see "Classroom Management"
+ And I should see "Total Registered Teachers: 1"
+ And I should see "Perry Zhong"
+ And I should see "Yes"
+ And I follow "❌"
+ And I confirm the action
+ Then I should see a "info" flash message "Registration was successfully cancelled."
+ And I should see "Total Registered Teachers: 0"
+ And I should not see "Perry Zhong"
+ And I should not see "Yes"
+
+# Advanced scenarios for Professional Development and Registration Management
+ Scenario: Admin creates a duplicate Professional Development registration should fail
+ Given the following professional developments registrations exist
+ | id | professional_development_id | teacher_id | role | attended | created_at | updated_at |
+ | 1 | 10 | 100 | attendee | yes | 2023-02-01 00:00:00 | 2023-02-01 00:00:00 |
+ Given I am on the BJC home page
+ And I have an admin email
+ And I follow "Log In"
+ Then I can log in with Google
+ When I go to the professional developments page
+ And I follow the first "Classroom Management" link
+ And I should see "Total Registered Teachers: 1"
+ And I press "Add Registration"
+ And I fill in "Teacher ID" with "100"
+ And I select "Attendee" from "Role"
+ And I select "Yes" from "Attended"
+ And I press "Add"
+ Then I should see a "danger" flash message "Teacher already has a registration for this PD"
+ And I should see "Total Registered Teachers: 1"
+
+ Scenario: Admin attempts to create a Professional Development with an end date earlier than the start date
+ Given I am on the BJC home page
+ And I have an admin email
+ And I follow "Log In"
+ Then I can log in with Google
+ Then I should see "New Requests"
+ Then I go to the new professional development page
+ Then I should see "Create a new Professional Development"
+ And I fill in "Professional Development Name" with "Future Educational Strategies"
+ And I fill in "City" with "Los Angeles"
+ And I select "CA" from "State" dropdown
+ And I select "United States" from "Country"
+ And I fill in "Start Date" with "2023-10-01"
+ And I fill in "End Date" with "2023-09-30"
+ And I select "High School" from "Grade Level"
+ And I press "Submit"
+ Then I should see a "danger" flash message "End date must be after the start date"
+ And I should see "Add a Professional Development Workshop"
+
+ Scenario: Admin attempts to create a Professional Development without mandatory fields
+ Given I am on the BJC home page
+ And I have an admin email
+ And I follow "Log In"
+ Then I can log in with Google
+ Then I should see "New Requests"
+ Then I go to the new professional development page
+ Then I should see "Add a Professional Development Workshop"
+ And I press "Submit"
+ Then I should be on the new professional development page
+ And I should see "Add a Professional Development Workshop"
diff --git a/features/step_definitions/page_steps.rb b/features/step_definitions/page_steps.rb
index 90c8796e..ca791de5 100644
--- a/features/step_definitions/page_steps.rb
+++ b/features/step_definitions/page_steps.rb
@@ -70,3 +70,16 @@
page_slug = Page.find_by(title: link_text).url_slug
page.execute_script("document.getElementById('pagelink_#{page_slug}').click();")
end
+
+
+When(/^I confirm the action$/) do
+ # Confirm the alert that appears after triggering a confirmation dialog
+ page.accept_alert do
+ end
+end
+
+When(/^I dismiss the action$/) do
+ # Dismiss the alert that appears after triggering a confirmation dialog
+ page.dismiss_confirm do
+ end
+end
diff --git a/features/step_definitions/professional_development_steps.rb b/features/step_definitions/professional_development_steps.rb
new file mode 100644
index 00000000..13ab317a
--- /dev/null
+++ b/features/step_definitions/professional_development_steps.rb
@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+require "cucumber/rspec/doubles"
+
+Given(/the following professional developments exist/) do |professional_developments_table|
+ professional_developments_table.symbolic_hashes.each do |development|
+ ProfessionalDevelopment.create!(development)
+ end
+end
+
+Given(/the following professional developments registrations exist/) do |pd_registrations_table|
+ pd_registrations_table.symbolic_hashes.each do |registration_hash|
+ development = ProfessionalDevelopment.find_by(id: registration_hash[:professional_development_id])
+ raise "ProfessionalDevelopment not found: #{registration_hash[:professional_development_name]}" unless development
+ registration_details = registration_hash.merge(professional_development: development)
+ PdRegistration.create!(registration_details)
+ end
+end
+
+Then(/^I should arrive at the professional development show page titled "([^"]*)"$/) do |title|
+ development = ProfessionalDevelopment.find_by(name: title)
+ expect(current_path).to eq(professional_development_path(development))
+end
+
+Then(/^I visit the professional development show page titled "([^"]*)"$/) do |title|
+ development = ProfessionalDevelopment.find_by(name: title)
+ visit(professional_development_path(development))
+end
From a6a1c01b433d183c4f422f309ff2c0401d229413 Mon Sep 17 00:00:00 2001
From: Jingchao Zhong <92573736+perryzjc@users.noreply.github.com>
Date: Fri, 5 Apr 2024 17:02:43 -0700
Subject: [PATCH 38/88] Remove debugging msg
---
app/controllers/pd_registrations_controller.rb | 2 +-
app/controllers/professional_developments_controller.rb | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/app/controllers/pd_registrations_controller.rb b/app/controllers/pd_registrations_controller.rb
index e1bfe1d7..aad1afd5 100644
--- a/app/controllers/pd_registrations_controller.rb
+++ b/app/controllers/pd_registrations_controller.rb
@@ -62,7 +62,7 @@ def set_pd_registration
def set_professional_development
@professional_development = ProfessionalDevelopment.find_by(id: params[:professional_development_id])
unless @professional_development
- redirect_to professional_developments_path, alert: "Professional Development not found. test"
+ redirect_to professional_developments_path, alert: "Professional Development not found."
end
end
diff --git a/app/controllers/professional_developments_controller.rb b/app/controllers/professional_developments_controller.rb
index 67b6808f..673ec330 100644
--- a/app/controllers/professional_developments_controller.rb
+++ b/app/controllers/professional_developments_controller.rb
@@ -51,7 +51,7 @@ def destroy
def set_professional_development
@professional_development = ProfessionalDevelopment.find(params[:id])
rescue ActiveRecord::RecordNotFound
- redirect_to professional_developments_url, alert: "Professional development not found. test2"
+ redirect_to professional_developments_url, alert: "Professional development not found."
end
def professional_development_params
From 9e122edf2c0ab6882f0004e76e64415fbb276ea4 Mon Sep 17 00:00:00 2001
From: Jingchao Zhong <92573736+perryzjc@users.noreply.github.com>
Date: Fri, 5 Apr 2024 18:38:34 -0700
Subject: [PATCH 39/88] Fix indexing & uniqueness check for pd
---
app/models/professional_development.rb | 5 -----
.../20240404190834_create_professional_developments.rb | 2 --
db/schema.rb | 1 -
3 files changed, 8 deletions(-)
diff --git a/app/models/professional_development.rb b/app/models/professional_development.rb
index 8962e17f..9467b217 100644
--- a/app/models/professional_development.rb
+++ b/app/models/professional_development.rb
@@ -15,16 +15,11 @@
# created_at :datetime not null
# updated_at :datetime not null
#
-# Indexes
-#
-# index_professional_developments_on_name_and_start_date (name,start_date) UNIQUE
-#
class ProfessionalDevelopment < ApplicationRecord
VALID_STATES = %w[AL AK AS AZ AR CA CO CT DE DC FM FL GA GU HI ID IL IN IA KS KY LA ME MH MD MA MI MN MS MO MT NE NV
NH NJ NM NY NC ND MP OH OK OR PW PA PR RI SC SD TN TX UT VT VI VA WA WV WI WY].freeze
validates :name, :city, :country, :start_date, :end_date, presence: true
- validates :name, uniqueness: { scope: :start_date, message: "should be unique per start date" }
validates :state, presence: true, if: -> { country == "US" }
validates :state, inclusion: { in: VALID_STATES, message: "%{value} is not a valid state" },
if: -> { country == "US" }
diff --git a/db/migrate/20240404190834_create_professional_developments.rb b/db/migrate/20240404190834_create_professional_developments.rb
index 61ba5f50..f551d470 100644
--- a/db/migrate/20240404190834_create_professional_developments.rb
+++ b/db/migrate/20240404190834_create_professional_developments.rb
@@ -11,7 +11,5 @@ def change
t.timestamps
end
-
- add_index :professional_developments, [:name, :start_date], unique: true
end
end
diff --git a/db/schema.rb b/db/schema.rb
index 44f6d7f9..0f32ca27 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -102,7 +102,6 @@
t.integer "grade_level", null: false
t.datetime "created_at", precision: 6, null: false
t.datetime "updated_at", precision: 6, null: false
- t.index ["name", "start_date"], name: "index_professional_developments_on_name_and_start_date", unique: true
end
create_table "schools", id: :serial, force: :cascade do |t|
From 700c87ea4bcc6a08668b50a3d9d76ebdf318f0db Mon Sep 17 00:00:00 2001
From: Jingchao Zhong <92573736+perryzjc@users.noreply.github.com>
Date: Fri, 5 Apr 2024 18:39:31 -0700
Subject: [PATCH 40/88] Add rspec tests
---
Gemfile | 3 +
Gemfile.lock | 5 +
.../pd_registrations_controller_spec.rb | 120 ++++++++++++++++++
...ofessional_developments_controller_spec.rb | 119 +++++++++++++++++
4 files changed, 247 insertions(+)
create mode 100644 spec/controllers/pd_registrations_controller_spec.rb
create mode 100644 spec/controllers/professional_developments_controller_spec.rb
diff --git a/Gemfile b/Gemfile
index 0b73d21e..e1dac083 100644
--- a/Gemfile
+++ b/Gemfile
@@ -96,4 +96,7 @@ group :test do
# Accessibility Testing
gem "axe-core-rspec"
gem "axe-core-cucumber"
+
+ # Test suite speedup
+ gem "rails-controller-testing"
end
diff --git a/Gemfile.lock b/Gemfile.lock
index 24ef2246..9d16489e 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -399,6 +399,10 @@ GEM
bundler (>= 1.15.0)
railties (= 6.1.7.6)
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.2.0)
activesupport (>= 5.0.0)
minitest
@@ -595,6 +599,7 @@ DEPENDENCIES
puma (~> 5)
rack-mini-profiler (~> 2.0)
rails (= 6.1.7.6)
+ rails-controller-testing
rspec-rails
rubocop
rubocop-faker
diff --git a/spec/controllers/pd_registrations_controller_spec.rb b/spec/controllers/pd_registrations_controller_spec.rb
new file mode 100644
index 00000000..1a91a95f
--- /dev/null
+++ b/spec/controllers/pd_registrations_controller_spec.rb
@@ -0,0 +1,120 @@
+# frozen_string_literal: true
+
+require "rails_helper"
+
+RSpec.describe "PdRegistrations", type: :request do
+ fixtures :all
+
+ let(:admin_user) { teachers(:admin) }
+ let(:regular_user) { teachers(:validated_teacher) }
+ let(:professional_development) { ProfessionalDevelopment.create!(name: "Effective Teaching Strategies", city: "City", state: "State", country: "Country", start_date: Date.today, end_date: Date.tomorrow, grade_level: "university") }
+ let(:teacher1) { teachers(:bob) }
+ let(:teacher2) { teachers(:long) }
+ let(:valid_registration_attributes) {
+ {
+ teacher_id: teacher1.id,
+ professional_development_id: professional_development.id,
+ attended: true,
+ role: "attendee"
+ }
+ }
+ let(:valid_registration_attributes2) {
+ {
+ teacher_id: teacher2.id,
+ professional_development_id: professional_development.id,
+ attended: true,
+ role: "attendee"
+ }
+ }
+ let(:invalid_registration_attributes) {
+ {
+ teacher_id: nil,
+ professional_development_id: nil,
+ attended: false,
+ role: ""
+ }
+ }
+ let!(:pd_registration) { PdRegistration.create!(valid_registration_attributes) }
+
+ shared_examples "admin access" do
+ it "allows operation" do
+ action
+ expect(flash[:notice]).to match(/successfully/)
+ end
+ end
+
+ shared_examples "regular user denied" do
+ it "denies operation" do
+ action
+ expect(flash[:danger]).to be_present
+ end
+ end
+
+ describe "CRUD operations for PD Registrations" do
+ describe "CREATE" do
+ let(:action) { post professional_development_pd_registrations_path(
+ professional_development_id: professional_development.id),
+ params: { pd_registration: valid_registration_attributes2 } }
+
+ context "as an admin" do
+ before { log_in(admin_user) }
+ include_examples "admin access"
+ end
+
+ context "as a regular user" do
+ before { log_in(regular_user) }
+ include_examples "regular user denied"
+ end
+ end
+
+ describe "UPDATE" do
+ context "with valid attributes" do
+ let(:action) { patch professional_development_pd_registration_path(
+ professional_development_id: professional_development.id, id: pd_registration.id),
+ params: { pd_registration: { role: "leader" } } }
+
+ context "as an admin" do
+ before { log_in(admin_user) }
+ include_examples "admin access"
+ end
+
+ context "as a regular user" do
+ before { log_in(regular_user) }
+ include_examples "regular user denied"
+ end
+ end
+
+ context "with invalid attributes" do
+ let(:action) { patch professional_development_pd_registration_path(
+ professional_development_id: professional_development.id, id: pd_registration.id),
+ params: { pd_registration: invalid_registration_attributes } }
+
+ it "fails to update and redirects for admin" do
+ log_in(admin_user)
+ action
+ expect(response).to render_template(:show)
+ expect(flash[:alert]).to be_present
+ end
+ end
+ end
+
+ describe "DELETE" do
+ let(:action) { delete professional_development_pd_registration_path(
+ professional_development_id: professional_development.id, id: pd_registration.id) }
+
+ context "as an admin" do
+ before { log_in(admin_user) }
+ it "deletes the PD registration" do
+ expect { action }.to change(PdRegistration, :count).by(-1)
+ expect(response).to redirect_to(professional_development_path(professional_development))
+ expect(flash[:notice]).to match(/successfully cancelled/)
+ end
+ end
+
+ context "as a regular user" do
+ before { log_in(regular_user) }
+ include_examples "regular user denied"
+ end
+ end
+ end
+end
diff --git a/spec/controllers/professional_developments_controller_spec.rb b/spec/controllers/professional_developments_controller_spec.rb
new file mode 100644
index 00000000..a6952181
--- /dev/null
+++ b/spec/controllers/professional_developments_controller_spec.rb
@@ -0,0 +1,119 @@
+# frozen_string_literal: true
+
+require "rails_helper"
+
+RSpec.describe "ProfessionalDevelopments", type: :request do
+ fixtures :all
+
+ let(:admin_user) { teachers(:admin) }
+ let(:regular_user) { teachers(:validated_teacher) }
+ let(:valid_attributes) {
+ {
+ name: "Effective Teaching Strategies",
+ city: "City",
+ state: "State",
+ country: "Country",
+ start_date: Date.today,
+ end_date: Date.tomorrow,
+ grade_level: "university",
+ }
+ }
+ let(:valid_attributes2) {
+ {
+ name: "PD2",
+ city: "City",
+ state: "State",
+ country: "Country",
+ start_date: Date.today,
+ end_date: Date.tomorrow,
+ grade_level: "university",
+ }
+ }
+ let(:invalid_attributes) {
+ {
+ name: "",
+ city: "",
+ state: "",
+ country: "",
+ start_date: Date.tomorrow,
+ end_date: Date.today - 1.day,
+ grade_level: ""
+ }
+ }
+ let!(:professional_development) { ProfessionalDevelopment.create!(valid_attributes) }
+
+ shared_examples "admin access" do
+ it "allows operation" do
+ action
+ expect(flash[:notice]).to match(/successfully/)
+ end
+ end
+
+ shared_examples "regular user denied" do
+ it "denies operation and redirects" do
+ action
+ expect(flash[:danger]).to be_present
+ end
+ end
+
+ describe "CRUD operations" do
+ describe "CREATE" do
+ let(:action) { post professional_developments_path, params: { professional_development: valid_attributes2 } }
+
+ context "as an admin" do
+ before { log_in(admin_user) }
+ include_examples "admin access"
+ end
+
+ context "as a regular user" do
+ before { log_in(regular_user) }
+ include_examples "regular user denied"
+ end
+ end
+
+ describe "UPDATE" do
+ context "with valid attributes" do
+ let(:action) { patch professional_development_path(professional_development), params: { professional_development: { name: "Updated PD Name" } } }
+
+ context "as an admin" do
+ before { log_in(admin_user) }
+ include_examples "admin access"
+ end
+
+ context "as a regular user" do
+ before { log_in(regular_user) }
+ include_examples "regular user denied"
+ end
+ end
+
+ context "with invalid attributes" do
+ let(:action) { patch professional_development_path(professional_development), params: { professional_development: invalid_attributes } }
+
+ it "fails to update and re-renders edit for admin" do
+ log_in(admin_user)
+ action
+ expect(response).to render_template(:edit)
+ expect(flash[:alert]).to be_present
+ end
+ end
+ end
+
+ describe "DELETE" do
+ let(:action) { delete professional_development_path(professional_development) }
+
+ context "as an admin" do
+ before { log_in(admin_user) }
+ it "deletes the professional development" do
+ expect { action }.to change(ProfessionalDevelopment, :count).by(-1)
+ expect(response).to redirect_to(professional_developments_path)
+ expect(flash[:notice]).to match(/deleted successfully/)
+ end
+ end
+
+ context "as a regular user" do
+ before { log_in(regular_user) }
+ include_examples "regular user denied"
+ end
+ end
+ end
+end
From dbeb705b89baf3b759f76b4fc9d39471adb0f5e1 Mon Sep 17 00:00:00 2001
From: Jingchao Zhong <92573736+perryzjc@users.noreply.github.com>
Date: Fri, 5 Apr 2024 18:40:00 -0700
Subject: [PATCH 41/88] Add admin check for pd registrations
---
app/controllers/pd_registrations_controller.rb | 1 +
1 file changed, 1 insertion(+)
diff --git a/app/controllers/pd_registrations_controller.rb b/app/controllers/pd_registrations_controller.rb
index aad1afd5..55c171ef 100644
--- a/app/controllers/pd_registrations_controller.rb
+++ b/app/controllers/pd_registrations_controller.rb
@@ -2,6 +2,7 @@
class PdRegistrationsController < ApplicationController
before_action :require_login
+ before_action :require_admin
before_action :set_pd_registration, only: [:show, :edit, :update, :destroy]
before_action :set_professional_development, only: [:new, :create, :edit, :update, :destroy]
From afe77cca77873e882a352f87839790943de2975b Mon Sep 17 00:00:00 2001
From: Jingchao Zhong <92573736+perryzjc@users.noreply.github.com>
Date: Sun, 7 Apr 2024 12:54:12 -0700
Subject: [PATCH 42/88] Backup model updates
---
app/models/email_address.rb | 42 +++++++++++++
app/models/teacher.rb | 63 +++++++------------
...240407190126_create_new_email_addresses.rb | 15 +++++
...dapt_teachers_after_new_email_addresses.rb | 11 ++++
db/schema.rb | 43 +++++++++++--
5 files changed, 129 insertions(+), 45 deletions(-)
create mode 100644 app/models/email_address.rb
create mode 100644 db/migrate/20240407190126_create_new_email_addresses.rb
create mode 100644 db/migrate/20240407190132_adapt_teachers_after_new_email_addresses.rb
diff --git a/app/models/email_address.rb b/app/models/email_address.rb
new file mode 100644
index 00000000..d2f84a60
--- /dev/null
+++ b/app/models/email_address.rb
@@ -0,0 +1,42 @@
+# frozen_string_literal: true
+
+# == Schema Information
+#
+# Table name: email_addresses
+#
+# id :bigint not null, primary key
+# email :string not null
+# primary :boolean default(FALSE), not null
+# created_at :datetime not null
+# updated_at :datetime not null
+# teacher_id :bigint not null
+#
+# Indexes
+#
+# index_email_addresses_on_email (email) UNIQUE
+# index_email_addresses_on_teacher_id (teacher_id)
+# index_email_addresses_on_teacher_id_and_primary (teacher_id,primary) UNIQUE WHERE ("primary" = true)
+#
+# Foreign Keys
+#
+# fk_rails_... (teacher_id => teachers.id)
+#
+class EmailAddress < ApplicationRecord
+ belongs_to :teacher
+
+ validates :email, presence: true, uniqueness: true
+ validate :only_one_primary_email_per_teacher
+
+ before_save :downcase_email
+
+ private
+ def only_one_primary_email_per_teacher
+ if primary? && EmailAddress.where(teacher_id:, primary: true).where.not(id:).exists?
+ errors.add(:primary, "There can only be one primary email per teacher.")
+ end
+ end
+
+ def downcase_email
+ self.email = email.downcase
+ end
+end
diff --git a/app/models/teacher.rb b/app/models/teacher.rb
index 8a023449..607f70b5 100644
--- a/app/models/teacher.rb
+++ b/app/models/teacher.rb
@@ -8,14 +8,12 @@
# admin :boolean default(FALSE)
# application_status :string default("not_reviewed")
# education_level :integer default(NULL)
-# email :string
# first_name :string
# ip_history :inet default([]), is an Array
# languages :string default(["\"English\""]), is an Array
# last_name :string
# last_session_at :datetime
# more_info :string
-# personal_email :string
# personal_website :string
# session_count :integer default(0)
# snap :string
@@ -26,12 +24,9 @@
#
# Indexes
#
-# index_teachers_on_email (email) UNIQUE
-# index_teachers_on_email_and_first_name (email,first_name)
-# index_teachers_on_email_and_personal_email (email,personal_email) UNIQUE
-# index_teachers_on_school_id (school_id)
-# index_teachers_on_snap (snap) UNIQUE WHERE ((snap)::text <> ''::text)
-# index_teachers_on_status (status)
+# index_teachers_on_school_id (school_id)
+# index_teachers_on_snap (snap) UNIQUE WHERE ((snap)::text <> ''::text)
+# index_teachers_on_status (status)
#
# Foreign Keys
#
@@ -40,10 +35,10 @@
class Teacher < ApplicationRecord
WORLD_LANGUAGES = [ "Afrikaans", "Albanian", "Arabic", "Armenian", "Basque", "Bengali", "Bulgarian", "Catalan", "Cambodian", "Chinese (Mandarin)", "Croatian", "Czech", "Danish", "Dutch", "English", "Estonian", "Fiji", "Finnish", "French", "Georgian", "German", "Greek", "Gujarati", "Hebrew", "Hindi", "Hungarian", "Icelandic", "Indonesian", "Irish", "Italian", "Japanese", "Javanese", "Korean", "Latin", "Latvian", "Lithuanian", "Macedonian", "Malay", "Malayalam", "Maltese", "Maori", "Marathi", "Mongolian", "Nepali", "Norwegian", "Persian", "Polish", "Portuguese", "Punjabi", "Quechua", "Romanian", "Russian", "Samoan", "Serbian", "Slovak", "Slovenian", "Spanish", "Swahili", "Swedish ", "Tamil", "Tatar", "Telugu", "Thai", "Tibetan", "Tonga", "Turkish", "Ukrainian", "Urdu", "Uzbek", "Vietnamese", "Welsh", "Xhosa" ].freeze
- validates :first_name, :last_name, :email, :status, presence: true
- validates :email, uniqueness: true
- validates :personal_email, uniqueness: true, if: -> { personal_email.present? }
- validate :ensure_unique_personal_email, if: -> { email_changed? || personal_email_changed? }
+ has_many :email_addresses, dependent: :destroy
+ accepts_nested_attributes_for :email_addresses, allow_destroy: true
+
+ validates :first_name, :last_name, :status, presence: true
validate :valid_languages
before_validation :sort_and_clean_languages
@@ -127,7 +122,7 @@ def full_name
def email_name
# We need to normalize names for emails.
- "#{full_name} <#{email}>".delete(",")
+ "#{full_name} <#{primary_email}>".delete(",")
end
def snap_username
@@ -218,15 +213,9 @@ def short_application_status
end
def self.user_from_omniauth(omniauth)
- teachers = Teacher.where("LOWER(email) = :email or LOWER(personal_email) = :email",
- email: omniauth.email.downcase)
- if teachers.length > 1
- raise "Too Many Teachers Found"
- elsif teachers.length == 1
- teachers.first
- else
- nil
- end
+ teacher = EmailAddress.find_by(email: omniauth.email.downcase)&.teacher
+ raise "Teacher not found" unless teacher
+ teacher
end
def try_append_ip(ip)
@@ -241,8 +230,9 @@ def email_attributes
teacher_first_name: self.first_name,
teacher_last_name: self.last_name,
teacher_full_name: self.full_name,
- teacher_email: self.email,
- teacher_personal_email: self.personal_email,
+ teacher_email: self.primary_email,
+ # TODO: change to personal_emails
+ teacher_personal_email: self.non_primary_emails,
teacher_more_info: self.more_info,
teacher_snap: self.snap_username,
teacher_snap_username: self.snap_username,
@@ -292,21 +282,16 @@ def self.csv_export
end
end
- private
- def ensure_unique_personal_email
- # We want to ensure that both email and personal_email are unique across
- # BOTH columns (i.e. the email only appears once in the combined set)
- # However, it's perfectly valid for personal_email to be nil
- # TODO: This really suggests email needs to be its own table...
- teacher_with_email = Teacher.where.not(id: self.id).exists?(["email = :value OR personal_email = :value", { value: self.email }])
- if teacher_with_email
- errors.add(:email, "must be unique across both email and personal_email columns")
- end
- return unless self.personal_email.present?
+ def primary_email
+ email_addresses.find_by(primary: true)&.email
+ end
- teacher_personal_email = Teacher.where.not(id: self.id).exists?(["email = :value OR personal_email = :value", { value: self.personal_email }])
- if teacher_personal_email
- errors.add(:personal_email, "must be unique across both email and personal_email columns")
- end
+ def personal_emails
+ non_primary_emails
+ end
+
+ private
+ def non_primary_emails
+ email_addresses.where(primary: false)&.pluck(:email)
end
end
diff --git a/db/migrate/20240407190126_create_new_email_addresses.rb b/db/migrate/20240407190126_create_new_email_addresses.rb
new file mode 100644
index 00000000..09fdc0f7
--- /dev/null
+++ b/db/migrate/20240407190126_create_new_email_addresses.rb
@@ -0,0 +1,15 @@
+class CreateNewEmailAddresses < ActiveRecord::Migration[6.1]
+ def change
+ create_table :email_addresses do |t|
+ t.references :teacher, null: false, foreign_key: true
+ t.string :email, null: false
+ t.boolean :primary, default: false, null: false
+
+ t.timestamps
+ end
+
+ add_index :email_addresses, :email, unique: true
+ # quickly find primary emails for teachers
+ add_index :email_addresses, [:teacher_id, :primary], unique: true, where: '"primary" = TRUE'
+ end
+end
diff --git a/db/migrate/20240407190132_adapt_teachers_after_new_email_addresses.rb b/db/migrate/20240407190132_adapt_teachers_after_new_email_addresses.rb
new file mode 100644
index 00000000..c5e566fa
--- /dev/null
+++ b/db/migrate/20240407190132_adapt_teachers_after_new_email_addresses.rb
@@ -0,0 +1,11 @@
+class AdaptTeachersAfterNewEmailAddresses < ActiveRecord::Migration[6.1]
+ def change
+ # Remove indexes related to email and personal_email
+ remove_index :teachers, name: "index_teachers_on_email_and_first_name"
+ remove_index :teachers, name: "index_teachers_on_email_and_personal_email"
+ remove_index :teachers, name: "index_teachers_on_email"
+
+ remove_column :teachers, :email
+ remove_column :teachers, :personal_email
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 39c506c2..1410e82b 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: 2024_03_14_160959) do
+ActiveRecord::Schema.define(version: 2024_04_07_190132) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
@@ -53,6 +53,17 @@
t.index ["blob_id", "variation_digest"], name: "index_active_storage_variant_records_uniqueness", unique: true
end
+ create_table "email_addresses", force: :cascade do |t|
+ t.bigint "teacher_id", null: false
+ t.string "email", null: false
+ t.boolean "primary", default: false, null: false
+ t.datetime "created_at", precision: 6, null: false
+ t.datetime "updated_at", precision: 6, null: false
+ t.index ["email"], name: "index_email_addresses_on_email", unique: true
+ t.index ["teacher_id", "primary"], name: "index_email_addresses_on_teacher_id_and_primary", unique: true, where: "(\"primary\" = true)"
+ t.index ["teacher_id"], name: "index_email_addresses_on_teacher_id"
+ end
+
create_table "email_templates", force: :cascade do |t|
t.text "body"
t.string "path"
@@ -82,6 +93,28 @@
t.index ["url_slug"], name: "index_pages_on_url_slug", unique: true
end
+ create_table "pd_registrations", force: :cascade do |t|
+ t.integer "teacher_id", null: false
+ t.integer "professional_development_id", null: false
+ t.boolean "attended", default: false
+ t.string "role", null: false
+ t.datetime "created_at", precision: 6, null: false
+ t.datetime "updated_at", precision: 6, null: false
+ t.index ["teacher_id", "professional_development_id"], name: "index_pd_reg_on_teacher_id_and_pd_id", unique: true
+ end
+
+ create_table "professional_developments", force: :cascade do |t|
+ t.string "name", null: false
+ t.string "city", null: false
+ t.string "state"
+ t.string "country", null: false
+ t.date "start_date", null: false
+ t.date "end_date", null: false
+ t.integer "grade_level", null: false
+ t.datetime "created_at", precision: 6, null: false
+ t.datetime "updated_at", precision: 6, null: false
+ end
+
create_table "schools", id: :serial, force: :cascade do |t|
t.string "name"
t.string "city"
@@ -103,7 +136,6 @@
create_table "teachers", id: :serial, force: :cascade do |t|
t.string "first_name"
t.string "last_name"
- t.string "email"
t.string "snap"
t.integer "school_id"
t.datetime "created_at", default: -> { "now()" }
@@ -117,11 +149,7 @@
t.datetime "last_session_at"
t.inet "ip_history", default: [], array: true
t.integer "session_count", default: 0
- t.string "personal_email"
t.string "languages", default: ["English"], array: true
- t.index ["email", "first_name"], name: "index_teachers_on_email_and_first_name"
- t.index ["email", "personal_email"], name: "index_teachers_on_email_and_personal_email", unique: true
- t.index ["email"], name: "index_teachers_on_email", unique: true
t.index ["school_id"], name: "index_teachers_on_school_id"
t.index ["snap"], name: "index_teachers_on_snap", unique: true, where: "((snap)::text <> ''::text)"
t.index ["status"], name: "index_teachers_on_status"
@@ -129,7 +157,10 @@
add_foreign_key "active_storage_attachments", "active_storage_blobs", column: "blob_id"
add_foreign_key "active_storage_variant_records", "active_storage_blobs", column: "blob_id"
+ add_foreign_key "email_addresses", "teachers"
add_foreign_key "pages", "teachers", column: "creator_id"
add_foreign_key "pages", "teachers", column: "last_editor_id"
+ add_foreign_key "pd_registrations", "professional_developments"
+ add_foreign_key "pd_registrations", "teachers"
add_foreign_key "teachers", "schools"
end
From 47f2799ee99f5a14929adb1efa8c72fef7e9d21b Mon Sep 17 00:00:00 2001
From: Jingchao Zhong <92573736+perryzjc@users.noreply.github.com>
Date: Sun, 7 Apr 2024 20:49:18 -0700
Subject: [PATCH 43/88] Finish model updates
---
app/models/email_address.rb | 15 ++++++++++++++-
app/models/teacher.rb | 6 ++++--
2 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/app/models/email_address.rb b/app/models/email_address.rb
index d2f84a60..c9a66d86 100644
--- a/app/models/email_address.rb
+++ b/app/models/email_address.rb
@@ -24,10 +24,16 @@
class EmailAddress < ApplicationRecord
belongs_to :teacher
- validates :email, presence: true, uniqueness: true
+ # Regular expression for validating the format of an email address
+ # https://stackoverflow.com/a/7791100/23305580
+ EMAIL_REGEX = /\A[a-zA-Z0-9.!#$%&'*+\/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*\z/
+
+
+ validates :email, presence: true, uniqueness: true, format: { with: EMAIL_REGEX }
validate :only_one_primary_email_per_teacher
before_save :downcase_email
+ before_save :flag_teacher_if_email_changed
private
def only_one_primary_email_per_teacher
@@ -39,4 +45,11 @@ def only_one_primary_email_per_teacher
def downcase_email
self.email = email.downcase
end
+
+ def flag_teacher_if_email_changed
+ if self.email_changed? && !self.new_record?
+ teacher.email_changed_flag = true
+ teacher.handle_relevant_changes
+ end
+ end
end
diff --git a/app/models/teacher.rb b/app/models/teacher.rb
index 607f70b5..03bba45e 100644
--- a/app/models/teacher.rb
+++ b/app/models/teacher.rb
@@ -42,6 +42,9 @@ class Teacher < ApplicationRecord
validate :valid_languages
before_validation :sort_and_clean_languages
+ # Custom attribute for tracking email changes. This is not persisted in the database.
+ attribute :email_changed_flag, :boolean, default: false
+
enum application_status: {
validated: "Validated",
denied: "Denied",
@@ -213,8 +216,7 @@ def short_application_status
end
def self.user_from_omniauth(omniauth)
- teacher = EmailAddress.find_by(email: omniauth.email.downcase)&.teacher
- raise "Teacher not found" unless teacher
+ teacher = EmailAddress.find_by(email: omniauth.primary_email.downcase)&.teacher
teacher
end
From 8f7d9ec6d2f0b9a2fa18574720fa5fd74908fccd Mon Sep 17 00:00:00 2001
From: Jingchao Zhong <92573736+perryzjc@users.noreply.github.com>
Date: Sun, 7 Apr 2024 20:50:19 -0700
Subject: [PATCH 44/88] Update seed data
---
db/seed_data.rb | 44 ++++++++++++++++++++++++++++++++++++++++----
db/seeds.rb | 9 ++++++++-
2 files changed, 48 insertions(+), 5 deletions(-)
diff --git a/db/seed_data.rb b/db/seed_data.rb
index f5fd7128..b9319af5 100644
--- a/db/seed_data.rb
+++ b/db/seed_data.rb
@@ -163,21 +163,57 @@ def self.teachers
{
first_name: "Michael",
last_name: "Ball",
- email: "ball@berkeley.edu",
admin: true,
status: 0,
application_status: "Validated",
- school: School.find_by(name: "UC Berkeley")
+ school: School.find_by(name: "UC Berkeley"),
+
+ # Note: email field does not exist in the new schema of the Teacher model
+ # Include it in the seed data is to simulate the behavior of creating a new teacher,
+ # because we need to use it to compared with the EmailAddress model,
+ # to determine the existence of the teacher
+ email: "ball@berkeley.edu",
},
{
first_name: "Lauren",
last_name: "Mock",
- email: "lmock@berkeley.edu",
admin: true,
status: 0,
application_status: "Validated",
- school: School.find_by(name: "UC Berkeley")
+ school: School.find_by(name: "UC Berkeley"),
+
+ email: "lmock@berkeley.edu",
}
]
end
+
+ def self.email_addresses
+ [
+ {
+ email: "ball@berkeley.edu",
+ primary: true,
+ teacher: Teacher.find_by(first_name: "Michael"),
+ },
+ {
+ email: "lmock@berkeley.edu",
+ primary: true,
+ teacher: Teacher.find_by(first_name: "Lauren"),
+ },
+ {
+ email: "ball2@berkeley.edu",
+ primary: false,
+ teacher: Teacher.find_by(first_name: "Michael"),
+ },
+ {
+ email: "ball3@berkeley.edu",
+ primary: false,
+ teacher: Teacher.find_by(first_name: "Michael"),
+ },
+ {
+ email: "lmock2@berkeley.edu",
+ primary: false,
+ teacher: Teacher.find_by(first_name: "Lauren"),
+ },
+ ]
+ end
end
diff --git a/db/seeds.rb b/db/seeds.rb
index 2f4a8fc3..9c3ec387 100644
--- a/db/seeds.rb
+++ b/db/seeds.rb
@@ -10,5 +10,12 @@
SeedData.create_schools
SeedData.teachers.each do |teacher|
- Teacher.find_or_create_by(email: teacher[:email]).update(teacher)
+ teacher_id = EmailAddress.find_by(email: teacher[:email])&.teacher_id
+ # Seed data includes email field for comparison purposes, but it is not part of the Teacher model
+ teacher_attrs = teacher.except(:email)
+ Teacher.find_or_create_by(id: teacher_id).update(teacher_attrs)
+end
+
+SeedData.email_addresses.each do |email_address|
+ EmailAddress.find_or_create_by(email: email_address[:email]).update(email_address)
end
From b0b20cdb324d7dc59715b17d451b5ecf95d72f84 Mon Sep 17 00:00:00 2001
From: Jingchao Zhong <92573736+perryzjc@users.noreply.github.com>
Date: Sun, 7 Apr 2024 20:50:56 -0700
Subject: [PATCH 45/88] Update controllers
---
app/controllers/application_controller.rb | 2 +-
app/controllers/teachers_controller.rb | 89 ++++++++++++++++++-----
2 files changed, 70 insertions(+), 21 deletions(-)
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 05aa0ea0..391c9ce7 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -51,6 +51,6 @@ def check_teacher_admin
end
def set_sentry_user
- Sentry.set_user(id: session[:user_id], email: current_user&.email)
+ Sentry.set_user(id: session[:user_id], email: current_user&.primary_email)
end
end
diff --git a/app/controllers/teachers_controller.rb b/app/controllers/teachers_controller.rb
index d64b2430..39b6b832 100644
--- a/app/controllers/teachers_controller.rb
+++ b/app/controllers/teachers_controller.rb
@@ -42,7 +42,9 @@ def new
@school = @teacher.school # maybe delegate this
@readonly = false
if omniauth_data.present?
+ email_data = { email: omniauth_data.delete("email"), primary: true }
@teacher.assign_attributes(omniauth_data)
+ @teacher.email_addresses.build(email_data)
end
end
@@ -50,14 +52,15 @@ def new
# If you are logged in and not an admin, this should fail.
def create
# Find by email, but allow updating other info.
- @teacher = Teacher.find_by(email: teacher_params[:email])
+ teacher_id = EmailAddress.find_by(email: teacher_params[:primary_email])&.teacher_id
+ @teacher = Teacher.find_by(id: teacher_id)
if @teacher && defined?(current_user.id) && (current_user.id == @teacher.id)
params[:id] = current_user.id
update
return
elsif @teacher
redirect_to login_path,
- notice: "You already have signed up with '#{@teacher.email}'. Please log in."
+ notice: "You already have signed up with '#{@teacher.primary_email}'. Please log in."
return
end
@@ -70,13 +73,16 @@ def create
end
end
- @teacher = Teacher.new(teacher_params)
+ teacher_attr = teacher_params.except(:primary_email)
+ @teacher = Teacher.new(teacher_attr)
+ @teacher.email_addresses.build(email: teacher_params[:primary_email], primary: true)
+
@teacher.try_append_ip(request.remote_ip)
@teacher.session_count += 1
@teacher.school = @school
if @teacher.save
@teacher.not_reviewed!
- flash[:success] = "Thanks for signing up for BJC, #{@teacher.first_name}! You'll hear from us shortly. Your email address is: #{@teacher.email}."
+ flash[:success] = "Thanks for signing up for BJC, #{@teacher.first_name}! You'll hear from us shortly. Your email address is: #{@teacher.primary_email}."
TeacherMailer.form_submission(@teacher).deliver_now
TeacherMailer.teacher_form_submission(@teacher).deliver_now
redirect_to root_path
@@ -95,7 +101,18 @@ def edit
def update
load_school
ordered_schools
- @teacher.assign_attributes(teacher_params)
+
+ personal_emails_params = params[:teacher].keys.select { |key| key.to_s.start_with?("personal_email") }
+
+ primary_email = params[:teacher].delete(:primary_email)
+ personal_emails = params[:teacher].extract!(*personal_emails_params).values
+
+ # Now, `params[:teacher]` does not contain primary_email or any personal_emailX fields
+ teacher_attr = teacher_params
+ @teacher.assign_attributes(teacher_attr)
+
+ update_primary_email(primary_email)
+ update_personal_emails(personal_emails)
if teacher_params[:school_id].present?
@teacher.school = @school
else
@@ -111,13 +128,14 @@ def update
redirect_to root_path, alert: "Failed to update your information. You have already been denied. If you have questions, please email contact@bjc.berkeley.edu."
return
end
- if (@teacher.email_changed? || @teacher.snap_changed?) && !is_admin?
- redirect_to edit_teacher_path(current_user.id), alert: "Failed to update your information. If you want to change your email or Snap! username, please email contact@bjc.berkeley.edu."
+ if (@teacher.email_changed_flag || @teacher.snap_changed?) && !is_admin?
+ @teacher.email_changed_flag = false
+ redirect_to edit_teacher_path(params[:id]), alert: "Failed to update your information. If you want to change your email or Snap! username, please email contact@bjc.berkeley.edu."
return
end
- if !@teacher.save
- redirect_to edit_teacher_path(current_user.id),
- alert: "An error occurred: #{@teacher.errors.full_messages.join(', ')}"
+ unless @teacher.save
+ redirect_to edit_teacher_path(params[:id]),
+ alert: "An error occurred: #{@teacher.errors.full_messages.join(', ')}"
return
end
if !@teacher.validated? && !current_user.admin?
@@ -125,12 +143,12 @@ def update
TeacherMailer.teacher_form_submission(@teacher).deliver_now
end
if is_admin?
- redirect_to edit_teacher_path(current_user.id), notice: "Saved #{@teacher.full_name}"
+ redirect_to edit_teacher_path(params[:id]), notice: "Saved #{@teacher.full_name}"
return
else
@teacher.try_append_ip(request.remote_ip)
end
- redirect_to edit_teacher_path(current_user.id), notice: "Successfully updated your information"
+ redirect_to edit_teacher_path(params[:id]), notice: "Successfully updated your information"
end
def send_email_if_application_status_changed_and_email_resend_enabled
@@ -209,13 +227,15 @@ def load_school
end
def teacher_params
- teacher_attributes = [:first_name, :last_name, :school, :email, :status, :snap,
- :more_info, :personal_website, :education_level, :school_id]
- if is_admin?
- teacher_attributes << [:personal_email, :application_status,
- :request_reason, :skip_email]
- end
- params.require(:teacher).permit(*teacher_attributes, languages: [])
+ teacher_attributes = [:first_name, :last_name, :school, :primary_email, :status, :snap,
+ :more_info, :personal_website, :education_level, :school_id, languages: []]
+ admin_attributes = [:application_status, :request_reason, :skip_email]
+ teacher_attributes.push(*admin_attributes) if is_admin?
+
+ # Dynamically extract and permit personal_emailX attributes from params
+ personal_email_attributes = params[:teacher].keys.select { |key| key.to_s.start_with?("personal_email") }
+ permitted_attributes = teacher_attributes + personal_email_attributes
+ params.require(:teacher).permit(*permitted_attributes)
end
def school_params
@@ -230,7 +250,7 @@ def omniauth_data
def ordered_schools
if params[:id].present?
load_teacher
- @ordered_schools ||= [ @teacher.school ] +
+ @ordered_schools ||= [@teacher.school] +
School.all.order(:name).reject { |s| s.id == @teacher.school_id }
else
@ordered_schools ||= School.all.order(:name)
@@ -260,4 +280,33 @@ def sanitize_params
def load_pages
@pages ||= Page.where(viewer_permissions: Page.viewable_pages(current_user))
end
+
+ def update_primary_email(primary_email)
+ return unless primary_email.present?
+
+ # First, ensure the current primary email is marked as not primary if it's not the same as the new one
+ @teacher.email_addresses.where(primary: true).each do |email_record|
+ if email_record.email != primary_email
+ email_record.update(primary: false)
+ end
+ end
+
+ primary_email_record = @teacher.email_addresses.find_or_initialize_by(email: primary_email)
+ primary_email_record.primary = true
+
+ primary_email_record.save if primary_email_record.changed?
+ end
+
+ def update_personal_emails(personal_emails)
+ personal_emails = personal_emails.reject(&:empty?)
+ return if personal_emails.empty?
+
+ current_emails = @teacher.email_addresses.pluck(:email)
+
+ new_emails = personal_emails - current_emails
+
+ new_emails.each do |email|
+ @teacher.email_addresses.build(email:, primary: false)
+ end
+ end
end
From a2bf8dc0f8437c0e34da0ff30793ec954e0c4cc9 Mon Sep 17 00:00:00 2001
From: Jingchao Zhong <92573736+perryzjc@users.noreply.github.com>
Date: Sun, 7 Apr 2024 20:51:15 -0700
Subject: [PATCH 46/88] Update frontend
---
app/views/schools/show.html.erb | 2 +-
app/views/teachers/_form.html.erb | 9 +++++----
app/views/teachers/_teacher.erb | 6 +++---
app/views/teachers/_teacher_info.html.erb | 10 +++++-----
4 files changed, 14 insertions(+), 13 deletions(-)
diff --git a/app/views/schools/show.html.erb b/app/views/schools/show.html.erb
index 5a579716..092554f0 100644
--- a/app/views/schools/show.html.erb
+++ b/app/views/schools/show.html.erb
@@ -66,7 +66,7 @@
<% @school.teachers.each do |teacher| %>
diff --git a/app/views/teachers/_form.html.erb b/app/views/teachers/_form.html.erb
index 20903a9a..c4115b8e 100644
--- a/app/views/teachers/_form.html.erb
+++ b/app/views/teachers/_form.html.erb
@@ -57,8 +57,8 @@ status ONLY IF the person viewing this page is an admin. %>
@@ -81,13 +81,14 @@ status ONLY IF the person viewing this page is an admin. %>
<%# For now... only admins can enter/edit personal emails. %>
- <%- if current_user&.admin? || @teacher.personal_email.present? %>
+
+ <%- if current_user&.admin? || @teacher.personal_emails.present? %>
From e7e88b4b1576d11d1f39adf8b71ca6e8011aab1a Mon Sep 17 00:00:00 2001
From: Jingchao Zhong <92573736+perryzjc@users.noreply.github.com>
Date: Sun, 7 Apr 2024 20:52:44 -0700
Subject: [PATCH 47/88] Update fixtures
---
spec/factories/email_addresses.rb | 9 +++++++
spec/fixtures/email_addresses.yml | 41 +++++++++++++++++++++++++++++++
spec/fixtures/teachers.yml | 19 +++-----------
3 files changed, 53 insertions(+), 16 deletions(-)
create mode 100644 spec/factories/email_addresses.rb
create mode 100644 spec/fixtures/email_addresses.yml
diff --git a/spec/factories/email_addresses.rb b/spec/factories/email_addresses.rb
new file mode 100644
index 00000000..684d0f57
--- /dev/null
+++ b/spec/factories/email_addresses.rb
@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+FactoryBot.define do
+ factory :email_address do
+ association :teacher
+ sequence(:email) { |n| "teacher-#{n}@example.edu" }
+ primary { false }
+ end
+end
diff --git a/spec/fixtures/email_addresses.yml b/spec/fixtures/email_addresses.yml
new file mode 100644
index 00000000..ddbc8fdf
--- /dev/null
+++ b/spec/fixtures/email_addresses.yml
@@ -0,0 +1,41 @@
+admin_email:
+# teacher: admin
+ teacher_id: 1
+ email: 'ye@berkeley.edu'
+ primary: true
+
+validated_teacher_email:
+# teacher: validated_teacher
+ teacher_id: 2
+ email: 'validated@teacher.edu'
+ primary: true
+
+bob_email:
+# teacher: bob
+ teacher_id: 3
+ email: 'bob@gmail.com'
+ primary: true
+
+long_email:
+# teacher: long
+ teacher_id: 4
+ email: 'short@long.com'
+ primary: true
+
+reimu_email:
+# teacher: reimu
+ teacher_id: 5
+ email: 'reimu@touhou.com'
+ primary: true
+
+barney_email:
+# teacher: barney
+ teacher_id: 6
+ email: 'barneydinosaur@gmail.com'
+ primary: true
+
+barney_personal_email1:
+# teacher: barney
+ teacher_id: 6
+ email: 'bigpurpletrex@gmail.com'
+ primary: false
diff --git a/spec/fixtures/teachers.yml b/spec/fixtures/teachers.yml
index befd9393..26e5dc6b 100644
--- a/spec/fixtures/teachers.yml
+++ b/spec/fixtures/teachers.yml
@@ -6,14 +6,12 @@
# admin :boolean default(FALSE)
# application_status :string default("not_reviewed")
# education_level :integer default(NULL)
-# email :string
# first_name :string
# ip_history :inet default([]), is an Array
# languages :string default(["\"English\""]), is an Array
# last_name :string
# last_session_at :datetime
# more_info :string
-# personal_email :string
# personal_website :string
# session_count :integer default(0)
# snap :string
@@ -24,12 +22,9 @@
#
# Indexes
#
-# index_teachers_on_email (email) UNIQUE
-# index_teachers_on_email_and_first_name (email,first_name)
-# index_teachers_on_email_and_personal_email (email,personal_email) UNIQUE
-# index_teachers_on_school_id (school_id)
-# index_teachers_on_snap (snap) UNIQUE WHERE ((snap)::text <> ''::text)
-# index_teachers_on_status (status)
+# index_teachers_on_school_id (school_id)
+# index_teachers_on_snap (snap) UNIQUE WHERE ((snap)::text <> ''::text)
+# index_teachers_on_status (status)
#
# Foreign Keys
#
@@ -43,7 +38,6 @@ admin:
last_name: Wang
status: 4
more_info: A CS169 Student
- email: 'ye@berkeley.edu'
snap: ye
application_status: Validated
education_level: -1
@@ -56,7 +50,6 @@ validated_teacher:
last_name: Teacher
status: 4
more_info: A CS169 Student
- email: validated@teacher.edu
snap: validated
application_status: Validated
education_level: -1
@@ -67,7 +60,6 @@ bob:
first_name: Bob
last_name: Johnson
snap: BobJohnson
- email: 'bob@gmail.com'
status: 1
more_info: ''
application_status: Denied
@@ -79,7 +71,6 @@ long:
first_name: Short
last_name: Long
snap: song
- email: 'short@long.com'
status: 3
school: berkeley
more_info: ''
@@ -91,7 +82,6 @@ reimu:
first_name: Reimu
last_name: Hakurei
snap: reimu
- email: 'reimu@touhou.com'
status: 4
more_info: Best Touhou Character
school: berkeley
@@ -103,11 +93,8 @@ barney:
first_name: Barney
last_name: Dinosaur
snap: barney
- email: 'barneydinosaur@gmail.com'
- personal_email: 'bigpurpletrex@gmail.com'
status: 1
more_info: ''
application_status: Denied
school: berkeley
education_level: -1
-
From 9e2d47e1fff175fd3c1ee916288260048a2531d8 Mon Sep 17 00:00:00 2001
From: Jingchao Zhong <92573736+perryzjc@users.noreply.github.com>
Date: Sun, 7 Apr 2024 20:53:26 -0700
Subject: [PATCH 48/88] Update cucumber tests
---
features/access.feature | 2 +-
features/admin.feature | 32 +++++++++++-----------
features/email_template.feature | 2 +-
features/form_submission.feature | 8 +++---
features/pages_admin.feature | 2 +-
features/pages_permissions.feature | 2 +-
features/school_form_submission.feature | 6 ++--
features/step_definitions/admin_steps.rb | 7 +++--
features/step_definitions/form_steps.rb | 9 ++++--
features/step_definitions/teacher_steps.rb | 16 ++++++++---
features/teacher.feature | 32 +++++++++++-----------
features/teacher_info_request.feature | 8 +++---
12 files changed, 70 insertions(+), 56 deletions(-)
diff --git a/features/access.feature b/features/access.feature
index 3b37e633..5d6df362 100644
--- a/features/access.feature
+++ b/features/access.feature
@@ -5,7 +5,7 @@ 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 |
+ | first_name | last_name | admin | primary_email |
| Alice | Admin | true | testadminuser@berkeley.edu |
| Todd | Teacher | false | testteacher@berkeley.edu |
diff --git a/features/admin.feature b/features/admin.feature
index fcec4b12..c23dd49d 100644
--- a/features/admin.feature
+++ b/features/admin.feature
@@ -6,7 +6,7 @@ Feature: basic admin functionality
Background: Has an Admin in DB
Given the following teachers exist:
- | first_name | last_name | admin | email |
+ | first_name | last_name | admin | primary_email |
| Admin | User | true | testadminuser@berkeley.edu |
Scenario: Logging in as an admin
@@ -79,7 +79,7 @@ Feature: basic admin functionality
| name | country | city | state | website | grade_level | school_type |
| UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | university | public |
Given the following teachers exist:
- | first_name | last_name | admin | email | school | snap |
+ | first_name | last_name | admin | primary_email | school | snap |
| Joseph | Mamoa | false | testteacher@berkeley.edu | UC Berkeley | alonzo |
Given I am on the BJC home page
Given I have an admin email
@@ -99,7 +99,7 @@ Feature: basic admin functionality
| name | country | city | state | website |
| UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu |
Given the following teachers exist:
- | first_name | last_name | admin | email | school | languages |
+ | first_name | last_name | admin | primary_email | school | languages |
| Joseph | Mamoa | false | testteacher@berkeley.edu | UC Berkeley | - English\n- French\n- Spanish |
Given I am on the BJC home page
And I have an admin email
@@ -113,7 +113,7 @@ Feature: basic admin functionality
| name | country | city | state | website |
| UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu |
Given the following teachers exist:
- | first_name | last_name | admin | email | school | languages |
+ | first_name | last_name | admin | primary_email | school | languages |
| Joseph | Mamoa | false | testteacher@berkeley.edu | UC Berkeley | - English\n- Hindi |
Given I am on the BJC home page
And I have an admin email
@@ -134,7 +134,7 @@ Feature: basic admin functionality
| name | country | city | state | website | grade_level | school_type |
| UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | university | public |
Given the following teachers exist:
- | first_name | last_name | admin | email | school | snap | application_status |
+ | first_name | last_name | admin | primary_email | school | snap | application_status |
| Bobby | John | false | testteacher@berkeley.edu | UC Berkeley | bobby | denied |
Given I am on the BJC home page
And I have an admin email
@@ -169,7 +169,7 @@ Feature: basic admin functionality
| name | country | city | state | website | grade_level | school_type |
| UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | university | public |
Given the following teachers exist:
- | first_name | last_name | admin | email | school | snap | application_status |
+ | first_name | last_name | admin | primary_email | school | snap | application_status |
| Bobby | John | false | testteacher@berkeley.edu | UC Berkeley | bobby | denied |
Given I am on the BJC home page
And I have an admin email
@@ -187,7 +187,7 @@ Feature: basic admin functionality
| name | country | city | state | website | grade_level | school_type |
| UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | university | public |
Given the following teachers exist:
- | first_name | last_name | admin | email | school | snap | application_status |
+ | first_name | last_name | admin | primary_email | school | snap | application_status |
| Bobby | John | false | testteacher@berkeley.edu | UC Berkeley | bobby | denied |
Given I am on the BJC home page
And I have an admin email
@@ -207,7 +207,7 @@ Feature: basic admin functionality
| name | country | city | state | website | grade_level | school_type |
| UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | university | public |
Given the following teachers exist:
- | first_name | last_name | admin | email | school |
+ | first_name | last_name | admin | primary_email | school |
| Joseph | Mamoa | false | testteacher@berkeley.edu | UC Berkeley |
Given I am on the BJC home page
Given I have an admin email
@@ -229,7 +229,7 @@ Feature: basic admin functionality
| name | country | city | state | website | grade_level | school_type |
| UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | university | public |
Given the following teachers exist:
- | first_name | last_name | admin | email | school |
+ | first_name | last_name | admin | primary_email | school |
| Joseph | Mamoa | false | testteacher@berkeley.edu | UC Berkeley |
When I go to the edit page for Joseph Mamoa
Then should see "You need to log in to access this."
@@ -239,7 +239,7 @@ Feature: basic admin functionality
| name | country | city | state | website | grade_level | school_type |
| UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | university | public |
Given the following teachers exist:
- | first_name | last_name | admin | email | school | application_status |
+ | first_name | last_name | admin | primary_email | school | application_status |
| Victor | Validateme | false | testteacher1@berkeley.edu | UC Berkeley | Validated |
| Danny | Denyme | false | testteacher2@berkeley.edu | UC Berkeley | Denied |
| Peter | Pendme | false | testteacher3@berkeley.edu | UC Berkeley | Not Reviewed |
@@ -263,7 +263,7 @@ Feature: basic admin functionality
| name | country | city | state | website | grade_level | school_type |
| UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | university | public |
Given the following teachers exist:
- | first_name | last_name | admin | email | school | snap |
+ | first_name | last_name | admin | primary_email | school | snap |
| Joseph | Test | false | testteacher@berkeley.edu | UC Berkeley | alonzo |
Given I am on the BJC home page
Given I have an admin email
@@ -284,7 +284,7 @@ Feature: basic admin functionality
| name | country | city | state | website | grade_level | school_type |
| UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | university | public |
Given the following teachers exist:
- | first_name | last_name | admin | email | school | snap |
+ | first_name | last_name | admin | primary_email | school | snap |
| Joseph | Mamoa New | false | testteacher@berkeley.edu | UC Berkeley | alonzo |
Given I am on the BJC home page
Given I have an admin email
@@ -308,7 +308,7 @@ Feature: basic admin functionality
| name | country | city | state | website | grade_level | school_type |
| UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | university | public |
Given the following teachers exist:
- | first_name | last_name | admin | email | school | snap | application_status |
+ | first_name | last_name | admin | primary_email | school | snap | application_status |
| Joseph | Mamoa | false | testteacher@berkeley.edu | UC Berkeley | alonzo | validated |
Given I am on the BJC home page
Given I have an admin email
@@ -345,7 +345,7 @@ Feature: basic admin functionality
| name | country | city | state | website | grade_level | school_type |
| UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | university | public |
And the following teachers exist:
- | first_name | last_name | admin | email | school |
+ | first_name | last_name | admin | primary_email | school |
| Joseph | Mamoa | false | testteacher@berkeley.edu | UC Berkeley |
And I am on the BJC home page
And I have an admin email
@@ -368,7 +368,7 @@ Feature: basic admin functionality
| name | country | city | state | website | grade_level | school_type |
| UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | university | public |
And the following teachers exist:
- | first_name | last_name | admin | email | school |
+ | first_name | last_name | admin | primary_email | school |
| Jane | Doe | false | janedoe@berkeley.edu | UC Berkeley |
Given I am on the BJC home page
And I have an admin email
@@ -376,7 +376,7 @@ Feature: basic admin functionality
Then I can log in with Google
When I go to the teachers page
And I go to the edit page for Jane Doe
- And I fill in "teacher_email" with ""
+ And I fill in "teacher_primary_email" with ""
And I press "Update"
Then I should be on the edit page for Jane Doe
diff --git a/features/email_template.feature b/features/email_template.feature
index e527204c..0d561f1a 100644
--- a/features/email_template.feature
+++ b/features/email_template.feature
@@ -4,7 +4,7 @@ Feature: email template features
Background: I am logged in as an admin, and email templates are generated
Given the following teachers exist:
- | first_name | last_name | admin | email |
+ | first_name | last_name | admin | primary_email |
| Joseph | Mamoa | true | testadminuser@berkeley.edu |
Given I am on the BJC home page
Given I have an admin email
diff --git a/features/form_submission.feature b/features/form_submission.feature
index 574e26d6..092e9a58 100644
--- a/features/form_submission.feature
+++ b/features/form_submission.feature
@@ -27,7 +27,7 @@ Feature: submit a form as a teacher
And I select "Public" from "School Type"
And I press "Submit"
Then I see a confirmation "Thanks for signing up for BJC"
- And I send a form submission email to both admin and teacher with email "TESTkpzhu@berkeley.edu"
+ And I send a form submission email to both admin and teacher with email "testkpzhu@berkeley.edu"
Scenario: Not Correctly filling out and unsuccessful form submission
Given I am on the BJC home page
@@ -125,7 +125,7 @@ Feature: submit a form as a teacher
Scenario: Filling out new form with existing email should not update information
Given the following teachers exist:
- | first_name | last_name | admin | email |
+ | first_name | last_name | admin | primary_email |
| Alice | Adams | false | alice@berkeley.edu |
And I am on the BJC home page
And I enter my "First Name" as "Mallory"
@@ -148,7 +148,7 @@ Feature: submit a form as a teacher
Scenario: Filling out new form with existing Snap should not create new teacher
Given the following teachers exist:
- | first_name | last_name | admin | email | snap |
+ | first_name | last_name | admin | primary_email | snap |
| Alice | Adams | false | alice@berkeley.edu | aliceadams |
And I am on the BJC home page
And I enter my "First Name" as "Mallory"
@@ -235,7 +235,7 @@ Feature: submit a form as a teacher
| name | country | city | state | website | grade_level | school_type |
| UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | university | public |
And the following teachers exist:
- | first_name | last_name | admin | email | school | application_status |
+ | first_name | last_name | admin | primary_email | school | application_status |
| Joseph | Mamoa | false | testteacher@berkeley.edu | UC Berkeley | info_needed |
And I am on the BJC home page
And I have a teacher Google email
diff --git a/features/pages_admin.feature b/features/pages_admin.feature
index 3189dbea..822d2d8e 100644
--- a/features/pages_admin.feature
+++ b/features/pages_admin.feature
@@ -4,7 +4,7 @@ Feature: pages features as an admin
Background: I am logged in as an admin
Given the following teachers exist:
- | first_name | last_name | admin | email |
+ | first_name | last_name | admin | primary_email |
| Joseph | Mamoa | true | testadminuser@berkeley.edu |
Given I am on the BJC home page
Given I have an admin email
diff --git a/features/pages_permissions.feature b/features/pages_permissions.feature
index 21537976..09991457 100644
--- a/features/pages_permissions.feature
+++ b/features/pages_permissions.feature
@@ -4,7 +4,7 @@ Feature: pages features a verified teacher
Background: Has admin and teacher in DB along with pages of each permission type
Given the following teachers exist:
- | first_name | last_name | admin | email | application_status |
+ | first_name | last_name | admin | primary_email | application_status |
| Joseph | Mamoa | true | testadminuser@berkeley.edu | Not Reviewed |
| Todd | Teacher | false | testteacher@berkeley.edu | Validated |
Given the following pages exist:
diff --git a/features/school_form_submission.feature b/features/school_form_submission.feature
index 0710f69a..ad9175e9 100644
--- a/features/school_form_submission.feature
+++ b/features/school_form_submission.feature
@@ -9,7 +9,7 @@ Scenario: Viewing the schools page should show the all current schools
| UC Irvine | US | Irvine | CA | https://www.uci.edu |
| UC Scam Diego | US | La Jolla | CA | https://www.ucsd.edu |
And the following teachers exist:
- | first_name | last_name | admin | email | school |
+ | first_name | last_name | admin | primary_email | school |
| Admin | User | true | testadminuser@berkeley.edu | UC Berkeley |
| Joseph | Mamoa | false | jmomoa@berkeley.edu | UC Berkeley |
| Dwayne | Johnson | false | djohnson@berkeley.edu | UC Berkeley |
@@ -26,7 +26,7 @@ Scenario: Viewing the schools page should show the all current schools
Scenario: Admins can see Tags and NCES ID
Given the following teachers exist:
- | first_name | last_name | admin | email |
+ | first_name | last_name | admin | primary_email |
| Joseph | Mamoa | true | testadminuser@berkeley.edu |
Given I am on the BJC home page
Given I have an admin email
@@ -39,7 +39,7 @@ Scenario: Admins can see Tags and NCES ID
Scenario: Admins can create new schools
Given the following teachers exist:
- | first_name | last_name | admin | email |
+ | first_name | last_name | admin | primary_email |
| Joseph | Mamoa | true | testadminuser@berkeley.edu |
Given I am on the BJC home page
Given I have an admin email
diff --git a/features/step_definitions/admin_steps.rb b/features/step_definitions/admin_steps.rb
index 40e85d69..5f17e2e7 100644
--- a/features/step_definitions/admin_steps.rb
+++ b/features/step_definitions/admin_steps.rb
@@ -18,7 +18,7 @@
name: "Admin User",
first_name: "Admin",
last_name: "User",
- email: "testadminuser@berkeley.edu",
+ primary_email: "testadminuser@berkeley.edu",
school: "UC Berkeley"
}
})
@@ -34,7 +34,7 @@
name: "Random User",
first_name: "Random",
last_name: "User",
- email: "randomemail@berkeley.edu",
+ primary_email: "randomemail@berkeley.edu",
school: "UC Berkeley",
}
})
@@ -79,6 +79,9 @@
# sent first, as this is how it is implemented in the code
admin_fs_email = ActionMailer::Base.deliveries[-2]
teacher_fs_email = ActionMailer::Base.deliveries.last
+
+ expect(admin_fs_email).not_to eq nil
+ expect(teacher_fs_email).not_to eq nil
admin_fs_email.subject.should eq "Form Submission"
admin_fs_email.to[0].should eq "lmock@berkeley.edu"
admin_fs_email.to[1].should eq "contact@bjc.berkeley.edu"
diff --git a/features/step_definitions/form_steps.rb b/features/step_definitions/form_steps.rb
index b1dbc9b7..eed1d6a0 100644
--- a/features/step_definitions/form_steps.rb
+++ b/features/step_definitions/form_steps.rb
@@ -9,7 +9,7 @@
end
And(/^"(.*)" is not in the database$/) do |email|
- expect(Teacher.exists?(email:)).to be false
+ expect(EmailAddress.exists?(email:)).to be false
end
Given(/^I enter (?:my)? "(.*)" as "(.*)"$/) do |field_name, input|
@@ -91,11 +91,14 @@
end
Then(/the "(.*)" of the user with email "(.*)" should be "(.*)"/) do |field, email, expected|
- expect(Teacher.find_by(email:)[field]).to eq(expected)
+ email = EmailAddress.find_by(email:)
+ expect(email.teacher[field]).to eq(expected)
end
Then(/^I should find a teacher with email "([^"]*)" and school country "([^"]*)" in the database$/) do |email, country|
- teacher = Teacher.includes(:school).where(email:, 'schools.country': country).first
+ teacher = Teacher.joins(:email_addresses, :school)
+ .where(email_addresses: { email: }, schools: { country: })
+ .first
expect(teacher).not_to be_nil, "No teacher found with email #{email} and country #{country}"
end
diff --git a/features/step_definitions/teacher_steps.rb b/features/step_definitions/teacher_steps.rb
index 07798cee..ea3dad51 100644
--- a/features/step_definitions/teacher_steps.rb
+++ b/features/step_definitions/teacher_steps.rb
@@ -12,7 +12,7 @@
name: "Joseph",
first_name: "Joseph",
last_name: "Mamoa",
- email: "testteacher@berkeley.edu",
+ primary_email: "testteacher@berkeley.edu",
school: "UC Berkeley",
},
credentials: {
@@ -45,7 +45,6 @@
teachers_default = {
first_name: "Alonzo",
last_name: "Church",
- email: "alonzo@snap.berkeley.edu",
snap: "",
status: "Other - Please specify below.",
education_level: 1,
@@ -53,7 +52,13 @@
admin: false,
personal_website: "https://snap.berkeley.edu",
application_status: "Not Reviewed",
- languages: ["English"]
+ languages: ["English"],
+
+ # Note: primary email field does not exist in the new schema of the Teacher model
+ # Include it in the seed data is to simulate the behavior of creating a new teacher,
+ # because we need to use it to compared with the EmailAddress model,
+ # to determine the existence of the teacher
+ primary_email: "alonzo@snap.berkeley.edu",
}
teachers_table.symbolic_hashes.each do |teacher|
@@ -68,10 +73,13 @@
end
end
+ email = teacher.delete(:primary_email)
+
school_name = teacher.delete(:school)
school = School.find_by(name: school_name || "UC Berkeley")
teacher[:school_id] = school.id
- Teacher.create!(teacher)
+ teacher = Teacher.create!(teacher)
+ EmailAddress.create!(email:, teacher:, primary: true)
School.reset_counters(school.id, :teachers)
end
end
diff --git a/features/teacher.feature b/features/teacher.feature
index 8edbb36c..da7bab66 100644
--- a/features/teacher.feature
+++ b/features/teacher.feature
@@ -27,7 +27,7 @@ Scenario: Logging in as a teacher with Google account should be able to edit the
| UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu |
| Cupertino High School | US | Cupertino | CA | https://chs.fuhsd.org |
Given the following teachers exist:
- | first_name | last_name | admin | email | school |
+ | first_name | last_name | admin | primary_email | school |
| Joseph | Mamoa | false | testteacher@berkeley.edu | UC Berkeley |
Given I have a teacher Google email
Given I am on the BJC home page
@@ -54,7 +54,7 @@ Scenario: Logging in as a teacher with Microsoft account should be able to edit
| name | country | city | state | website |
| UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu |
Given the following teachers exist:
- | first_name | last_name | admin | email | school |
+ | first_name | last_name | admin | primary_email | school |
| Joseph | Mamoa | false | testteacher@berkeley.edu | UC Berkeley |
Given I have a teacher Microsoft email
Given I am on the BJC home page
@@ -68,7 +68,7 @@ Scenario: Logging in as a teacher with Snap account should be able to edit their
| name | country | city | state | website |
| UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu |
Given the following teachers exist:
- | first_name | last_name | admin | email | school |
+ | first_name | last_name | admin | primary_email | school |
| Joseph | Mamoa | false | testteacher@berkeley.edu | UC Berkeley |
Given I have a teacher Snap email
Given I am on the BJC home page
@@ -82,7 +82,7 @@ Scenario: Logging in as a teacher with Snap account should be able to edit their
| name | country | city | state | website |
| UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu |
Given the following teachers exist:
- | first_name | last_name | admin | email | school |
+ | first_name | last_name | admin | primary_email | school |
| Joseph | Mamoa | false | testteacher@berkeley.edu | UC Berkeley |
Given I have a teacher Clever email
Given I am on the BJC home page
@@ -96,7 +96,7 @@ Scenario: Logged in teacher with Not_Reviewed application status can update thei
| name | country | city | state | website |
| UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu |
Given the following teachers exist:
- | first_name | last_name | admin | email | school | snap |
+ | first_name | last_name | admin | primary_email | school | snap |
| Joseph | Mamoa | false | testteacher@berkeley.edu | UC Berkeley | alonzo |
Given I have a teacher Google email
Given I am on the BJC home page
@@ -123,7 +123,7 @@ Scenario: Logged in teacher with Not_Reviewed application status can update thei
| name | country | city | state | website |
| UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu |
Given the following teachers exist:
- | first_name | last_name | admin | email | school | snap |
+ | first_name | last_name | admin | primary_email | school | snap |
| Joseph | Mamoa | false | testteacher@berkeley.edu | UC Berkeley | alonzo |
Given I have a teacher Google email
Given I am on the BJC home page
@@ -149,7 +149,7 @@ Scenario: Logged in teacher can only edit their own information
| name | country | city | state | website |
| UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu |
Given the following teachers exist:
- | first_name | last_name | admin | email |
+ | first_name | last_name | admin | primary_email |
| Joseph | Mamoa | false | testteacher@berkeley.edu |
| Jane | Austin | false | testteacher2@berkeley.edu |
Given I have a teacher Google email
@@ -164,7 +164,7 @@ Scenario: Logging in as a teacher with not_reviewed status should see "Update" i
| name | country | city | state | website |
| UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu |
Given the following teachers exist:
- | first_name | last_name | admin | email |
+ | first_name | last_name | admin | primary_email |
| Joseph | Mamoa | false | testteacher@berkeley.edu |
Given I have a teacher Google email
Given I am on the BJC home page
@@ -178,7 +178,7 @@ Scenario: Frontend should not allow Teacher to edit their email
| name | country | city | state | website |
| UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu |
Given the following teachers exist:
- | first_name | last_name | admin | email | snap |
+ | first_name | last_name | admin | primary_email | snap |
| Jane | Austin | false | testteacher@berkeley.edu | Jane |
Given I have a teacher Google email
Given I am on the BJC home page
@@ -196,7 +196,7 @@ Scenario: Validated teacher should see resend button
| name | country | city | state | website |
| UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu |
Given the following teachers exist:
- | first_name | last_name | admin | email | snap | application_status |
+ | first_name | last_name | admin | primary_email | snap | application_status |
| Jane | Austin | false | testteacher@berkeley.edu | Jane | validated |
Given I have a teacher Google email
Given I am on the BJC home page
@@ -210,7 +210,7 @@ Scenario: teacher with not_reviewed status should not see resend button
| name | country | city | state | website |
| UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu |
Given the following teachers exist:
- | first_name | last_name | admin | email | snap | application_status |
+ | first_name | last_name | admin | primary_email | snap | application_status |
| Jane | Austin | false | testteacher@berkeley.edu | Jane | Not Reviewed |
Given I have a teacher Google email
Given I am on the BJC home page
@@ -224,7 +224,7 @@ Scenario: Denied teacher should not see resend button
| name | country | city | state | website |
| UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu |
Given the following teachers exist:
- | first_name | last_name | admin | email | snap | application_status |
+ | first_name | last_name | admin | primary_email | snap | application_status |
| Jane | Austin | false | testteacher@berkeley.edu | Jane | denied |
Given I have a teacher Google email
Given I am on the BJC home page
@@ -238,7 +238,7 @@ Scenario: Denied teacher cannot edit their information
| name | country | city | state | website |
| UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu |
Given the following teachers exist:
- | first_name | last_name | admin | email | snap | application_status | more_info |
+ | first_name | last_name | admin | primary_email | snap | application_status | more_info |
| Jane | Austin | false | testteacher@berkeley.edu | Jane | denied | Original Information |
Given I have a teacher Google email
Given I am on the BJC home page
@@ -254,7 +254,7 @@ Scenario: Validated teacher should not see Tags or NCES ID
| name | country | city | state | website |
| UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu |
Given the following teachers exist:
- | first_name | last_name | admin | email | snap | application_status |
+ | first_name | last_name | admin | primary_email | snap | application_status |
| Jane | Austin | false | testteacher@berkeley.edu | Jane | validated |
Given I have a teacher Google email
Given I am on the BJC home page
@@ -269,7 +269,7 @@ Scenario: Teacher with not_reviewed status should not see Tags or NCES ID
| name | country | city | state | website |
| UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu |
Given the following teachers exist:
- | first_name | last_name | admin | email | snap | application_status |
+ | first_name | last_name | admin | primary_email | snap | application_status |
| Jane | Austin | false | testteacher@berkeley.edu | Jane | Not Reviewed |
Given I have a teacher Google email
Given I am on the BJC home page
@@ -284,7 +284,7 @@ Scenario: Denied teacher should not see Tags or NCES ID
| name | country | city | state | website |
| UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu |
Given the following teachers exist:
- | first_name | last_name | admin | email | snap | application_status |
+ | first_name | last_name | admin | primary_email | snap | application_status |
| Jane | Austin | false | testteacher@berkeley.edu | Jane | denied |
Given I have a teacher Google email
Given I am on the BJC home page
diff --git a/features/teacher_info_request.feature b/features/teacher_info_request.feature
index 367d60a4..a78bfd99 100644
--- a/features/teacher_info_request.feature
+++ b/features/teacher_info_request.feature
@@ -6,7 +6,7 @@ Feature: Request additional information for teacher application
Background: Has an Admin in DB
Given the following teachers exist:
- | first_name | last_name | admin | email |
+ | first_name | last_name | admin | primary_email |
| Joseph | Mamoa | true | testadminuser@berkeley.edu |
Scenario: Admin requests more information and teacher sees notification upon login
@@ -14,7 +14,7 @@ Feature: Request additional information for teacher application
| name | country | city | state | website | grade_level | school_type |
| UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | university | public |
And the following teachers exist:
- | first_name | last_name | admin | email | school |
+ | first_name | last_name | admin | primary_email | school |
| Joseph | Mamoa | false | testteacher@berkeley.edu | UC Berkeley |
And I am on the BJC home page
And I have an admin email
@@ -25,7 +25,7 @@ Feature: Request additional information for teacher application
And I should see "Request Info from Joseph Mamoa"
And I fill in "request_reason" with "Please provide more details on your teaching experience"
And I press "Submit"
- Then I can send a request info email
+ Then I send a request info email
And I follow "Logout"
Given I am on the BJC home page
@@ -39,7 +39,7 @@ Feature: Request additional information for teacher application
| name | country | city | state | website | grade_level | school_type |
| UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | university | public |
And the following teachers exist:
- | first_name | last_name | admin | email | school | application_status |
+ | first_name | last_name | admin | primary_email | school | application_status |
| Joseph | Mamoa | false | testteacher@berkeley.edu | UC Berkeley | info_needed |
And I am on the BJC home page
And I have a teacher Google email "testteacher@berkeley.edu"
From 96ba6e5e1d78f11ffb26e4b3eb9700432b5280d6 Mon Sep 17 00:00:00 2001
From: Jingchao Zhong <92573736+perryzjc@users.noreply.github.com>
Date: Sun, 7 Apr 2024 20:53:40 -0700
Subject: [PATCH 49/88] Update rspec tests
---
spec/controllers/teachers_controller_spec.rb | 26 ++++++++++++++------
spec/controllers/teachers_signup_spec.rb | 6 +++--
spec/factories/teachers.rb | 16 ++++++------
spec/models/teacher_spec.rb | 17 +++++--------
spec/rails_helper.rb | 2 +-
spec/support/sessions_helper.rb | 2 +-
6 files changed, 37 insertions(+), 32 deletions(-)
diff --git a/spec/controllers/teachers_controller_spec.rb b/spec/controllers/teachers_controller_spec.rb
index 6127f54b..4501ff31 100644
--- a/spec/controllers/teachers_controller_spec.rb
+++ b/spec/controllers/teachers_controller_spec.rb
@@ -14,7 +14,8 @@
ApplicationController.any_instance.stub(:is_admin?).and_return(false)
short_app = Teacher.find_by(first_name: "Short")
post :create, params: { teacher: { first_name: "First", last_name: "Last", status: 0, education_level: 0,
- email: "new@user.com", password: "pa33word!", more_info: "info", school_id: short_app.school_id } }
+ primary_email: "new@user.com", password: "pa33word!", more_info: "info",
+ school_id: short_app.school_id } }
user = Teacher.find_by(first_name: "First")
expect(user).not_to be_nil
expect(user.session_count).to eq 1
@@ -24,7 +25,8 @@
ApplicationController.any_instance.stub(:is_admin?).and_return(false)
short_app = Teacher.find_by(first_name: "Short")
post :create, params: { teacher: { first_name: "First", last_name: "Last", status: 0, education_level: 0,
- email: "new@user.com", password: "pa33word!", more_info: "info", school_id: short_app.school_id } }
+ primary_email: "new@user.com", password: "pa33word!", more_info: "info",
+ school_id: short_app.school_id } }
user = Teacher.find_by(first_name: "First")
expect(user).not_to be_nil
expect(user.ip_history).to include(request.remote_ip)
@@ -35,7 +37,8 @@
short_app = Teacher.find_by(first_name: "Short")
session_count_orig = short_app.session_count
post :create, params: { teacher: { first_name: "Short", last_name: "Last", status: 0, education_level: 0,
- email: short_app.email, password: "pa33word!", more_info: "info", school_id: short_app.school_id } }
+ primary_email: short_app.primary_email, password: "pa33word!", more_info: "info",
+ school_id: short_app.school_id } }
expect(Teacher.find_by(first_name: "Short").session_count).to eq session_count_orig
end
@@ -112,7 +115,7 @@
post :update, params: { id: short_app.id, teacher: { id: short_app.id, email: "wrong@berkeley.edu", school_id: short_app.school_id } }
post :update, params: { id: short_app.id, teacher: { id: short_app.id, snap: "wrong", school_id: short_app.school_id } }
short_app = Teacher.find_by(first_name: "Short")
- expect(short_app.email).to eq "short@long.com"
+ expect(short_app.primary_email).to eq "short@long.com"
expect(short_app.snap).to eq "song"
end
@@ -158,14 +161,18 @@
teacher: {
first_name: "Valid",
last_name: "Example",
- email: "valid_example@valid_example.edu",
+ primary_email: "valid_example@validexample.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)
+
+ email_address = EmailAddress.find_by(email: "valid_example@validexample.edu")
+ expect(email_address).not_to be_nil
+ teacher = email_address.teacher
+ expect(teacher.admin).to be(false)
end
it "rejects malicious auto-approve signup attempt" do
@@ -173,14 +180,17 @@
teacher: {
first_name: "Valid",
last_name: "Example",
- email: "valid_example@valid_example.edu",
+ primary_email: "valid_example@validexample.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
+ email_address = EmailAddress.find_by(email: "valid_example@validexample.edu")
+ expect(email_address).not_to be_nil
+ teacher = email_address.teacher
+ expect(teacher.not_reviewed?).to be(true)
end
end
diff --git a/spec/controllers/teachers_signup_spec.rb b/spec/controllers/teachers_signup_spec.rb
index b3f6ca1b..548f0e42 100644
--- a/spec/controllers/teachers_signup_spec.rb
+++ b/spec/controllers/teachers_signup_spec.rb
@@ -60,7 +60,7 @@
teacher: {
first_name: "valid_example",
last_name: "valid_example",
- email: "valid_example@valid_example.edu",
+ primary_email: "valid_example@validexample.edu",
status: 0,
snap: "valid_example"
}
@@ -74,7 +74,9 @@
school: {
id: 1
},
- teacher: Teacher.first.attributes
+ teacher: Teacher.first.attributes.merge(
+ primary_email: EmailAddress.find_by(
+ teacher_id: Teacher.first.id).email)
}
expect(response).to redirect_to(login_path)
expect(flash[:notice]).to match(/Please log in/)
diff --git a/spec/factories/teachers.rb b/spec/factories/teachers.rb
index 6db86bc2..4d2c2cce 100644
--- a/spec/factories/teachers.rb
+++ b/spec/factories/teachers.rb
@@ -8,14 +8,12 @@
# admin :boolean default(FALSE)
# application_status :string default("not_reviewed")
# education_level :integer default(NULL)
-# email :string
# first_name :string
# ip_history :inet default([]), is an Array
# languages :string default(["\"English\""]), is an Array
# last_name :string
# last_session_at :datetime
# more_info :string
-# personal_email :string
# personal_website :string
# session_count :integer default(0)
# snap :string
@@ -26,12 +24,9 @@
#
# Indexes
#
-# index_teachers_on_email (email) UNIQUE
-# index_teachers_on_email_and_first_name (email,first_name)
-# index_teachers_on_email_and_personal_email (email,personal_email) UNIQUE
-# index_teachers_on_school_id (school_id)
-# index_teachers_on_snap (snap) UNIQUE WHERE ((snap)::text <> ''::text)
-# index_teachers_on_status (status)
+# index_teachers_on_school_id (school_id)
+# index_teachers_on_snap (snap) UNIQUE WHERE ((snap)::text <> ''::text)
+# index_teachers_on_status (status)
#
# Foreign Keys
#
@@ -42,10 +37,13 @@
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 }
+
+ after(:create) do |teacher|
+ create(:email_address, teacher:, primary: true)
+ end
end
end
diff --git a/spec/models/teacher_spec.rb b/spec/models/teacher_spec.rb
index e0e8c626..870b7e2e 100644
--- a/spec/models/teacher_spec.rb
+++ b/spec/models/teacher_spec.rb
@@ -8,14 +8,12 @@
# admin :boolean default(FALSE)
# application_status :string default("not_reviewed")
# education_level :integer default(NULL)
-# email :string
# first_name :string
# ip_history :inet default([]), is an Array
# languages :string default(["\"English\""]), is an Array
# last_name :string
# last_session_at :datetime
# more_info :string
-# personal_email :string
# personal_website :string
# session_count :integer default(0)
# snap :string
@@ -26,12 +24,9 @@
#
# Indexes
#
-# index_teachers_on_email (email) UNIQUE
-# index_teachers_on_email_and_first_name (email,first_name)
-# index_teachers_on_email_and_personal_email (email,personal_email) UNIQUE
-# index_teachers_on_school_id (school_id)
-# index_teachers_on_snap (snap) UNIQUE WHERE ((snap)::text <> ''::text)
-# index_teachers_on_status (status)
+# index_teachers_on_school_id (school_id)
+# index_teachers_on_snap (snap) UNIQUE WHERE ((snap)::text <> ''::text)
+# index_teachers_on_status (status)
#
# Foreign Keys
#
@@ -78,15 +73,15 @@
it "does not change a info_need status to not_reviewed if submitted without changes" do
expect do
- # Same email, which means no any change compared to last time
- teacher.update(email: "reimu@touhou.com")
+ # Same primary email, which means no any change compared to last time
+ teacher.email_addresses.find_by(email: "reimu@touhou.com").update(primary: true)
end.not_to change(teacher, :application_status)
end
it "changes a info_needed status to not_reviewed" do
expect do
# Changes the status to not_reviewed only when there are meaningful changes
- teacher.update(email: "reimu_different_email@touhou.com")
+ teacher.email_addresses.find_by(email: "reimu@touhou.com").update(email: "reimu_different_email@touhou.com")
end.to change(teacher, :application_status)
.from("info_needed").to("not_reviewed")
end
diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb
index 175c3543..4b62603a 100644
--- a/spec/rails_helper.rb
+++ b/spec/rails_helper.rb
@@ -90,7 +90,7 @@ def self.get_lat_lng(_)
uid: "123456789",
info: {
name: "Tony Stark",
- email: "tony@stark.com"
+ primary_email: "tony@stark.com"
},
credentials: {
token: "token",
diff --git a/spec/support/sessions_helper.rb b/spec/support/sessions_helper.rb
index a5906f02..339ed0d8 100644
--- a/spec/support/sessions_helper.rb
+++ b/spec/support/sessions_helper.rb
@@ -9,7 +9,7 @@ def log_in(user, provider: "google_oauth2")
uid: "123456789",
info: {
name: user.full_name,
- email: user.email
+ primary_email: user.primary_email
}
})
From 9de1f365d6c7e83b9fd851a7806b4b6a85438acd Mon Sep 17 00:00:00 2001
From: Jingchao Zhong <92573736+perryzjc@users.noreply.github.com>
Date: Sun, 7 Apr 2024 21:27:05 -0700
Subject: [PATCH 50/88] Fix code for email usage in the session
---
app/models/teacher.rb | 7 ++++++-
features/step_definitions/admin_steps.rb | 4 ++--
features/step_definitions/teacher_steps.rb | 2 +-
spec/rails_helper.rb | 2 +-
spec/support/sessions_helper.rb | 2 +-
5 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/app/models/teacher.rb b/app/models/teacher.rb
index 03bba45e..01f3ad7f 100644
--- a/app/models/teacher.rb
+++ b/app/models/teacher.rb
@@ -216,7 +216,7 @@ def short_application_status
end
def self.user_from_omniauth(omniauth)
- teacher = EmailAddress.find_by(email: omniauth.primary_email.downcase)&.teacher
+ teacher = EmailAddress.find_by(email: omniauth.email.downcase)&.teacher
teacher
end
@@ -284,6 +284,11 @@ def self.csv_export
end
end
+ def email
+ # Default return primary email
+ primary_email
+ end
+
def primary_email
email_addresses.find_by(primary: true)&.email
end
diff --git a/features/step_definitions/admin_steps.rb b/features/step_definitions/admin_steps.rb
index 5f17e2e7..33aa424a 100644
--- a/features/step_definitions/admin_steps.rb
+++ b/features/step_definitions/admin_steps.rb
@@ -18,7 +18,7 @@
name: "Admin User",
first_name: "Admin",
last_name: "User",
- primary_email: "testadminuser@berkeley.edu",
+ email: "testadminuser@berkeley.edu",
school: "UC Berkeley"
}
})
@@ -34,7 +34,7 @@
name: "Random User",
first_name: "Random",
last_name: "User",
- primary_email: "randomemail@berkeley.edu",
+ email: "randomemail@berkeley.edu",
school: "UC Berkeley",
}
})
diff --git a/features/step_definitions/teacher_steps.rb b/features/step_definitions/teacher_steps.rb
index ea3dad51..b67f6529 100644
--- a/features/step_definitions/teacher_steps.rb
+++ b/features/step_definitions/teacher_steps.rb
@@ -12,7 +12,7 @@
name: "Joseph",
first_name: "Joseph",
last_name: "Mamoa",
- primary_email: "testteacher@berkeley.edu",
+ email: "testteacher@berkeley.edu",
school: "UC Berkeley",
},
credentials: {
diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb
index 4b62603a..175c3543 100644
--- a/spec/rails_helper.rb
+++ b/spec/rails_helper.rb
@@ -90,7 +90,7 @@ def self.get_lat_lng(_)
uid: "123456789",
info: {
name: "Tony Stark",
- primary_email: "tony@stark.com"
+ email: "tony@stark.com"
},
credentials: {
token: "token",
diff --git a/spec/support/sessions_helper.rb b/spec/support/sessions_helper.rb
index 339ed0d8..f4e35723 100644
--- a/spec/support/sessions_helper.rb
+++ b/spec/support/sessions_helper.rb
@@ -9,7 +9,7 @@ def log_in(user, provider: "google_oauth2")
uid: "123456789",
info: {
name: user.full_name,
- primary_email: user.primary_email
+ email: user.primary_email
}
})
From a486c259a1bb8c0ed82895405586661e7de02237 Mon Sep 17 00:00:00 2001
From: Jingchao Zhong <92573736+perryzjc@users.noreply.github.com>
Date: Mon, 8 Apr 2024 11:29:24 -0700
Subject: [PATCH 51/88] Change email regex to the rail default email regex
---
app/models/email_address.rb | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/app/models/email_address.rb b/app/models/email_address.rb
index c9a66d86..909c4992 100644
--- a/app/models/email_address.rb
+++ b/app/models/email_address.rb
@@ -24,12 +24,8 @@
class EmailAddress < ApplicationRecord
belongs_to :teacher
- # Regular expression for validating the format of an email address
- # https://stackoverflow.com/a/7791100/23305580
- EMAIL_REGEX = /\A[a-zA-Z0-9.!#$%&'*+\/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*\z/
-
-
- validates :email, presence: true, uniqueness: true, format: { with: EMAIL_REGEX }
+ # Rail's bulit-in validation for email format regex
+ validates :email, presence: true, uniqueness: true, format: { with: URI::MailTo::EMAIL_REGEXP }
validate :only_one_primary_email_per_teacher
before_save :downcase_email
From d62937ca488605e989e87e0b23b12df5e8c298ac Mon Sep 17 00:00:00 2001
From: Jingchao Zhong <92573736+perryzjc@users.noreply.github.com>
Date: Mon, 8 Apr 2024 11:46:17 -0700
Subject: [PATCH 52/88] Remove email validation due to original email format
problem
---
app/models/email_address.rb | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/app/models/email_address.rb b/app/models/email_address.rb
index 909c4992..fb38be10 100644
--- a/app/models/email_address.rb
+++ b/app/models/email_address.rb
@@ -24,8 +24,7 @@
class EmailAddress < ApplicationRecord
belongs_to :teacher
- # Rail's bulit-in validation for email format regex
- validates :email, presence: true, uniqueness: true, format: { with: URI::MailTo::EMAIL_REGEXP }
+ validates :email, presence: true, uniqueness: true
validate :only_one_primary_email_per_teacher
before_save :downcase_email
From 2c154298816be6ec0bd2b283c3dece298f896c20 Mon Sep 17 00:00:00 2001
From: JacksonXu33
Date: Mon, 8 Apr 2024 21:31:37 -0700
Subject: [PATCH 53/88] fix update path bug for admins, and some refactor
---
app/controllers/teachers_controller.rb | 63 ++++++++++++++------------
1 file changed, 35 insertions(+), 28 deletions(-)
diff --git a/app/controllers/teachers_controller.rb b/app/controllers/teachers_controller.rb
index d64b2430..1b986e08 100644
--- a/app/controllers/teachers_controller.rb
+++ b/app/controllers/teachers_controller.rb
@@ -107,30 +107,22 @@ def update
@teacher.school = @school
end
send_email_if_application_status_changed_and_email_resend_enabled
- if @teacher.denied? && !is_admin?
- redirect_to root_path, alert: "Failed to update your information. You have already been denied. If you have questions, please email contact@bjc.berkeley.edu."
- return
- end
- if (@teacher.email_changed? || @teacher.snap_changed?) && !is_admin?
- redirect_to edit_teacher_path(current_user.id), alert: "Failed to update your information. If you want to change your email or Snap! username, please email contact@bjc.berkeley.edu."
- return
- end
- if !@teacher.save
- redirect_to edit_teacher_path(current_user.id),
- alert: "An error occurred: #{@teacher.errors.full_messages.join(', ')}"
+
+ if fail_to_update
return
end
+
if !@teacher.validated? && !current_user.admin?
TeacherMailer.form_submission(@teacher).deliver_now
TeacherMailer.teacher_form_submission(@teacher).deliver_now
end
+
if is_admin?
- redirect_to edit_teacher_path(current_user.id), notice: "Saved #{@teacher.full_name}"
- return
+ redirect_to teachers_path, notice: "Saved #{@teacher.full_name}"
else
@teacher.try_append_ip(request.remote_ip)
+ redirect_to edit_teacher_path(current_user.id), notice: "Successfully updated your information"
end
- redirect_to edit_teacher_path(current_user.id), notice: "Successfully updated your information"
end
def send_email_if_application_status_changed_and_email_resend_enabled
@@ -201,6 +193,23 @@ def deny_access
redirect_to new_teacher_path, alert: "Email address or Snap username already in use. Please use a different email or Snap username."
end
+ def fail_to_update
+ failed = false
+ if @teacher.denied? && !is_admin?
+ redirect_to root_path, alert: "Failed to update your information. You have already been denied. If you have questions, please email contact@bjc.berkeley.edu."
+ failed = true
+ elsif (@teacher.email_changed? || @teacher.snap_changed?) && !is_admin?
+ flash.now[:alert] = "Failed to update your information. If you want to change your email or Snap! username, please email contact@bjc.berkeley.edu."
+ render "edit"
+ failed = true
+ elsif !@teacher.save
+ flash.now[:alert] = "Failed to update data. #{@teacher.errors.full_messages.to_sentence}"
+ render "edit"
+ failed = true
+ end
+ failed
+ end
+
def load_school
if teacher_params[:school_id].present?
@school ||= School.find(teacher_params[:school_id])
@@ -238,22 +247,20 @@ def ordered_schools
end
def sanitize_params
- if params[:teacher]
- if params[:teacher][:status]
- params[:teacher][:status] = params[:teacher][:status].to_i
- end
- if params[:teacher][:education_level]
- params[:teacher][:education_level] = params[:teacher][:education_level].to_i
- end
+ teacher = params[:teacher]
+ if teacher && teacher[:status]
+ teacher[:status] = teacher[:status].to_i
+ end
+ if teacher && teacher[:education_level]
+ teacher[:education_level] = teacher[:education_level].to_i
end
- if params[:school]
- if params[:school][:grade_level]
- params[:school][:grade_level] = params[:school][:grade_level].to_i
- end
- if params[:school][:school_type]
- params[:school][:school_type] = params[:school][:school_type].to_i
- end
+ school = params[:school]
+ if school && school[:grade_level]
+ school[:grade_level] = school[:grade_level].to_i
+ end
+ if school && school[:school_type]
+ school[:school_type] = school[:school_type].to_i
end
end
From 6d4a956761bc1306a3d77060088400aaf914f4ef Mon Sep 17 00:00:00 2001
From: Jingchao Zhong <92573736+perryzjc@users.noreply.github.com>
Date: Tue, 9 Apr 2024 12:54:59 -0700
Subject: [PATCH 54/88] Revert back the email validation
---
app/models/email_address.rb | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/app/models/email_address.rb b/app/models/email_address.rb
index fb38be10..909c4992 100644
--- a/app/models/email_address.rb
+++ b/app/models/email_address.rb
@@ -24,7 +24,8 @@
class EmailAddress < ApplicationRecord
belongs_to :teacher
- validates :email, presence: true, uniqueness: true
+ # Rail's bulit-in validation for email format regex
+ validates :email, presence: true, uniqueness: true, format: { with: URI::MailTo::EMAIL_REGEXP }
validate :only_one_primary_email_per_teacher
before_save :downcase_email
From 81b15db8be6087a00f2ed6986fec3d7371b64f49 Mon Sep 17 00:00:00 2001
From: Jingchao Zhong <92573736+perryzjc@users.noreply.github.com>
Date: Tue, 9 Apr 2024 13:21:54 -0700
Subject: [PATCH 55/88] Adjust personal email handling for this PR
---
app/models/teacher.rb | 4 +++-
app/views/teachers/_teacher.erb | 7 ++++---
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/app/models/teacher.rb b/app/models/teacher.rb
index 01f3ad7f..aae6381a 100644
--- a/app/models/teacher.rb
+++ b/app/models/teacher.rb
@@ -299,6 +299,8 @@ def personal_emails
private
def non_primary_emails
- email_addresses.where(primary: false)&.pluck(:email)
+ # email_addresses.where(primary: false)&.pluck(:email)
+ # below code is temporary for current PR, to make sure the frontend same as before (only one personal email)
+ email_addresses.where(primary: false)&.pluck(:email)&.first
end
end
diff --git a/app/views/teachers/_teacher.erb b/app/views/teachers/_teacher.erb
index af1cfc0d..aae3c6af 100644
--- a/app/views/teachers/_teacher.erb
+++ b/app/views/teachers/_teacher.erb
@@ -4,9 +4,10 @@
<%= teacher.primary_email %>
- <%- if teacher.personal_emails.present? %>
- <%= teacher.personal_emails %>
- <%- end %>
+
+ <%#- if teacher.personal_emails.present? %>
+
+ <%#- end %>
<%= teacher.display_education_level %>
Date: Tue, 9 Apr 2024 18:47:31 -0700
Subject: [PATCH 56/88] Kept original email column in teacher for this PR
---
app/models/teacher.rb | 17 +++++++---
...dapt_teachers_after_new_email_addresses.rb | 11 -------
db/schema.rb | 31 ++++---------------
db/seed_data.rb | 3 +-
db/seeds.rb | 28 ++++++++++++++---
spec/factories/email_addresses.rb | 21 +++++++++++++
spec/factories/teachers.rb | 11 +++++--
spec/fixtures/email_addresses.yml | 21 +++++++++++++
spec/fixtures/teachers.yml | 11 +++++--
spec/models/teacher_spec.rb | 11 +++++--
10 files changed, 109 insertions(+), 56 deletions(-)
delete mode 100644 db/migrate/20240407190132_adapt_teachers_after_new_email_addresses.rb
diff --git a/app/models/teacher.rb b/app/models/teacher.rb
index aae6381a..4a34778e 100644
--- a/app/models/teacher.rb
+++ b/app/models/teacher.rb
@@ -8,12 +8,14 @@
# admin :boolean default(FALSE)
# application_status :string default("not_reviewed")
# education_level :integer default(NULL)
+# email :string
# first_name :string
# ip_history :inet default([]), is an Array
# languages :string default(["\"English\""]), is an Array
# last_name :string
# last_session_at :datetime
# more_info :string
+# personal_email :string
# personal_website :string
# session_count :integer default(0)
# snap :string
@@ -24,9 +26,12 @@
#
# Indexes
#
-# index_teachers_on_school_id (school_id)
-# index_teachers_on_snap (snap) UNIQUE WHERE ((snap)::text <> ''::text)
-# index_teachers_on_status (status)
+# index_teachers_on_email (email) UNIQUE
+# index_teachers_on_email_and_first_name (email,first_name)
+# index_teachers_on_email_and_personal_email (email,personal_email) UNIQUE
+# index_teachers_on_school_id (school_id)
+# index_teachers_on_snap (snap) UNIQUE WHERE ((snap)::text <> ''::text)
+# index_teachers_on_status (status)
#
# Foreign Keys
#
@@ -290,7 +295,9 @@ def email
end
def primary_email
- email_addresses.find_by(primary: true)&.email
+ # ||:email this code is temporary for this PR: https://github.com/cs169/BJC-Teacher-Tracker-App/pull/49
+ # to make sure at least original data in db still work and passed the existing tests
+ email_addresses.find_by(primary: true)&.email || self[:email]
end
def personal_emails
@@ -301,6 +308,6 @@ def personal_emails
def non_primary_emails
# email_addresses.where(primary: false)&.pluck(:email)
# below code is temporary for current PR, to make sure the frontend same as before (only one personal email)
- email_addresses.where(primary: false)&.pluck(:email)&.first
+ email_addresses.where(primary: false)&.pluck(:email)&.first || self.personal_email
end
end
diff --git a/db/migrate/20240407190132_adapt_teachers_after_new_email_addresses.rb b/db/migrate/20240407190132_adapt_teachers_after_new_email_addresses.rb
deleted file mode 100644
index c5e566fa..00000000
--- a/db/migrate/20240407190132_adapt_teachers_after_new_email_addresses.rb
+++ /dev/null
@@ -1,11 +0,0 @@
-class AdaptTeachersAfterNewEmailAddresses < ActiveRecord::Migration[6.1]
- def change
- # Remove indexes related to email and personal_email
- remove_index :teachers, name: "index_teachers_on_email_and_first_name"
- remove_index :teachers, name: "index_teachers_on_email_and_personal_email"
- remove_index :teachers, name: "index_teachers_on_email"
-
- remove_column :teachers, :email
- remove_column :teachers, :personal_email
- end
-end
diff --git a/db/schema.rb b/db/schema.rb
index 1410e82b..fc5c4fcd 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: 2024_04_07_190132) do
+ActiveRecord::Schema.define(version: 2024_04_07_190126) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
@@ -93,28 +93,6 @@
t.index ["url_slug"], name: "index_pages_on_url_slug", unique: true
end
- create_table "pd_registrations", force: :cascade do |t|
- t.integer "teacher_id", null: false
- t.integer "professional_development_id", null: false
- t.boolean "attended", default: false
- t.string "role", null: false
- t.datetime "created_at", precision: 6, null: false
- t.datetime "updated_at", precision: 6, null: false
- t.index ["teacher_id", "professional_development_id"], name: "index_pd_reg_on_teacher_id_and_pd_id", unique: true
- end
-
- create_table "professional_developments", force: :cascade do |t|
- t.string "name", null: false
- t.string "city", null: false
- t.string "state"
- t.string "country", null: false
- t.date "start_date", null: false
- t.date "end_date", null: false
- t.integer "grade_level", null: false
- t.datetime "created_at", precision: 6, null: false
- t.datetime "updated_at", precision: 6, null: false
- end
-
create_table "schools", id: :serial, force: :cascade do |t|
t.string "name"
t.string "city"
@@ -136,6 +114,7 @@
create_table "teachers", id: :serial, force: :cascade do |t|
t.string "first_name"
t.string "last_name"
+ t.string "email"
t.string "snap"
t.integer "school_id"
t.datetime "created_at", default: -> { "now()" }
@@ -149,7 +128,11 @@
t.datetime "last_session_at"
t.inet "ip_history", default: [], array: true
t.integer "session_count", default: 0
+ t.string "personal_email"
t.string "languages", default: ["English"], array: true
+ t.index ["email", "first_name"], name: "index_teachers_on_email_and_first_name"
+ t.index ["email", "personal_email"], name: "index_teachers_on_email_and_personal_email", unique: true
+ t.index ["email"], name: "index_teachers_on_email", unique: true
t.index ["school_id"], name: "index_teachers_on_school_id"
t.index ["snap"], name: "index_teachers_on_snap", unique: true, where: "((snap)::text <> ''::text)"
t.index ["status"], name: "index_teachers_on_status"
@@ -160,7 +143,5 @@
add_foreign_key "email_addresses", "teachers"
add_foreign_key "pages", "teachers", column: "creator_id"
add_foreign_key "pages", "teachers", column: "last_editor_id"
- add_foreign_key "pd_registrations", "professional_developments"
- add_foreign_key "pd_registrations", "teachers"
add_foreign_key "teachers", "schools"
end
diff --git a/db/seed_data.rb b/db/seed_data.rb
index b9319af5..ce084203 100644
--- a/db/seed_data.rb
+++ b/db/seed_data.rb
@@ -153,8 +153,9 @@ def self.create_schools
School.find_or_create_by(
name: "UC Berkeley",
city: "Berkeley",
+ country: "US",
+ website: "https://bjc.berkeley.edu",
state: "CA",
- website: "https://bjc.berkeley.edu"
)
end
diff --git a/db/seeds.rb b/db/seeds.rb
index 9c3ec387..9497fb63 100644
--- a/db/seeds.rb
+++ b/db/seeds.rb
@@ -9,11 +9,29 @@
SeedData.create_schools
-SeedData.teachers.each do |teacher|
- teacher_id = EmailAddress.find_by(email: teacher[:email])&.teacher_id
- # Seed data includes email field for comparison purposes, but it is not part of the Teacher model
- teacher_attrs = teacher.except(:email)
- Teacher.find_or_create_by(id: teacher_id).update(teacher_attrs)
+SeedData.teachers.each do |teacher_attr|
+ email_address = EmailAddress.find_or_initialize_by(email: teacher_attr[:email])
+ puts "teacher_attr: #{teacher_attr}"
+
+ if email_address.new_record?
+ puts "email_address.new_record?: #{email_address.new_record?}"
+
+ teacher = Teacher.create(teacher_attr)
+ if teacher.persisted?
+ email_address.teacher_id = teacher.id
+ email_address_saved = email_address.save
+ puts "New teacher created and email_address saved: #{email_address_saved}"
+ else
+ puts "Failed to create teacher. Errors: #{teacher.errors.full_messages.join(", ")}"
+ end
+ else
+ teacher = Teacher.find_by(id: email_address.teacher_id)
+ if teacher&.update(teacher_attr)
+ puts "Teacher updated successfully."
+ else
+ puts "Failed to update teacher. Errors: #{teacher.errors.full_messages.join(", ")}"
+ end
+ end
end
SeedData.email_addresses.each do |email_address|
diff --git a/spec/factories/email_addresses.rb b/spec/factories/email_addresses.rb
index 684d0f57..57e34dc9 100644
--- a/spec/factories/email_addresses.rb
+++ b/spec/factories/email_addresses.rb
@@ -1,5 +1,26 @@
# frozen_string_literal: true
+# == Schema Information
+#
+# Table name: email_addresses
+#
+# id :bigint not null, primary key
+# email :string not null
+# primary :boolean default(FALSE), not null
+# created_at :datetime not null
+# updated_at :datetime not null
+# teacher_id :bigint not null
+#
+# Indexes
+#
+# index_email_addresses_on_email (email) UNIQUE
+# index_email_addresses_on_teacher_id (teacher_id)
+# index_email_addresses_on_teacher_id_and_primary (teacher_id,primary) UNIQUE WHERE ("primary" = true)
+#
+# Foreign Keys
+#
+# fk_rails_... (teacher_id => teachers.id)
+#
FactoryBot.define do
factory :email_address do
association :teacher
diff --git a/spec/factories/teachers.rb b/spec/factories/teachers.rb
index 4d2c2cce..36ce3284 100644
--- a/spec/factories/teachers.rb
+++ b/spec/factories/teachers.rb
@@ -8,12 +8,14 @@
# admin :boolean default(FALSE)
# application_status :string default("not_reviewed")
# education_level :integer default(NULL)
+# email :string
# first_name :string
# ip_history :inet default([]), is an Array
# languages :string default(["\"English\""]), is an Array
# last_name :string
# last_session_at :datetime
# more_info :string
+# personal_email :string
# personal_website :string
# session_count :integer default(0)
# snap :string
@@ -24,9 +26,12 @@
#
# Indexes
#
-# index_teachers_on_school_id (school_id)
-# index_teachers_on_snap (snap) UNIQUE WHERE ((snap)::text <> ''::text)
-# index_teachers_on_status (status)
+# index_teachers_on_email (email) UNIQUE
+# index_teachers_on_email_and_first_name (email,first_name)
+# index_teachers_on_email_and_personal_email (email,personal_email) UNIQUE
+# index_teachers_on_school_id (school_id)
+# index_teachers_on_snap (snap) UNIQUE WHERE ((snap)::text <> ''::text)
+# index_teachers_on_status (status)
#
# Foreign Keys
#
diff --git a/spec/fixtures/email_addresses.yml b/spec/fixtures/email_addresses.yml
index ddbc8fdf..e84403dd 100644
--- a/spec/fixtures/email_addresses.yml
+++ b/spec/fixtures/email_addresses.yml
@@ -1,3 +1,24 @@
+# == Schema Information
+#
+# Table name: email_addresses
+#
+# id :bigint not null, primary key
+# email :string not null
+# primary :boolean default(FALSE), not null
+# created_at :datetime not null
+# updated_at :datetime not null
+# teacher_id :bigint not null
+#
+# Indexes
+#
+# index_email_addresses_on_email (email) UNIQUE
+# index_email_addresses_on_teacher_id (teacher_id)
+# index_email_addresses_on_teacher_id_and_primary (teacher_id,primary) UNIQUE WHERE ("primary" = true)
+#
+# Foreign Keys
+#
+# fk_rails_... (teacher_id => teachers.id)
+#
admin_email:
# teacher: admin
teacher_id: 1
diff --git a/spec/fixtures/teachers.yml b/spec/fixtures/teachers.yml
index 26e5dc6b..0c7ae2ae 100644
--- a/spec/fixtures/teachers.yml
+++ b/spec/fixtures/teachers.yml
@@ -6,12 +6,14 @@
# admin :boolean default(FALSE)
# application_status :string default("not_reviewed")
# education_level :integer default(NULL)
+# email :string
# first_name :string
# ip_history :inet default([]), is an Array
# languages :string default(["\"English\""]), is an Array
# last_name :string
# last_session_at :datetime
# more_info :string
+# personal_email :string
# personal_website :string
# session_count :integer default(0)
# snap :string
@@ -22,9 +24,12 @@
#
# Indexes
#
-# index_teachers_on_school_id (school_id)
-# index_teachers_on_snap (snap) UNIQUE WHERE ((snap)::text <> ''::text)
-# index_teachers_on_status (status)
+# index_teachers_on_email (email) UNIQUE
+# index_teachers_on_email_and_first_name (email,first_name)
+# index_teachers_on_email_and_personal_email (email,personal_email) UNIQUE
+# index_teachers_on_school_id (school_id)
+# index_teachers_on_snap (snap) UNIQUE WHERE ((snap)::text <> ''::text)
+# index_teachers_on_status (status)
#
# Foreign Keys
#
diff --git a/spec/models/teacher_spec.rb b/spec/models/teacher_spec.rb
index 870b7e2e..4f7f90bc 100644
--- a/spec/models/teacher_spec.rb
+++ b/spec/models/teacher_spec.rb
@@ -8,12 +8,14 @@
# admin :boolean default(FALSE)
# application_status :string default("not_reviewed")
# education_level :integer default(NULL)
+# email :string
# first_name :string
# ip_history :inet default([]), is an Array
# languages :string default(["\"English\""]), is an Array
# last_name :string
# last_session_at :datetime
# more_info :string
+# personal_email :string
# personal_website :string
# session_count :integer default(0)
# snap :string
@@ -24,9 +26,12 @@
#
# Indexes
#
-# index_teachers_on_school_id (school_id)
-# index_teachers_on_snap (snap) UNIQUE WHERE ((snap)::text <> ''::text)
-# index_teachers_on_status (status)
+# index_teachers_on_email (email) UNIQUE
+# index_teachers_on_email_and_first_name (email,first_name)
+# index_teachers_on_email_and_personal_email (email,personal_email) UNIQUE
+# index_teachers_on_school_id (school_id)
+# index_teachers_on_snap (snap) UNIQUE WHERE ((snap)::text <> ''::text)
+# index_teachers_on_status (status)
#
# Foreign Keys
#
From a1fc144ed9df323ba1e026576fa69d3995e03b38 Mon Sep 17 00:00:00 2001
From: Jingchao Zhong <92573736+perryzjc@users.noreply.github.com>
Date: Thu, 11 Apr 2024 12:05:21 -0700
Subject: [PATCH 57/88] Change order to improve readability
---
app/models/teacher.rb | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/app/models/teacher.rb b/app/models/teacher.rb
index 4a34778e..09203aa8 100644
--- a/app/models/teacher.rb
+++ b/app/models/teacher.rb
@@ -297,7 +297,7 @@ def email
def primary_email
# ||:email this code is temporary for this PR: https://github.com/cs169/BJC-Teacher-Tracker-App/pull/49
# to make sure at least original data in db still work and passed the existing tests
- email_addresses.find_by(primary: true)&.email || self[:email]
+ self[:email] || email_addresses.find_by(primary: true)&.email
end
def personal_emails
@@ -308,6 +308,6 @@ def personal_emails
def non_primary_emails
# email_addresses.where(primary: false)&.pluck(:email)
# below code is temporary for current PR, to make sure the frontend same as before (only one personal email)
- email_addresses.where(primary: false)&.pluck(:email)&.first || self.personal_email
+ personal_email || email_addresses.where(primary: false)&.pluck(:email)&.first
end
end
From a825a5fa3a2068b3eb80bb44e41224d9a6cb85dd Mon Sep 17 00:00:00 2001
From: Jingchao Zhong <92573736+perryzjc@users.noreply.github.com>
Date: Thu, 11 Apr 2024 12:12:01 -0700
Subject: [PATCH 58/88] Clean up naming & data handling
---
app/controllers/teachers_controller.rb | 3 +--
app/models/email_address.rb | 6 +++---
2 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/app/controllers/teachers_controller.rb b/app/controllers/teachers_controller.rb
index 39b6b832..5e8c1afa 100644
--- a/app/controllers/teachers_controller.rb
+++ b/app/controllers/teachers_controller.rb
@@ -108,8 +108,7 @@ def update
personal_emails = params[:teacher].extract!(*personal_emails_params).values
# Now, `params[:teacher]` does not contain primary_email or any personal_emailX fields
- teacher_attr = teacher_params
- @teacher.assign_attributes(teacher_attr)
+ @teacher.assign_attributes(teacher_params)
update_primary_email(primary_email)
update_personal_emails(personal_emails)
diff --git a/app/models/email_address.rb b/app/models/email_address.rb
index 909c4992..7bf8dc0d 100644
--- a/app/models/email_address.rb
+++ b/app/models/email_address.rb
@@ -28,7 +28,7 @@ class EmailAddress < ApplicationRecord
validates :email, presence: true, uniqueness: true, format: { with: URI::MailTo::EMAIL_REGEXP }
validate :only_one_primary_email_per_teacher
- before_save :downcase_email
+ before_save :normalize_email
before_save :flag_teacher_if_email_changed
private
@@ -38,8 +38,8 @@ def only_one_primary_email_per_teacher
end
end
- def downcase_email
- self.email = email.downcase
+ def normalize_email
+ self.email = email.strip.downcase
end
def flag_teacher_if_email_changed
From 63dd1ab0ac665078cb161a3caa733f1ff99b08d5 Mon Sep 17 00:00:00 2001
From: Jingchao Zhong <92573736+perryzjc@users.noreply.github.com>
Date: Thu, 11 Apr 2024 12:36:50 -0700
Subject: [PATCH 59/88] Shorten code finding teacher
---
app/controllers/teachers_controller.rb | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/app/controllers/teachers_controller.rb b/app/controllers/teachers_controller.rb
index 5e8c1afa..d22531fc 100644
--- a/app/controllers/teachers_controller.rb
+++ b/app/controllers/teachers_controller.rb
@@ -52,8 +52,7 @@ def new
# If you are logged in and not an admin, this should fail.
def create
# Find by email, but allow updating other info.
- teacher_id = EmailAddress.find_by(email: teacher_params[:primary_email])&.teacher_id
- @teacher = Teacher.find_by(id: teacher_id)
+ @teacher = @teacher = EmailAddress.find_by(email: teacher_params[:primary_email])&.teacher
if @teacher && defined?(current_user.id) && (current_user.id == @teacher.id)
params[:id] = current_user.id
update
From 793733b2b0e0877489086131774d7e07812d1e48 Mon Sep 17 00:00:00 2001
From: Jingchao Zhong <92573736+perryzjc@users.noreply.github.com>
Date: Thu, 11 Apr 2024 13:27:32 -0700
Subject: [PATCH 60/88] Clarify and shorten primary email update logic
---
app/controllers/teachers_controller.rb | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/app/controllers/teachers_controller.rb b/app/controllers/teachers_controller.rb
index d22531fc..48fc3f73 100644
--- a/app/controllers/teachers_controller.rb
+++ b/app/controllers/teachers_controller.rb
@@ -283,11 +283,7 @@ def update_primary_email(primary_email)
return unless primary_email.present?
# First, ensure the current primary email is marked as not primary if it's not the same as the new one
- @teacher.email_addresses.where(primary: true).each do |email_record|
- if email_record.email != primary_email
- email_record.update(primary: false)
- end
- end
+ @teacher.email_addresses.find_by(primary: true)&.update(primary: false)
primary_email_record = @teacher.email_addresses.find_or_initialize_by(email: primary_email)
primary_email_record.primary = true
From 74b8b83f2369ab7cbd9c0c5447f5d09815fef589 Mon Sep 17 00:00:00 2001
From: Jingchao Zhong <92573736+perryzjc@users.noreply.github.com>
Date: Thu, 11 Apr 2024 13:36:05 -0700
Subject: [PATCH 61/88] Fix typo
---
app/controllers/teachers_controller.rb | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/app/controllers/teachers_controller.rb b/app/controllers/teachers_controller.rb
index 48fc3f73..690ee7da 100644
--- a/app/controllers/teachers_controller.rb
+++ b/app/controllers/teachers_controller.rb
@@ -51,8 +51,7 @@ def new
# TODO: This needs to be re-written.
# If you are logged in and not an admin, this should fail.
def create
- # Find by email, but allow updating other info.
- @teacher = @teacher = EmailAddress.find_by(email: teacher_params[:primary_email])&.teacher
+ @teacher = EmailAddress.find_by(email: teacher_params[:primary_email])&.teacher
if @teacher && defined?(current_user.id) && (current_user.id == @teacher.id)
params[:id] = current_user.id
update
From ee559cca5247e615f777439c39c87c0e9bb7df43 Mon Sep 17 00:00:00 2001
From: Jingchao Zhong <92573736+perryzjc@users.noreply.github.com>
Date: Thu, 11 Apr 2024 14:37:27 -0700
Subject: [PATCH 62/88] Refract param handling
---
app/controllers/teachers_controller.rb | 21 ++++++----------
app/views/teachers/_form.html.erb | 10 ++++----
features/admin.feature | 2 +-
spec/controllers/teachers_controller_spec.rb | 26 ++++++++++++++------
spec/controllers/teachers_signup_spec.rb | 11 ++++++---
5 files changed, 39 insertions(+), 31 deletions(-)
diff --git a/app/controllers/teachers_controller.rb b/app/controllers/teachers_controller.rb
index 690ee7da..58d25ddb 100644
--- a/app/controllers/teachers_controller.rb
+++ b/app/controllers/teachers_controller.rb
@@ -51,7 +51,7 @@ def new
# TODO: This needs to be re-written.
# If you are logged in and not an admin, this should fail.
def create
- @teacher = EmailAddress.find_by(email: teacher_params[:primary_email])&.teacher
+ @teacher = EmailAddress.find_by(email: params.dig(:email, :primary))&.teacher
if @teacher && defined?(current_user.id) && (current_user.id == @teacher.id)
params[:id] = current_user.id
update
@@ -71,9 +71,8 @@ def create
end
end
- teacher_attr = teacher_params.except(:primary_email)
- @teacher = Teacher.new(teacher_attr)
- @teacher.email_addresses.build(email: teacher_params[:primary_email], primary: true)
+ @teacher = Teacher.new(teacher_params)
+ @teacher.email_addresses.build(email: params[:email][:primary], primary: true)
@teacher.try_append_ip(request.remote_ip)
@teacher.session_count += 1
@@ -100,10 +99,8 @@ def update
load_school
ordered_schools
- personal_emails_params = params[:teacher].keys.select { |key| key.to_s.start_with?("personal_email") }
-
- primary_email = params[:teacher].delete(:primary_email)
- personal_emails = params[:teacher].extract!(*personal_emails_params).values
+ primary_email = params.dig(:email, :primary)
+ personal_emails = params[:email]&.select { |key, value| key.start_with?("personal_") }&.values
# Now, `params[:teacher]` does not contain primary_email or any personal_emailX fields
@teacher.assign_attributes(teacher_params)
@@ -224,15 +221,12 @@ def load_school
end
def teacher_params
- teacher_attributes = [:first_name, :last_name, :school, :primary_email, :status, :snap,
+ teacher_attributes = [:first_name, :last_name, :school, :status, :snap,
:more_info, :personal_website, :education_level, :school_id, languages: []]
admin_attributes = [:application_status, :request_reason, :skip_email]
teacher_attributes.push(*admin_attributes) if is_admin?
- # Dynamically extract and permit personal_emailX attributes from params
- personal_email_attributes = params[:teacher].keys.select { |key| key.to_s.start_with?("personal_email") }
- permitted_attributes = teacher_attributes + personal_email_attributes
- params.require(:teacher).permit(*permitted_attributes)
+ params.require(:teacher).permit(*teacher_attributes)
end
def school_params
@@ -291,6 +285,7 @@ def update_primary_email(primary_email)
end
def update_personal_emails(personal_emails)
+ return unless personal_emails.present?
personal_emails = personal_emails.reject(&:empty?)
return if personal_emails.empty?
diff --git a/app/views/teachers/_form.html.erb b/app/views/teachers/_form.html.erb
index c4115b8e..0d8e1889 100644
--- a/app/views/teachers/_form.html.erb
+++ b/app/views/teachers/_form.html.erb
@@ -57,9 +57,9 @@ status ONLY IF the person viewing this page is an admin. %>
+ <%= teacher.primary_email %>
+ <% if teacher.personal_emails.present? %>
+ <% teacher.personal_emails.each do |email| %>
+ <%= email %>
+ <% end %>
+ <% end %>
+
<%= teacher.display_status %>
<%= teacher.display_application_status %>
<%= teacher.created_at.strftime("%b %d, %Y") %>
diff --git a/app/views/teachers/_form.html.erb b/app/views/teachers/_form.html.erb
index 8e228987..7c3cf65e 100644
--- a/app/views/teachers/_form.html.erb
+++ b/app/views/teachers/_form.html.erb
@@ -82,21 +82,6 @@ status ONLY IF the person viewing this page is an admin. %>
<%# For now... only admins can enter/edit personal emails. %>
- <% if @current_user&.admin? %>
-
From 5cc4676bd5a44714ca9fc51f8c23b94a64c684c7 Mon Sep 17 00:00:00 2001
From: Jingchao Zhong <92573736+perryzjc@users.noreply.github.com>
Date: Thu, 18 Apr 2024 19:19:14 -0700
Subject: [PATCH 74/88] Add cucumber tests
---
features/email_addresses.feature | 99 +++++++++++++++++++
.../step_definitions/email_addresses_steps.rb | 23 +++++
2 files changed, 122 insertions(+)
create mode 100644 features/email_addresses.feature
create mode 100644 features/step_definitions/email_addresses_steps.rb
diff --git a/features/email_addresses.feature b/features/email_addresses.feature
new file mode 100644
index 00000000..ae066b58
--- /dev/null
+++ b/features/email_addresses.feature
@@ -0,0 +1,99 @@
+Feature: Managing personal email addresses for teachers
+
+ Background:
+ Background: Has an Admin in DB
+ Given the following teachers exist:
+ | first_name | last_name | admin | primary_email |
+ | Admin | User | true | testadminuser@berkeley.edu |
+ Given the following schools exist:
+ | name | country | city | state | website | grade_level | school_type |
+ | UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | university | public |
+ Given the following teachers exist:
+ | first_name | last_name | admin | primary_email | school | snap | application_status |
+ | Joseph | Mamoa | false | jose.primary@email.com | UC Berkeley | false | Validated |
+ Given the following emails exist:
+ | first_name | last_name | email | primary |
+ | Joseph | Mamoa | jose.personal1@email.com | false |
+ | Joseph | Mamoa | jose.personal2@email.com | false |
+
+
+ Scenario: Admin views teacher personal emails
+ Given I am on the BJC home page
+ And I have an admin email
+ And I follow "Log In"
+ Then I can log in with Google
+ When I follow "Teachers"
+ And I follow "Joseph Mamoa"
+ Then I should see "Admin View Joseph Mamoa"
+ Then I should see "jose.personal1@email.com"
+ Then I should see "jose.personal2@email.com"
+ Then I should see "Edit Personal Emails"
+
+ Scenario: Admin adds a personal email address to a teacher
+ Given I am on the BJC home page
+ And I have an admin email
+ And I follow "Log In"
+ Then I can log in with Google
+ When I follow "Teachers"
+ And I follow "Joseph Mamoa"
+ And I follow "Edit Personal Emails"
+ And I follow "Add Email"
+ And I fill in "teacher[email_addresses_attributes][2][email]" with "jose.personal3@email.com"
+ And I press "Update Emails"
+ Then I should see a "info" flash message "Personal email addresses updated successfully."
+ Then I should see "Admin View Joseph Mamoa"
+ Then I should see "jose.personal3@email.com"
+
+ Scenario: Admin updates an existing personal email address
+ Given I am on the BJC home page
+ And I have an admin email
+ And I follow "Log In"
+ Then I can log in with Google
+ When I follow "Teachers"
+ And I follow "Joseph Mamoa"
+ And I follow "Edit Personal Emails"
+ And I fill in "teacher[email_addresses_attributes][1][email]" with "jose.personalxxxxxx@email.com"
+ And I press "Update Emails"
+ Then I should see a "info" flash message "Personal email addresses updated successfully."
+ Then I should not see "jose.personal2@email.com"
+ And I should see "jose.personal1@email.com"
+ And I should see "jose.personalxxxxxx@email.com"
+
+ Scenario: Admin deletes a personal email address
+ Given I am on the BJC home page
+ And I have an admin email
+ And I follow "Log In"
+ Then I can log in with Google
+ When I follow "Teachers"
+ And I follow "Joseph Mamoa"
+ And I follow "Edit Personal Emails"
+ And I press "Remove" next to "jose.personal1@email.com"
+ And I press "Update Emails"
+ Then I should see a "info" flash message "Personal email addresses updated successfully."
+ Then I should not see "jose.personal1@email.com"
+ And I should see "jose.personal2@email.com"
+
+ Scenario: Admin tries to add an existing personal email address to the same teacher
+ Given I am on the BJC home page
+ And I have an admin email
+ And I follow "Log In"
+ Then I can log in with Google
+ When I follow "Teachers"
+ And I follow "Joseph Mamoa"
+ And I follow "Edit Personal Emails"
+ And I follow "Add Email"
+ And I fill in "teacher[email_addresses_attributes][2][email]" with "jose.personal1@email.com"
+ And I press "Update Emails"
+ Then I should see a "danger" flash message "An error occurred: Email has already been taken"
+
+ Scenario: Admin tries to add an email that exists for another teacher
+ Given I am on the BJC home page
+ And I have an admin email
+ And I follow "Log In"
+ Then I can log in with Google
+ When I follow "Teachers"
+ And I follow "Joseph Mamoa"
+ And I follow "Edit Personal Emails"
+ And I fill in "teacher[email_addresses_attributes][0][email]" with "testadminuser@berkeley.edu"
+ And I press "Update Emails"
+ Then I should see a "danger" flash message "An error occurred: Email has already been taken"
diff --git a/features/step_definitions/email_addresses_steps.rb b/features/step_definitions/email_addresses_steps.rb
new file mode 100644
index 00000000..d7c9f537
--- /dev/null
+++ b/features/step_definitions/email_addresses_steps.rb
@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+require "cucumber/rspec/doubles"
+
+Given(/the following emails exist/) do |emails_table|
+ emails_table.symbolic_hashes.each do |email_hash|
+ teacher = Teacher.find_by(first_name: email_hash[:first_name], last_name: email_hash[:last_name])
+ raise "Teacher not found for email: #{email_hash[:email]}" unless teacher
+
+ EmailAddress.create!(
+ email: email_hash[:email],
+ teacher:,
+ primary: email_hash[:primary] == "true"
+ )
+ end
+end
+
+When(/^I press "([^"]*)" next to "([^"]*)"$/) do |button_text, email_text|
+ email_field = all('.email-field').find { |field| field.has_css?('input[type="email"][value="' + email_text + '"]') }
+ within(email_field) do
+ click_link(button_text)
+ end
+end
From 7f31e97892182d06f646a65ddab0d115ff86f4ac Mon Sep 17 00:00:00 2001
From: Jingchao Zhong <92573736+perryzjc@users.noreply.github.com>
Date: Thu, 18 Apr 2024 19:33:01 -0700
Subject: [PATCH 75/88] Fix rubocop
---
features/step_definitions/email_addresses_steps.rb | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/features/step_definitions/email_addresses_steps.rb b/features/step_definitions/email_addresses_steps.rb
index d7c9f537..4c8f0171 100644
--- a/features/step_definitions/email_addresses_steps.rb
+++ b/features/step_definitions/email_addresses_steps.rb
@@ -16,7 +16,7 @@
end
When(/^I press "([^"]*)" next to "([^"]*)"$/) do |button_text, email_text|
- email_field = all('.email-field').find { |field| field.has_css?('input[type="email"][value="' + email_text + '"]') }
+ email_field = all(".email-field").find { |field| field.has_css?('input[type="email"][value="' + email_text + '"]') }
within(email_field) do
click_link(button_text)
end
From 9b318f928a57ea1d95b440d1d0b2e868150418c4 Mon Sep 17 00:00:00 2001
From: Jingchao Zhong <92573736+perryzjc@users.noreply.github.com>
Date: Thu, 18 Apr 2024 19:40:32 -0700
Subject: [PATCH 76/88] Add rspec for email validation
---
.../email_addresses_controller_spec.rb | 30 +++++++++++++++++++
1 file changed, 30 insertions(+)
create mode 100644 spec/controllers/email_addresses_controller_spec.rb
diff --git a/spec/controllers/email_addresses_controller_spec.rb b/spec/controllers/email_addresses_controller_spec.rb
new file mode 100644
index 00000000..95880990
--- /dev/null
+++ b/spec/controllers/email_addresses_controller_spec.rb
@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+
+require "rails_helper"
+
+RSpec.describe EmailAddress, type: :model do
+ describe "validations" do
+ it "is invalid with an incorrect email format" do
+ teacher = Teacher.create(first_name: "John", last_name: "Doe", admin: false)
+ email_address = EmailAddress.new(email: "invalid-email", teacher:, primary: false)
+
+ expect(email_address.valid?).to be false
+ expect(email_address.errors[:email]).to include("is invalid")
+ end
+
+ it "is invalid without an email" do
+ teacher = Teacher.create(first_name: "John", last_name: "Doe", admin: false)
+ email_address = EmailAddress.new(teacher:, primary: false)
+
+ expect(email_address.valid?).to be false
+ expect(email_address.errors[:email]).to include("can't be blank")
+ end
+
+ it "is valid with a correct email format" do
+ teacher = Teacher.create(first_name: "John", last_name: "Doe", admin: false)
+ email_address = EmailAddress.new(email: "valid.email@berkeley.edu", teacher:, primary: false)
+
+ expect(email_address.valid?).to be true
+ end
+ end
+end
From b0fa3d51254a4146d7a8bc6653595d5ff1f2b77f Mon Sep 17 00:00:00 2001
From: Jingchao Zhong <92573736+perryzjc@users.noreply.github.com>
Date: Fri, 19 Apr 2024 10:46:22 -0700
Subject: [PATCH 77/88] Fix cucumber tests to adapt new schema from new email
address model
---
features/professional_development.feature | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/features/professional_development.feature b/features/professional_development.feature
index 3dbc6558..77ec66c6 100644
--- a/features/professional_development.feature
+++ b/features/professional_development.feature
@@ -6,7 +6,7 @@ Feature: Professional Development and Registration Management
Background: Admin and Teacher exist
Given the following teachers exist:
- | first_name | last_name | admin | email | id |
+ | first_name | last_name | admin | primary_email | id |
| Perry | Zhong | true | testadminuser@berkeley.edu | 100 |
| Joseph | Mamoa | false | testteacher@berkeley.edu | 101 |
And the following professional developments exist:
From 436485fe1c5bf21960d9a1de5b2cb54c02a4bf77 Mon Sep 17 00:00:00 2001
From: Jingchao Zhong <92573736+perryzjc@users.noreply.github.com>
Date: Fri, 19 Apr 2024 11:04:35 -0700
Subject: [PATCH 78/88] Remove unused variable
---
app/controllers/pd_registrations_controller.rb | 1 -
1 file changed, 1 deletion(-)
diff --git a/app/controllers/pd_registrations_controller.rb b/app/controllers/pd_registrations_controller.rb
index 55c171ef..8dcc2b2d 100644
--- a/app/controllers/pd_registrations_controller.rb
+++ b/app/controllers/pd_registrations_controller.rb
@@ -7,7 +7,6 @@ class PdRegistrationsController < ApplicationController
before_action :set_professional_development, only: [:new, :create, :edit, :update, :destroy]
def index
- @pd_registrations = PdRegistration.where(professional_development_id: @professional_development.id)
end
def show
From 417adeaa03b481c1945924494a65caf05fe3bcf4 Mon Sep 17 00:00:00 2001
From: Jingchao Zhong <92573736+perryzjc@users.noreply.github.com>
Date: Fri, 19 Apr 2024 11:55:17 -0700
Subject: [PATCH 79/88] Add more rspec tests
---
.../pd_registrations_controller_spec.rb | 128 ++++++++++--------
1 file changed, 73 insertions(+), 55 deletions(-)
diff --git a/spec/controllers/pd_registrations_controller_spec.rb b/spec/controllers/pd_registrations_controller_spec.rb
index 1a91a95f..0ad6dc75 100644
--- a/spec/controllers/pd_registrations_controller_spec.rb
+++ b/spec/controllers/pd_registrations_controller_spec.rb
@@ -13,7 +13,6 @@
let(:valid_registration_attributes) {
{
teacher_id: teacher1.id,
- professional_development_id: professional_development.id,
attended: true,
role: "attendee"
}
@@ -21,7 +20,6 @@
let(:valid_registration_attributes2) {
{
teacher_id: teacher2.id,
- professional_development_id: professional_development.id,
attended: true,
role: "attendee"
}
@@ -29,91 +27,111 @@
let(:invalid_registration_attributes) {
{
teacher_id: nil,
- professional_development_id: nil,
attended: false,
role: ""
}
}
- let!(:pd_registration) { PdRegistration.create!(valid_registration_attributes) }
+ let!(:pd_registration) { PdRegistration.create!(valid_registration_attributes.merge(professional_development_id: professional_development.id)) }
+
+ describe "Authorization" do
+ shared_examples "admin access" do
+ it "allows operation for admin" do
+ log_in(admin_user)
+ action
+ expect(response).to have_http_status(:redirect)
+ expect(flash[:notice]).to match(/successfully/)
+ end
+ end
- shared_examples "admin access" do
- it "allows operation" do
- action
- expect(flash[:notice]).to match(/successfully/)
+ shared_examples "unauthorized access" do
+ it "denies operation for regular user" do
+ log_in(regular_user)
+ action
+ expect(response).to redirect_to(root_path)
+ expect(flash[:danger]).to be_present
+ end
end
- end
- shared_examples "regular user denied" do
- it "denies operation" do
- action
- expect(flash[:danger]).to be_present
+ describe "CREATE" do
+ let(:action) { post professional_development_pd_registrations_path(professional_development), params: { pd_registration: valid_registration_attributes2 } }
+ include_examples "admin access"
+ include_examples "unauthorized access"
+ end
+
+ describe "UPDATE" do
+ let(:action) { patch professional_development_pd_registration_path(professional_development, pd_registration), params: { pd_registration: { role: "leader" } } }
+ include_examples "admin access"
+ include_examples "unauthorized access"
+ end
+
+ describe "DELETE" do
+ let(:action) { delete professional_development_pd_registration_path(professional_development, pd_registration) }
+ include_examples "admin access"
+ include_examples "unauthorized access"
end
end
- describe "CRUD operations for PD Registrations" do
- describe "CREATE" do
- let(:action) { post professional_development_pd_registrations_path(
- professional_development_id: professional_development.id),
- params: { pd_registration: valid_registration_attributes2 } }
+ describe "Admin operations" do
+ before { log_in(admin_user) }
- context "as an admin" do
- before { log_in(admin_user) }
- include_examples "admin access"
+ describe "CREATE" do
+ context "with valid attributes" do
+ it "creates a new registration successfully" do
+ expect {
+ post professional_development_pd_registrations_path(professional_development), params: { pd_registration: valid_registration_attributes2 }
+ }.to change(PdRegistration, :count).by(1)
+ expect(response).to redirect_to(professional_development_path(professional_development))
+ expect(flash[:notice]).to match(/successfully created/)
+ end
end
- context "as a regular user" do
- before { log_in(regular_user) }
- include_examples "regular user denied"
+ context "with invalid attributes" do
+ it "fails to create and renders errors" do
+ post professional_development_pd_registrations_path(professional_development), params: { pd_registration: invalid_registration_attributes }
+ expect(response).to render_template("professional_developments/show")
+ expect(flash.now[:alert]).to be_present
+ end
end
end
describe "UPDATE" do
context "with valid attributes" do
- let(:action) { patch professional_development_pd_registration_path(
- professional_development_id: professional_development.id, id: pd_registration.id),
- params: { pd_registration: { role: "leader" } } }
-
- context "as an admin" do
- before { log_in(admin_user) }
- include_examples "admin access"
- end
-
- context "as a regular user" do
- before { log_in(regular_user) }
- include_examples "regular user denied"
+ it "updates the registration successfully" do
+ patch professional_development_pd_registration_path(professional_development, pd_registration), params: { pd_registration: { role: "leader" } }
+ expect(response).to redirect_to(professional_development_path(professional_development))
+ expect(flash[:notice]).to match(/successfully updated/)
end
end
context "with invalid attributes" do
- let(:action) { patch professional_development_pd_registration_path(
- professional_development_id: professional_development.id, id: pd_registration.id),
- params: { pd_registration: invalid_registration_attributes } }
-
- it "fails to update and redirects for admin" do
- log_in(admin_user)
- action
- expect(response).to render_template(:show)
- expect(flash[:alert]).to be_present
+ it "fails to update and renders errors" do
+ patch professional_development_pd_registration_path(professional_development, pd_registration), params: { pd_registration: invalid_registration_attributes }
+ expect(response).to render_template("professional_developments/show")
+ expect(flash.now[:alert]).to include("Teacher must exist and Role is not a valid role")
end
end
end
describe "DELETE" do
- let(:action) { delete professional_development_pd_registration_path(
- professional_development_id: professional_development.id, id: pd_registration.id) }
-
- context "as an admin" do
- before { log_in(admin_user) }
- it "deletes the PD registration" do
- expect { action }.to change(PdRegistration, :count).by(-1)
+ context "when deletion is successful" do
+ it "deletes the registration successfully and redirects" do
+ expect {
+ delete professional_development_pd_registration_path(professional_development, pd_registration)
+ }.to change(PdRegistration, :count).by(-1)
expect(response).to redirect_to(professional_development_path(professional_development))
expect(flash[:notice]).to match(/successfully cancelled/)
end
end
- context "as a regular user" do
- before { log_in(regular_user) }
- include_examples "regular user denied"
+ context "when attempting to delete a non-existent registration" do
+ it "does not change the count of registrations and redirects with an error" do
+ expect {
+ # using -1 to simulate a non-existent ID
+ delete professional_development_pd_registration_path(professional_development, -1)
+ }.not_to change(PdRegistration, :count)
+ expect(response).to redirect_to(professional_development_path)
+ expect(flash[:alert]).to match("Registration not found.")
+ end
end
end
end
From ed3d7d5cfd9ca58a6304ddbe414dbdeceadb0d05 Mon Sep 17 00:00:00 2001
From: MT
Date: Thu, 25 Apr 2024 17:11:12 -0700
Subject: [PATCH 80/88] finish tooltip bug fix
---
app/javascript/packs/datatables.js | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/app/javascript/packs/datatables.js b/app/javascript/packs/datatables.js
index 35183279..fc6496ad 100644
--- a/app/javascript/packs/datatables.js
+++ b/app/javascript/packs/datatables.js
@@ -22,5 +22,12 @@ $(function() {
});
$tables.draw();
- $(".custom-checkbox").on("change", () => $tables.draw());
+ $(".custom-checkbox").on("change", () => {
+ console.log("custom-checkbox change");
+ $tables.draw();
+ });
+ $tables.on('draw', function() {
+ $('[data-toggle="tooltip"]').tooltip('dispose');
+ $('[data-toggle="tooltip"]').tooltip();
+ });
});
From 9efdd797806240370591cd2f052095c46a0cd0de Mon Sep 17 00:00:00 2001
From: Michael Ball
Date: Fri, 10 May 2024 18:26:34 +0200
Subject: [PATCH 81/88] Little cleanup CS169L work
---
app/models/pd_registration.rb | 8 +-------
app/models/professional_development.rb | 5 +----
app/models/teacher.rb | 5 +----
app/views/professional_developments/edit.html.erb | 14 +++++---------
.../professional_developments/index.html.erb | 4 +---
app/views/professional_developments/show.html.erb | 1 -
app/views/schools/_form.html.erb | 2 +-
app/views/teachers/_form.html.erb | 15 +++++++--------
8 files changed, 17 insertions(+), 37 deletions(-)
diff --git a/app/models/pd_registration.rb b/app/models/pd_registration.rb
index fdd0d292..5ecf2d55 100644
--- a/app/models/pd_registration.rb
+++ b/app/models/pd_registration.rb
@@ -30,11 +30,5 @@ class PdRegistration < ApplicationRecord
validates :attended, inclusion: { in: [true, false] }
def teacher_name
- teacher = Teacher.find_by(id: teacher_id)
- if teacher.present?
- "#{teacher.first_name} #{teacher.last_name}"
- else
- "Teacher not found"
- end
- end
+ self.teacher.full_name
end
diff --git a/app/models/professional_development.rb b/app/models/professional_development.rb
index 9467b217..8f37d5d2 100644
--- a/app/models/professional_development.rb
+++ b/app/models/professional_development.rb
@@ -16,12 +16,9 @@
# updated_at :datetime not null
#
class ProfessionalDevelopment < ApplicationRecord
- VALID_STATES = %w[AL AK AS AZ AR CA CO CT DE DC FM FL GA GU HI ID IL IN IA KS KY LA ME MH MD MA MI MN MS MO MT NE NV
- NH NJ NM NY NC ND MP OH OK OR PW PA PR RI SC SD TN TX UT VT VI VA WA WV WI WY].freeze
-
validates :name, :city, :country, :start_date, :end_date, presence: true
validates :state, presence: true, if: -> { country == "US" }
- validates :state, inclusion: { in: VALID_STATES, message: "%{value} is not a valid state" },
+ validates :state, inclusion: { in: School::VALID_STATES, message: "%{value} is not a valid state" },
if: -> { country == "US" }
validate :end_date_after_start_date
diff --git a/app/models/teacher.rb b/app/models/teacher.rb
index 265a48cf..a46a502e 100644
--- a/app/models/teacher.rb
+++ b/app/models/teacher.rb
@@ -218,8 +218,7 @@ def short_application_status
end
def self.user_from_omniauth(omniauth)
- teacher = EmailAddress.find_by(email: omniauth.email.downcase)&.teacher
- teacher
+ EmailAddress.find_by(email: omniauth.email.downcase)&.teacher
end
def try_append_ip(ip)
@@ -292,8 +291,6 @@ def email
end
def primary_email
- # ||:email this code is temporary for this PR: https://github.com/cs169/BJC-Teacher-Tracker-App/pull/49
- # to make sure at least original data in db still work and passed the existing tests
email_addresses.find_by(primary: true)&.email
end
diff --git a/app/views/professional_developments/edit.html.erb b/app/views/professional_developments/edit.html.erb
index 64f4f467..a5371faf 100644
--- a/app/views/professional_developments/edit.html.erb
+++ b/app/views/professional_developments/edit.html.erb
@@ -1,12 +1,8 @@
<%= provide(:h1, "Update #{@professional_development.name}") %>
-<% if @professional_development.nil? %>
-
<% end %>
diff --git a/app/views/professional_developments/index.html.erb b/app/views/professional_developments/index.html.erb
index ed1bb5b4..c5f45451 100644
--- a/app/views/professional_developments/index.html.erb
+++ b/app/views/professional_developments/index.html.erb
@@ -1,4 +1,4 @@
-<%= provide(:title, "BJC Schools") %>
+<%= provide(:title, "BJC PD") %>
<%= provide(:header_button, "New Professional Development") %>
@@ -9,8 +9,6 @@
Start Date
End Date
Grade Level
-
-
Actions
diff --git a/app/views/professional_developments/show.html.erb b/app/views/professional_developments/show.html.erb
index 2ab41ba4..28f17cca 100644
--- a/app/views/professional_developments/show.html.erb
+++ b/app/views/professional_developments/show.html.erb
@@ -162,7 +162,6 @@
function setFormActionForUpdate(pdId, registrationId) {
form.action = `/professional_developments/${pdId}/pd_registrations/${registrationId}`;
- console.debug("Setting form action to:", form.action);
form.method = 'post';
ensureMethodInput('patch');
}
diff --git a/app/views/schools/_form.html.erb b/app/views/schools/_form.html.erb
index 6cb701aa..7bfe25d5 100644
--- a/app/views/schools/_form.html.erb
+++ b/app/views/schools/_form.html.erb
@@ -119,7 +119,7 @@
} else {
stateTextfieldContainer.show();
- stateSelect.removeAttr('required') //else make state select not required
+ stateSelect.removeAttr('required');
stateSelectContainer.hide();
diff --git a/app/views/teachers/_form.html.erb b/app/views/teachers/_form.html.erb
index 7c3cf65e..9f5098f9 100644
--- a/app/views/teachers/_form.html.erb
+++ b/app/views/teachers/_form.html.erb
@@ -80,7 +80,6 @@ status ONLY IF the person viewing this page is an admin. %>
- <%# For now... only admins can enter/edit personal emails. %>
@@ -111,13 +110,13 @@ status ONLY IF the person viewing this page is an admin. %>
<%= f.label :languages, "What language(s) are spoken in the classroom?",
class: "label-required" %>
<%= f.select(
- :languages,
- options_for_select(Teacher.language_options, @teacher.languages),
- {},
- multiple: true,
- include_blank: "Select an option",
- class: 'selectize', required: true
-) %>
+ :languages,
+ options_for_select(Teacher.language_options, @teacher.languages),
+ {},
+ multiple: true,
+ include_blank: "Select an option",
+ class: 'selectize', required: true
+ ) %>
From b77845a03404224df957365344ec2d03db7ae198 Mon Sep 17 00:00:00 2001
From: Michael Ball
Date: Fri, 10 May 2024 23:19:49 +0200
Subject: [PATCH 82/88] Update db/seed_data.rb
---
db/seed_data.rb | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/db/seed_data.rb b/db/seed_data.rb
index ce084203..fb5c9c03 100644
--- a/db/seed_data.rb
+++ b/db/seed_data.rb
@@ -169,7 +169,7 @@ def self.teachers
application_status: "Validated",
school: School.find_by(name: "UC Berkeley"),
- # Note: email field does not exist in the new schema of the Teacher model
+ # Note: email field does not exist in the schema of the Teacher model
# Include it in the seed data is to simulate the behavior of creating a new teacher,
# because we need to use it to compared with the EmailAddress model,
# to determine the existence of the teacher
From d1d0ed5e0a1b63130d4fee8567e47bc5bf76859c Mon Sep 17 00:00:00 2001
From: Michael Ball
Date: Fri, 10 May 2024 23:20:11 +0200
Subject: [PATCH 83/88] Update app/models/teacher.rb
---
app/models/teacher.rb | 2 --
1 file changed, 2 deletions(-)
diff --git a/app/models/teacher.rb b/app/models/teacher.rb
index a46a502e..cdf61128 100644
--- a/app/models/teacher.rb
+++ b/app/models/teacher.rb
@@ -300,8 +300,6 @@ def personal_emails
private
def non_primary_emails
- # email_addresses.where(primary: false)&.pluck(:email)
- # below code is temporary for current PR, to make sure the frontend same as before (only one personal email)
email_addresses.where(primary: false)&.pluck(:email)
end
end
From 74c67d6ee3f8c6fe833aa0a60c4ebaab7f634549 Mon Sep 17 00:00:00 2001
From: Michael Ball
Date: Fri, 10 May 2024 23:47:51 +0200
Subject: [PATCH 84/88] trying to fix datatables
---
app/javascript/packs/datatables.js | 1 -
app/views/teachers/_table_headers.erb | 2 +-
app/views/teachers/_teacher.erb | 9 ++++++---
app/views/teachers/index.html.erb | 4 ++--
db/schema.rb | 2 --
5 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/app/javascript/packs/datatables.js b/app/javascript/packs/datatables.js
index fc6496ad..f3811e4d 100644
--- a/app/javascript/packs/datatables.js
+++ b/app/javascript/packs/datatables.js
@@ -23,7 +23,6 @@ $(function() {
$tables.draw();
$(".custom-checkbox").on("change", () => {
- console.log("custom-checkbox change");
$tables.draw();
});
$tables.on('draw', function() {
diff --git a/app/views/teachers/_table_headers.erb b/app/views/teachers/_table_headers.erb
index 8c070619..047016fe 100644
--- a/app/views/teachers/_table_headers.erb
+++ b/app/views/teachers/_table_headers.erb
@@ -1,7 +1,7 @@
<% Teacher.application_statuses.each do |key, value| %>
-
- >
-
-
+
+ >
+
+
+
<% end %>
diff --git a/script/normalize_school_data.rb b/script/normalize_school_data.rb
index c2ec9b3f..d3eeb8b8 100644
--- a/script/normalize_school_data.rb
+++ b/script/normalize_school_data.rb
@@ -30,7 +30,6 @@ def set_country_for_international_schools!
schools.each do |school|
domain = URI.parse(school.website).host
country = domain.split(".").last.upcase
- # check if country is valid
if ISO3166::Country.find_country_by_alpha2(country)
school.update(country:, state: nil)
else
From 34208a6a9485bfc450172f55b4af612e383ba92b Mon Sep 17 00:00:00 2001
From: Michael Ball
Date: Sat, 11 May 2024 01:15:54 +0200
Subject: [PATCH 88/88] OK, I think the cucumber specs are good now...
---
features/data_tables.feature | 34 +++++++---------------------------
1 file changed, 7 insertions(+), 27 deletions(-)
diff --git a/features/data_tables.feature b/features/data_tables.feature
index 97635f90..2456a01f 100644
--- a/features/data_tables.feature
+++ b/features/data_tables.feature
@@ -6,41 +6,21 @@ Feature: Admin Data Tables functionality
Background: Has an Admin in DB
Given the following teachers exist:
- | first_name | last_name | admin | email |
- | Admin | User | true | testadminuser@berkeley.edu |
-
- # Scenario: Updating application status persists changes in database
- # Given the following schools exist:
- # | name | country | city | state | website | grade_level | school_type |
- # | UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | university | public |
- # Given the following teachers exist:
- # | first_name | last_name | admin | email | school | snap | application_status |
- # | Bobby | John | false | testteacher@berkeley.edu | UC Berkeley | bobby | denied |
- # Given I am on the BJC home page
- # And I have an admin email
- # And I follow "Log In"
- # Then I can log in with Google
- # When I go to the teachers page
- # And I go to the edit page for Bobby John
- # And I set my application status as "Validated"
- # And I press "Update"
- # Then I see a confirmation "Saved"
- # When I go to the teachers page
- # And I check "Validated"
- # Then I should see "Bobby John"
+ | first_name | last_name | admin | primary_email |
+ | Admin | User | true | testadminuser@berkeley.edu |
Scenario: Updating application status persists changes in database
Given the following schools exist:
- | name | country | city | state | website | grade_level | school_type |
- | UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | university | public |
+ | name | country | city | state | website | grade_level | school_type |
+ | UC Berkeley | US | Berkeley | CA | https://www.berkeley.edu | university | public |
Given the following teachers exist:
- | first_name | last_name | admin | primary_email | school | snap | application_status |
- | Bobby | John | false | testteacher@berkeley.edu | UC Berkeley | bobby | denied |
+ | first_name | last_name | admin | primary_email | school | snap | application_status |
+ | Bobby | John | false | testteacher@berkeley.edu | UC Berkeley | bobby | not_reviewed |
Given I am on the BJC home page
And I have an admin email
And I follow "Log In"
Then I can log in with Google
- When I go to the teachers page
+ # When I go to the teachers page
And I go to the edit page for Bobby John
And I set my application status as "Validated"
And I press "Update"