Repository navigation
Sync form submissions to HubSpot as CRM contacts - #48
Open
JeremyCaney wants to merge 67 commits into
Open
JeremyCaney wants to merge 67 commits into
JeremyCaney wants to merge 67 commits into
Conversation
Junie is JetBrain's AI. We're not currently using it, but JetBrains Rider configured empty placeholders on the project. `docs` is where Claude likes to store plans, by convention, which don't need to be committed with the repository.
We'll be allowing the `QuoteFormBindingModel` to be mapped to a `Topic` as part of the HubSpot integration. As part of this, we don't want the `Address` properties to be mapped with a prefix so they can reuse the same existing attributes defined on the existing `ExtendedContact` content type.
Introduces `HubSpotFieldMapping`, a strongly typed representation of a single field mapping from a HubSpot sync manifest. Each instance describes how one topic attribute (or constant value) maps to a HubSpot contact property, including its HubSpot type, whether it's required, whether it's the unique key (used to determine if an existing record gets updated), and an optional value map for translating source values to HubSpot's internal option values. This is the first of several new types under `Areas/Forms/HubSpot/` that will make up a manifest-driven mapping layer; subsequent commits will add the manifest, registry, and payload construction types.
Introduces the `HubSpotFormManifest` record, which groups a form's `HubSpotFieldMapping` entries (1f08096) under a `FormIdentifier` and exposes `UniqueKeyField` to look up whichever mapping determines whether a HubSpot contact is created or updated.
Defines the contract for looking up a `HubSpotFormManifest` (01b4d04) by its form identifier via the `TryGetManifest()` method. Implementations will be responsible for loading and validating the manifest JSON at startup; this commit adds only the interface.
Defines the contract for translating a `Topic` into a HubSpot contact properties dictionary via its `Build()` method, driven by a `HubSpotFormManifest` (01b4d04). This commit adds only the interface; the implementation—including per-field type handling for string, enumeration, and enumerationSet values—will follow separately.
Introduces `HubSpotSyncResult`, the outcome of a single HubSpot sync attempt. Since sync should never throw an exception, since the form will otherwise still e.g., send an email to GoldSim and/or write a topic to OnTopic, so this conveys success or failure, the resulting contact ID, or an error message, so callers can log failures without interrupting the form submission.
Defines the contract for creating or updating a HubSpot contact from a `Topic` via the `SyncAsync()` method, returning a `HubSpotSyncResult` (1a73057) rather than throwing an exception, per design. This completes the introduction of core data types and interfaces required for the HubSpot sync service: `HubSpotFieldMapping` (1f08096), `HubSpotFormManifest` (01b4d04), `IHubSpotMappingRegistry` (9cc66c6), `IHubSpotPayloadBuilder` (a399f92), and `HubSpotSyncResult` (1a73057). Subsequent commits will introduce concrete implementations of the interfaces.
Implements the `IHubSpotPayloadBuilder` (a399f92), translating a Topic's attributes into a HubSpot property dictionary per a `HubSpotFormManifest`'s field mappings (01b4d04). Handles string, enumeration, and enumerationSet HubSpotType values, converting `HubSpotFieldMapping.MultiValueDelimiter`-separated source values into HubSpot's semicolon-delimited multi-checkbox format. A missing required value, an unrecognized `HubSpotType`, or a value with no `ValueMap` entry throws an exception immediately since these represent configuration errors that should be made clear during development.
Implements the `IHubSpotMappingRegistry` interface (9cc66c6), loading every `HubSpotFormManifest` (01b4d04) from the `.json` files under `Areas/Forms/HubSpot/Mappings/` during first load via `System.Text.Json`, so a malformed or misconfigured manifest fails at startup rather than on first use. Missing required members (`HubSpotFieldMapping`'s (1f08096) `HubSpotProperty` and `HubSpotType`, and `HubSpotFormManifest`'s `FormIdentifier` and Fields) are enforced automatically via C#'s `[required]` attribute; `ValidateManifest()` additionally checks that each manifest has exactly one `IsUniqueKey` field and that every `HubSpotFieldMapping` sets exactly one of `SourceAttribute` or `ConstantValue`. The mappings directory is optional at this stage, since no manifests exist yet. That will be introduced in a subsequent commit.
`HubSpotContactSyncService` needs to handle the scenario for when the sync settings aren't configured (e.g., no `HUBSPOT_ACCESS_TOKEN`) as a distinct outcome from success or failure—an absent token is a supported runtime state, not an error, and shouldn't populate `ErrorMessage` as if it were. This adds `IsSkipped` alongside the existing `IsSuccessful` (1a73057) to `HubSpotSyncResult`, and updates `ErrorMessage`'s doc comment to reflect that it's unset in both the skipped and successful cases.
Implements the new `IHubSpotContactSyncService` (1add9f1) by "upserting" a contact via HubSpot's CRM v3 batch upsert endpoint (`/crm/v3/objects/contacts/batch/upsert`). Uses `HubSpotFormManifest`'s `UniqueKeyField` (01b4d04) as the `idProperty`/`id` pair and `IHubSpotPayloadBuilder` (a399f92) for the properties payload. If no access token is configured, this returns a skipped `HubSpotSyncResult` (2da90d9) without attempting a call, per the design; every other failure—an unreachable host, a non-2xx response (`ErrorMessage` gets the raw response body), or an exception raised while building the payload—is caught within `SyncAsync()` and reported via the result instead being thrown, honoring the interface's contract even though `HubSpotPayloadBuilder` (63532f0) itself throws eagerly on manifest-level errors. The `IHttpClientFactory` registration this depends on is deferred to the `Program` wiring, which will be committed later.
Replaces the `IHttpClientFactory` dependency in` HubSpotContactSyncService` (1add9f1) with an injected `HttpClient`, matching the long-lived, manually constructed client pattern already used by `RecaptchaValidator`. `GoldSimActivator` constructs controllers before the DI container is built, so `IHttpClientFactory` isn't reliably resolvable at that point without larger changes to how the controller activator is wired up. As a result, I also removed the now-unused `HttpClientName` const and the `CreateClient()` call.
`SendCustomerReceipt()` was creating and disposing a new `HttpClient` per call, the pattern `IHttpClientFactory` exists specifically to avoid. Since the target URL varies per request, but a `HttpClient` doesn't require a fixed `BaseAddress` to be reused, this switches to a shared `static readonly` instance, matching `RecaptchaValidator`'s existing convention.
Adds the same `IHttpClientFactory`-migration `TODO` now present on `HubSpotContactSyncService` (5d5f2f3) and `FormsController` (8c133db), since `RecaptchaValidator`'s `_client` field is the pattern all three are following in the interim, and will also need to be updated once `IHttpClientFactory` is adopted at a later date. (This is expected to happen with OnTopic 6.0.0 later this year.)
The `GoldSimActivator` now constructs the HubSpot dependency chain as singletons—a `HttpClient` with its base address set to `https://api.hubapi.com` and a `HubSpotPayloadBuilder` (63532f0) feed into `HubSpotContactSyncService` (5d5f2f3), alongside a `HubSpotMappingRegistry` (006415c)—following the same constructor injection pattern already used for `PostmarkSmtpService`. Both the `IHubSpotMappingRegistry` (9cc66c6) and `IHubSpotContactSyncService` (1add9f1) are passed into `FormsController`'s constructor, which stores them for use in the actual (forthcoming) sync call. This commit only wires the dependencies; `ProcessForm()` doesn't call `SyncAsync()` yet—but will soon!
`ProcessForm()` will need the already-mapped `Topic` to pass into the upcoming HubSpot sync call when `SaveAsTopic` is true, rather than re-mapping the binding model a second time. The call site currently discards the returned value; the next commit will use it.
The `ProcessForm()` method now looks up a manifest for the current form via `IHubSpotMappingRegistry` (9cc66c6), keyed by the same content type identifier `SaveToTopic()` already derives from the binding model's type name (e.g. `TrialFormBindingModel` becomes `TrialForm`). When a manifest exists, it reuses the `Topic` already mapped by `SaveToTopic()` (d045d27) if `SaveAsTopic` was set, and otherwise maps an unpersisted one, then calls `IHubSpotContactSyncService.SyncAsync()` (1add9f1). Forms with no manifest are unaffected. The returned `HubSpotSyncResult` is discarded for now, since there's no logging provider yet to do anything with it. I still defined the response object in anticipation of adding proper logging at a future date.
The `HubSpotMappingRegistry` (006415c) reads manifest files from `ContentRootPath` at startup, so the `.json` files under `Areas/Forms/HubSpot/Mappings/` need to be deployed next to the built assembly. They're implicitly included as `None` items by the Web SDK's default, so this only needed a `CopyToOutputDirectory` update rather than a new `Include`.
Each manifest file deserializes into a `HubSpotFormManifest` (01b4d04) made up of `HubSpotFieldMapping` entries (1f08096), loaded and validated at startup by the `HubSpotMappingRegistry` (006415c). This introduces mapping manifests for the `TrialForm`, `DemoForm`, `QuoteForm`, `StudentAcademicForm`, and `InstructorAcademicForm`. Each maps the email as the unique key, along with whatever contact fields the form actually collects—`TrialForm` and `DemoForm` have no `street`, `city`, or `zip` source data, for example, so those properties are simply absent rather than mapped to nothing. The `goldsim_contact_type` and `datacor_product` are constants (`User` or `Trainer`, and `GoldSim`, respectively), since neither is collected on any form today. `country`, `state`, and `jobtitle` are intentionally left out for now; country and state are pending data migration, while no form currently collects a job title.
As part of the integration with Datacor, we migrated from our inherited country values to the official ISO 3155-1 standard. This has already been updated in the live OnTopic database. The default value for the `Country` field needs to follow suit.
Add the `[Metadata("State")]` attribute to the `Province` property in `Address`, `DemoFormBindingModel`, and `TrialFormBindingModel`, so a `Select` editor template can pull its options from the `State` metadata lookup topic instead of free text.
This will be exposed as a select field in subsequent commits.
Add a second `Province` control—a `Select` sourced from the `State` metadata lookup (820447d)—alongside the existing free-text input, in the `Demo`, `Trial`, and `_Address` (shared by `Quote`, `StudentAcademic`, and `InstructorAcademic) Razor templates. In the `Demo` and `Trial` templates, `Province` is moved after the `_ContactLocation` partial so both controls render together. Neither control is toggled yet; that follows in the next commit.
Add logic to the `Forms` JavaScript that shows and enables whichever `Province` control matches the selected `Country`, based on the `state-lookup` and `province-input` classes (5e0a3e4), and disables the other so it's excluded from submission. This matches against the exact "United States" string set as `Contact.Country`'s default (106a8cb). It also selects by class rather than field name since the underlying property may be nested (e.g. `BindingModel.Address.Province`) or top-level (e.g. `BindingModel.Province`) depending on the form, thus allowing one script to work for both cases. On toggle, the current value is carried over to the control that's about to become active so that values aren't lost. (Though if the state doesn't exist, it obviously won't bind to a record in the dropdown box.)
Reorder the `Country` field so that it occurs _before_ the `Postal` field, even though normally it would be at the end. This is because the type of input that `Providence` field will be depends on what `Country` value is selected, with a state dropdown list for United States, and a text input for everyone else (186f6ad). As such, we want the user to select the country _before_ they are asked to input their state/providence. This was ordered to account for the grid: If the `Country` takes up two columns, we don't want to simply place it above e.g., `Street1` or `Providence` and, thus, disrupt the flow of the columns.
This allows us to align it next to phone number, which makes more sense now that `Country` has been reordered and takes up two columns as part of that (b5c56d3).
Add `sourceAttribute` mappings for the `Province` and `Country` properties to `state` and `country` contact attributes weren't added to the original manifests (a2b9d22) since neither field's value reliably matched what HubSpot expects. They can now be mapped properly since we've fixed the `Country` default value (106a8cb) and bound `Province` to the `State` metadata lookup (820447d), thus aligning both fields with HubSpot's expected values. (This also required updated OnTopic data for the `Country` and `State` lookup metadata, handled independently of this.)
The HubSpot-only branch in `ProcessForm()` (c2c05b9), called `IReverseTopicMappingService.MapAsync()` directly on the binding model whenever a topic wasn't otherwise saved, thus skipping the `Key` and `ContentType` assignment that `SaveToTopic()` had been performing. As a result, any topic not configured to save (`SaveAsTopic` as false) threw an `ArgumentNullException`, requiring a `Key` when its HubSpot sync ran. To resolve this, I extracted a new `MapToTopic()` method, shared by both callers, so `Key` and `ContentType` properties are always populated before mapping, without duplicating logic.
Document the manifest-driven HubSpot contact sync (a2b9d22), covering the manifest location and naming (`HubSpotMappingRegistry`, 006415c), the fields consumed by `HubSpotPayloadBuilder` (63532f0, `HubSpotFieldMapping` 1f08096, `HubSpotFormManifest` 01b4d04), `valueMap` for `enumeration` and `enumerationSet` types, and how `HubSpotContactSyncService` (fe34d2f) handles a missing token or a sync failure without blocking the submission.
The `enumerationSet` values ("Multiple checkboxes") technically allows multiple values to be set, by appending supported values via a semi-colon (`;`) delimiter. This adds a configuration option to `HupSpotFieldMapping` allowing values set on `enumerationSet` fields to be appended, if appropriate.
This isn't wired up yet, but will be in subsequent updates.
This extends the `ValidateManifest()` method to ensure that any field that's configured to `IsAppended` (05d8511) is an `enumerationSet`, as that's the only HubSpot type that permits appending values.
This takes the new `IsAppended` configuration on `HubSpotFieldMapping` (05d8511) and uses it to optionally append values onto any existing values, should they exist. This is only supported on `enumerationSet` fields ("Multiple Checkboxes"), which is ensured via `ValidateManifest()` (6130bb5) when the manifests are loaded.
This introduces a new `SourceFlagMap` (6e85fd6)—keyed by the "Modules" source flag—to a newly established `Shared/Lookups.json`, as loaded (f787190), mapped (886bf1b), and validated (7e897d8) via the `HubSpotMappingRegistry`. This will allow a `"sourceFlags": "Modules"` configuration to be added to any of the manifests which need to map a multi-select or checkbox list of modules.
When we originally setup the HubSpot synchronization, we only supported the core contact fields supported by all Datacor forms, such as name, contact information, and address. We're now extending that to support (almost) all of the fields supported on the forms, which are GoldSim specific (prefixed with `goldsim_info_`). This includes a checkbox list of modules, as supported by the recent `Shared/Lookups.json` capabilities (2ceebf3, 6e85fd6, f787190, 886bf1b, 7e897d8, b28251c, 5f6ca30) as well as appending values to `enumerationSet` types (05d8511, 6130bb5, b02bdca, 2ceebf3).
We already map the Datacor product (always GoldSim). This maps the specific GoldSim product the customer is interested in. This currently only applies to the Quote form, and currently only accepts GoldSim or GoldSim research.
While we already infer this from whether or not we have training information, and the checkbox is only intended to enable (and require) the form fields, Datacor would also like this information submitted as a boolean value in HubSpot. We could also infer this as part of the mapping, but as this is a one-off technical requirement that we don't see a need for elsewhere, it's simpler to just handle it as a standard mapping. Since the trial form is persisted to OnTopic, I'd also added the `IsTrainingCourse` attribute to the `TrialForm` content type in the database itself. Because this is now part of the `BindingModel`, this also required some modification t how the default state of the button is handled. Positively, this ensures that it's reenabled when there's a server-side validation error, which it wasn't previously.
This maps the `IsTrainingCourse` boolean (4ae1b8e) to the (wordy!) `goldsim_info_are_you_using_this_trial_version_for_a_training_course` HubSpot attribute. This this is the first standalone checkbox we've added, I also acknowledge that option in the `README` for HubSpot.
Previously, while unlikely, two entries submitted at the same time would corrupt the database, causing it to fail on the next load. This had previously been handled by increasing the resolution to milliseconds, but we've still encountered issues with this due to duplicates of the same submission. It's unclear how that happened, but given the severity of the impact, this will prevent it from corrupting the database.
The Form Type field has, in Hubspot, been updated to accept multiple values, which prevents the value from getting overwritten if a customer e.g., submits a Trial and a Quote request. This updates our configuration to use the new `IsAppended` property (30d1322) for this value.
Datacor has requested we add this. We'll still need to add this to the forms and so, until then, they're not `[Required]`. Unfortunately, there's no common base class here, so we need to add this three times.
This places the department next to the institution, and the phone number next to the email address.
This has more to do with their institution and email address than it does their physical mailing address, so I'm moving it to be in place.
The `JobTitle` fields (b9ded24) aren't yet added to the views and thus aren't yet required, but we can safely map them in the interim to HubSpot; this will only take affect once the values are available.
The `_Profile.cshtml` partial isn't referenced by any view at this point, so it's being removed ahead of inlining the contact blocks into each form.
The `MetadataLookupViewComponent` previously found the `[Metadata]` attribute by reflecting on the container type's property. It now reads the attributes ASP.NET Core collected for the property, which include those from a `[ModelMetadataType]` class. This allows `[Metadata]` to optionally be declared on a shared metadata class.
The Demo form never collects Modules, and only honored the required `AcceptTermsOfUse` with a hidden input. Deriving from `Profile` instead of `ExtendedProfile` removes both fields, along with the hidden input, and is thus a cleaner, more honest approach that avoids confusion and further complicated dependencies.
The academic sponsor never collects a country, which previously required overriding `Country` with `[Required(AllowEmptyStrings = true)]` and assigning it an empty string. Deriving from `CoreContact` instead of `Contact` removes the field entirely. This requires that `PhoneNumber` be declared directly, with the same validation as `Contact`, plus the sponsor label. That's still a cleaner solution that entangling dependencies that require overrides, and especially given how cheap the definitions are to add. (And even more so here since we had to override phone number regardless.)
The new `JobTitle` property (b9ded24) should be declared on the Quote, Trial, and Demo binding models, which don't share a base class that could host it without "polluting" other forms. The new `FormFields` class defines its annotations and documentation once. Each model then opts in with `[ModelMetadataType]` and inherits the documentation with `<inheritdoc cref>`. It still hasn't been exposed to the views yet; that will still happen in a future commit.
`Province` is declared on the Trial and Demo binding models as well as `Address`, each with identical annotations, including `[Metadata("State")]`. These are now defined once in `FormFields` (6ff7dcc), and `Address` opts in with `[ModelMetadataType]`.
This relies on `MetadataLookupViewComponent` reading `[Metadata]` from model metadata, as was recently implemented (2c34a4c).
`OtherTools` is declared on both the Trial and Demo binding models, with nearly identical annotations. These are now defined once in the `FormFields` catalog (6ff7dcc). As part of this, the Demo form's label adopts the Trial form's wording, which drops a stray asterisk and the space before the question mark, which was likely a typo.
The instructions for using an institutional email address were embedded in the `_Contact` partial. Moving them to their own partial allows forms that layout their contact fields individually while still centralizing the email instructions.
The `_Contact` and `_ContactLocation` partials locked in the order of the contact fields, which made it difficult to adjust the layout of individual forms. This was a factor of me getting too cute with the inheritance structure, which has caused a lot of problems when adding new fields previously. The Trial and Demo forms now render each contact field directly, with the email instructions coming from the `_EmailInstructions` partial (bb08706). Since the `EditorFor()` calls are cheap, and the metadata is centralized in the models, this duplication is not a big issue. Neither of the old partials has any remaining consumers, so both are removed.
Following the lead of the Trial and Demo forms (8d6e677), the Quote form now renders its contact fields directly instead of through the `_ExtendedContact` partial. Country now follows Email, rather than spanning its own row after Fax. The `_ExtendedContact` partial has no remaining users, so it's removed.
The `JobTitle` property (b9ded24) is now rendered next to the organization on each form, using the annotations defined in `FormFields` (6ff7dcc). It remains optional for now. The prior shuffling (b3e294b, 8d6e677, bb08706) was meant to make room for this since, previously, not all consumers of `_Contact` needed `JobTitle` (8d6e677), notably the training contact on the Trial form. This thus allows for it to be added as appropriate, without interfering from the rest of the inheritance.
The Trial form now renders its trainer's contact fields directly instead of through the `_CoreContact` partial, following the other contact fields (8d6e677). The `_CoreContact` partial has no remaining users, so it's removed. This completes the shuffling intended to make room for `JobTitle` (f16879d).
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.
Form submissions are now optionally synced to HubSpot as CRM contacts, alongside the existing email receipt (via
!DisableEmailReceipt) and saving to OnTopic (viaSaveAsTopic). The HubSpot syn is driven entirely by per-form JSON manifests, so wiring up a new form or adding a field requires no code changes once a manifest exists.To support this, this also adds a
State/Provincetoggle based on theCountryselection so their values can map cleanly to the new HubSpot contact fields.Features
Infrastructure
Added the infrastructure for the HubSpot sync:
HubSpotFormManifest(01b4d04) ofHubSpotFieldMapping(1f08096) instancesHubSpotMappingRegistry(006415c):IHubSpotMappingRegistry(9cc66c6)HubSpotPayloadBuilder(63532f0):IHubSpotPayloadBuilder(a399f92)HubSpotContactSyncService(fe34d2f):IHubSpotContactSyncService(1add9f1)HttpClientrather thanIHttpClientFactory, matching existing pattern (5d5f2f3)HubSpotSyncResult(1a73057), including itsIsSkippedproperty (2da90d9)Integration
FormsControllerviaGoldSimActivator(d045d27)ProcessForm()looks up a manifest by content type and callsSyncAsync()(c2c05b9)Topicalready mapped bySaveToTopic()when available, so it doesn't need to map twice (9dd5d68)MapToTopic()centralizes the mapping of binding model to topics to centralize that logic (267b43f)Configuration
DemoForm,TrialForm,QuoteForm,StudentAcademicForm, andInstructorAcademicForm(a2b9d22)ProvinceandCountrymappings added once their values were normalized to match HubSpot (e5cbae6)Presentation
Added a
State/Provincetoggle based onCountryso the appropriate control is dynamically displayed based on whether or not the United States is selected:Selectcontrol was added alongside the existing text input inDemo,Trial, and_Address(5e0a3e4)Provincebinds to theStatemetadata lookup data to populate the options (820447d)Forms.jsshows and enables whichever control matches the selectedCountry, disabling the other so it's excluded from submission (186f6ad)Countrywas reordered ahead ofPostalso the toggle is set before the user reachesProvince(b5c56d3)FaxNumbermoved next toPhoneNumberinExtendedContactto make room (df82708)Country's default value updated to ISO 3155-1 to match the migrated OnTopic data (106a8cb)Documentation
Areas/Forms/HubSpot/README.mddocumenting manifest structure, field mapping,valueMap, and sync failure behavior (859c1df)Cleanup
Addressinto its parent so its properties map without a prefix (14bea59)QuoteFormBindingModelto reuse the sameExtendedContactattributesFormsController'sSendCustomerReceipt()to a sharedstatic readonly HttpClient(8c133db)IHttpClientFactorymigration onRecaptchaValidatorfor consistency (a555a5a).gitignorefor JetBrains/Junie placeholders and Claude'sdocsplanning folder (d6d2b34)