Skip to content

Commit

Permalink
merge main / PR fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
cycomachead committed May 11, 2024
2 parents 4272ddc + e0df660 commit 716296c
Show file tree
Hide file tree
Showing 17 changed files with 91 additions and 85 deletions.
3 changes: 1 addition & 2 deletions app/javascript/packs/datatables.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +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.
return enabled.length === 0 || enabled.includes(searchData[7]);
return enabled.length === 0 || enabled.includes(searchData[5]);
});

let $tables = $('.js-dataTable').DataTable({
Expand All @@ -23,7 +23,6 @@ $(function() {

$tables.draw();
$(".custom-checkbox").on("change", () => {
console.log("custom-checkbox change");
$tables.draw();
});
$tables.on('draw', function() {
Expand Down
7 changes: 1 addition & 6 deletions app/models/pd_registration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,6 @@ 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
self.teacher.full_name
end
end
5 changes: 1 addition & 4 deletions app/models/professional_development.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
7 changes: 1 addition & 6 deletions app/models/teacher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -223,8 +223,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)
Expand Down Expand Up @@ -297,8 +296,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

Expand All @@ -308,8 +305,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
2 changes: 1 addition & 1 deletion app/views/professional_developments/_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@

<div class='col-4' id="state_select_container">
<%= 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' } %>
</div>

<div class='col-4' id="state_textfield_container">
Expand Down
14 changes: 5 additions & 9 deletions app/views/professional_developments/edit.html.erb
Original file line number Diff line number Diff line change
@@ -1,12 +1,8 @@
<%= provide(:h1, "Update #{@professional_development.name}") %>

<% if @professional_development.nil? %>
<p>Professional Development not found.</p>
<% else %>
<%= form_for @professional_development do |f| %>
<%= render 'professional_developments/form', f: f, professional_development: @professional_development %>
<div>
<%= f.submit 'Submit', {class: 'btn btn-primary', id: 'submit_button'} %>
</div>
<% end %>
<%= form_for @professional_development do |f| %>
<%= render 'professional_developments/form', f: f, professional_development: @professional_development %>
<div>
<%= f.submit 'Submit', {class: 'btn btn-primary', id: 'submit_button'} %>
</div>
<% end %>
4 changes: 1 addition & 3 deletions app/views/professional_developments/index.html.erb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<%= provide(:title, "BJC Schools") %>
<%= provide(:title, "BJC PD") %>
<%= provide(:header_button, "New Professional Development") %>

<table class="table table-striped js-dataTable">
Expand All @@ -9,8 +9,6 @@
<th scope="col">Start Date</th>
<th scope="col">End Date</th>
<th scope="col">Grade Level</th>
<!-- skip this for now-->
<!-- <th scope="col">Registration Open</th>-->
<th scope="col">Actions</th>
</tr>
</thead>
Expand Down
1 change: 0 additions & 1 deletion app/views/professional_developments/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
Expand Down
2 changes: 1 addition & 1 deletion app/views/schools/_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@

} else {
stateTextfieldContainer.show();
stateSelect.removeAttr('required') //else make state select not required
stateSelect.removeAttr('required');
stateSelectContainer.hide();


Expand Down
31 changes: 15 additions & 16 deletions app/views/teachers/_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ status ONLY IF the person viewing this page is an admin. %>
<%= label_tag :request_reason, "Request Reason: ", id: 'request_or_deny_reason_text', class: "label-optional" %>
<%= text_field_tag :request_reason, nil, class: 'form-control' %>
</div>
</div>
</div>
</div>
<% end %>

Expand Down Expand Up @@ -80,7 +80,6 @@ status ONLY IF the person viewing this page is an admin. %>
</div>
</div>

<%# For now... only admins can enter/edit personal emails. %>

<div class='form-group row'>
<div class='col-12'>
Expand All @@ -103,7 +102,7 @@ status ONLY IF the person viewing this page is an admin. %>
</div>
</div>
<% else %>
<!-- Note that edit teacher page CANNOT allow direct adding/removing
<!-- Note that edit teacher page CANNOT allow direct adding/removing
of files because the nested forms result in undefined
behavior in Rails, including random controller
actions getting called from unrelated submit buttons -->
Expand All @@ -116,7 +115,7 @@ status ONLY IF the person viewing this page is an admin. %>
<%= f.label :more_files, "Upload More Files: " %>
<%= f.file_field :more_files, multiple: true, direct_upload: true %>
</div>
</div>
</div>
<% end %>
<div class='form-group row'>
<div class='col-12'>
Expand All @@ -132,15 +131,15 @@ status ONLY IF the person viewing this page is an admin. %>

<div class='form-group row'>
<div class='col-12'>
<%= f.label :languages, "What language(s) are spoken in the classroom?",
<%= 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
) %>
</div>
</div>
Expand All @@ -164,7 +163,7 @@ status ONLY IF the person viewing this page is an admin. %>

<%= f.submit (f.object.new_record? ? 'Submit' : 'Update'), class: 'btn btn-primary' %>
<!-- This apparent Rails bug of requiring two ends after a submit is likely related
to the use of the nested forms (here, the school form inside the teacher form)
to the use of the nested forms (here, the school form inside the teacher form)
which can result in inexplicable, and sometimes undefined behavior -->
<% end %>
<% end %>
Expand All @@ -177,14 +176,14 @@ which can result in inexplicable, and sometimes undefined behavior -->
persist: false,
create: false
});
//on loading the page, immediately update the reason field depending on
//on loading the page, immediately update the reason field depending on
//the value of the application status and check teacher status option
updateReasonField();
listenForStatusOptionChange();
});

//JS to update the text description for the denial/request reason input
//field and hide/display relevant linked fields depending on what is
//field and hide/display relevant linked fields depending on what is
//selected in the application status dropdown
function updateReasonField() {
$("#request_or_deny_reason").show();
Expand All @@ -198,7 +197,7 @@ which can result in inexplicable, and sometimes undefined behavior -->
$( "#request_or_deny_reason_text").text("Info Request Reason: ");
break;
case "Validated":
//show the skip email notification but not
//show the skip email notification but not
//the deny/request more info reason box
$("#request_or_deny_reason").hide();
break;
Expand All @@ -224,6 +223,6 @@ which can result in inexplicable, and sometimes undefined behavior -->
} else {
$('#upload_file_field').hide();
}
}
}
}
</script>
3 changes: 1 addition & 2 deletions app/views/teachers/_table_headers.erb
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@
<% end %>
<th scope="col">Name</th>
<th scope="col">Email</th>
<th scope="col">Education</th>
<th scope="col">Status</th>
<th scope="col">Role</th>
<th scope="col" width="15ch">Snap<i>!</i></th>
<th scope="col">School</th>
<th scope="col" data-col="status" width="7ch">Approved?</th>
Expand Down
11 changes: 7 additions & 4 deletions app/views/teachers/_teacher.erb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
<% end %>
<% end %>
</td>
<td><%= teacher.display_education_level %></td>
<td
<%- if teacher.display_status.length > 25 %>
data-toggle="tooltip"
Expand All @@ -35,10 +34,14 @@
<td>
<%= link_to(teacher.school.name, school_path(teacher.school)) %>
<%= teacher.school.location %>
<br><%= teacher.school.display_grade_level %>
</td>
<td data-search="<%= teacher.application_status %>" data-col="status" >
<%= teacher.short_application_status %>
</td>
<td
data-search="<%= teacher.display_application_status %>" data-col="status"
data-toggle="tooltip"
data-placement="bottom"
title="<%= teacher.display_application_status %>"
><%= teacher.short_application_status %></td>
<td data-sort="<%= teacher.created_at %>">
<%= teacher.created_at.strftime("%m/%d/%Y") %>
</td>
19 changes: 5 additions & 14 deletions app/views/teachers/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -6,22 +6,13 @@
<a href="/teachers.csv" class="btn btn-primary">Export All</a>
</div>

<% Teacher.application_statuses.each do |key, value| %>
<div class="custom-control custom-checkbox custom-control-inline">
<input class="custom-control-input" type="checkbox" name="statusFilter" id="Validated" value="validated" checked=true>
<label class="custom-control-label" for="Validated">Validated</label>
</div>
<div class="custom-control custom-checkbox custom-control-inline">
<input class="custom-control-input" type="checkbox" name="statusFilter" id="Denied" value="denied">
<label class="custom-control-label" for="Denied">Denied</label>
</div>
<div class="custom-control custom-checkbox custom-control-inline">
<input class="custom-control-input" type="checkbox" name="statusFilter" id="NotReviewed" value="not_reviewed">
<label class="custom-control-label" for="NotReviewed">Not Reviewed</label>
</div>
<div class="custom-control custom-checkbox custom-control-inline">
<input class="custom-control-input" type="checkbox" name="statusFilter" id="InfoNeeded" value="info_needed">
<label class="custom-control-label" for="InfoNeeded">Info Needed</label>
<input class="custom-control-input" type="checkbox" name="statusFilter" id="<%= key %>" value="<%= value %>" <%= key == 'validated' ? 'checked=true' : '' %>>

<label class="custom-control-label" for="<%= key %>"><%= value %></label>
</div>
<% end %>

<table class="table table-striped js-dataTable js-teachersTable">
<thead class="thead-dark">
Expand Down
2 changes: 1 addition & 1 deletion db/seed_data.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 2 additions & 8 deletions db/seeds.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,24 +11,18 @@

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}"
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
Expand Down
14 changes: 7 additions & 7 deletions features/data_tables.feature
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +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 |
| 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"
Expand Down
Loading

0 comments on commit 716296c

Please sign in to comment.