Skip to content
This repository has been archived by the owner on Nov 28, 2022. It is now read-only.

Removed third step to invite staff users from self-hoster setup #2286

Merged
merged 12 commits into from
Mar 8, 2022

Conversation

sanne-san
Copy link
Member

No issue

  • The step to invite staff users during site setup for self-hosters is unnecessary and out of style with the rest of admin, and is therefore removed.

@matthanley
Copy link
Contributor

❤️ really looking forward to not having to skip this every time!

@sanne-san sanne-san force-pushed the updated-selfhoster-setup-flow branch 2 times, most recently from c12586a to ffbdba1 Compare March 8, 2022 11:38
@codecov-commenter
Copy link

Codecov Report

Merging #2286 (631f32b) into main (6629062) will decrease coverage by 0.48%.
The diff coverage is 59.18%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2286      +/-   ##
==========================================
- Coverage   41.19%   40.71%   -0.49%     
==========================================
  Files         581      578       -3     
  Lines       22818    22642     -176     
  Branches     4041     4007      -34     
==========================================
- Hits         9400     9218     -182     
- Misses      13418    13424       +6     
Impacted Files Coverage Δ
app/controllers/signin.js 82.60% <ø> (-0.29%) ⬇️
app/controllers/setup.js 56.52% <56.52%> (-43.48%) ⬇️
app/helpers/site-icon-style.js 100.00% <100.00%> (ø)
app/router.js 100.00% <100.00%> (ø)
app/routes/setup.js 86.95% <100.00%> (ø)
app/routes/setup/index.js 0.00% <0.00%> (-100.00%) ⬇️
app/components/gh-html-iframe.js 45.45% <0.00%> (-38.64%) ⬇️
app/components/gh-site-iframe.js 46.00% <0.00%> (-22.00%) ⬇️
app/utils/link-component.js 87.50% <0.00%> (-12.50%) ⬇️
app/components/gh-notification.js 95.45% <0.00%> (-4.55%) ⬇️
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6629062...631f32b. Read the comment docs.

@sanne-san sanne-san force-pushed the updated-selfhoster-setup-flow branch from 1e7f099 to c173226 Compare March 8, 2022 16:50
@sanne-san sanne-san merged commit c7c6f9c into main Mar 8, 2022
@sanne-san sanne-san deleted the updated-selfhoster-setup-flow branch March 8, 2022 17:30
kevinansfield added a commit that referenced this pull request Mar 10, 2022
#2286

- `session.authenticate()` returns from it's promise as soon as the authenticate request is completed but it was assumed that it returned after the `session.handleAuthentication()` promise was also completed. A side-effect of that was that depending on network timing, the setup flow could transition to the dashboard before we had loaded all of the necessary user, config, and settings requests
  - normally that's not a problem because `handleAuthentication()` kicks off a transition once authentication is fully complete, in the setup flow we're handling the transition manually so need a way to manage the full async flow from outside of the session service
  - it didn't show up as a problem previously because the setup flow transitioned to a third setup screen that didn't require all of the post-auth data to exist
- moved the async parts of `session.handleAuthentication()` into a task and updated to return the currently running task instance if one was already running
  - lets code that is relying on the full authentication flow to have completed call `await this.session.handleAuthentication()` without causing a double-load of the post-auth API requests
- updated setup flow
  - removed manual `session.populateUser()` call as that was a workaround for the async timing issue and caused a double-fetch of the current user API endpoint
  - added an `await this.session.handleAuthentication()` call to the manual post-auth handler so we don't transition until the full auth flow is complete
kevinansfield added a commit that referenced this pull request May 27, 2022
refs #2286

- the UI to upload a profile image during setup/signup was removed as part of the auth screens redesign but the related code was left behind
tigefa4u pushed a commit to tigefa4u/Ghost that referenced this pull request Aug 3, 2022
TryGhost/Admin#2286

- `session.authenticate()` returns from it's promise as soon as the authenticate request is completed but it was assumed that it returned after the `session.handleAuthentication()` promise was also completed. A side-effect of that was that depending on network timing, the setup flow could transition to the dashboard before we had loaded all of the necessary user, config, and settings requests
  - normally that's not a problem because `handleAuthentication()` kicks off a transition once authentication is fully complete, in the setup flow we're handling the transition manually so need a way to manage the full async flow from outside of the session service
  - it didn't show up as a problem previously because the setup flow transitioned to a third setup screen that didn't require all of the post-auth data to exist
- moved the async parts of `session.handleAuthentication()` into a task and updated to return the currently running task instance if one was already running
  - lets code that is relying on the full authentication flow to have completed call `await this.session.handleAuthentication()` without causing a double-load of the post-auth API requests
- updated setup flow
  - removed manual `session.populateUser()` call as that was a workaround for the async timing issue and caused a double-fetch of the current user API endpoint
  - added an `await this.session.handleAuthentication()` call to the manual post-auth handler so we don't transition until the full auth flow is complete
tigefa4u pushed a commit to tigefa4u/Ghost that referenced this pull request Aug 3, 2022
refs TryGhost/Admin#2286

- the UI to upload a profile image during setup/signup was removed as part of the auth screens redesign but the related code was left behind
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants