Configure audiences to log warning on invalid events#553
Configure audiences to log warning on invalid events#553codemonium wants to merge 29 commits intomainfrom
Conversation
This change lets applications that use audiences configure the groups that must be present in a user provisioning event for the event to be considered valid. Some applications do not behave correctly if certain groups are missing.
gregblake
left a comment
gregblake
left a comment
There was a problem hiding this comment.
I think these changes are close to what we need. I added some feedback and questions on comments below.
The addition of the required_group_types method caused the ExternalUser
model to be 101 lines. Rubocop is configured to keep classes at a
maximum of 100 lines.
This change extracts the TERRITORY_ABBBRS constant from the ExternalUser
model. This has two benefits: 1) it resolves the Rubocop violation, and
2) it handles territory abbreviations (which could change on a
per-application basis) as application config. No longer hard coding this
value allows apps that use audiences to do customize the TERRITORY_ABBR
data. For example:
```ruby
Audiences.configure do |config|
config.territory_abbreviations = { "Custom Territory" => "CT" }
end
```
The validation logic now uses ActiveRecord::RecordInvalid.
config.required_group_types is a better match for config.group_types
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Some applications need to subscribe_to this event (for example, to add a new group to a given criteria).
Using dependency injection here also makes the tests simpler.
gregblake
changed the title
With the changes on this commit, the #process method calls Audiences.logger.error(e) on any exception, and then raises the exception. This is consistent with the current behavior on the main branch, when an actual exception is raised. Note that the `#valid_group_types?` method still only logs a warning when there is an user provisioning event where the user is missing one or more required groups. Other changes on this commit: #log_upsert_action was extracted from #process, to keep the Metrics/AbcSize Assignment Branch condition size under the Rubocop threshold.
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 24 hours if no further activity occurs. |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 24 hours if no further activity occurs. |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 24 hours if no further activity occurs. |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 24 hours if no further activity occurs. |
3 participants
This change lets applications that use audiences configure the groups that must be present in a user provisioning event for the event to be considered valid. Some applications do not behave correctly if certain groups are missing.