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 new device flag and refresh meta #923

Merged
merged 13 commits into from
Apr 14, 2021
Merged

Fix new device flag and refresh meta #923

merged 13 commits into from
Apr 14, 2021

Conversation

FSM1
Copy link
Contributor

@FSM1 FSM1 commented Apr 13, 2021 •
edited
Loading

Closes #903

This PR ensures that the isNewDevice is set correctly in all instances.

Also made sure that metadata and shares are refreshed when the Security page is loading, to prevent stale metadata being displayed.

@render
Copy link

render bot commented Apr 13, 2021

Your Render PR Server URL is https://chainsafex-dev-pr-923.onrender.com.

Follow its progress at https://dashboard.render.com/static/srv-c1qqtc3s1gh86uf24ls0.

@render
Copy link

render bot commented Apr 13, 2021

Your Render PR Server URL is https://files-ui-dev-pr-923.onrender.com.

Follow its progress at https://dashboard.render.com/static/srv-c1qqtcbs1gh86uf24m1g.

@render
Copy link

render bot commented Apr 13, 2021

Your Render PR Server URL is https://files-landing-dev-pr-923.onrender.com.

Follow its progress at https://dashboard.render.com/static/srv-c1qqtcrs1gh86uf24mm0.

@FSM1 FSM1 requested review from tanmoyAtb, Tbaut and RyRy79261 April 13, 2021 14:46
@FSM1 FSM1 added the Status: Review Needed 👀 Added to PRs when they need more review label Apr 13, 2021
Tbaut
Tbaut reviewed Apr 13, 2021
View reviewed changes
Copy link
Collaborator

@Tbaut Tbaut left a comment

Choose a reason for hiding this comment

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

Made some comments on the code, will test it out now

packages/files-ui/src/Components/Modules/Settings/SavedBrowsers/index.tsx Outdated Show resolved Hide resolved
packages/files-ui/src/Components/Modules/Settings/SavedBrowsers/index.tsx Outdated Show resolved Hide resolved
packages/files-ui/src/Components/Modules/Settings/Security/index.tsx Outdated Show resolved Hide resolved
packages/files-ui/src/Contexts/ThresholdKeyContext.tsx Show resolved Hide resolved
packages/files-ui/src/Contexts/ThresholdKeyContext.tsx Outdated Show resolved Hide resolved
@Tbaut
Copy link
Collaborator

Tbaut commented Apr 13, 2021 •
edited
Loading

Here's what I tried. Logged in with Metamask, delete any saved browser. Relaunch my browser (so as to clear any session storage) and login with the same account:
I'm being asked my password or passphrase, all good. but then:

  • I'm presented the screen for new accounts (with all the auth methods etc)
  • I clicked "let's go" and then the app crashed:
    image

FSM1 and others added 4 commits April 13, 2021 17:12
@FSM1
Copy link
Contributor Author

FSM1 commented Apr 13, 2021

Here's what I tried. Logged in with Metamask, delete any saved browser. Relaunch my browser (so as to clear any session storage) and login with the same account:
I'm being asked my password or passphrase, all good. but then:

  • I'm presented the screen for new accounts (with all the auth methods etc)
  • I clicked "let's go" and then the app crashed:
    image

Ahhh nice catch. I've added a handler for this and am quickly testing out some of the other logic

FSM1 and others added 2 commits April 13, 2021 19:23
@FSM1
protect users from themselves
b055669
@FSM1 @Tbaut
Update packages/files-ui/src/Components/Modules/Settings/SavedBrowser…
95c28a9 
…s/index.tsx

Co-authored-by: Thibaut Sardan <33178835+Tbaut@users.noreply.github.com>
@FSM1 FSM1 requested a review from Tbaut April 13, 2021 17:24
Tbaut
Tbaut approved these changes Apr 14, 2021
View reviewed changes
Copy link
Collaborator

@Tbaut Tbaut left a comment

Choose a reason for hiding this comment

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

Just found a couple nits, works but perfectly!

packages/files-ui/src/Components/Modules/LoginModule/SignInMethods.tsx Outdated Show resolved Hide resolved
packages/files-ui/src/Components/Modules/LoginModule/SignInMethods.tsx Outdated Show resolved Hide resolved
packages/files-ui/src/Components/Modules/Settings/SavedBrowsers/BrowserPanel.tsx Outdated Show resolved Hide resolved
@FSM1 @Tbaut
Apply suggestions from code review
c8a5f07 
Co-authored-by: Thibaut Sardan <33178835+Tbaut@users.noreply.github.com>
@FSM1 FSM1 merged commit 4427c59 into dev Apr 14, 2021
@FSM1 FSM1 deleted the fix/tkey-new-device-flag-is-not-set-903 branch April 14, 2021 12:12
@FSM1 FSM1 mentioned this pull request Apr 22, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Tbaut Tbaut Tbaut approved these changes

@RyRy79261 RyRy79261 RyRy79261 approved these changes

@tanmoyAtb tanmoyAtb Awaiting requested review from tanmoyAtb

Assignees
No one assigned
Labels
Status: Review Needed 👀 Added to PRs when they need more review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

TKey isNewDevice flag is sometimes not set correctly
3 participants
@FSM1 @Tbaut @RyRy79261