Add Ability to Delete All Saved Passwords #2265

aataraxiaa · 2024-02-26T16:01:48Z

Task/Issue URL: https://app.asana.com/0/72649045549333/1206560791755490/f
Tech Design URL: NA
CC:

Description:
This PR adds the ability to delete all passwords via a single action. It also includes changes to the Settings screen.

Steps to test this PR:

Delete All Option Not Available if No Passwords Saved

Open the Autofill menu
Ensure you have no passwords saved
Open the Autofill more menu
Observe the ‘Delete All Passwords…’ menu item is disabled

Delete All (Single Password) - Sync Disabled

Open the Autofill menu
Add a single password
Open the Autofill more menu
Select the ‘Delete All Passwords’ menu item
Observe an alert appears with the relevant copy (e.g "(1)” is shown as the number of passwords, copy contains no references to sync)
Choose ‘Delete'
Observe the user is asked to authenticate
Authenticate
Observe an alert appears with confirmation of delete copy
Open the Autofill menu
Observe all passwords are deleted

Delete All (Multiple Passwords) - Sync Disabled

Open the Autofill menu
Create or Import multiple passwords
Open the Autofill more menu
Select the ‘Delete All Passwords’ menu item
Observe an alert appears with the relevant copy (e.g “(1000)” is shown as the number of passwords, copy contains no references to sync)
Choose ‘Delete'
Observe the user is asked to authenticate
Authenticate
Observe an alert appears with confirmation of delete copy
Open the Autofill menu
Observe all passwords are deleted

Delete All (Single Password) - Sync Enabled

Repeat Delete All (Single Password) - Sync Disabled test steps, with the only change being the copy displayed should be relevant to sync

Delete All (Multiple Passwords) - Sync Enabled

Repeat Delete All (Multiple Passwords) - Sync Disabled test steps, with the only change being the copy displayed should be relevant to sync

Settings -> Autofill -> Import/Export Hide/Show

Navigate to Settings -> Autofill
Select DuckDuckGo as the password manager
Observe the import and export password radio buttons are visible
Select Bitwarden as the password manager (The setup dialog appears)
Observe the import and export password radio buttons are hidden

Authentication

Repeat a Delete all flow but this time lock the macbook at the stage where you are asked to authenticate
Confirm authentication is still required upon unlocking the device

Additional Smoke Tests

Lightly test search & confirm everything still works correctly

<!—
Tagging instructions
If this PR isn't ready to be merged for whatever reason it should be marked with the DO NOT MERGE label (particularly if it's a draft)
If it's pending Product Review/PFR, please add the Pending Product Review label.

If at any point it isn't actively being worked on/ready for review/otherwise moving forward (besides the above PR/PFR exception) strongly consider closing it (or not opening it in the first place). If you decide not to close it, make sure it's labelled to make it clear the PRs state and comment with more information.
—>

—

Internal references:

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

github-actions · 2024-02-26T16:03:31Z

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

Generated by 🚫 dangerJS against 4187dee

… and associated unit tests

…fillActionExecutor

…jection/testing

…view model, rename notification

…esign Readiness Review feedback

…management, add scaffolding for alternate text based on Sync being enabled

aataraxiaa · 2024-02-27T10:47:33Z

DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved

@@ -14,8 +14,7 @@
      "kind" : "remoteSourceControl",
      "location" : "https://github.com/duckduckgo/BrowserServicesKit",
      "state" : {
-        "revision" : "04c35220aa94bd005171086acccadd677400e7d5",
-        "version" : "111.0.2"
+        "revision" : "f087cf8389926b58692870bd5a459bad492ce66d"


I’ll update this to point to the latest version once the related BSK PR is merged.

aataraxiaa · 2024-02-27T10:48:26Z

DuckDuckGo/Common/Localizables/UserText.swift

@@ -1059,6 +1059,63 @@ struct UserText {
        }
    }

+    // MARK: Autofill Item Deletion (Autofill -> More Menu, Settings -> Autofill)


Note: It’s possible copy with change after review.

There is actually a dedicated UserText+PasswordManager.swift that contains all the autofill related copy; perhaps these strings can be moved there too? 🙂

aataraxiaa · 2024-02-27T10:52:02Z

DuckDuckGo/Autofill/AutofillActionExecutor.swift

