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

Account App public views #3453

Merged

Conversation

kschiffer
Copy link
Member

@kschiffer kschiffer commented Nov 9, 2020

Summary

This PR adds the new public views to the account app scaffold feature/1422-account-app branch and introduces some related changes and fixes.

image

More screenshots

image
image
image

References #1422

Changes

  • Add public views to replace old OAuth app views
  • Update tests
  • Related scaffolding
    • Add store selectors
    • Update API definitions
  • Related minor changes/fixes
    • Add functionality to pass titles to the form errors and info notifications
    • Retain the next parameter when registering a new account
    • Fix crashing <FullViewError />

Testing

Manual testing and e2e tests.

Regressions

The diff is quite large so there's a good chance that this breaks things. I tried to test everything but I'd still like you to do some monkey testing yourselves. I also updated the e2e tests.

Notes for Reviewers

  • Like last time, it's necessary to update the config variables to mount this in development mode:
export export TTN_LW_IS_OAUTH_UI_JS_FILE="libs.bundle.js account.js"
  • Like last time, this PR targets feature/1422-account-app which I keep rebased on latest minor. When all parts are added, we will merge this branch into the minor.
  • I put the update password integration tests on skip() since I had to remove the password updating when logged in for now. I will reintroduce it in the upcoming PR, which will add the groundwork for the new authenticated views
  • I missed renaming the static assets from account-*.* to oauth-*.* in the last PR. This is necessary to ensure config compatibility. I'll fix this in another PR, since there will also be some changes to the branding/assets itself.

Checklist

  • Scope: The referenced issue is addressed, there are no unrelated changes.
  • Compatibility: The changes are backwards compatible with existing API, storage, configuration and CLI, according to the compatibility commitments in README.md for the chosen target branch.
  • Documentation: Relevant documentation is added or updated.
  • Changelog: Significant features, behavior changes, deprecations and fixes are added to CHANGELOG.md.
  • Commits: Commit messages follow guidelines in CONTRIBUTING.md, there are no fixup commits left.

@kschiffer kschiffer added prio/medium ui/web This is related to a web interface labels Nov 9, 2020
@kschiffer kschiffer added this to the November 2020 milestone Nov 9, 2020
@kschiffer kschiffer self-assigned this Nov 9, 2020
@kschiffer kschiffer force-pushed the feature/1422-account-app-new-groundwork branch from fc0395b to ed0cc10 Compare November 9, 2020 17:35
Copy link
Contributor

@bafonins bafonins left a comment

Choose a reason for hiding this comment

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

  1. Reset password view does not show an error when non-existent user id is provided
  2. Update jsconfig.json paths to account for the new account folder
  3. If I navigate to /oauth/anything I end up on a broken screen without any errors

pkg/webui/account/containers/header/index.js Outdated Show resolved Hide resolved
pkg/webui/account/views/app/index.js Outdated Show resolved Hide resolved
pkg/webui/account/views/front/front.styl Outdated Show resolved Hide resolved
pkg/webui/account/views/update-password/index.js Outdated Show resolved Hide resolved
@johanstokking
Copy link
Member

Looks great!

@kschiffer kschiffer force-pushed the feature/1422-account-app-new-groundwork branch from ed0cc10 to 0059c7f Compare December 4, 2020 16:14
@kschiffer kschiffer force-pushed the feature/1422-account-app-new-groundwork branch from 0059c7f to e65af93 Compare December 4, 2020 16:28
@kschiffer kschiffer merged commit 0894b8e into feature/1422-account-app Dec 4, 2020
@kschiffer kschiffer deleted the feature/1422-account-app-new-groundwork branch December 4, 2020 17:22
@kschiffer kschiffer mentioned this pull request Jan 20, 2021
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ui/web This is related to a web interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants