Iteration 4 - Add Course Filter#415
Draft
kienthuynh wants to merge 69 commits intobeautyjoy:mainfrom
Draft
Conversation
Iteration 1 - Implemented Verification Notes as an optional teacher field end-to-end
Iteration 3 - Redirect admin to intended page after login
Iteration 1 - Reapply "Implement school merging"
Iteration 2 - add school location to teacher view
- Add mailbluster_id and mailbluster_synced_at columns to teachers table - Add unique index on teachers.mailbluster_id - Add emails_sent, emails_delivered, bounced columns to email_addresses table - Email delivery tracking on email_addresses (not teachers) per architecture decision
- Create app/services/mailbluster_service.rb - Implement create_or_update_lead, read_lead, delete_lead methods - Implement sync_all_teachers for bulk synchronization - Use HTTParty for API calls (already in Gemfile) - MailBluster API uses MD5 hash of email as lead identifier - Add US state timezone mapping for lead timezone - Store mailbluster_id on teacher after successful sync - Include teacher school data and tags in lead payload
- Create config/initializers/mailbluster.rb - Log warning if MAILBLUSTER_API_KEY is not set (except in test env) - Document how to configure the API key
- When admin validates a teacher, sync them to MailBluster as a lead - Only runs if MAILBLUSTER_API_KEY is configured - Creates or updates the lead with teacher info, school data, and tags
- After creating a new email address, sync teacher to MailBluster - Only syncs if API key is configured AND teacher is validated - Keeps MailBluster lead data up-to-date with new email additions
- POST /teachers/:id/sync_mailbluster (individual teacher sync) - POST /teachers/sync_all_mailbluster (bulk sync all teachers)
- sync_mailbluster: sync individual teacher to MailBluster (admin only) - sync_all_mailbluster: bulk sync all validated teachers (admin only) - Both check if API key is configured before proceeding - Flash messages report sync results (synced/failed/skipped counts) - First 5 errors shown in alert flash for debugging
- Button at bottom of teachers page triggers bulk sync of all validated teachers - Confirmation dialog prevents accidental bulk operations - Uses warning (yellow) style to indicate impactful action
- Display MailBluster ID with link to MailBluster profile - Show last synced timestamp - Show per-email delivery stats (sent, delivered, bounced) - Individual 'Sync to MailBluster' button for admin users - Only visible to admin users
- Add mailbluster_synced?, mailbluster_profile_url helper methods - Add primary_email_sent, primary_email_delivered, primary_email_bounced - Update csv_export to include mailbluster_id and delivery stats columns - Eager load email_addresses in CSV export for performance - Delivery data columns ready for future AWS bounce tracking integration
- Add bounced scope to find all bounced email addresses - Add with_undelivered scope to find emails with delivery gaps - Add undelivered_count and has_undelivered? instance methods - These prepare the model for future AWS bounce data integration
- email_address_label now shows 'bounced' badge for bounced emails - Add mailbluster_sync_status helper for displaying sync state - Refactor email_address_label to support multiple badges
- Add 'MB Sync' column header to table_headers partial - Display sync status badge (Synced/Not Synced) in teacher row partial - Visible on both teachers index and merge modal tables
- Test create_or_update_lead with success and failure scenarios - Test read_lead by email with MD5 hash lookup - Test delete_lead - Test sync_teacher (individual sync) - Test sync_all_teachers (bulk sync with synced/failed/skipped counts) - Test configured? method - Test payload contents (name, email, tags, subscribed, overrideExisting) - Test error handling for API failures and exceptions - Mock HTTParty responses to avoid real API calls
- Test sync_mailbluster: success, failure, and unconfigured API - Test sync_all_mailbluster: results display, errors, unconfigured API - Test validate action triggers MailBluster sync - Fix: exclude sync_all_mailbluster from load_teacher before_action - All 37 controller tests pass
- Teacher model tests: mailbluster_synced?, profile_url, email stats helpers - Teacher CSV export tests: verify new mailbluster and delivery columns - EmailAddress model tests: undelivered_count, has_undelivered?, scopes - EmailAddress tests: bounced scope, with_undelivered scope, default values - All 46 model tests pass
- Test sync triggers when adding email to validated teacher - Test sync skipped for non-validated teachers - Test sync skipped when API key is not configured - All 6 tests pass
- Test admin sees MailBluster sync info on teacher show page - Test admin sees 'Sync All to MailBluster' button on index - Test MB Sync column visible in teachers table
…ions - Use unique email addresses (mb_validated@, mb_pending@) to avoid collision with spec/fixtures/email_addresses.yml entries - Use 'should see a button named' instead of 'should see' for button_to elements whose value attribute isn't in Capybara's text content - All 3 scenarios now pass
- Add scenario for email delivery stats on teacher show page - Add scenario for MailBluster section on pending teacher page - Add scenario verifying non-admin cannot see sync controls - Total: 6 Cucumber scenarios covering MailBluster UI elements
- mailbluster:sync_all - Sync all validated teachers to MailBluster - mailbluster:sync_teacher[id] - Sync a single teacher by ID - mailbluster:status - Show sync status summary and API config
- Return structured hash with :success and :error keys - Rescue StandardError for network/unexpected errors - Display error details in flash message on sync failure - Add spec for unexpected error handling (connection timeout)
- Sync on update action when status changes to/from validated - Handle both validation and de-validation transitions - Add controller specs for auto-sync on status change
- Auto-delete lead from MailBluster before teacher destruction - Add spec for MailBluster lead deletion on destroy
Fix personal email deletion confirm and last-email guard
Custom CSS-only approach using ::after pseudo-elements with Unicode triangles. White on dark headers, half-opacity unsorted indicator, full-opacity for active sort direction. No arrow on non-orderable columns (sorting_disabled). Applies globally to all DataTables.
Sortable DataTable column headers had no visual indication of sort direction or clickability.
- Add ajax-datatables-rails gem - Create SchoolDatatable class with view_columns and data methods - Update SchoolsController#index to respond to JSON format - Add request spec verifying DataTables JSON structure
- Add datatable_params helper for realistic DataTables request params - Verify recordsTotal/recordsFiltered match school count - Verify each record returns name, location, country, website, teachers_count, grade_level, and DT_RowId
Override filter_records in SchoolDatatable to include state in global search (not covered by view_columns since Location is a composite display column).
- Replace server-rendered rows with empty tbody for AJAX population - Add id="schools-table" and data-source attribute for JS init - Remove js-dataTable class so shared init doesn't interfere - Remove @Schools assignment from HTML path (data comes via JSON) - Update index spec to match new behavior
- Add schools_index.js with serverSide DataTables init on #schools-table - Bind to both document.ready and turbolinks:load - Return HTML links and action buttons from SchoolDatatable#data - Mark HTML output as html_safe to prevent double-escaping - Add Cucumber scenarios for page load and search behavior - Update RSpec assertions for HTML-wrapped field values
|
looks good to me |
Feature/mailbluster
…ools Implement server-side DataTables for Schools index
Iteration 3 - When Searching w/ a Filter, UI should Indicate Hidden Results
Iteration 4 - Update Email Tags
Added a contributions section with a link to Hagen Haeussler's LinkedIn profile.
…Template View teacher url to the form submission email and RSPEC
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Important
This feature is built upon the hidden-results feature (#412), so it should be merged in only AFTER the above PR is merged.
Only the last commit in this PR relate to the course filter feature if you want to preview changes.
Summary
CSP: csp_teacher, mixed_class, teals_volunteer, teals_teacher, excite
Sparks: middle_school_bjc
Other: Any other status
Implementation Notes
Testing
@cycomachead @armandofox @mdawn65