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

Avivash/File System Recovery Shorthand Method #461

Merged
merged 18 commits into from
Jan 13, 2023
Merged

Conversation

avivash
Copy link
Member

@avivash avivash commented Jan 12, 2023

Summary

This PR fixes/implements the following bugs/features

  • The recoverFileSystem shorthand

Moving the FileSystem recovery functionality into webnative. Devs can now call the recoverFileSystem shorthand method directly from the program. It is important to note the newUsername and oldUsername that are passed in are the hashed versions, as webnative doesn't yet have a notion of the unhashed versions. (That will be completed in #385)

const { success } = await $sessionStore.program.recoverFileSystem({
  newUsername,
  oldUsername,
  readKey
})

Note: I'm not sure if we currently have the infrastructure in the codebase for this, but it would be cool if we could emit events at various parts of the recovery flow to update the message on the client 🤔 Answered by Steven!

Test plan (required)

Load WAT using this webnative branch and go through the recovery flow. It should work the same as it did before.

https://www.loom.com/share/b2ecfe8fa8a04f7e96dab76d8dc749a6

Closing issues

Closes #427

After Merge

  • Does this change invalidate any docs or tutorials? If so ensure the changes needed are either made or recorded
  • Does this change require a release to be made? Is so please create and deploy the release

Sibling PR

oddsdk/odd-app-template#113

@avivash avivash self-assigned this Jan 12, 2023
src/index.ts Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
@avivash
Copy link
Member Author

avivash commented Jan 12, 2023

ahh, i'll update the changelog!

Copy link
Contributor

@icidasset icidasset left a comment

Choose a reason for hiding this comment

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

Couple comments, looks great otherwise! 🥳

src/filesystem.ts Outdated Show resolved Hide resolved
src/filesystem.ts Outdated Show resolved Hide resolved
src/filesystem.ts Outdated Show resolved Hide resolved
src/filesystem.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
@icidasset
Copy link
Contributor

No event emitter yet in Webnative, but it's on the short-term todo list! Also, bit confused about the example shown here, this should work with regular usernames too right?

@avivash
Copy link
Member Author

avivash commented Jan 13, 2023

No event emitter yet in Webnative, but it's on the short-term todo list!

Nice!

Also, bit confused about the example shown here, this should work with regular usernames too right?

Ahh yes good point! I should remove the term hashed from the example. I'll update it in both PRs 👌🏼

Copy link
Contributor

@icidasset icidasset left a comment

Choose a reason for hiding this comment

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

One more nitpick.
Otherwise I think it's good to go.

src/filesystem.ts Outdated Show resolved Hide resolved
Copy link
Member

@bgins bgins left a comment

Choose a reason for hiding this comment

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

Looking good! ✨

Left a few comments. One thing we want to consider how developers will create a recovery kit to use this shorthand function. More on that in the comments.

CHANGELOG.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
src/filesystem.ts Outdated Show resolved Hide resolved
@avivash avivash merged commit b494f60 into main Jan 13, 2023
@avivash avivash deleted the avivash/fs-recovery branch January 13, 2023 23:02
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.

Add Filesystem Recovery
3 participants