Add required information to DBP NA<->FE API for removal timeline support #3268

THISISDINOSAUR · 2024-09-12T19:20:12Z

Task/Issue URL: https://app.asana.com/0/1203581873609357/1208164179397484/f
Tech Design URL:
Initial API additions: https://app.asana.com/0/481882893211075/1208203761791777/f
Parent - child api info and matching logic: https://app.asana.com/0/72649045549333/1208303545057273/f
Where to put the matching logic: https://app.asana.com/0/481882893211075/1208325118544624/f
CC:

Description:

Adds four new fields as per the first tech design. Also moves some of the construction of these objects to constructors.
Also removes an old date field that isn't officially in the api, and apparently varied what it was used for. It seemed unused on the native side, and the FE side doesn't know it exist.
It then also adds "parent child" matching logic so we can identify orphaned child records that will never be removed.

Steps to test this PR:

In DataBrokerProtectionWebUIURLSettings, change the production URL to "https://use-devtesting27.duckduckgo.com/dbp", Alex made an FE build we can use to test
In the DBP UI, click on individual records, this will show you things like the opt out submitted date
Make sure these values make sense, e.g. we don't ever show 1970, the timeline follows linear time, nothing violates causality etc.

Definition of Done:

Does this PR satisfy our Definition of Done?

Internal references:

Pull Request Review Checklist
Software Engineering Expectations
Technical Design Template
Pull Request Documentation

THISISDINOSAUR · 2024-09-12T19:25:20Z

...ckages/DataBrokerProtection/Sources/DataBrokerProtection/Model/DBPUICommunicationModel.swift

+                  addresses: extractedProfile.addresses?.map {DBPUIUserProfileAddress(addressCityState: $0) } ?? [],
+                  alternativeNames: extractedProfile.alternativeNames ?? [String](),
+                  relatives: extractedProfile.relatives ?? [String](),
+                  foundDate: optOutJobData.createdDate.timeIntervalSince1970,


I don't love that this information is gained from the optOutJobData, but what you can't see from looking just at this struct is if you see further above is that all these extracted profiles are taken from the opt out jobs the first place.
So I don't think there's anything I can do about it outside of making more serious challenges to our data architecture (which I'd love to do. In making this PR I really noticed how my issues with the DB scheme trickle all the way up to cause a lot of the things I don't like in the UI mapping stuff)

… if it exists

github-actions · 2024-09-18T17:03:33Z

	Warnings
⚠️	PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS against cea7624

THISISDINOSAUR · 2024-09-19T12:27:34Z

...ckages/DataBrokerProtection/Sources/DataBrokerProtection/Model/DBPUICommunicationModel.swift

+}
+
+extension DBPUIDataBrokerProfileMatch {
+    init(optOutJobData: OptOutJobData,


I don't really like how long this construct got, but didn't have any nice ideas to simplify it, so taking suggestions!

Tried rewriting this a different way, but it was mostly just different, not necessarily better. However, leaving one suggestion.

THISISDINOSAUR · 2024-09-19T12:28:56Z

LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/UI/UIMapper.swift

@@ -22,36 +22,6 @@ import os.log

 struct MapperToUI {

-    func mapToUI(_ dataBroker: DataBroker, extractedProfile: ExtractedProfile) -> DBPUIDataBrokerProfileMatch {


All this has now moved into constructors on the individual UI objects

THISISDINOSAUR · 2024-09-19T12:29:46Z

LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/UI/UIMapper.swift

@@ -74,21 +44,39 @@ struct MapperToUI {
                                             totalScans: totalScans,
                                             scannedBrokers: partiallyScannedBrokers)

-        let matches = mapMatchesToUI(brokerProfileQueryData)
+        let matches = mapMatchesToUI(withoutDeprecated)

        return .init(resultsFound: matches, scanProgress: scanProgress)
    }

    private func mapMatchesToUI(_ brokerProfileQueryData: [BrokerProfileQueryData]) -> [DBPUIDataBrokerProfileMatch] {


I don't like how much this and the maintenance scans function duplicate each other. However I'm also afraid to touch them since there seem to be some subtle differences which I assume are deliberate

aataraxiaa

Nice one @THISISDINOSAUR ! Great work adding tests to the UI model layer 🥳. Left a few comments but nothing blocking. However, I’ll wait to approve until I can test. I ran and the UI just hung. Maybe you can check and if we can confirm it’s only a me issue, I’ll try again.

aataraxiaa · 2024-09-23T18:11:42Z

...ckages/DataBrokerProtection/Sources/DataBrokerProtection/Model/DBPUICommunicationModel.swift

@@ -135,7 +146,79 @@ struct DBPUIDataBrokerProfileMatch: Codable {
    let addresses: [DBPUIUserProfileAddress]
    let alternativeNames: [String]
    let relatives: [String]
-    let date: Double? // Used in some methods to set the removedDate or found date
+    let foundDate: Double


aataraxiaa · 2024-09-23T18:21:42Z

...ckages/DataBrokerProtection/Sources/DataBrokerProtection/Model/DBPUICommunicationModel.swift

+}
+
+extension DBPUIDataBrokerProfileMatch {
+    init(optOutJobData: OptOutJobData,


Tried rewriting this a different way, but it was mostly just different, not necessarily better. However, leaving one suggestion.

...ckages/DataBrokerProtection/Sources/DataBrokerProtection/Model/DBPUICommunicationModel.swift

aataraxiaa · 2024-09-23T18:24:29Z

LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Model/ExtractedProfile.swift

+        if age != extractedProfile.age {
+            return false
+        }
+
+        if name != extractedProfile.name {
+            return false
+        }
+
+        if !(alternativeNames ?? []).isASubSetOrSuperSetOf(extractedProfile.alternativeNames ?? []) {
+            return false
+        }
+
+        if !(addresses ?? []).isASubSetOrSuperSetOf(extractedProfile.addresses ?? []) {
+            return false
+        }
+
+        if !(relatives ?? []).isASubSetOrSuperSetOf(extractedProfile.relatives ?? []) {
+            return false
+        }


Prob just a preference, but guard statements seem to fit this pattern better IMO (where we return true at the end if all conditions pass). Again, maybe just subjective preference.

I thought about this and I think I prefer not using guards because to me guard suggests something went wrong or some prerequisite weren't satisfied. I think that is a biased view on my part though and probably not shared universally

Fair, one subjective view -vs- another 😅! Fine with me to stick with what you have.

aataraxiaa · 2024-09-23T18:24:44Z

LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/UI/UIMapper.swift

@@ -22,36 +22,6 @@ import os.log

 struct MapperToUI {

-    func mapToUI(_ dataBroker: DataBroker, extractedProfile: ExtractedProfile) -> DBPUIDataBrokerProfileMatch {


aataraxiaa · 2024-09-23T18:24:59Z

LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/UI/UIMapper.swift

@@ -74,21 +44,39 @@ struct MapperToUI {
                                             totalScans: totalScans,
                                             scannedBrokers: partiallyScannedBrokers)

-        let matches = mapMatchesToUI(brokerProfileQueryData)
+        let matches = mapMatchesToUI(withoutDeprecated)

        return .init(resultsFound: matches, scanProgress: scanProgress)
    }

    private func mapMatchesToUI(_ brokerProfileQueryData: [BrokerProfileQueryData]) -> [DBPUIDataBrokerProfileMatch] {


aataraxiaa

Tested and looks good @THISISDINOSAUR ! 👍🏼

# Conflicts: # LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/UI/DBPUICommunicationLayer.swift

THISISDINOSAUR added 3 commits September 12, 2024 20:16

Add new fields to DBP->FE API, and associated constructors

1c82d0f

Make UIMapper use new constructors

6388873

Remove unused data field

24d3fc9

THISISDINOSAUR commented Sep 12, 2024

View reviewed changes

THISISDINOSAUR added the DO NOT MERGE label Sep 13, 2024

THISISDINOSAUR added 9 commits September 13, 2024 12:36

Merge branch 'main' into elle/add-removal-timeline-support

19ba1c8

Refactor DBPUIDateBrokerProfileMatch inits to reduce duplication

2f45004

Fix DBPUIDataBrokerProfileMatch creation when created date is default

9e326ef

Add unit tests for DBPUIDataBrokerProfileMatch creation

fa5f675

Change estimatedRemovalDate to be calculated from optOutSubmittedDate…

59988dd

… if it exists

Add match testing function to Extracted profile

0865a78

Add unit tests for matching function

9aa5b41

Add parent matching to DBPUIDataBrokerProfileMatch

1af1ebe

Add UIMapper tests for profile matching

29c8bb8

THISISDINOSAUR removed the DO NOT MERGE label Sep 18, 2024

Add parent URL field to UIDataBroker

c2d4d9f

THISISDINOSAUR commented Sep 19, 2024

View reviewed changes

Fix incorrect data being given when looking for parent matches

06b1020

THISISDINOSAUR requested a review from aataraxiaa September 23, 2024 15:55

aataraxiaa reviewed Sep 23, 2024

View reviewed changes

Simplify parent match logic

88fb01b

aataraxiaa approved these changes Sep 24, 2024

View reviewed changes

THISISDINOSAUR added 3 commits September 25, 2024 14:35

Fix estimated removal date not being propagated to completed opt outs

608bd9f

bump native - FE api version number

b389b6b

Merge branch 'main' into elle/add-removal-timeline-support

cea7624

# Conflicts: # LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/UI/DBPUICommunicationLayer.swift

THISISDINOSAUR merged commit f8a3914 into main Sep 25, 2024
18 checks passed

THISISDINOSAUR deleted the elle/add-removal-timeline-support branch September 25, 2024 18:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add required information to DBP NA<->FE API for removal timeline support #3268

Add required information to DBP NA<->FE API for removal timeline support #3268

THISISDINOSAUR commented Sep 12, 2024 •

edited

Loading

THISISDINOSAUR Sep 12, 2024

github-actions bot commented Sep 18, 2024 •

edited

Loading

THISISDINOSAUR Sep 19, 2024

aataraxiaa Sep 23, 2024

THISISDINOSAUR Sep 19, 2024

aataraxiaa Sep 23, 2024

THISISDINOSAUR Sep 19, 2024

aataraxiaa Sep 23, 2024

aataraxiaa left a comment

aataraxiaa Sep 23, 2024

aataraxiaa Sep 23, 2024

aataraxiaa Sep 23, 2024

THISISDINOSAUR Sep 24, 2024

aataraxiaa Sep 24, 2024

aataraxiaa Sep 23, 2024

aataraxiaa Sep 23, 2024

aataraxiaa left a comment

		@@ -22,36 +22,6 @@ import os.log

		struct MapperToUI {

		func mapToUI(_ dataBroker: DataBroker, extractedProfile: ExtractedProfile) -> DBPUIDataBrokerProfileMatch {

Add required information to DBP NA<->FE API for removal timeline support #3268

Add required information to DBP NA<->FE API for removal timeline support #3268

Conversation

THISISDINOSAUR commented Sep 12, 2024 • edited Loading

Internal references:

Choose a reason for hiding this comment

github-actions bot commented Sep 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aataraxiaa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aataraxiaa left a comment

Choose a reason for hiding this comment

THISISDINOSAUR commented Sep 12, 2024 •

edited

Loading

github-actions bot commented Sep 18, 2024 •

edited

Loading