-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Prevent broker data model mismatch during development #3629
base: main
Are you sure you want to change the base?
Conversation
"The saveProfile() issue happens due to broker data model mismatch during JSON decoding" |
@@ -22,6 +22,12 @@ import UserScript | |||
import os.log | |||
import Combine | |||
|
|||
func testableAssertionFailure(_ message: @autoclosure () -> String = String(), file: StaticString = #file, line: UInt = #line) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO this should probably live somewhere more general rather than localized to DBP. Do we do anything like this anywhere else on macOS?
@@ -174,12 +181,17 @@ public struct DataBroker: Codable, Sendable { | |||
|
|||
version = try container.decode(String.self, forKey: .version) | |||
steps = try container.decode([Step].self, forKey: .steps) | |||
// Hotfix for https://app.asana.com/0/1203581873609357/1208895331283089/f | |||
|
|||
// Fallback for older versions of the JSON file not having a maxAttempts property. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be good to explain why this is necessary given we do DB migrations
This PR has been inactive for more than 7 days and will be automatically closed 7 days from now. |
This PR has been closed after 14 days of inactivity. Feel free to reopen it if you plan to continue working on it or have further discussions. |
Task/Issue URL: https://app.asana.com/0/1203581873609357/1208902266278746/f
Tech Design URL:
CC:
Description:
The
saveProfile()
issue happens due to broker data model mismatch during JSON decoding which preventsfetchAllBrokerProfileQueryData()
from working. It fires a bunch of generic pixels but that behaviour is too forgiving and relatively easy to miss.Adding an
assertionFailure()
to make this more visible during development.Optional E2E tests:
Check this to run the Personal Information Removal end to end tests. If updating CCF, or any PIR related code, tick this.
Steps to test this PR:
DataBrokerScheduleConfig
init(from decoder: Decoder)
comment out the try catch block forschedulingConfig
and rely entirely onschedulingConfig = try container.decode(DataBrokerScheduleConfig.self, forKey: .schedulingConfig)
assertionFailure
should kick in and show the issue(If
assertionFailure
is commented out, we have the same behaviour as in the incident: The first PIR screen is Edit Profile, and any attempt to update it would result in An error occurred)Definition of Done:
Internal references:
Pull Request Review Checklist
Software Engineering Expectations
Technical Design Template
Pull Request Documentation