-
Notifications
You must be signed in to change notification settings - Fork 295
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
[DDW-536] Service agreement can be skipped by accessing to Settings #2304
[DDW-536] Service agreement can be skipped by accessing to Settings #2304
Conversation
…ipped-by-accessing-to-settings
Hi @yakovkaravelov. Tested on build 15928. Works really well when on the Service Level Agreement screen. However Service Level Agreement can be skipped with scenario 1(If a wallet is saved in state directory from before) and 2 as these options are available still from the initial settings screen
|
@ManusMcCole This was fixed. Please test again, thanks. |
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.
Goodjob Yakov. Looks good 👍
The test cases were updated to validate "Daedalus Diagnostics" (cases 4, 8, 12) and also to validate all the options after submitting the initial settings and restarting Daedalus (cases 13 to 16). @yakovkaravelov On build 15934 the cases 1 to 12 passed (pending to implement 13 to 16). |
@gnpf @nikolaglumac This is ready for review/test. |
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.
@yakovkaravelov please see my comments. Thanks!
@nikolaglumac They were renamed, thanks. |
…ipped-by-accessing-to-settings
@ManusMcCole please re-test this one 🙏 |
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.
👍
await this.getTermsOfUseAcceptanceRequest.execute(); | ||
if (this.getTermsOfUseAcceptanceRequest.result) { | ||
await enableApplicationMenuNavigationChannel.send(); | ||
} else { |
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.
@yakovkaravelov I don't think we need this else clause as the menu navigation is disabled by default?
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.
@yakovkaravelov nice work!
Now with the latest changes, I believe we actually don't even need the "disable application menu channel". The navigation is disabled by default and we only need to enable it either when user accepts the terms of use OR if we detect the terms of use have already been accepted in the past.
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.
Looks and works great now @yakovkaravelov 🎉
This PR disabled certain menu items when accepting terms of use.
Jira Ticket
Todos
Settings
,Wallet Settings
,About Daedalus
, andDaedalus Diagnostics
menu items when accepting terms of useTesting Checklist
App
Test Cases
Scenario 1 - Validate "Wallet settings" is disabled on the "Initial Settings" screen
Scenario 2 - Validate "Settings" is disabled is disabled on "Initial Settings" screen
Scenario 3 - Validate "About" is disabled is disabled on "Initial Settings" screen
Scenario 4 - Validate "Daedalus Diagnostics" is disabled is disabled on "Initial Settings" screen
Scenario 5 - Validate "Wallet settings" is disabled on "Service Agreement" screen
Scenario 6 - Validate "Settings" is disabled is disabled on "Service Agreement" screen
Scenario 7 - Validate "About" is disabled is disabled on "Service Agreement" screen
Scenario 8 - Validate "Daedalus diagnostics" is disabled is disabled on "Service Agreement" screen
Scenario 9 - Validate "Wallet settings" is enabled after agreeing on "Service Agreement" screen
Scenario 10 - Validate "Settings" is enabled after agreeing on "Service Agreement" screen
Scenario 11 - Validate "About" is enabled after agreeing on "Service Agreement" screen
Scenario 12 - Validate "Daedalus Diagnostics" is enabled after agreeing on "Service Agreement" screen
Scenario 13 - Validate "Wallet Settings" is disabled after submitting initial settings and restarting Daedalus
Scenario 14 - Validate "Settings" is disabled after submitting initial settings and restarting Daedalus
Scenario 15 - Validate "About" is disabled after submitting initial settings and restarting Daedalus
Scenario 16 - Validate "Daedalus Diagnostics" is disabled after submitting initial settings and restarting Daedalus
Testing Summary
English
Japanese
Review Checklist
Basics
feature
/bug
/chore
,release-x.x.x
)yarn test
)yarn dev
)yarn package
/ CI builds)yarn flow:test
)yarn lint
)yarn prettier:check
)yarn manage:translations
produces no changes)yarn storybook
)yarn.lock
file is updatedCode Quality
Testing
After Review
done
column on the YouTrack board