diff --git a/Gemfile b/Gemfile index af9ef1bf..cbf77e55 100644 --- a/Gemfile +++ b/Gemfile @@ -6,7 +6,7 @@ git_source(:github) { |repo| "https://github.com/#{repo}.git" } ruby "3.2.3" # TODO: use file: '.tool-versions' when heroku supports it. (bundler >= 2.5) -gem "rails", "6.1.7.6" +gem "rails", "6.1.7.7" # Production app server gem "puma", "~> 5" diff --git a/Gemfile.lock b/Gemfile.lock index 503f12ff..c7650204 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -24,62 +24,62 @@ GIT GEM remote: https://rubygems.org/ specs: - actioncable (6.1.7.6) - actionpack (= 6.1.7.6) - activesupport (= 6.1.7.6) + actioncable (6.1.7.7) + actionpack (= 6.1.7.7) + activesupport (= 6.1.7.7) nio4r (~> 2.0) websocket-driver (>= 0.6.1) - actionmailbox (6.1.7.6) - actionpack (= 6.1.7.6) - activejob (= 6.1.7.6) - activerecord (= 6.1.7.6) - activestorage (= 6.1.7.6) - activesupport (= 6.1.7.6) + actionmailbox (6.1.7.7) + actionpack (= 6.1.7.7) + activejob (= 6.1.7.7) + activerecord (= 6.1.7.7) + activestorage (= 6.1.7.7) + activesupport (= 6.1.7.7) mail (>= 2.7.1) - actionmailer (6.1.7.6) - actionpack (= 6.1.7.6) - actionview (= 6.1.7.6) - activejob (= 6.1.7.6) - activesupport (= 6.1.7.6) + actionmailer (6.1.7.7) + actionpack (= 6.1.7.7) + actionview (= 6.1.7.7) + activejob (= 6.1.7.7) + activesupport (= 6.1.7.7) mail (~> 2.5, >= 2.5.4) rails-dom-testing (~> 2.0) - actionpack (6.1.7.6) - actionview (= 6.1.7.6) - activesupport (= 6.1.7.6) + actionpack (6.1.7.7) + actionview (= 6.1.7.7) + activesupport (= 6.1.7.7) rack (~> 2.0, >= 2.0.9) rack-test (>= 0.6.3) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.0, >= 1.2.0) - actiontext (6.1.7.6) - actionpack (= 6.1.7.6) - activerecord (= 6.1.7.6) - activestorage (= 6.1.7.6) - activesupport (= 6.1.7.6) + actiontext (6.1.7.7) + actionpack (= 6.1.7.7) + activerecord (= 6.1.7.7) + activestorage (= 6.1.7.7) + activesupport (= 6.1.7.7) nokogiri (>= 1.8.5) - actionview (6.1.7.6) - activesupport (= 6.1.7.6) + actionview (6.1.7.7) + activesupport (= 6.1.7.7) builder (~> 3.1) erubi (~> 1.4) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.1, >= 1.2.0) - activejob (6.1.7.6) - activesupport (= 6.1.7.6) + activejob (6.1.7.7) + activesupport (= 6.1.7.7) globalid (>= 0.3.6) - activemodel (6.1.7.6) - activesupport (= 6.1.7.6) - activerecord (6.1.7.6) - activemodel (= 6.1.7.6) - activesupport (= 6.1.7.6) + activemodel (6.1.7.7) + activesupport (= 6.1.7.7) + activerecord (6.1.7.7) + activemodel (= 6.1.7.7) + activesupport (= 6.1.7.7) activerecord-import (1.4.0) activerecord (>= 4.2) - activestorage (6.1.7.6) - actionpack (= 6.1.7.6) - activejob (= 6.1.7.6) - activerecord (= 6.1.7.6) - activesupport (= 6.1.7.6) + activestorage (6.1.7.7) + actionpack (= 6.1.7.7) + activejob (= 6.1.7.7) + activerecord (= 6.1.7.7) + activesupport (= 6.1.7.7) marcel (~> 1.0) mini_mime (>= 1.1.0) - activesupport (6.1.7.6) + activesupport (6.1.7.7) concurrent-ruby (~> 1.0, >= 1.0.2) i18n (>= 1.6, < 2) minitest (>= 5.1) @@ -374,7 +374,7 @@ GEM puma (5.6.8) nio4r (~> 2.0) racc (1.7.3) - rack (2.2.8) + rack (2.2.8.1) rack-mini-profiler (2.3.3) rack (>= 1.2.0) rack-protection (3.2.0) @@ -384,20 +384,20 @@ GEM rack rack-test (2.1.0) rack (>= 1.3) - rails (6.1.7.6) - actioncable (= 6.1.7.6) - actionmailbox (= 6.1.7.6) - actionmailer (= 6.1.7.6) - actionpack (= 6.1.7.6) - actiontext (= 6.1.7.6) - actionview (= 6.1.7.6) - activejob (= 6.1.7.6) - activemodel (= 6.1.7.6) - activerecord (= 6.1.7.6) - activestorage (= 6.1.7.6) - activesupport (= 6.1.7.6) + rails (6.1.7.7) + actioncable (= 6.1.7.7) + actionmailbox (= 6.1.7.7) + actionmailer (= 6.1.7.7) + actionpack (= 6.1.7.7) + actiontext (= 6.1.7.7) + actionview (= 6.1.7.7) + activejob (= 6.1.7.7) + activemodel (= 6.1.7.7) + activerecord (= 6.1.7.7) + activestorage (= 6.1.7.7) + activesupport (= 6.1.7.7) bundler (>= 1.15.0) - railties (= 6.1.7.6) + railties (= 6.1.7.7) sprockets-rails (>= 2.0.0) rails-controller-testing (1.0.5) actionpack (>= 5.0.1.rc1) @@ -410,9 +410,9 @@ GEM rails-html-sanitizer (1.6.0) loofah (~> 2.21) nokogiri (~> 1.14) - railties (6.1.7.6) - actionpack (= 6.1.7.6) - activesupport (= 6.1.7.6) + railties (6.1.7.7) + actionpack (= 6.1.7.7) + activesupport (= 6.1.7.7) method_source rake (>= 12.2) thor (~> 1.0) @@ -421,7 +421,7 @@ GEM rb-fsevent (0.11.0) rb-inotify (0.10.1) ffi (~> 1.0) - rdoc (6.6.2) + rdoc (6.6.3.1) psych (>= 4.0.0) regexp_parser (2.8.2) reline (0.4.2) @@ -528,7 +528,7 @@ GEM temple (0.8.2) terminal-table (3.0.2) unicode-display_width (>= 1.1.1, < 3) - thor (1.3.0) + thor (1.3.1) thread_safe (0.3.6) tilt (2.0.10) timeout (0.4.1) @@ -599,7 +599,7 @@ DEPENDENCIES pronto-rubocop puma (~> 5) rack-mini-profiler (~> 2.0) - rails (= 6.1.7.6) + rails (= 6.1.7.7) rails-controller-testing rspec-rails rubocop diff --git a/app/controllers/concerns/school_params.rb b/app/controllers/concerns/school_params.rb new file mode 100644 index 00000000..5bb01362 --- /dev/null +++ b/app/controllers/concerns/school_params.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module SchoolParams + private + def unique_school_params + { + name: school_params[:name], + country: school_params[:country], + city: school_params[:city], + state: school_params[:state] + } + end + + def school_params + params.require(:school).permit(:name, :country, :city, :state, :website, :grade_level, :school_type, :country, { tags: [] }, :nces_id) + end +end diff --git a/app/controllers/email_templates_controller.rb b/app/controllers/email_templates_controller.rb index 8fb28361..76a78b65 100644 --- a/app/controllers/email_templates_controller.rb +++ b/app/controllers/email_templates_controller.rb @@ -18,7 +18,7 @@ def update flash[:success] = "Updated #{@email_template.title} template successfully." redirect_to email_templates_path else - flash.now[:alert] = "An error occurred: #{@email_template.errors.full_messages.join(', ')}" + flash.now[:alert] = "An error occurred: #{@email_template.errors.full_messages.join(", ")}" render "edit" end end @@ -36,7 +36,7 @@ def create flash[:success] = "Created #{@email_template.title} successfully." redirect_to email_templates_path else - flash[:alert] = "An error occurred: #{@email_template.errors.full_messages.join(', ')}" + flash.now[:alert] = "An error occurred: #{@email_template.errors.full_messages.join(", ")}" render "new" end end diff --git a/app/controllers/main_controller.rb b/app/controllers/main_controller.rb index 1441cbe1..1f3a343d 100644 --- a/app/controllers/main_controller.rb +++ b/app/controllers/main_controller.rb @@ -14,7 +14,6 @@ def index elsif is_teacher? && !current_user.validated? message = "Your application is #{mapped_application_status(current_user.application_status)}. You may update your information. Please check your email for more information." # Both `info_needed` and `not_reviewed` flash message should be flash[:warn] style - # flash[:warn] for yellow warning message in bootstrap style redirect_to edit_teacher_path(current_user.id), warn: message # if this user is denied, redirect to new else @@ -35,8 +34,7 @@ def mapped_application_status(status) status_map = { "info_needed" => "requested to be updated", "not_reviewed" => "not reviewed" - } - # Fetch the mapped status if present; otherwise, return the original status + }.with_indifferent_access status_map.fetch(status, status) end end diff --git a/app/controllers/merge_controller.rb b/app/controllers/merge_controller.rb new file mode 100644 index 00000000..d860662d --- /dev/null +++ b/app/controllers/merge_controller.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +class MergeController < ApplicationController + before_action :require_admin + + def preview + @from_teacher = Teacher.find(params[:from]) + @into_teacher = Teacher.find(params[:into]) + @result_teacher = merge_teachers(@from_teacher, @into_teacher) + end + + def execute + @from_teacher = Teacher.find(params[:from]) + @into_teacher = Teacher.find(params[:into]) + @merged_teacher = merge_teachers(@from_teacher, @into_teacher) + + merged_attributes = @merged_teacher.attributes.except("id") + @into_teacher.update!(merged_attributes) + @from_teacher.destroy + redirect_to teachers_path, notice: "Teachers merged successfully." + end + + private + # Returns a merged teacher without saving it to the database. + # Rendered in the preview, and only saved to the DB in a call to merge. + def merge_teachers(from_teacher, into_teacher) + merged_attributes = {} + into_teacher.attributes.each do |attr_name, attr_value| + from_teacher_attr_value = from_teacher.attributes[attr_name] + if attr_value.blank? + merged_attributes[attr_name] = from_teacher_attr_value + elsif from_teacher_attr_value.blank? + merged_attributes[attr_name] = attr_value + else + case attr_name + when "session_count" + merged_attributes[attr_name] = attr_value + from_teacher_attr_value + when "ip_history" + merged_attributes[attr_name] = (attr_value + from_teacher_attr_value).uniq + when "last_session_at" + # The resulting last session time is the most recent one + merged_attributes[attr_name] = attr_value > from_teacher_attr_value ? attr_value : from_teacher_attr_value + when "created_at" + # The resulting record creation time is the least recent one + merged_attributes[attr_name] = attr_value < from_teacher_attr_value ? attr_value : from_teacher_attr_value + else + merged_attributes[attr_name] = attr_value + end + end + end + + merged_teacher = Teacher.new(merged_attributes) + merged_teacher + end +end diff --git a/app/controllers/schools_controller.rb b/app/controllers/schools_controller.rb index 500f6ca0..ecdbfc37 100644 --- a/app/controllers/schools_controller.rb +++ b/app/controllers/schools_controller.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true class SchoolsController < ApplicationController + include SchoolParams before_action :require_admin def index @@ -12,11 +13,11 @@ def show end def search - School.all.collect { |school| ["#{school.name}, #{school.country}, #{school.city}, #{school.state}", school.name] } + School.search_list end def create - @school = School.find_by(name: school_params[:name], country: school_params[:country], city: school_params[:city], state: school_params[:state]) + @school = School.find_by(**unique_school_params) if @school @school.assign_attributes(school_params) else @@ -64,10 +65,6 @@ def destroy end private - def school_params - params.require(:school).permit(:name, :country, :city, :state, :website, :grade_level, :school_type, :country, { tags: [] }, :nces_id) - end - def load_ordered_schools @ordered_schools ||= School.all.order(:name) end diff --git a/app/controllers/teachers_controller.rb b/app/controllers/teachers_controller.rb index 2ed2ede8..997d4fed 100644 --- a/app/controllers/teachers_controller.rb +++ b/app/controllers/teachers_controller.rb @@ -6,13 +6,14 @@ require "activerecord-import" class TeachersController < ApplicationController + include SchoolParams include CsvProcess before_action :load_pages, only: [:new, :create, :edit, :update] - before_action :load_teacher, except: [:new, :index, :create, :import] + before_action :load_teacher, except: [:new, :index, :create, :import, :search] before_action :sanitize_params, only: [:new, :create, :edit, :update] before_action :require_login, except: [:new, :create] - before_action :require_admin, only: [:validate, :deny, :destroy, :index, :show] + before_action :require_admin, only: [:validate, :deny, :destroy, :index, :show, :search] before_action :require_edit_permission, only: [:edit, :update, :resend_welcome_email] rescue_from ActiveRecord::RecordNotUnique, with: :deny_access @@ -31,6 +32,7 @@ def index end def show + @all_teachers_except_current = Teacher.where.not(id: @teacher.id) @school = @teacher.school @status = is_admin? ? "Admin" : "Teacher" end @@ -57,9 +59,10 @@ def create load_school if @school.new_record? + return unless params[:school] @school = School.new(school_params) unless @school.save - flash[:alert] = "An error occurred! #{@school.errors.full_messages.join(', ')}" + flash[:alert] = "An error occurred: #{@school.errors.full_messages.join(', ')}" render "new" && return end end @@ -103,7 +106,7 @@ def update @school.update(school_params) if school_params unless @school.save flash[:alert] = "An error occurred: #{@school.errors.full_messages.join(', ')}" - render "new" && return + render "edit" && return end @teacher.school = @school end @@ -193,6 +196,7 @@ def import private def load_teacher + @teachers = Teacher.all @teacher ||= Teacher.find(params[:id]) end @@ -247,7 +251,7 @@ def load_school if teacher_params[:school_id].present? @school ||= School.find(teacher_params[:school_id]) end - @school ||= School.find_or_create_by(name: school_params[:name], city: school_params[:city], country: school_params[:country], state: school_params[:state]) + @school ||= School.find_or_create_by(**unique_school_params) end def teacher_params @@ -259,11 +263,6 @@ def teacher_params params.require(:teacher).permit(*teacher_attributes) end - def school_params - return unless params[:school] - params.require(:school).permit(:name, :country, :city, :state, :website, :grade_level, :school_type) - end - def omniauth_data @omniauth_data ||= session[:auth_data]&.slice("first_name", "last_name", "email") end diff --git a/app/helpers/merge_helper.rb b/app/helpers/merge_helper.rb new file mode 100644 index 00000000..e3765bb1 --- /dev/null +++ b/app/helpers/merge_helper.rb @@ -0,0 +1,4 @@ +# frozen_string_literal: true + +module MergeHelper +end diff --git a/app/javascript/styles/application.scss b/app/javascript/styles/application.scss index 5f898631..bfecc5a6 100644 --- a/app/javascript/styles/application.scss +++ b/app/javascript/styles/application.scss @@ -24,6 +24,7 @@ $fa-font-path: '~@fortawesome/fontawesome-free/webfonts'; @import 'layout'; @import 'forms'; @import 'edit_teachers'; +@import 'merge_modal'; @import './sidebar.scss'; @import "./selectize.scss"; diff --git a/app/javascript/styles/merge_modal.scss b/app/javascript/styles/merge_modal.scss new file mode 100644 index 00000000..56a3ad09 --- /dev/null +++ b/app/javascript/styles/merge_modal.scss @@ -0,0 +1,41 @@ +/* Modal styling */ +.merge_modal { + display: none; /* Hide the modal by default */ + position: fixed; + z-index: 1000; /* Ensure modal appears on top of other elements */ + left: 0; + top: 0; + width: 100%; + height: 100%; + overflow: auto; /* Enable scrolling if content exceeds the modal size */ + background-color: rgba(0, 0, 0, 0.4); /* Semi-transparent black background */ +} + +/* Modal content */ +.merge_modal-content { + background-color: #fefefe; + margin: 15% auto; /* Center modal vertically and horizontally */ + padding: 20px; /* You can increase the padding to add more hspace */ + border: 1px solid #888; + border-radius: 5px; + width: 95%; /* You can adjust the width of the modal content as needed */ + position: relative; /* Enable positioning of the close button */ +} + +/* Close button */ +.close { + position: absolute; /* Position the close button absolutely */ + top: 10px; /* Adjust this value to move the close button up or down */ + right: 10px; /* Position the close button on the right side of the modal content */ + color: #aaa; + font-size: 28px; + font-weight: bold; + cursor: pointer; +} + +.close:hover, +.close:focus { + color: black; + text-decoration: none; + cursor: pointer; +} \ No newline at end of file diff --git a/app/mailers/teacher_mailer.rb b/app/mailers/teacher_mailer.rb index a1bd199c..706a49d6 100644 --- a/app/mailers/teacher_mailer.rb +++ b/app/mailers/teacher_mailer.rb @@ -59,7 +59,6 @@ def liquid_assigns base_rules = { bjc_password: Rails.application.secrets[:bjc_password], piazza_password: Rails.application.secrets[:piazza_password], - # TODO: Review if below two are needed, or can they be refractored? denial_reason: @denial_reason, request_reason: @request_reason } diff --git a/app/models/school.rb b/app/models/school.rb index 0b47b6c1..eb1c282c 100644 --- a/app/models/school.rb +++ b/app/models/school.rb @@ -26,8 +26,7 @@ # class School < ApplicationRecord - VALID_STATES = [ "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 - + VALID_STATES = ISO3166::Country["US"].subdivisions.keys.freeze validates :name, :city, :website, :country, presence: true validates :country, inclusion: { in: ISO3166::Country.all.map(&:alpha2), message: "%{value} is not a valid country" } validates :state, presence: true, if: -> { country == "US" } @@ -61,7 +60,7 @@ def website end def location - "#{city}, #{state}" + "#{city}, #{state}, #{country}" end # TODO: Consider renaming this. @@ -78,11 +77,11 @@ def display_grade_level end def selectize_options - [name_location, to_json(only: [:id, :name, :city, :state, :website]) ] + [name_location, to_json(only: [:id, :name, :city, :state, :country, :website]) ] end def name_location - "#{name} (#{city}, #{state})" + "#{name} (#{city}, #{state}, #{country})" end def update_gps_data @@ -101,6 +100,18 @@ def maps_marker_data } end + def format_school(data) + name, city, state, country = data + country_str = country == "US" ? "" : ", #{country}" + "#{name} (#{city}, #{state}#{country_str})" + end + + def self.search_list + School.pluck(:name, :city, :state, :country).map do |data| + format_school(data) + end + end + def self.grade_level_options School.grade_levels.map { |key, _val| [key.to_s.titlecase, key] } end diff --git a/app/views/main/dashboard.html.erb b/app/views/main/dashboard.html.erb index bb83db7c..2242543f 100644 --- a/app/views/main/dashboard.html.erb +++ b/app/views/main/dashboard.html.erb @@ -4,14 +4,14 @@ - <%= render 'teachers/table_headers' %> + <%= render 'teachers/table_headers', include_id: false %> <% @unreviewed_teachers.each do |teacher| %> - <%= render 'teachers/teacher', teacher: teacher %> + <%= render 'teachers/teacher', teacher: teacher, merge_table: false %> +<% end %> diff --git a/app/views/teachers/_teacher.erb b/app/views/teachers/_teacher.erb index 1b926d81..788fc988 100644 --- a/app/views/teachers/_teacher.erb +++ b/app/views/teachers/_teacher.erb @@ -1,5 +1,12 @@ +<% if merge_table %> + +<% end %>
Actions
<%= button_to("✔️", validate_teacher_path(teacher.id), diff --git a/app/views/merge/merge.html.erb b/app/views/merge/merge.html.erb new file mode 100644 index 00000000..db971aaa --- /dev/null +++ b/app/views/merge/merge.html.erb @@ -0,0 +1,2 @@ +

Merge#merge

+

Find me in app/views/merge/merge.html.erb

diff --git a/app/views/merge/preview.html.erb b/app/views/merge/preview.html.erb new file mode 100644 index 00000000..f792464e --- /dev/null +++ b/app/views/merge/preview.html.erb @@ -0,0 +1,67 @@ + + + + Merge <%= @from_teacher.full_name %> into <%= @into_teacher.full_name %> + + + +
+
+ <%= content_tag :h1, "Preview Merge of #{@from_teacher.full_name} into #{@into_teacher.full_name}" %> +
+
+ <%= link_to "Switch Merge Order", preview_merge_path(from: @into_teacher.id, into: @from_teacher.id), method: :get, class: "btn btn-primary switch-merge-button btn-orange" %> +
+
+ <%= link_to "Confirm Merge", merge_path(from: @from_teacher.id, into: @into_teacher.id), method: :patch, class: "btn btn-primary confirm-merge-button btn-green" %> +
+
+ +
+ <%= tag.div class: 'teacher' do %> + <%= render 'teachers/teacher_info', teacher: @from_teacher, render_edit: true, render_smaller: true, render_delete: false, display_deletion_warning_icon: true, is_show_page: false %> + <% end %> + <%= tag.div class: 'teacher' do %> + <%= render 'teachers/teacher_info', teacher: @into_teacher, render_edit: true, render_smaller: true, render_delete: false, display_deletion_warning_icon: false, is_show_page: false %> + <% end %> + <%= tag.div class: 'teacher' do %> + <%= render 'teachers/teacher_info', teacher: @result_teacher, render_edit: false, render_smaller: true, render_delete: false, display_deletion_warning_icon: false, is_show_page: false %> + <% end %> +
+ + diff --git a/app/views/schools/_form.html.erb b/app/views/schools/_form.html.erb index 453b53e0..e6b87526 100644 --- a/app/views/schools/_form.html.erb +++ b/app/views/schools/_form.html.erb @@ -2,7 +2,7 @@
<%= f.label :name, "School Name", class: "label-required" %> - <%= f.text_field :name, placeholder: 'Name of School', class: 'form-control', + <%= f.text_field :name, placeholder: 'UC Berkeley', class: 'form-control', required: false, id: 'school_name' %>
diff --git a/app/views/teachers/_table_headers.erb b/app/views/teachers/_table_headers.erb index f519ec1e..13c40fa1 100644 --- a/app/views/teachers/_table_headers.erb +++ b/app/views/teachers/_table_headers.erb @@ -1,3 +1,6 @@ +<% if include_id %> +
IDName Email Education<%= teacher.id %> - <%= link_to(teacher.full_name, teacher_path(teacher)) %> + <% if merge_table %> + <%= link_to teacher.full_name, preview_merge_path(@teacher.id, teacher.id), method: :get %> + <% else %> + <%= link_to(teacher.full_name, teacher_path(teacher)) %> + <% end %> <%= link_to('(Web)', teacher.personal_website, target: '_blank') if teacher.personal_website.present? %> diff --git a/app/views/teachers/_teacher_info.html.erb b/app/views/teachers/_teacher_info.html.erb index fdc93a6b..4f492a5d 100644 --- a/app/views/teachers/_teacher_info.html.erb +++ b/app/views/teachers/_teacher_info.html.erb @@ -1,13 +1,35 @@
-

<%= teacher.full_name %>

+

<%= teacher.full_name %> + <% if display_deletion_warning_icon %> +

+ ❌ +

+ <% end %> +

- <%= link_to "Edit Information", edit_teacher_path(teacher), class: "btn btn-primary mr-2" %> - <%= link_to "Edit Personal Emails", edit_teacher_email_address_path(teacher), class: "btn btn-secondary mr-2" %> - <%= link_to("Delete", teacher_path(teacher), method: "delete", class: "btn btn-danger", data: {confirm: "Are you sure?"}) %> + <% if is_show_page && @is_admin %> + + <% end %> + <% if render_edit %> + <% button_class = render_smaller ? " btn-sm" : "" %> + <%= link_to "Edit Information", edit_teacher_path(teacher), class: "btn btn-primary mr-2" + button_class %> + <%= link_to "Edit Personal Emails", edit_teacher_email_address_path(teacher), class: "btn btn-secondary mr-2" + button_class %> + <% end %> + <% if render_delete %> + <%= link_to("Delete", teacher_path(teacher), method: "delete", class: "btn btn-danger", data: {confirm: "Are you sure?"}) %> + <% end %>
+ <% if @is_admin %> +
+
Teacher ID:
+
+ <%= teacher.id %> +
+
+ <% end %>
Email:
@@ -18,7 +40,7 @@
-
Personal Email:
+
Personal Emails:
<% teacher.personal_emails.each do |email| %>
@@ -80,7 +102,7 @@
School Location:
- <%= "#{teacher.school.location}, #{teacher.school.country}" %> + <%= "#{teacher.school.location}" %>
@@ -111,6 +133,7 @@
\ No newline at end of file diff --git a/config/routes.rb b/config/routes.rb index 6c68c98a..4c04a42c 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -30,4 +30,11 @@ get "/auth/failure", to: "sessions#omniauth_failure", as: "omniauth_failure" get "/dashboard", to: "main#dashboard", as: "dashboard" + + # routes for merging teachers (accessible to ADMIN ONLY) + # get 'merge/:id/:merge_id', to: 'merge#preview', as: 'preview_merge' + # patch 'merge/:id/:merge_id', to: 'merge#merge', as: 'merge' + + get "merge/:from/:into/preview", to: "merge#preview", as: "preview_merge" + patch "merge/:from/:into/execute", to: "merge#execute", as: "merge" end diff --git a/features/access.feature b/features/access.feature index 5d6df362..27fda4ba 100644 --- a/features/access.feature +++ b/features/access.feature @@ -66,6 +66,14 @@ Scenario: Other user's show page as a Teacher Given I am on the show page for Alice Admin Then I should be on the edit page for Todd Teacher +Scenario: Try to access merge page as a registered teacher + Given I have a teacher Google email + Given I am on the BJC home page + And I follow "Log In" + Then I can log in with Google + When I go to the merge preview page for Todd into Alice + Then I should see "Only admins can access this page" + #New User Scenario: Schools page as a new user Given I am on the schools page diff --git a/features/admin.feature b/features/admin.feature index 65780ce3..0322def0 100644 --- a/features/admin.feature +++ b/features/admin.feature @@ -362,7 +362,6 @@ Feature: basic admin functionality And I press "Submit" 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 | @@ -380,6 +379,82 @@ Feature: basic admin functionality And I press "Update" Then I should be on the edit page for Jane Doe + Scenario: Admin can switch merge order + 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 | primary_email | primary_email | school | + | Jane | Doe | false | janedoe@berkeley.edu | jd@berkeley.edu | UC Berkeley | + | Bobby | John | false | bobbyjohn@berkeley.edu | bj@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 + And I go to the merge preview page for Jane into Bobby + Then I should see "Preview Merge of Jane Doe into Bobby John" + When I follow "Switch Merge Order" + Then I should see "Preview Merge of Bobby John into Jane Doe" + + Scenario: Merging teachers only updates blank fields with those of teacher being merged + 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 | personal_website | admin | primary_email | school | application_status | + | Jane | Doe | abc@berkeley.edu | false | janedoe@berkeley.edu | UC Berkeley | validated | + | Bobby | John | | false | bobbyjohn@berkeley.edu | UC Berkeley | 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 merge preview page for Jane into Bobby + And I follow "Confirm Merge" + Then I see a confirmation "Teachers merged successfully" + And the following entries should not exist in the teachers database: + | first_name | last_name | personal_website | admin | primary_email | school | application_status | + | Jane | Doe | https://abc.berkeley.edu | false | janedoe@berkeley.edu | UC Berkeley | validated | + | Bobby | John | | false | bobbyjohn@berkeley.edu | UC Berkeley | denied | + And the following entries should exist in the teachers database: + | first_name | last_name | personal_website | admin | primary_email | school | application_status | + | Bobby | John | abc@berkeley.edu | false | bobbyjohn@berkeley.edu | UC Berkeley | denied | + + Scenario: Merging teachers sums session counts, concatenates IP histories, and saves most recent datetime + 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 | session_count | ip_history | last_session_at | admin | primary_email | school | + | Jane | Doe | 169 | 1.2.3.4, 4.5.6.7 | 2023-04-10 12:30:00 | false | janedoe@berkeley.edu | UC Berkeley | + | Bobby | John | 365 | 4.5.6.7, 7.8.9.10 | 2023-01-11 12:00:00 | false | bobbyjohn@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 merge preview page for Jane into Bobby + And I follow "Confirm Merge" + Then I see a confirmation "Teachers merged successfully" + And the following entries should exist in the teachers database: + | first_name | last_name | session_count | ip_history | last_session_at | admin | primary_email | school | + | Bobby | John | 534 | 1.2.3.4, 4.5.6.7, 7.8.9.10 | 2023-04-10 12:30:00 | false | bobbyjohn@berkeley.edu | UC Berkeley | + + Scenario: Admin can access merge page from teacher show page + 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 | primary_email | school | + | Jane | Doe | false | janedoe@berkeley.edu | UC Berkeley | + | Bobby | John | false | bobbyjohn@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 show page for Bobby John + And I press "Merge" + Then I should see "Choose A User To Merge Into" + When I follow the first "Jane Doe" link + Then I should be on the merge preview page for Bobby into Jane # Scenario: Admin can import csv file. The loader should filter invalid record and create associate school. # Given the following schools exist: diff --git a/features/email_template.feature b/features/email_template.feature index 0d561f1a..cbce2267 100644 --- a/features/email_template.feature +++ b/features/email_template.feature @@ -63,3 +63,11 @@ Scenario: Editing email template to have blank body or to field displays flash e When I fill in TinyMCE email form with "" And I press "Submit" Then I should see "An error occurred: Body can't be blank" + +Scenario: Updating email template with valid parameters succeeds + Given I am on the email templates index + And I follow "Welcome Email" + And I fill in "email_template_to" with "abc@berkeley.edu" + And I fill in TinyMCE email form with "ABC" + And I press "Submit" + Then I should see "Updated Welcome Email template successfully" diff --git a/features/step_definitions/teacher_steps.rb b/features/step_definitions/teacher_steps.rb index b67f6529..ed960e26 100644 --- a/features/step_definitions/teacher_steps.rb +++ b/features/step_definitions/teacher_steps.rb @@ -48,17 +48,19 @@ snap: "", status: "Other - Please specify below.", education_level: 1, - more_info: "I'm teaching a college course", + more_info: "default more_info", admin: false, - personal_website: "https://snap.berkeley.edu", + personal_website: "", application_status: "Not Reviewed", languages: ["English"], - + session_count: 1, + last_session_at: DateTime.now, + ip_history: [IPAddr.new("1.2.3.4")], # 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", + primary_email: "alonzo@snap.berkeley.edu" } teachers_table.symbolic_hashes.each do |teacher| @@ -67,6 +69,16 @@ if key == :languages languages = if teacher[key].present? then YAML.safe_load(teacher[key]) else nil end teacher[key] = languages.presence || value + elsif key == :last_session_at + # conversion to DateTime object needed + teacher[key] = if teacher[key].present? then DateTime.parse(teacher[key]) else value end + elsif key == :session_count + # type cast from string --> int needed + teacher[key] = (teacher[key].presence || value).to_i + elsif key == :ip_history + # type cast from comma-separated list of ip addresses to array of IPAddr objects + ip_addr_array = if teacher[key].present? then teacher[key].split(/\s*,\s*/).map { |ip_str| IPAddr.new(ip_str.strip) } else nil end + teacher[key] = ip_addr_array.presence || value else # Handle other fields as usual teacher[key] = teacher[key].presence || value @@ -83,3 +95,31 @@ School.reset_counters(school.id, :teachers) end end + +Then(/the following entries should exist in the teachers database/) do |entries_table| + entries_table.symbolic_hashes.each do |params| + keys_to_exclude = [:school, :session_count, :last_session_at, :ip_history, :primary_email] + # teacher should be present and school should be valid + teacher = Teacher.find_by(params.except(*keys_to_exclude)) + expect(!teacher.blank?).to be true + expect(!(School.find_by(name: params[:school]).blank?)).to be true + if params[:session_count].present? + expect(teacher.session_count).to eq(params[:session_count].to_i) + end + if params[:last_session_at].present? + expect(teacher.last_session_at).to eq(DateTime.parse(params[:last_session_at])) + end + if params[:ip_history].present? + ip_addr_array = params[:ip_history].split(/\s*,\s*/).map { |ip_str| IPAddr.new(ip_str.strip) } + expect(teacher.ip_history.all? { |addr| ip_addr_array.include?(addr) }).to be true + end + end +end + +Then(/the following entries should not exist in the teachers database/) do |entries_table| + entries_table.symbolic_hashes.each do |teacher_params| + teacher_params.except!(:school, :primary_email) + # matches on all fields except school name + expect(Teacher.find_by(teacher_params).blank?).to be true + end +end diff --git a/features/support/paths.rb b/features/support/paths.rb index ddb14c44..29b566b0 100644 --- a/features/support/paths.rb +++ b/features/support/paths.rb @@ -21,6 +21,8 @@ def path_to(page_name) when /^the teachers page$/ then teachers_path when /^the edit page for (.*) (.*)$/ edit_teacher_path(Teacher.find_by(first_name: $1, last_name: $2)) + when /^the merge preview page for (.*) into (.*)$/ + preview_merge_path(Teacher.find_by(first_name: $1), Teacher.find_by(first_name: $2)) when /^the show page for (.*) (.*)$/ teacher_path(Teacher.find_by(first_name: $1, last_name: $2)) when /^the schools page$/ then schools_path diff --git a/yarn.lock b/yarn.lock index 218f5baa..28d6981d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1773,13 +1773,13 @@ bn.js@^5.0.0, bn.js@^5.2.1: resolved "https://registry.yarnpkg.com/bn.js/-/bn.js-5.2.1.tgz#0bc527a6a0d18d0aa8d5b0538ce4a77dccfa7b70" integrity sha512-eXRvHzWyYPBuB4NBy0cmYQjGitUrtqwbvlzP3G6VFnNRbsZQIxQ10PbKKHt8gZ/HW/D/747aDl+QkDqg3KQLMQ== -body-parser@1.20.1: - version "1.20.1" - resolved "https://registry.yarnpkg.com/body-parser/-/body-parser-1.20.1.tgz#b1812a8912c195cd371a3ee5e66faa2338a5c668" - integrity sha512-jWi7abTbYwajOytWCQc37VulmWiRae5RyTpaCyDcS5/lMdtwSz5lOpDE67srw/HYe35f1z3fDQw+3txg7gNtWw== +body-parser@1.20.2: + version "1.20.2" + resolved "https://registry.yarnpkg.com/body-parser/-/body-parser-1.20.2.tgz#6feb0e21c4724d06de7ff38da36dad4f57a747fd" + integrity sha512-ml9pReCu3M61kGlqoTm2umSXTlRTuGTx0bfYj+uIUKKYycG5NtSbeetV3faSU6R7ajOPw0g/J1PvK4qNy7s5bA== dependencies: bytes "3.1.2" - content-type "~1.0.4" + content-type "~1.0.5" debug "2.6.9" depd "2.0.0" destroy "1.2.0" @@ -1787,7 +1787,7 @@ body-parser@1.20.1: iconv-lite "0.4.24" on-finished "2.4.1" qs "6.11.0" - raw-body "2.5.1" + raw-body "2.5.2" type-is "~1.6.18" unpipe "1.0.0" @@ -2335,7 +2335,7 @@ content-disposition@0.5.4: dependencies: safe-buffer "5.2.1" -content-type@~1.0.4: +content-type@~1.0.4, content-type@~1.0.5: version "1.0.5" resolved "https://registry.yarnpkg.com/content-type/-/content-type-1.0.5.tgz#8b773162656d1d1086784c8f23a54ce6d73d7918" integrity sha512-nTjqfcBFEipKdXCv4YDQWCfmcLZKm81ldF0pAopTvyrFGVbcR6P/VAAd5G7N+0tTr8QqiU0tFadD6FK4NtJwOA== @@ -2350,10 +2350,10 @@ cookie-signature@1.0.6: resolved "https://registry.yarnpkg.com/cookie-signature/-/cookie-signature-1.0.6.tgz#e303a882b342cc3ee8ca513a79999734dab3ae2c" integrity sha512-QADzlaHc8icV8I7vbaJXJwod9HWYp8uCqf1xa4OfNu1T7JVxQIrUgOWtHdNDtPiywmFbiS12VjotIXLrKM3orQ== -cookie@0.5.0: - version "0.5.0" - resolved "https://registry.yarnpkg.com/cookie/-/cookie-0.5.0.tgz#d1f5d71adec6558c58f389987c366aa47e994f8b" - integrity sha512-YZ3GUyn/o8gfKJlnlX7g7xq4gyO6OSuhGPKaaGssGB2qgDUS0gPgtTvoyZLTt9Ab6dC4hfc9dV5arkvc/OCmrw== +cookie@0.6.0: + version "0.6.0" + resolved "https://registry.yarnpkg.com/cookie/-/cookie-0.6.0.tgz#2798b04b071b0ecbff0dbb62a505a8efa4e19051" + integrity sha512-U71cyTamuh1CRNCfpGY6to28lxvNwPG4Guz/EVjgf3Jmzv0vlDp1atT9eS5dDjMYHucpHbWns6Lwf3BKz6svdw== copy-concurrently@^1.0.0: version "1.0.5" @@ -3113,16 +3113,16 @@ expand-tilde@^2.0.0, expand-tilde@^2.0.2: homedir-polyfill "^1.0.1" express@^4.17.3: - version "4.18.2" - resolved "https://registry.yarnpkg.com/express/-/express-4.18.2.tgz#3fabe08296e930c796c19e3c516979386ba9fd59" - integrity sha512-5/PsL6iGPdfQ/lKM1UuielYgv3BUoJfz1aUwU9vHZ+J7gyvwdQXFEBIEIaxeGf0GIcreATNyBExtalisDbuMqQ== + version "4.19.2" + resolved "https://registry.yarnpkg.com/express/-/express-4.19.2.tgz#e25437827a3aa7f2a827bc8171bbbb664a356465" + integrity sha512-5T6nhjsT+EOMzuck8JjBHARTHfMht0POzlA60WV2pMD3gyXw2LZnZ+ueGdNxG+0calOJcWKbpFcuzLZ91YWq9Q== dependencies: accepts "~1.3.8" array-flatten "1.1.1" - body-parser "1.20.1" + body-parser "1.20.2" content-disposition "0.5.4" content-type "~1.0.4" - cookie "0.5.0" + cookie "0.6.0" cookie-signature "1.0.6" debug "2.6.9" depd "2.0.0" @@ -3305,9 +3305,9 @@ flush-write-stream@^1.0.0: readable-stream "^2.3.6" follow-redirects@^1.0.0: - version "1.15.4" - resolved "https://registry.yarnpkg.com/follow-redirects/-/follow-redirects-1.15.4.tgz#cdc7d308bf6493126b17ea2191ea0ccf3e535adf" - integrity sha512-Cr4D/5wlrb0z9dgERpUL3LrmPKVDsETIJhaCMeDfuFYcqa5bldGV6wBsAN6X/vxlXQtFBMrXdXxdL8CbDTGniw== + version "1.15.6" + resolved "https://registry.yarnpkg.com/follow-redirects/-/follow-redirects-1.15.6.tgz#7f815c0cda4249c74ff09e95ef97c23b5fd0399b" + integrity sha512-wWN62YITEaOpSK584EZXJafH1AGpO8RVgElfkuXbTOrPX4fIfOyEpW/CsiNd8JdYrAoOvafRTOEnvsO++qCqFA== for-in@^1.0.2: version "1.0.2" @@ -5919,10 +5919,10 @@ range-parser@^1.2.1, range-parser@~1.2.1: resolved "https://registry.yarnpkg.com/range-parser/-/range-parser-1.2.1.tgz#3cf37023d199e1c24d1a55b84800c2f3e6468031" integrity sha512-Hrgsx+orqoygnmhFbKaHE6c296J+HTAQXoxEF6gNupROmmGJRoyzfG3ccAveqCBrwr/2yxQ5BVd/GTl5agOwSg== -raw-body@2.5.1: - version "2.5.1" - resolved "https://registry.yarnpkg.com/raw-body/-/raw-body-2.5.1.tgz#fe1b1628b181b700215e5fd42389f98b71392857" - integrity sha512-qqJBtEyVgS0ZmPGdCFPWJ3FreoqvG4MVQln/kCgF7Olq95IbOp0/BWyMwbdtn4VTvkM8Y7khCQ2Xgk/tcrCXig== +raw-body@2.5.2: + version "2.5.2" + resolved "https://registry.yarnpkg.com/raw-body/-/raw-body-2.5.2.tgz#99febd83b90e08975087e8f1f9419a149366b68a" + integrity sha512-8zGqypfENjCIqGhgXToC8aB2r7YrBX+AQAfIPs/Mlk+BtPTztOvTS01NRW/3Eh60J+a48lt8qsCzirQ6loCVfA== dependencies: bytes "3.1.2" http-errors "2.0.0" @@ -7173,9 +7173,9 @@ webpack-cli@^3.3.12: yargs "^13.3.2" webpack-dev-middleware@^5.3.1: - version "5.3.3" - resolved "https://registry.yarnpkg.com/webpack-dev-middleware/-/webpack-dev-middleware-5.3.3.tgz#efae67c2793908e7311f1d9b06f2a08dcc97e51f" - integrity sha512-hj5CYrY0bZLB+eTO+x/j67Pkrquiy7kWepMHmUMoPsmcUaeEnQJqFzHJOyxgWlq746/wUuA64p9ta34Kyb01pA== + version "5.3.4" + resolved "https://registry.yarnpkg.com/webpack-dev-middleware/-/webpack-dev-middleware-5.3.4.tgz#eb7b39281cbce10e104eb2b8bf2b63fce49a3517" + integrity sha512-BVdTqhhs+0IfoeAf7EoH5WE+exCmqGerHfDM0IL096Px60Tq2Mn9MAbnaGUe6HiMa41KMCYF19gyzZmBcq/o4Q== dependencies: colorette "^2.0.10" memfs "^3.4.3"