Sync Schools and Roles to Salesforce on Create and Update#677
Sync Schools and Roles to Salesforce on Create and Update#677
Conversation
d6ab230 to
4837a1c
Compare
2e6f173 to
fb20637
Compare
Test coverage90.14% line coverage reported by SimpleCov. |
e084550 to
8241770
Compare
There was a problem hiding this comment.
Pull request overview
Adds Salesforce (Heroku Connect) synchronization for Schools and Roles by introducing a dedicated salesforce_connect database connection, Salesforce-backed ActiveRecord models, and GoodJob jobs/queues to sync on create/update (plus backfill rake tasks).
Changes:
- Add multi-database config (
salesforce_connect) and local/CI Docker services to run Heroku Connect Postgres. - Add Salesforce AR models (
Salesforce::Base,Salesforce::School/Role/Contact) and sync jobs (SalesforceSyncJob+ per-model jobs). - Enqueue sync jobs from
School/Rolelifecycle callbacks and add specs/factories/rake tasks to validate & backfill.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/models/school_spec.rb | Adds expectations that Salesforce sync jobs are enqueued on School create/update. |
| spec/models/role_spec.rb | Adds expectations that Salesforce sync job is enqueued on Role create/update. |
| spec/jobs/salesforce/school_sync_job_spec.rb | Verifies SchoolSyncJob field mappings, truncation, and feature-flag discard behavior. |
| spec/jobs/salesforce/role_sync_job_spec.rb | Verifies RoleSyncJob field mappings, student skip behavior, and feature-flag discard behavior. |
| spec/jobs/salesforce/contact_sync_job_spec.rb | Verifies ContactSyncJob updates the opt-in field and retries when contact missing. |
| spec/factories/salesforce/school.rb | Factory for Salesforce school records. |
| spec/factories/salesforce/role.rb | Factory for Salesforce role records. |
| spec/factories/salesforce/contact.rb | Factory for Salesforce contact records. |
| lib/tasks/salesforce_sync.rake | Adds rake tasks to enqueue backfill sync jobs for all Schools/Roles/Contacts. |
| docker-compose.yml | Adds salesforce_connect service and wires api dependency/env for local dev. |
| config/initializers/good_job.rb | Adds a dedicated salesforce_sync queue with concurrency. |
| config/database.yml | Adds salesforce_connect DB configuration for all envs (tasks disabled). |
| app/models/school.rb | Enqueues Salesforce School + Contact sync jobs on create/update commit. |
| app/models/role.rb | Enqueues Salesforce Role sync job on create/update commit. |
| app/models/salesforce/base.rb | Establishes Salesforce model base class connected to salesforce_connect. |
| app/models/salesforce/school.rb | Maps Salesforce school table and primary key. |
| app/models/salesforce/role.rb | Maps Salesforce role/affiliation table and primary key. |
| app/models/salesforce/contact.rb | Maps Salesforce contact table and primary key. |
| app/jobs/salesforce/salesforce_sync_job.rb | Adds shared job behaviors: concurrency keying, retries, discard flag, truncation helper. |
| app/jobs/salesforce/school_sync_job.rb | Implements School-to-Salesforce attribute mapping and save. |
| app/jobs/salesforce/role_sync_job.rb | Implements Role-to-Salesforce attribute mapping (skips student roles) and save. |
| app/jobs/salesforce/contact_sync_job.rb | Syncs school creator UX-contact opt-in to Salesforce contact. |
| .github/workflows/ci.yml | Adds GHCR permissions and Heroku Connect service container for tests. |
| .env.example | Documents Salesforce connect env vars for local development. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
239ff00 to
63a7445
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
f5813b2 to
7afc8bb
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
f1eaa91 to
b824e69
Compare
| sf_contact = Salesforce::Contact.find_by(pi_accounts_unique_id__c: school.creator_id) | ||
| raise SalesforceRecordNotFound, "Contact not found for creator_id: #{school.creator_id}" unless sf_contact | ||
|
|
||
| sf_contact.experiencecsagreetouxcontact__c = school.creator_agree_to_ux_contact |
There was a problem hiding this comment.
Why is this field prefixed by experiencecs? I thought the intention was to sync this data for all schools, not just experience CS
There was a problem hiding this comment.
Hmm. That's the name I was given from the Salesforce team. I'll ask if we should consider changing it.
There was a problem hiding this comment.
We have changed this to EditorAgreeToUXContact__c and updated the heroku-connect image too.
zetter-rpf
left a comment
There was a problem hiding this comment.
Is it important to handle deletions of schools and roles?
I think schools are unlikely to be deleted, but roles may be if a school requests it.
This might be something you want to consider in a seperate PR.
|
Is is possible to configure this so that the salesforce database isn't needed to run the app and tests? When we spoke before I said it would be useful if docker wasn't required to run the app as we don't all use it in the team. I can currently boot the server and the console on this branch without the salesforce database running, but see errors when I run the tests: Maybe this is because it's trying to run the sync job inline during the test run? |
- Replace pluck(:id).each with find_each in all three sync tasks to avoid loading all IDs into memory at once - Scope role backfill to non-student roles to avoid enqueuing no-op jobs for student roles (which RoleSyncJob immediately discards)
07cab8f to
1badbdc
Compare
…B is unset In development and test environments, if SALESFORCE_CONNECT_DB is not set (i.e. the Heroku Connect Docker service is not running), the salesforce_connect AR connection now falls back to the main database. This prevents ActiveRecord::NoDatabaseError when developers run the test suite locally without Docker. When SALESFORCE_CONNECT_DB is set (Docker / CI), the connection is directed to the real Heroku Connect datastore as before.
1badbdc to
ba50e3a
Compare
Step 1: Tag all three Salesforce job specs with requires_salesforce_db: true. Add a filter_run_excluding rule in rails_helper so these specs are automatically skipped when SALESFORCE_CONNECT_DB is not set (i.e. the developer is running tests locally without the heroku-connect Docker service). CI and Docker environments, which set SALESFORCE_CONNECT_DB, continue to run the full Salesforce spec suite unchanged. Step 2: Make SALESFORCE_ENABLED opt-in (default false) Previously salesforce_sync? returned true unless SALESFORCE_ENABLED was explicitly set to 'false'. This caused GoodJob (which runs inline in test mode) to execute SchoolSyncJob and RoleSyncJob immediately whenever a school or role was created in any test — hitting Salesforce tables that don't exist in the test database. Change the check to opt-in: salesforce_sync? now returns true only when SALESFORCE_ENABLED == 'true'. Production and Docker setups already set this explicitly, so behaviour there is unchanged. Update the model specs to set SALESFORCE_ENABLED=true for the describe block that asserts jobs are enqueued, and update the feature flags spec to reflect the new default-false behaviour. Step 3: Guard after_commit callbacks with feature flag condition Move the FeatureFlags.salesforce_sync? check (and the student? check for Role) from inside do_salesforce_sync into the after_commit :if condition. This prevents the callback from firing at all when Salesforce sync is disabled, avoiding any interaction with the queue adapter or Salesforce models in tests that don't enable the feature.
ba50e3a to
b6bd57b
Compare
|
@zetter-rpf I've fully split out the Salesforce tests from the code. If you run: POSTGRES_HOST=localhost HOSTNAME=$(hostname) POSTGRES_USER=postgres POSTGRES_DB=choco_cake_test bundle exec rspecYou can successfully run the test suite locally without docker and without the Salesforce integration. In CI it will run inside docker and test the Salesforce integration directly. |
This PR reflects a renaming of the `Contact` field in Salesforce from `ExperienceCSAgreeToUXContact__c` to `EditorAgreeToUXContact__c` to reflect the fact that it's generic to schools using either product and not Experience-CS specific.
Status
What's changed?
Infrastructure
salesforce_connectdatabase configuration indatabase.ymlto connect to therpf-heroku-connectHeroku Connect datastore (read/write via a separate PostgreSQL connection, withdatabase_tasks: falseso Rails migrations leave it alone).salesforce_connectDocker service (image:ghcr.io/raspberrypifoundation/heroku-connect) todocker-compose.ymlfor local development, with a named volume and a health check.SALESFORCE_CONNECT_*environment variables to.env.exampleanddocker-compose.yml..github/workflows/ci.yml) to spin up theheroku-connectservice container, pass the Salesforce connection env vars, and addpackages: readpermission so the private image can be pulled.Models
Salesforce::Base— abstract Active Record base class that routes all reads and writes to thesalesforce_connectdatabase.Salesforce::School— maps to thesalesforce.editor__ctable (PK:editoruuid__c).Salesforce::Role— maps to thesalesforce.contact_editor_affiliation__ctable (PK:affiliation_id__c).Salesforce::Contact— maps to thesalesforce.contacttable (PK:pi_accounts_unique_id__c).Jobs
Salesforce::SalesforceSyncJob— base job class that:SALESFORCE_ENABLEDenv var and discards the job (no retry) if it is nottrue.SalesforceRecordNotFoundwith polynomial backoff, up to 10 attempts.…).salesforce_syncqueue.Salesforce::SchoolSyncJob— upserts aSchoolrecord intosalesforce.editor__c, mapping all address, status, and metadata fields. Defaultsuserorigin__cto'for_education'when blank.Salesforce::RoleSyncJob— upserts non-studentRolerecords intosalesforce.contact_editor_affiliation__c. Skips student roles entirely.Salesforce::ContactSyncJob— looks up the Salesforce Contact by the school creator's user ID and syncs thecreator_agree_to_ux_contactflag. RaisesSalesforceRecordNotFound(triggering retry) if no Contact exists yet.Model callbacks
School—after_commiton create/update enqueues bothSchoolSyncJobandContactSyncJob.Role—after_commiton create/update enqueuesRoleSyncJob.GoodJob queue configuration
salesforce_sync:10queue to allow up to 10 concurrent Salesforce sync workers.Rake tasks
salesforce_sync:school— bulk-enqueuesSchoolSyncJobfor every School (for backfilling).salesforce_sync:role— bulk-enqueuesRoleSyncJobfor every Role (for backfilling).salesforce_sync:contact— bulk-enqueuesContactSyncJobfor every School (for backfilling the UX contact flag).Tests
SchoolSyncJob,RoleSyncJob, andContactSyncJob, including field mapping, skipping/discarding behaviour, and error cases.SchoolandRoleverify that the correct jobs are enqueued after create/update.Points for consideration
salesforce_connectconnection credentials are injected via environment variables. No secrets are committed to the repo.after_commitcallback enqueues a background job (GoodJob), so there is no synchronous overhead on the request. Concurrency is capped per-record to avoid TOCTOU races.ContactSyncJobrequires the Salesforce Contact record to already exist (keyed bycreator_id). If it does not exist yet, the job retries up to 10 times with polynomial backoff.How to Test Locally
To test these changes locally, you can:
Observe the GoodJob dashboard - you should see new
SchoolSyncJobandRoleSyncJobjobs in thesalesforce_syncqueue. In the Rails Console, you can also inspect the number ofSalesforce::SchoolandSalesforce::Roleobjects that have been created - you should see them increase when you add a new school.Steps to perform after deploying to production
After deploying, run the rake backfill tasks to sync existing data: