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.

Sign up for GitHub

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

Jump to bottom

Fix bug domain not shown on signature #6464

Merged
merged 9 commits into from
Jun 7, 2023
Merged

Fix bug domain not shown on signature #6464

merged 9 commits into from
Jun 7, 2023

Conversation

segun
Copy link
Contributor

@segun segun commented May 25, 2023 •
edited
Loading

Description

This PR fixes a bug where domain URL is not displayed on any signature type.

Screenshots/Recordings

Before

After

Screenshot 2023-05-25 at 15 08 24

Issue

Fixes: #6429

Checklist

  • There is a related GitHub issue
  • Tests are included if applicable
  • Any added code is fully documented

@segun segun self-assigned this May 25, 2023
@segun segun requested a review from a team as a code owner May 25, 2023 11:55
@segun segun added needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) team-confirmations-secure-ux-PR PR from the confirmations team labels May 25, 2023
@github-actions
Copy link
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@segun segun force-pushed the fix-bug-domain-not-shown-on-signature-6429 branch from 37f2f64 to 05c0be2 Compare May 25, 2023 20:23
blackdevelopa
blackdevelopa previously approved these changes May 30, 2023
View reviewed changes
@blackdevelopa blackdevelopa mentioned this pull request May 30, 2023
Closed
3 tasks
@seaona seaona added the QA Passed A successful QA run through has been done label May 31, 2023
@seaona
Copy link
Contributor

seaona commented May 31, 2023

This PR looks good from QA @segun. I can see how the Balance is displayed, as well as the domain. Whenever we get x2 dev approvals it can be merged.

image

jpuri
jpuri previously approved these changes Jun 3, 2023
View reviewed changes
Copy link
Contributor

@jpuri jpuri left a comment

Choose a reason for hiding this comment

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

Changes look good, @digiwand has made some useful suggestion hrere.

segun added 9 commits June 5, 2023 17:39
@segun
Make sure origin is shown on SignatureRequest
004fb21 
Signed-off-by: Olusegun Akintayo <[email protected]>
@segun
if asset is undefined, show balance of selected account.
8649d1f 
Signed-off-by: Olusegun Akintayo <[email protected]>
@segun
update snapshot
67d84ef 
Signed-off-by: Olusegun Akintayo <[email protected]>
@segun
lint fix
47d52b8 
Signed-off-by: Olusegun Akintayo <[email protected]>
@segun
Test get balance if address is null
5615d0a 
Signed-off-by: Olusegun Akintayo <[email protected]>
@segun
test show origin header
5dd6ebd 
Signed-off-by: Olusegun Akintayo <[email protected]>
@segun
Lint fixes
e87ec70 
Signed-off-by: Olusegun Akintayo <[email protected]>
@segun
Podfile fix
ac2bdaf 
Signed-off-by: Olusegun Akintayo <[email protected]>
@segun
Fix PR comment: Combine the ifs
cf8545d 
Signed-off-by: Olusegun Akintayo <[email protected]>
@segun segun dismissed stale reviews from jpuri and blackdevelopa via cf8545d June 5, 2023 14:46
@segun segun force-pushed the fix-bug-domain-not-shown-on-signature-6429 branch from 05c0be2 to cf8545d Compare June 5, 2023 14:46
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jun 5, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

81.8% 81.8% Coverage
0.0% 0.0% Duplication

jpuri
jpuri approved these changes Jun 6, 2023
View reviewed changes
Copy link
Contributor

@jpuri jpuri left a comment

Choose a reason for hiding this comment

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

LGTM except for a small feedback.

jpuri
jpuri approved these changes Jun 6, 2023
View reviewed changes
Copy link
Contributor

@jpuri jpuri left a comment

Choose a reason for hiding this comment

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

LGTM except for a small feedback.

chrisleewilcox
chrisleewilcox previously requested changes Jun 6, 2023
View reviewed changes
Copy link
Contributor

@chrisleewilcox chrisleewilcox left a comment

Choose a reason for hiding this comment

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

@seaona @digiwand @jpuri when a PR is QA Passed then adding a review afterwards puts the PR at risk. Not sure the reviews were worth it after being QA Passed but I would like this to go through QA again and would advise QA to merge the PR after QA Passed to avoid further changes after it's been QA Passed.

@chrisleewilcox chrisleewilcox removed the QA Passed A successful QA run through has been done label Jun 6, 2023
@chrisleewilcox chrisleewilcox added the needs-qa Any New Features that needs a full manual QA prior to being added to a release. label Jun 6, 2023
@seaona
Copy link
Contributor

seaona commented Jun 6, 2023

hi @chrisleewilcox correct if there were changes in the PR this resets the x2 reviews as well as the QA approval

@chrisleewilcox chrisleewilcox dismissed their stale review June 6, 2023 17:30

Don't want to hold this up

@seaona
Copy link
Contributor

seaona commented Jun 7, 2023 •
edited
Loading

The new changes look good from QA side @segun

@seaona seaona added QA Passed A successful QA run through has been done release-7.1.0 Issue or pull request that will be included in release 7.1.0 and removed needs-qa Any New Features that needs a full manual QA prior to being added to a release. needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) labels Jun 7, 2023
@seaona
Copy link
Contributor

seaona commented Jun 7, 2023

@segun can we resolve the open conversations so PR can be merged?

@segun segun merged commit 6f92be9 into main Jun 7, 2023
@segun segun deleted the fix-bug-domain-not-shown-on-signature-6429 branch June 7, 2023 15:21
@github-actions github-actions bot locked and limited conversation to collaborators Jun 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@digiwand digiwand digiwand approved these changes

@jpuri jpuri jpuri approved these changes

@blackdevelopa blackdevelopa blackdevelopa left review comments

@chrisleewilcox chrisleewilcox chrisleewilcox left review comments

Assignees

@segun segun

Labels
QA Passed A successful QA run through has been done release-7.1.0 Issue or pull request that will be included in release 7.1.0 team-confirmations-secure-ux-PR PR from the confirmations team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug] Domain is not displayed on any Signature screen
6 participants
@segun @seaona @jpuri @chrisleewilcox @digiwand @blackdevelopa