Skip to content
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

feat(Onboarding): implement the KeycardFactoryReset flow #17167

Merged
merged 2 commits into from
Feb 4, 2025

Conversation

caybro
Copy link
Member

@caybro caybro commented Jan 30, 2025

What does the PR do

  • integrate it into the UI and StoryBook
  • a new keycardState is introduced: FactoryResetting (matching the backend)
  • a new store method introduced: startKeycardFactoryReset()

Fixes: #17094

Affected areas

Onboarding

Architecture compliance

Screenshot of functionality (including design for comparison)

  • I've checked the design and this PR matches it
Zaznam.obrazovky.z.2025-01-31.15-20-57.mp4

@caybro caybro linked an issue Jan 30, 2025 that may be closed by this pull request
@@ -47,7 +47,7 @@ SQUtils.QObject {

function finishWithFactoryReset() {
root.keycardFactoryResetRequested()
root.finished()
root.finished(false)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't want to proceed with the original flow when we request a Factory reset

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better to emit a different signal then?

@@ -86,16 +85,10 @@ Control {
}
MaybeOutlineButton {
width: parent.width
visible: root.keycardState === Onboarding.KeycardState.BlockedPIN
visible: root.keycardState === Onboarding.KeycardState.BlockedPIN || root.keycardState === Onboarding.KeycardState.BlockedPUK
Copy link
Member Author

@caybro caybro Jan 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Unblock with recovery phrase" is always visible on the Login screen

text: qsTr("Unblock with recovery phrase")
onClicked: root.unblockWithSeedphraseRequested()
}
MaybeOutlineButton {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.. and Factory reset never visible on the Login screen

@status-im-auto
Copy link
Member

status-im-auto commented Jan 30, 2025

Jenkins Builds

Click to see older builds (45)
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 0513de1 #1 2025-01-30 14:24:39 ~8 min macos/aarch64 🍎dmg
✔️ 0513de1 #1 2025-01-30 14:25:17 ~8 min tests/nim 📄log
0513de1 #1 2025-01-30 14:28:35 ~12 min tests/ui 📄log
✔️ 0513de1 #1 2025-01-30 14:31:50 ~15 min macos/x86_64 🍎dmg
✔️ 0513de1 #1 2025-01-30 14:38:11 ~21 min linux/x86_64 📦tgz
✔️ 0513de1 #1 2025-01-30 14:39:57 ~23 min linux-nix/x86_64 📦tgz
✔️ 0513de1 #1 2025-01-30 14:43:17 ~26 min windows/x86_64 💿exe
✔️ 0c3b4df #2 2025-01-30 15:09:18 ~4 min macos/aarch64 🍎dmg
✔️ 0c3b4df #2 2025-01-30 15:13:28 ~8 min tests/nim 📄log
✔️ 0c3b4df #2 2025-01-30 15:15:41 ~10 min macos/x86_64 🍎dmg
0c3b4df #2 2025-01-30 15:17:32 ~12 min tests/ui 📄log
✔️ 0c3b4df #2 2025-01-30 15:25:43 ~21 min linux-nix/x86_64 📦tgz
✔️ 0c3b4df #2 2025-01-30 15:26:09 ~21 min linux/x86_64 📦tgz
✔️ 0c3b4df #2 2025-01-30 15:27:06 ~22 min windows/x86_64 💿exe
✔️ 2beff66 #4 2025-01-30 17:11:49 ~7 min macos/aarch64 🍎dmg
✔️ 2beff66 #4 2025-01-30 17:13:15 ~9 min tests/nim 📄log
2beff66 #4 2025-01-30 17:16:50 ~12 min tests/ui 📄log
✔️ 2beff66 #4 2025-01-30 17:19:35 ~15 min macos/x86_64 🍎dmg
✔️ 2beff66 #4 2025-01-30 17:25:06 ~20 min linux/x86_64 📦tgz
✔️ 2beff66 #4 2025-01-30 17:27:02 ~22 min linux-nix/x86_64 📦tgz
✔️ 2beff66 #4 2025-01-30 17:30:10 ~25 min windows/x86_64 💿exe
2beff66 #5 2025-01-30 22:32:26 ~11 min tests/ui 📄log
✔️ 2beff66 #6 2025-01-30 23:02:58 ~12 min tests/ui 📄log
99db6d8 #5 2025-01-30 23:45:17 ~5 min macos/aarch64 📄log
✔️ 99db6d8 #5 2025-01-30 23:48:29 ~8 min tests/nim 📄log
✔️ 99db6d8 #7 2025-01-30 23:52:26 ~12 min tests/ui 📄log
✔️ 99db6d8 #5 2025-01-30 23:52:33 ~12 min macos/x86_64 🍎dmg
✔️ 99db6d8 #5 2025-01-30 23:57:46 ~17 min linux-nix/x86_64 📦tgz
✔️ 99db6d8 #5 2025-01-31 00:00:14 ~20 min linux/x86_64 📦tgz
✔️ 99db6d8 #5 2025-01-31 00:05:20 ~25 min windows/x86_64 💿exe
✔️ 866faf0 #7 2025-01-31 11:22:28 ~5 min macos/aarch64 🍎dmg
✔️ 866faf0 #6 2025-01-31 11:26:25 ~9 min tests/nim 📄log
✔️ 866faf0 #6 2025-01-31 11:28:22 ~11 min macos/x86_64 🍎dmg
866faf0 #8 2025-01-31 11:29:01 ~11 min tests/ui 📄log
✔️ 866faf0 #6 2025-01-31 11:37:16 ~20 min linux-nix/x86_64 📦tgz
✔️ 866faf0 #6 2025-01-31 11:37:37 ~20 min linux/x86_64 📦tgz
✔️ 866faf0 #6 2025-01-31 11:41:01 ~23 min windows/x86_64 💿exe
✔️ 866faf0 #9 2025-01-31 11:51:42 ~12 min tests/ui 📄log
✔️ 13d0c83 #8 2025-01-31 14:17:04 ~5 min macos/aarch64 🍎dmg
✔️ 13d0c83 #7 2025-01-31 14:21:00 ~9 min tests/nim 📄log
✔️ 13d0c83 #7 2025-01-31 14:24:15 ~12 min macos/x86_64 🍎dmg
✔️ 13d0c83 #10 2025-01-31 14:24:49 ~13 min tests/ui 📄log
✔️ 13d0c83 #7 2025-01-31 14:31:43 ~20 min linux-nix/x86_64 📦tgz
✔️ 13d0c83 #7 2025-01-31 14:33:04 ~21 min linux/x86_64 📦tgz
✔️ 13d0c83 #7 2025-01-31 14:38:23 ~26 min windows/x86_64 💿exe
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 7f39e36 #9 2025-02-03 13:06:38 ~5 min macos/aarch64 🍎dmg
✔️ 7f39e36 #8 2025-02-03 13:10:12 ~9 min tests/nim 📄log
✔️ 7f39e36 #8 2025-02-03 13:14:00 ~12 min macos/x86_64 🍎dmg
✔️ 7f39e36 #11 2025-02-03 13:14:15 ~13 min tests/ui 📄log
✔️ 7f39e36 #8 2025-02-03 13:20:08 ~19 min linux-nix/x86_64 📦tgz
✔️ 7f39e36 #8 2025-02-03 13:21:17 ~20 min linux/x86_64 📦tgz
✔️ 7f39e36 #8 2025-02-03 13:29:24 ~28 min windows/x86_64 💿exe
✔️ 82a21b1 #10 2025-02-04 21:22:21 ~6 min macos/aarch64 🍎dmg
✔️ 82a21b1 #9 2025-02-04 21:24:38 ~8 min tests/nim 📄log
✔️ 82a21b1 #9 2025-02-04 21:28:45 ~12 min macos/x86_64 🍎dmg
✔️ 82a21b1 #12 2025-02-04 21:28:58 ~12 min tests/ui 📄log
✔️ 82a21b1 #9 2025-02-04 21:35:10 ~19 min linux-nix/x86_64 📦tgz
✔️ 82a21b1 #9 2025-02-04 21:36:19 ~20 min linux/x86_64 📦tgz
✔️ 82a21b1 #9 2025-02-04 21:42:24 ~26 min windows/x86_64 💿exe

@caybro caybro force-pushed the 17094-onboarding-factory-reset-keycard-flow branch 3 times, most recently from 88c35d7 to 2beff66 Compare January 30, 2025 17:03
@caybro caybro requested a review from igor-sirotin January 30, 2025 17:06
@caybro caybro force-pushed the 17094-onboarding-factory-reset-keycard-flow branch 3 times, most recently from 866faf0 to 13d0c83 Compare January 31, 2025 14:11
@caybro caybro marked this pull request as ready for review January 31, 2025 14:14
@caybro caybro requested review from micieslak, alexjba and a team as code owners January 31, 2025 14:14
@caybro caybro removed the request for review from a team January 31, 2025 14:14
Copy link
Contributor

@igor-sirotin igor-sirotin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be glad if we can merge it after #17127 🙂

storybook/pages/OnboardingLayoutPage.qml Outdated Show resolved Hide resolved
@@ -66,6 +66,7 @@ class OnboardingEnums: public QObject
MaxPairingSlotsReached,
BlockedPIN, // PIN remaining attempts == 0
BlockedPUK, // PUK remaining attempts == 0
FactoryResetting,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If merged after #17127, please add it to here as well:

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, I can wait, will add this state

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@igor-sirotin what about the missing startKeycardFactoryReset() NIM method?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, I added the state in my PR: 2a25314

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@caybro What's the startKeycardFactoryReset, I have no idea about it

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should invoke the actual factory reset method on the backend side, via the respective store function

@@ -47,7 +47,7 @@ SQUtils.QObject {

function finishWithFactoryReset() {
root.keycardFactoryResetRequested()
root.finished()
root.finished(false)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better to emit a different signal then?

Copy link
Member

@micieslak micieslak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but Ii think some basic test should be added as for other flows

@caybro
Copy link
Member Author

caybro commented Feb 3, 2025

LGTM but Ii think some basic test should be added as for other flows

Yeah I will add an extra issue/task to extend the QML tests with these "unblock" flows; afaik we don't really test any of these yet

@caybro caybro force-pushed the 17094-onboarding-factory-reset-keycard-flow branch from 13d0c83 to 7f39e36 Compare February 3, 2025 13:00
@caybro
Copy link
Member Author

caybro commented Feb 3, 2025

Blocked by #17127

@caybro caybro marked this pull request as draft February 3, 2025 13:01
@caybro caybro added the blocked label Feb 3, 2025
- in other words, stop if we want some other flow instead, e.g. the
factory reset
- integrate it into the UI and StoryBook
- a new keycardState is introduced: `FactoryResetting` (matching the
backend)
- a new store method introduced: `startKeycardFactoryReset()`

Fixes: #17094
@caybro caybro force-pushed the 17094-onboarding-factory-reset-keycard-flow branch from 7f39e36 to 82a21b1 Compare February 4, 2025 21:15
@caybro caybro removed the blocked label Feb 4, 2025
@caybro caybro marked this pull request as ready for review February 4, 2025 21:15
@caybro caybro merged commit 09bdb95 into master Feb 4, 2025
9 checks passed
@caybro caybro deleted the 17094-onboarding-factory-reset-keycard-flow branch February 4, 2025 21:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Onboarding] "Factory reset" keycard flow
4 participants