From 9efdd797806240370591cd2f052095c46a0cd0de Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Fri, 10 May 2024 18:26:34 +0200 Subject: [PATCH 1/8] 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? %> -

Professional Development not found.

-<% else %> - <%= form_for @professional_development do |f| %> - <%= render 'professional_developments/form', f: f, professional_development: @professional_development %> -
- <%= f.submit 'Submit', {class: 'btn btn-primary', id: 'submit_button'} %> -
- <% end %> +<%= form_for @professional_development do |f| %> + <%= render 'professional_developments/form', f: f, professional_development: @professional_development %> +
+ <%= f.submit 'Submit', {class: 'btn btn-primary', id: 'submit_button'} %> +
<% 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 @@ - - 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 2/8] 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 3/8] 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 4/8] 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 @@ - + diff --git a/app/views/teachers/_teacher.erb b/app/views/teachers/_teacher.erb index 5ef1bbba..c7986865 100644 --- a/app/views/teachers/_teacher.erb +++ b/app/views/teachers/_teacher.erb @@ -29,9 +29,12 @@ <%= link_to(teacher.school.name, school_path(teacher.school)) %> <%= teacher.school.location %> - + diff --git a/app/views/teachers/index.html.erb b/app/views/teachers/index.html.erb index f3238106..0edd61e5 100644 --- a/app/views/teachers/index.html.erb +++ b/app/views/teachers/index.html.erb @@ -7,8 +7,8 @@
- - + +
diff --git a/db/schema.rb b/db/schema.rb index de073570..6fd36fa5 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,10 +10,8 @@ # # It's strongly recommended that you check this file into your version control system. - ActiveRecord::Schema.define(version: 2024_04_15_190239) do - # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" From 710e6d68d7f1ab01595e824bbe635b89427de14d Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Fri, 10 May 2024 23:51:25 +0200 Subject: [PATCH 5/8] fix typos/specs from code cleanup --- app/models/pd_registration.rb | 3 ++- app/views/professional_developments/_form.html.erb | 2 +- db/seeds.rb | 7 ++----- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/app/models/pd_registration.rb b/app/models/pd_registration.rb index 5ecf2d55..ca605884 100644 --- a/app/models/pd_registration.rb +++ b/app/models/pd_registration.rb @@ -30,5 +30,6 @@ class PdRegistration < ApplicationRecord validates :attended, inclusion: { in: [true, false] } def teacher_name - self.teacher.full_name + self.teacher.full_name + end end diff --git a/app/views/professional_developments/_form.html.erb b/app/views/professional_developments/_form.html.erb index efcedd0c..74c2a789 100644 --- a/app/views/professional_developments/_form.html.erb +++ b/app/views/professional_developments/_form.html.erb @@ -25,7 +25,7 @@
<%= f.label :state, class: "label-required", for: "professional_development_state" %> - <%= f.select :state, ProfessionalDevelopment::VALID_STATES, { include_blank: "State" }, { id: "state_select", class: 'form-control' } %> + <%= f.select :state, School::VALID_STATES, { include_blank: "State" }, { id: "state_select", class: 'form-control' } %>
diff --git a/db/seeds.rb b/db/seeds.rb index 445115e3..3f2efa74 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -11,23 +11,20 @@ SeedData.teachers.each do |teacher_attr| email_address = EmailAddress.find_or_initialize_by(email: teacher_attr.delete(: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}" + # 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." + # puts "Teacher updated successfully." else puts "Failed to update teacher. Errors: #{teacher.errors.full_messages.join(", ")}" end From e7b5f07eb2ec91f3a4fb7d3ac8eebf67da9111fd Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Sat, 11 May 2024 00:49:07 +0200 Subject: [PATCH 6/8] Fix some datatables bugs, add school data cleanup script --- app/javascript/packs/datatables.js | 3 ++- app/views/teachers/index.html.erb | 22 +++++----------- db/seeds.rb | 7 ++--- script/normalize_school_data.rb | 42 ++++++++++++++++++++++++++++++ 4 files changed, 52 insertions(+), 22 deletions(-) create mode 100644 script/normalize_school_data.rb diff --git a/app/javascript/packs/datatables.js b/app/javascript/packs/datatables.js index f3811e4d..151a9ebc 100644 --- a/app/javascript/packs/datatables.js +++ b/app/javascript/packs/datatables.js @@ -3,7 +3,8 @@ $(function() { $.fn.dataTable.ext.search.push((_, searchData) => { let enabled = $('input:checkbox[name="statusFilter"]:checked').map((_i, el) => el.value).get(); // Include all rows when no checkboxes are selected. - return enabled.length === 0 || enabled.includes(searchData[7]); + console.log(enabled.includes(searchData[6])); + return enabled.length === 0 || enabled.includes(searchData[6]); }); let $tables = $('.js-dataTable').DataTable({ diff --git a/app/views/teachers/index.html.erb b/app/views/teachers/index.html.erb index 0edd61e5..f021ab03 100644 --- a/app/views/teachers/index.html.erb +++ b/app/views/teachers/index.html.erb @@ -6,22 +6,12 @@ Export All
-
- - -
-
- - -
-
- - -
-
- - -
+<% Teacher.application_statuses.each do |key, value| %> +
+ > + +
+<% end %>
Start Date End Date Grade Level Actions
Name Email EducationStatusRole Snap! School Approved? - <%= teacher.short_application_status %> -<%= teacher.short_application_status %> <%= teacher.created_at.strftime("%m/%d/%Y") %>
diff --git a/db/seeds.rb b/db/seeds.rb index 3f2efa74..74e8b57f 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -16,16 +16,13 @@ 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}" + email_address.save 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 + if !teacher&.update(teacher_attr) puts "Failed to update teacher. Errors: #{teacher.errors.full_messages.join(", ")}" end end diff --git a/script/normalize_school_data.rb b/script/normalize_school_data.rb new file mode 100644 index 00000000..c2ec9b3f --- /dev/null +++ b/script/normalize_school_data.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +# Usage: rails runner script/normalize_school_data.rb +def set_usa_schools! + schools = School.where.not(state: "International") + puts "Updating #{schools.count} schools to have country 'US'" + schools.update_all(country: "US") +end + +set_usa_schools! + +def cleanup_school_websites + schools = School.where.not(website: nil) + schools.each do |school| + begin + uri = URI.parse(school.website).normalize.to_s + rescue URI::InvalidURIError + puts "Invalid URI: #{school.website}" + uri = school.website.lowercase.strip + end + school.update(website: uri) + end +end + +cleanup_school_websites + +def set_country_for_international_schools! + # Set the country code based on the domain name + schools = School.where(state: "International") + 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 + puts "Invalid country code. School: #{school.id} #{school.name}, #{country} #{school.website}" + end + end +end + +set_country_for_international_schools! From e977d68843da5e2c69a866b918075a2dff41af4a Mon Sep 17 00:00:00 2001 From: Michael Ball Date: Sat, 11 May 2024 01:06:58 +0200 Subject: [PATCH 7/8] Remove the education level from the teacher datatable --- app/javascript/packs/datatables.js | 3 +-- app/views/teachers/_table_headers.erb | 1 - app/views/teachers/_teacher.erb | 2 +- app/views/teachers/index.html.erb | 9 +++++---- script/normalize_school_data.rb | 1 - 5 files changed, 7 insertions(+), 9 deletions(-) diff --git a/app/javascript/packs/datatables.js b/app/javascript/packs/datatables.js index 151a9ebc..22ee1529 100644 --- a/app/javascript/packs/datatables.js +++ b/app/javascript/packs/datatables.js @@ -3,8 +3,7 @@ $(function() { $.fn.dataTable.ext.search.push((_, searchData) => { let enabled = $('input:checkbox[name="statusFilter"]:checked').map((_i, el) => el.value).get(); // Include all rows when no checkboxes are selected. - console.log(enabled.includes(searchData[6])); - return enabled.length === 0 || enabled.includes(searchData[6]); + return enabled.length === 0 || enabled.includes(searchData[5]); }); let $tables = $('.js-dataTable').DataTable({ diff --git a/app/views/teachers/_table_headers.erb b/app/views/teachers/_table_headers.erb index 047016fe..707a4958 100644 --- a/app/views/teachers/_table_headers.erb +++ b/app/views/teachers/_table_headers.erb @@ -1,6 +1,5 @@ - diff --git a/app/views/teachers/_teacher.erb b/app/views/teachers/_teacher.erb index c7986865..f32258d7 100644 --- a/app/views/teachers/_teacher.erb +++ b/app/views/teachers/_teacher.erb @@ -10,7 +10,6 @@ <% end %> <% end %> -
Name EmailEducation Role Snap! School<%= teacher.display_education_level %> 25 %> data-toggle="tooltip" @@ -28,6 +27,7 @@ <%= link_to(teacher.school.name, school_path(teacher.school)) %> <%= teacher.school.location %> +
<%= teacher.school.display_grade_level %> 		<% 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 8/8] 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"