Repository navigation
Build org-answer entries in one place - #2842
Merged
Merged
Conversation
The org name/position/website/type/address answers were shaped into the same hash in four places, so every new org field (like country) had to be added to each one and they could drift. FormSubmission.org_entry_from now builds it, and ProfileDiff.for_entry reads it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
maebeale
marked this pull request as ready for review
October 25, 2026 11:18
CaptureFromSubmission now reads org answers through FormSubmission::ORG_ENTRY_FIELD_IDENTIFIERS rather than literal field_value calls, so the coverage spec's scrape missed them. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Admins want the background charts to show where registrants work, not where they live. Cities, states, and countries now read the org linked on the registration, at the address picked on that registration's affiliation, else the org's oldest address. The drill-ins follow so a clicked row lists the same people; the roster's State/County filters keep matching home addresses. Also builds org_entry_from off the field-map constants so the identifier list and the hash can't drift. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
maebeale
changed the title
The location filters should mean the same thing as the location charts. The roster, bulk-email picker, and attendees State/County/City filters now match the registration's work address (RegistrationWorkAddresses), so the attendees States drill-in can reuse the state param. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
maebeale
changed the title
1 task done
Keeps this PR to the org-answer refactor so each reviews on its own. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
maebeale
changed the title
Building the entry up front meant the form-params path looked up each org field on its own, so a form with no org section paid nine queries for answers it never had. One batched lookup covers them all. The identifier-coverage guard counted the appended constants too, which left it passing on a scan that had stopped matching most of the pipeline — it now counts only what it scanned. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
maebeale
added a commit
that referenced
this pull request
Oct 25, 2026
Admins want the location charts and the State/County/City filters to show where registrants work, not where they live. Both now read the org linked on the registration, at the address picked on that registration's affiliation, else the org's oldest address (RegistrationWorkAddresses). Drill-ins follow so a clicked row lists the same people. Moved out of #2842 so it reviews separately from the org-answer refactor. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
maebeale
added a commit
that referenced
this pull request
Oct 25, 2026
Admins want the location charts and the State/County/City filters to show where registrants work, not where they live. Both now read the org linked on the registration, at the address picked on that registration's affiliation, else the org's oldest address (RegistrationWorkAddresses). Drill-ins follow so a clicked row lists the same people. Moved out of #2842 so it reviews separately from the org-answer refactor. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
maebeale
added a commit
that referenced
this pull request
Oct 25, 2026
* Chart and filter registrant locations by where they work Admins want the location charts and the State/County/City filters to show where registrants work, not where they live. Both now read the org linked on the registration, at the address picked on that registration's affiliation, else the org's oldest address (RegistrationWorkAddresses). Drill-ins follow so a clicked row lists the same people. Moved out of #2842 so it reviews separately from the org-answer refactor. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Fall back to the registrant's own address when no work address is known A registrant with no linked org (or an org with no address) dropped out of the location filters entirely; counting them where they live keeps them findable. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Point the work-locations feature entry at its own PR Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Name registrant locations for what they are, not just work With the fallback to a registrant's own address, "work" no longer describes the whole rule: RegistrationWorkAddresses -> RegistrationLocations and work_* scopes -> located_in_*. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Keep the location filters on one address, and off every id The three chained location scopes each tested any of a registration's addresses, so a registrant working in two states matched a city from one and the state from the other — the shared addresses join used to rule that out. One `located_in(state:, county:, city:)` call makes the invariant structural, and the reminder picker's City search now runs through it instead of matching rows the State and County dropdowns had already filtered separately. RegistrationLocations also takes a relation now. The attendees index built its filter options by plucking every reportable registration id and feeding the array back as IN lists across five queries; as subqueries that stays one statement no matter how large the population. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Let every location filter describe one address, and one field name Review of the work-location branch turned up three things. The attendees index resolved County in a second RegistrationLocations pass, so City and County could land on different addresses — the exact case located_in was built to prevent, and the one the feature entry promises is impossible. Its State and County dropdowns were filled by building a location row for every registration in the viewer's whole reportable population, where two grouped queries over the address tables answer the same question. And rows_by_registration had no callers. Rows now carry every field an address uses to say where it is — everything but the street and phone — so the next chart, count or filter asks for a field name rather than another method: registrant_ids_by(:country) replaces a hand-written country map, and locality, zip, school district and the LA districts come along for free. matching takes the whole filter set in one call, and filters_from folds a County option's own state into it so the roster scope and the index controller stop keeping separate copies of that rule. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Settle which address stands for a registrant when only one can The own-address fallback picked the primary flag, which is pre-checked on every address added and so rarely points at the current one. Address::STANDING ranks them instead: active work, else the most recently edited active personal, else a retired personal, else a retired work. Work and personal swap places below the active line on purpose — a current work address is the sharpest thing we know about someone and a retired one the stalest, because people change jobs and home outlasts the job. The list is written once in Ruby and compiled into the ORDER BY, because a hand-kept SQL twin drifts the first time the order changes. AddressDecorator sorts its already-loaded association by the same list instead of its own near-copy of half of it. Retired addresses now count, which is why registrant_addresses drops .active: a registrant holding nothing but a stale address belongs on the map at that address rather than nowhere. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Locate people by their active affiliations on a date, not by the registration One rule for every location chart, filter, and report: the work addresses of the affiliations active on a date (the event's start date on event pages, today elsewhere), else the person's own standing address. That lets pages that span events, the reports, and later the people index share the rule, while a past event still doesn't move when someone changes jobs. RegistrationLocations becomes PersonLocations with the same interface. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Label the city and country chips without "Org" Both now read PersonLocations, which falls back to a person's own address, so "Org city" / "Org country" could label a home-address match as an org one. Closes #2845 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
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
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.
🤖 suggested review level: 3 Read 📖 refactor across the org-linking paths, no intended behavior change
The org name, position, website, type, and address answers were turned into the same hash in four places, so a new org field had to be added four times. Now one method builds it.
FormSubmission.org_entry_from(answers)builds the entry fromORG_ENTRY_FIELDS/ORG_ADDRESS_FIELDS#org_entrywraps it and adds the submissionOrganizationServices::ProfileDiff.for_entrytakes that entry directlyCaptureFromSubmission,LinkSubmittedOrganization, and the importernilinstead of""; everything downstream already treats them the sameCaptureFromSubmissionresolves the org fields in oneform_fieldsquery — fewer than the per-field lookups it replaces, and it no longer matters that the entry is built before the org name decides whether any of it is usedsmart_form_fields_speccounts only the identifiers it scanned, so appendingORG_ENTRY_FIELD_IDENTIFIERScan't keep it passing on a broken scanFeatures seed (optional)
🤖 Generated with Claude Code