Allow hidden data to be sent separately #128
Merged
Conversation
This file contains 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
ericaporter
commented
Apr 8, 2024
ericaporter
commented
Apr 8, 2024
asatwal
reviewed
Apr 9, 2024
There was a problem hiding this comment.
@ericaporter - I've suggested a slightly different approach in order to minimise changes, but I might be over simplifying here or misunderstood something. We can discuss anyway. Thanks
ericaporter
commented
Apr 12, 2024
ericaporter
commented
Apr 12, 2024
ericaporter
force-pushed
the
separate-hidden-data
branch
from
8dcb87b to
799aa27
Compare
ericaporter
commented
Apr 12, 2024
ericaporter
commented
Apr 12, 2024
ericaporter
commented
Apr 12, 2024
asatwal
reviewed
Apr 12, 2024
* Add validation for hidden pii fields appearing on both lists * Prevent erroring if config file is missing * Update to runtime error, remove logging
asatwal
reviewed
Apr 18, 2024
There was a problem hiding this comment.
Looks good @ericaporter - I just have a few questions about on the specs.
spec/dfe/analytics/entities_spec.rb
Outdated
| @@ -143,8 +165,7 @@ | |||
| 'entity_table_name' => Candidate.table_name, | |||
| 'event_type' => 'update_entity', | |||
| 'data' => [ | |||
| { 'key' => 'email_address', 'value' => ['bar@baz.com'] }, | |||
| { 'key' => 'first_name', 'value' => ['Jason'] } | |||
| { 'key' => 'email_address', 'value' => ['bar@baz.com'] } | |||
There was a problem hiding this comment.
It seems like we should get first_name here as well as there are no hiddel_pii fields specified for this test ?
There was a problem hiding this comment.
So previously we were but did that functionality make sense? We only update the email address so shouldn't we just get the updated email address? (Unless there is a benefit to sending the unchanged fields?)
There was a problem hiding this comment.
Checked with Steven and it is essential to send complete record of allowable fields... so reverted to previous logic with a few updates to method names to try to make it more clear
ericaporter
added a commit
that referenced
this pull request
May 2, 2024
* Allow hidden data to be sent separately * Add validation for hidden pii fields appearing on both lists * Update to runtime error, remove logging * Add value to expectation * Ensure allowed_attributes send with updated_at attributes
ericaporter
added a commit
that referenced
this pull request
May 2, 2024
* Allow hidden data to be sent separately * Add validation for hidden pii fields appearing on both lists * Update to runtime error, remove logging * Add value to expectation * Ensure allowed_attributes send with updated_at attributes
ericaporter
added a commit
that referenced
this pull request
May 8, 2024
* Allow hidden data to be sent separately * Add validation for hidden pii fields appearing on both lists * Update to runtime error, remove logging * Add value to expectation * Ensure allowed_attributes send with updated_at attributes
ericaporter
added a commit
that referenced
this pull request
Jul 8, 2024
* Add DATA_hidden to events table creation SQL script * Add hidden PII required permissions to GCP custom IAM role setup instructions * Add policy tag setup instructions to GCP setup instructions * Update role configuration to match latest BAT configuration * Add hidden_pii.yml (#121) * Allow hidden data to be sent separately (#128) * Mask hidden_pii from logs * Add validation for hidden pii fields appearing on both lists * Change DATA_hidden to hidden_DATA in google_cloud_bigquery_setup.md * Change DATA_hidden to hidden_DATA in create-events-table.sql * Cover edge cases for validations (#146) * Update log masking method (#150) * Add reference to hidden_pii to cutom events (#153)
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.
Ticket
This PR introduces a with_hidden_data event field
Hidden data is kept separate to both allowed_data and allowed_pii