What went wrong previously
This document explains why the employer sync should be reimplemented rather than repaired piece by piece. It is a review of the design and delivery process, not of an individual engineer.
The detailed findings were made from the original specification, its 18 issue drafts, the merged employer-sync pull requests, PR #1852, its requested-changes review, the API and web request paths, and the sanitized JodGig database.
The short answer
Keep the data research and schema foundations. Replace the employer-sync algorithm and deliver it as complete user journeys.
The original work discovered important facts, but its implementation shape made correctness harder to see:
- one migration became 18 issues mixed with adjacent product work;
- small pull requests contained incomplete fragments rather than complete behaviour;
- the checkpoint watched only one of several source tables;
- important rules were calculated with only one user's data in memory;
- JodApp could store multiple or revoked memberships but did not use them correctly; and
- the acceptance tests described in the issues did not land with the code.
These are foundation problems. Fixing individual methods would still leave the wrong overall shape.
The project grew beyond the business outcome
| Original need | Work added to the launch path |
|---|---|
| Existing employers can log in and receive correct access | Old issue 5.1 proposed JodGig::Users::UpsertIdentitiesUserService and changing the scheduled talent sync, talent migration during login, and employer sync to call it |
| Copy and maintain employer access | Employer migration during login |
| Revoke removed access | Hourly scheduling and timestamp checkpoint |
| Verify the migration | Mobile collection prompt |
| Preserve login behaviour | Login-page redesign and additional frontend states |
The adjacent work may have value, but it is not required to prove employer identity and access. Each addition creates more code that must agree on eligibility, matching, errors and timing.
The new plan keeps identity, access, offboarding and verification in scope. The rest can be delivered independently.
The issue split hid the complete behaviour
A small pull request is useful only when it proves a complete statement about the system.
The work was split into a sync shell and separate private mapping functions. One pull request could create an identity but not a membership. The next could create a membership while outlet assignment remained commented out. Tests would have needed to call private functions because the public job still ended at unfinished methods.
This optimized the number of changed lines in each pull request, but it made the central question difficult to answer:
Can one real employer log in, enter the correct company, use the correct outlets, and lose access after offboarding?
The new phases are ordered by complete behaviour. Each phase ends with something a reviewer can verify. The final tests start the sync through the same code used by manual and scheduled runs.
The checkpoint observes the wrong thing
| JodGig source change | Does jodgig.users.updated_at reliably change? | Consequence of an incremental user checkpoint |
|---|---|---|
| Employer email or name | Yes | Usually detected |
| Company disabled | No | Old membership can remain active |
| External super-HQ grant removed | No | Removed company access can remain active |
| Area manager loses an outlet | No | Old outlet assignment can remain active |
| Outlet disabled or deleted | No | Old outlet access can remain active |
The source population contains only a few thousand employer-type rows. A daily full read is simpler than maintaining checkpoints for several tables and safer than pretending one timestamp represents all access changes.
Creation and revocation were mixed together
“Should this person be created?” and “Should their old access be removed?” require different answers.
An inactive person who has never migrated should be skipped. An active person who migrated last week and becomes inactive today must still be read so their old access can be revoked.
Removing inactive users from the query solves the first case and breaks the second. Removing the filter solves the second case and creates identities for people the audit says to skip.
The new planner reads the full employer-type population and emits an explicit migrate, update, revoke, skip, or needs_review decision.
External super-HQ was read from mutable user state
| Field | What it actually means | Safe migration use |
|---|---|---|
jodgig.users.role_id | Temporarily becomes HQ while the user is inside a selected company | Do not derive grants from it |
jodgig.users.company_id | The currently or most recently selected company | Default selection only, when it is still a valid grant |
jodgig.user_company row | A durable customer-company grant | Build memberships from valid live rows |
External super-HQ is real customer multi-company access. Internal super-HQ is JOD staff impersonation with access to all enabled companies. Treating them alike would either remove valid customer access or create customer memberships for staff.
The new plan migrates valid external grants and excludes internal impersonation.
PR #1852 attempted a company-wide decision inside a user loop
Owner selection needed all company members, but the method ran while processing one user and had no current product consumer.
The pull request attempted to choose one company owner after each user. This introduced stale association data, repeated company-wide reads, incorrect precedence and a rule that would not correct an already-wrong owner.
No current API or web path reads Org::Membership.is_owner. Legacy HQ or super-HQ access is not proof of company ownership.
The new plan leaves is_owner = false for migrated memberships. Ownership can be designed later when a feature has a real definition and consumer.
The destination application was not ready for the data
| Data written by the sync | Application behaviour found during review |
|---|---|
| Several memberships for one identity | Identity association and request path assumed one membership |
status = revoked | Employer request path checked that a row existed, not that it was active |
is_default | No reliable request-path reader |
is_owner | No current reader |
A migration is not correct merely because PostgreSQL accepts its rows. The application must interpret those rows as intended.
Phase 1 of the new plan fixes membership selection and authorization before migrated access is relied upon.
Important row-level defects remained
| Problem | Effect |
|---|---|
Notification migration created is_* columns but wrote allow_* names | Membership writes fail |
LOCATION mapped to location_manager instead of confirmed outlet_manager | Destination language and authorization disagree |
| Identity conflict emitted a warning but continued | A membership can be attached to the wrong person |
| Missing super-HQ company was used as if it existed | One bad pivot can crash that user's processing |
| Two different default rules ran for super-HQ | A person can receive two defaults |
| Outlet assignment call was commented out | Area and outlet managers receive incomplete access |
| Removed memberships and assignments were not compared with the source | Access survives after removal |
Any one of these can be fixed locally. Together with the algorithm problems above, they make a clean replacement easier to reason about than continued repair.
Tests did not protect the promised behaviour
The issue documents listed tests, but the merged employer selection and identity mapping changes contained no employer-sync tests.
The most valuable tests are complete journeys:
- active HQ employer migrates and reaches an employer endpoint;
- disabled employer does not migrate;
- previously active employer is revoked;
- external super-HQ receives exactly the valid company memberships;
- area and outlet managers receive the exact outlet scope;
- identity conflict writes nothing;
- native JodApp membership survives reconciliation; and
- an identical second run changes nothing.
The new rollout phase treats these as release gates rather than follow-up work.
What remains useful
| Keep | Replace or defer |
|---|---|
| Production data audit and eligibility groups | Employer sync service and timestamp checkpoint |
| Stable JodGig person ID on the identity | PR #1852 membership and owner implementation |
| Unique identity, membership and assignment keys | Employer login-time migration |
| Boolean membership columns | Old issue 5.1's JodGig::Users::UpsertIdentitiesUserService, which would also change two working talent migration paths |
| Notification preference columns | Login redesign and mobile prompt |
| Existing company and outlet syncs | Owner calculation |
| Legacy password verification service | Partial private-mapper delivery approach |
The goal is not to erase the last month. The useful discoveries become the input to a smaller implementation.
Why a new implementation is the safer choice
| Repair the current service | Build the replacement from the confirmed contract |
|---|---|
| Change an incremental copier into a full reconciliation while retaining old seams | Begin with a planner whose output already represents the complete current truth |
| Continue reasoning through partial mapping methods | Review one plain decision record per source person |
| Add tests around code before the complete sync can run | Test complete journeys through the same code used by manual and scheduled runs |
| Carry owner, JIT and checkpoint concepts until individually removed | Leave them out from the first line |
The old employer job should remain unscheduled while the replacement is built. It may be used as reference, but it should not write alongside the new job. After QA and production dry-run approval, remove the old employer-specific implementation.