+
+    var confirmationAlert: NSAlert {
+        let accounts = (try? secureVault.accounts()) ?? []
+        let syncEnabled = syncService.account != nil


Note: I think on iOS we went with checking if account.authenticated is true. I’m unsure which is the better check, but we check the accounts object at another point in the macOS codebase.

Good question, in the iOS codebase we're usually using syncService.authState == .inactive which is what I went with.

Ok, I've checked with Dominik and he says syncService.authState == .inactive is best to use as:

we have more states between inactive and active and every other than inactive means that sync is enabled or being enabled

Great, thanks @amddg44, I’ll update PR and Ship Review build

amddg44

LGTM! V nice work @aataraxiaa

…lection after data refresh

…ty for clarity

…cess Alert

DuckDuckGo/Localizable.xcstrings

@mallexxx

Task/Issue URL: https://app.asana.com/0/72649045549333/1206560791755490/f Description: Last remaining translation for Delete All Password feature Additional changes to Russian translations (as requested by @mallexxx)

github-actions bot added the bot: not in app board Added by automation for pull requests with tasks not added to macOS App Board Asana project label Feb 26, 2024

aataraxiaa marked this pull request as ready for review February 27, 2024 10:30

aataraxiaa removed the bot: not in app board Added by automation for pull requests with tasks not added to macOS App Board Asana project label Feb 27, 2024

aataraxiaa added 18 commits February 27, 2024 10:33

Implement AutofillActionExecutor, concrete Delete All Passwords type,…

cfc7281

… and associated unit tests

Add initial Strings (copy to be approved)

0be6ca3

Implement AutofillActionPresenter, create custom NSAlert, update Auto…

2569353

…fillActionExecutor

Add Autofill More menu item & validation, add new image for alert

5847268

Update AutofillActionExecutor test, add some documentation

b146c7d

Iterate on Action Executor and Presenter, implement Builder to aid in…

b121f4a

…jection/testing

Implement initial Settings UI (Design in Review), implement logic in …

288cfb8

…view model, rename notification

Use Builder in PasswordManagementVC and update notification reference

47db351

Add mocks and testing

3079176

Fix file -> target issues

d79e256

Revert Settings screen related changes and associated code based on D…

675b426

…esign Readiness Review feedback

Hide/show import/export buttons in Settings based on active password …

a259d4e

…management, add scaffolding for alternate text based on Sync being enabled

Remove checkbox, remove new icon

6427f97

Iterate on copy based on in-progress copy-review

b5be13b

Update copy strings

26da4cc

Update copy

5f259ef

Fix extra period in copy

09a96ed

Fix rebase package resolved

418020e

aataraxiaa force-pushed the pete/delete-all-passwords branch from 8a0288e to 418020e Compare February 27, 2024 10:46

aataraxiaa commented Feb 27, 2024

View reviewed changes

aataraxiaa requested a review from amddg44 February 27, 2024 10:52

aataraxiaa added 2 commits February 27, 2024 10:59

Improve naming

41d45c7

PR feedback: Check authState instead of nil for syncEnabled

eaed1af

amddg44 approved these changes Feb 27, 2024

View reviewed changes

aataraxiaa added 21 commits February 28, 2024 11:00

Update BSK reference to PR branch

8611884

Package errors

64808f8

Merge branch 'main' into pete/delete-all-passwords

48611d5

Merge latest main to pick up BSK changes

e03edcd

Update copy based on copy review

aac7d7c

Merge latest main

c08178d

Show import button on empty passwords

51ff740

Display approapriate empty state when switching autofill type

7d91ccc

PR feedback, build feedback: Fix old copy, implement autofill type se…

04cd79e

…lection after data refresh

Merge branch 'main' into pete/delete-all-passwords

16cf922

Update BSK reference after BSK main merge

df094e0

Undo previous change, implement not ideal fix, rename existing proper…

b871ee1

…ty for clarity

Improve comment

8a57841

Update renamed property reference in tests

adc5d6e

Ship Review Feedback: Refresh Autofill UI while before displaying suc…

e6d65a8

…cess Alert

Merge branch 'main' into pete/delete-all-passwords

73331ad

Import translations

91e4507

Fix string key

635f550

Merge latest main

c9cbd5b

Fix merge project package issues

9ff31f4

Remove bad references introduced by other work into main

670101f

aataraxiaa commented Mar 7, 2024

View reviewed changes

DuckDuckGo/Localizable.xcstrings Show resolved Hide resolved

aataraxiaa added 4 commits March 7, 2024 16:21

Translations for Delete All feature (#2335)

34b885b

Task/Issue URL: https://app.asana.com/0/72649045549333/1206560791755490/f Description: Last remaining translation for Delete All Password feature Additional changes to Russian translations (as requested by @mallexxx)

Merge main

01452cb

Fix merge conflict issues

76b7372

Undo submodule commit\

4187dee

aataraxiaa merged commit 7ef31da into main Mar 8, 2024
17 checks passed

aataraxiaa deleted the pete/delete-all-passwords branch March 8, 2024 10:28

aataraxiaa restored the pete/delete-all-passwords branch March 8, 2024 10:58

aataraxiaa deleted the pete/delete-all-passwords branch March 8, 2024 10:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Ability to Delete All Saved Passwords #2265

Add Ability to Delete All Saved Passwords #2265

aataraxiaa commented Feb 26, 2024 •

edited

Loading

github-actions bot commented Feb 26, 2024 •

edited

Loading

aataraxiaa Feb 27, 2024

aataraxiaa Feb 27, 2024

amddg44 Feb 27, 2024

aataraxiaa Feb 27, 2024

amddg44 Feb 27, 2024

amddg44 Feb 27, 2024

aataraxiaa Feb 27, 2024

amddg44 left a comment

Add Ability to Delete All Saved Passwords #2265

Add Ability to Delete All Saved Passwords #2265

Conversation

aataraxiaa commented Feb 26, 2024 • edited Loading

Internal references:

github-actions bot commented Feb 26, 2024 • edited Loading

aataraxiaa Feb 27, 2024

Choose a reason for hiding this comment

aataraxiaa Feb 27, 2024

Choose a reason for hiding this comment

amddg44 Feb 27, 2024

Choose a reason for hiding this comment

aataraxiaa Feb 27, 2024

Choose a reason for hiding this comment

amddg44 Feb 27, 2024

Choose a reason for hiding this comment

amddg44 Feb 27, 2024

Choose a reason for hiding this comment

aataraxiaa Feb 27, 2024

Choose a reason for hiding this comment

amddg44 left a comment

Choose a reason for hiding this comment

aataraxiaa commented Feb 26, 2024 •

edited

Loading

github-actions bot commented Feb 26, 2024 •

edited

Loading