Startup Wizard - Addition of Startup Wizard #284

camc314 · 2020-11-15T20:00:53Z

Closes Setup wizard #269
Left to do. Add ability to add and configure libraries in the wizard

heyhippari · 2020-11-28T10:42:23Z

Add ability to add and configure libraries in the wizard

For this step, keep in mind the mid-term plan is to remove that step from the wizard and replace it with 2 steps of plugin installs: one for features and one for providers.

So I'm not sure if it's worth spending much time on adding that here :)

components/wizard/WizardAdminAccount.vue

locales/en-US.json

camc314 · 2020-11-28T12:46:15Z

Add ability to add and configure libraries in the wizard

For this step, keep in mind the mid-term plan is to remove that step from the wizard and replace it with 2 steps of plugin installs: one for features and one for providers.

So I'm not sure if it's worth spending much time on adding that here :)

Wouldn't it be better to have the add libraries step, then based on the type of libraries have a step for adding the relevant plugins (e.g. MusicBrainz for music...)

heyhippari · 2020-11-28T13:13:11Z

We want the providers to be there before the library creation (otherwise scan starts and the user has to go edit the libraries) and the server will require a restart to load the plugins.

So the idea is to install the plugins, then restart and, finally, gently onboard them towards adding libraries.

heyhippari · 2021-01-05T20:04:58Z

@camc314 What's the status on this? Could we merge it without the library creation, then improve it later?

camc314 · 2021-01-05T21:01:04Z

I've removed library creation. I think it is good enough for now.

heyhippari · 2021-01-05T22:42:57Z

Brand new server, I seem to be unable to create the admin account, so I can't get further than that for testing.

Some early things:

I'm not sure how this will play with the server, but we should probably be able to jump back to previous steps.
Edit: I notice we can with the Previous button. Being able to click on the stepper at the top would be nice as well.
The forms are way too large. Something like the login/server pages would feel better
The "Previous" button should be the secondary color
I'm not sure the page descriptions like "This account will be used for managing the server" are necessary
The "confirm password" field is missing the visibility toggle
Translation keys should be nested. A main key firstTimeWizard with sub-keys per step or something would fit and keep it more organized
"Language and Locale" uses title case
The first step is missing the Dates and numbers settings (It's different from languages, so one can't be the other. Plus people might want a language with a specific locale, like English (US) for language and German for dates and times)
The buttons should be centered and forced to the same size (like what's done on the login/select user/server pages)

The traceback I get on the admin account is the following:

[2021-01-05 23:30:21.064 +01:00] [ERR] [105] Jellyfin.Server.Middleware.ExceptionMiddleware: Error processing request. URL "POST" "/Startup/User".
System.InvalidOperationException: Sequence contains no elements
   at System.Linq.ThrowHelper.ThrowNoElementsException()
   at System.Linq.Enumerable.First[TSource](IEnumerable`1 source)
   at Jellyfin.Api.Controllers.StartupController.UpdateStartupUser(StartupUserDto startupUserDto) in /tmp/makepkg/jellyfin-git/src/jellyfin/Jellyfin.Api/Controllers/StartupCont
roller.cs:line 133
   at lambda_method64(Closure , Object )
   at Microsoft.AspNetCore.Mvc.Infrastructure.ActionMethodExecutor.TaskOfActionResultExecutor.Execute(IActionResultTypeMapper mapper, ObjectMethodExecutor executor, Object controller, Object[] arguments)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.<InvokeActionMethodAsync>g__Awaited|12_0(ControllerActionInvoker invoker, ValueTask`1 actionResultValueTask)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.<InvokeNextActionFilterAsync>g__Awaited|10_0(ControllerActionInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.Rethrow(ActionExecutedContextSealed context)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.InvokeInnerFilterAsync()
--- End of stack trace from previous location ---
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeNextResourceFilter>g__Awaited|24_0(ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.Rethrow(ResourceExecutedContextSealed context)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.InvokeFilterPipelineAsync()
--- End of stack trace from previous location ---
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeAsync>g__Awaited|17_0(ResourceInvoker invoker, Task task, IDisposable scope)
   at Microsoft.AspNetCore.Routing.EndpointMiddleware.<Invoke>g__AwaitRequestTask|6_0(Endpoint endpoint, Task requestTask, ILogger logger)
   at Jellyfin.Server.Middleware.ServerStartupMessageMiddleware.Invoke(HttpContext httpContext, IServerApplicationHost serverApplicationHost, ILocalizationManager localizationManager) in /tmp/makepkg/jellyfin-git/src/jellyfin/Jellyfin.Server/Middleware/ServerStartupMessageMiddleware.cs:line 49
   at Jellyfin.Server.Middleware.WebSocketHandlerMiddleware.Invoke(HttpContext httpContext, IWebSocketManager webSocketManager) in /tmp/makepkg/jellyfin-git/src/jellyfin/Jellyfin.Server/Middleware/WebSocketHandlerMiddleware.cs:line 38
   at Jellyfin.Server.Middleware.IpBasedAccessValidationMiddleware.Invoke(HttpContext httpContext, INetworkManager networkManager, IServerConfigurationManager serverConfigurationManager) in /tmp/makepkg/jellyfin-git/src/jellyfin/Jellyfin.Server/Middleware/IpBasedAccessValidationMiddleware.cs:line 75
   at Jellyfin.Server.Middleware.LanFilteringMiddleware.Invoke(HttpContext httpContext, INetworkManager networkManager, IServerConfigurationManager serverConfigurationManager) in /tmp/makepkg/jellyfin-git/src/jellyfin/Jellyfin.Server/Middleware/LanFilteringMiddleware.cs:line 46
   at Microsoft.AspNetCore.Authorization.Policy.AuthorizationMiddlewareResultHandler.HandleAsync(RequestDelegate next, HttpContext context, AuthorizationPolicy policy, PolicyAuthorizationResult authorizeResult)
   at Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.Invoke(HttpContext context)
   at Swashbuckle.AspNetCore.ReDoc.ReDocMiddleware.Invoke(HttpContext httpContext)
   at Swashbuckle.AspNetCore.SwaggerUI.SwaggerUIMiddleware.Invoke(HttpContext httpContext)
   at Swashbuckle.AspNetCore.Swagger.SwaggerMiddleware.Invoke(HttpContext httpContext, ISwaggerProvider swaggerProvider)
   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
   at Jellyfin.Server.Middleware.RobotsRedirectionMiddleware.Invoke(HttpContext httpContext) in /tmp/makepkg/jellyfin-git/src/jellyfin/Jellyfin.Server/Middleware/RobotsRedirectionMiddleware.cs:line 45
   at Jellyfin.Server.Middleware.LegacyEmbyRouteRewriteMiddleware.Invoke(HttpContext httpContext) in /tmp/makepkg/jellyfin-git/src/jellyfin/Jellyfin.Server/Middleware/LegacyEmbyRouteRewriteMiddleware.cs:line 52
   at Microsoft.AspNetCore.ResponseCompression.ResponseCompressionMiddleware.Invoke(HttpContext context)
   at Jellyfin.Server.Middleware.ResponseTimeMiddleware.Invoke(HttpContext context) in /tmp/makepkg/jellyfin-git/src/jellyfin/Jellyfin.Server/Middleware/ResponseTimeMiddleware.cs:line 64
   at Jellyfin.Server.Middleware.ExceptionMiddleware.Invoke(HttpContext context) in /tmp/makepkg/jellyfin-git/src/jellyfin/Jellyfin.Server/Middleware/ExceptionMiddleware.cs:line 101

camc314 · 2021-01-06T12:04:41Z

I'm not sure how this will play with the server, but we should probably be able to jump back to previous steps.
Edit: I notice we can with the Previous button. Being able to click on the stepper at the top would be nice as well.

The only problem I see with this is if someone were to jump ahead to the final step, complete the wizard without completing the prior steps. This could cause issues as there may be no admin account

The forms are way too large. Something like the login/server pages would feel better

The purpose of it being larger is so that the text above is visible:
I think it is better this way as it means the names of all steps can be seen

The "Previous" button should be the secondary color

This makes the text even darker
Secondary:

Undefined:

I'm not sure the page descriptions like "This account will be used for managing the server" are necessary

Removed

The "confirm password" field is missing the visibility toggle

Added

Translation keys should be nested. A main key firstTimeWizard with sub-keys per step or something would fit and keep it more organized

"Language and Locale" uses title case

updated

The first step is missing the Dates and numbers settings (It's different from languages, so one can't be the other. Plus people might want a language with a specific locale, like English (US) for language and German for dates and times)

Where are you getting the Dates and numbers setting from? As far as I can see it is not pointed out in the startupwizard API endpoints.

The buttons should be centered and forced to the same size (like what's done on the login/select user/server pages)

With making the v-stepper wider than before, I think having the buttons size not take up the full width

The traceback I get on the admin account is the following:

I think this may be due to the device ID changing? I have moved setting the device profile to the wizard page instead of being set everytime the account is created/update.

heyhippari · 2021-01-06T12:25:02Z

The only problem I see with this is if someone were to jump ahead to the final step, complete the wizard without completing the prior steps. This could cause issues as there may be no admin account

Adding editable to v-stepper-step should make previous steps clickable without allowing to skip any of them (https://vuetifyjs.com/en/api/v-stepper-step/#props)

The purpose of it being larger is so that the text above is visible:
I think it is better this way as it means the names of all steps can be seen

Agreed, but couldn't we have two width for these? The horizontal one won't work on mobile anyway, so having it become vertical on, say, xs and sm maybe, but horizontal on larger screens would work here. Then have the form itself act like the others but the stepper stay a larger width could work, I think.

But this is not blocking at all, we can look at it in further PRs, so no worries.

This makes the text even darker

I was thinking of removing the text prop to make it a normal button and have it use the secondary color. We don't really use text buttons for anything elsewhere, so the normal ones may fit better.

Where are you getting the Dates and numbers setting from? As far as I can see it is not pointed out in the startupwizard API endpoints.

Hm, I was thing we had that in the startup, but I might have been mistaken. Seems like the setting I'm thinking about is client-side only on old web.

Not needed now, but a potential future improvement for this would be to mirror the selected language here to the user preferences for the client's display language (And same for the locale).

With making the v-stepper wider than before, I think having the buttons size not take up the full width

For sure, but with the narrower form (keeping the stepper as-is), this would be doable and would look more in sync with the other pages.

Again, not blocking though, we can mess with that at a later date.

I think this may be due to the device ID changing? I have moved setting the device profile to the wizard page instead of being set everytime the account is created/update.

Thanks, I'll re-test :)

heyhippari · 2021-01-06T12:32:36Z

The wizard works perfectly now 👍

Gonna review the code later today, once I have some free time 😃

One issue I could see during the test is the allowRemoteAccess key doesn't have a translation. Aside from that, all good!

camc314 · 2021-01-06T13:01:35Z

I've fixed making the steps editable. The editable prop allows skipping steps, so it is now computed, if a step should be editable or not.

I was thinking of removing the text prop to make it a normal button and have it use the secondary color. We don't really use text buttons for anything elsewhere, so the normal ones may fit better.

Updated, it looks better now the text prop is gone

Agreed, but couldn't we have two width for these? The horizontal one won't work on mobile anyway, so having it become vertical on, say, xs and sm maybe, but horizontal on larger screens would work here. Then have the form itself act like the others but the stepper stay a larger width could work, I think.

On smaller screens, the v-stepper hides the section titles

Ah I see what you mean, without dev tools open it is very wide lol. Should be a bit better now.

components/wizard/WizardAdminAccount.vue

components/wizard/WizardLanguage.vue

heyhippari · 2021-01-06T19:11:32Z

components/wizard/WizardMetadata.vue

+          }
+        });
+
+        this.pushSnackbarMessage({


components/wizard/WizardAdminAccount.vue

locales/en-US.json

heyhippari · 2021-01-06T19:18:27Z

pages/wizard.vue

+  <v-container fill-height>
+    <v-row align="center" justify="center">
+      <v-col cols="12" sm="12" md="12" xl="8">
+        <h1 class="text-h4 mb-6 text-center">{{ heading }}</h1>


Not sure if we should have a header here.

Technically, the stepper already has the title. By making the stepper vertical on small screen, we can keep the headers visible and not need this.

pages/wizard.vue

codecov-io · 2021-01-15T09:48:05Z

Codecov Report

Merging #284 (9c05297) into master (c3b0640) will decrease coverage by 0.19%.
The diff coverage is 0.00%.

@@            Coverage Diff            @@
##           master    #284      +/-   ##
=========================================
- Coverage    5.60%   5.40%   -0.20%     
=========================================
  Files          97     102       +5     
  Lines        2696    2793      +97     
  Branches      424     429       +5     
=========================================
  Hits          151     151              
- Misses       2538    2635      +97     
  Partials        7       7

Impacted Files	Coverage Δ
components/wizard/WizardAdminAccount.vue	`0.00% <0.00%> (ø)`
components/wizard/WizardLanguage.vue	`0.00% <0.00%> (ø)`
components/wizard/WizardMetadata.vue	`0.00% <0.00%> (ø)`
components/wizard/WizardRemoteAccess.vue	`0.00% <0.00%> (ø)`
pages/wizard.vue	`0.00% <0.00%> (ø)`
store/servers.ts	`0.00% <0.00%> (ø)`
... and 1 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 c3b0640...9c05297. Read the comment docs.

… false

camc314 · 2021-01-15T11:24:09Z

plugins/veeValidate.ts

+  // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+  // @ts-ignore
+  validate(value, { target }) {
+    return value === target;


This ts-comment is not ideal, but this is what the docs show to do to ensure that passwords are equal

https://logaretm.github.io/vee-validate/advanced/cross-field-validation.html#targeting-other-fields

We should have added a comment there with that before merging

sonarqubecloud · 2021-01-15T11:24:19Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
4 Code Smells

No Coverage information
0.0% Duplication

heyhippari

LGTM 🚀

camc314 changed the title ~~Your a wizard harry - Addition of Startup Wizard~~ Startup Wizard - Addition of Startup Wizard Nov 15, 2020

camc314 marked this pull request as draft November 15, 2020 20:38

dkanada reviewed Nov 28, 2020

View reviewed changes

components/wizard/WizardAdminAccount.vue Outdated Show resolved Hide resolved

dkanada reviewed Nov 28, 2020

View reviewed changes

locales/en-US.json Outdated Show resolved Hide resolved

ferferga mentioned this pull request Dec 1, 2020

feat(login): add public users #293

Merged

heyhippari added this to the Preview Release 1 milestone Dec 6, 2020

camc314 force-pushed the YourAWizardHarry branch from 5e8bdac to d047056 Compare December 14, 2020 09:37

camc314 force-pushed the YourAWizardHarry branch from 649bb1f to ec500f4 Compare January 5, 2021 21:00

camc314 marked this pull request as ready for review January 5, 2021 21:01

camc314 force-pushed the YourAWizardHarry branch from 9a5e62b to 7e4a2c4 Compare January 6, 2021 13:01

heyhippari suggested changes Jan 6, 2021

View reviewed changes

camc314 force-pushed the YourAWizardHarry branch from 7e4a2c4 to 9c05297 Compare January 15, 2021 09:46

camc314 added 6 commits January 15, 2021 11:16

feat(wizard): add initial page with v-stepper for wizard

909cd05

feat(wizard): redirect to startup wizard if StartupWizardCompleted is…

ff1c47b

… false

feat(wizard): add wizard page for admin account

3034e51

feat(wizard): add wizard page for remove access

49d0bef

feat(wizard): add wizard page for language

fc7ef33

feat(wizard): add wizard page for metadata

5711023

camc314 force-pushed the YourAWizardHarry branch from 9c05297 to 5711023 Compare January 15, 2021 11:19

fix(fix build): add ts-ignore

8454395

camc314 commented Jan 15, 2021

View reviewed changes

heyhippari approved these changes Jan 15, 2021

View reviewed changes

heyhippari merged commit ef41d6d into master Jan 15, 2021

heyhippari deleted the YourAWizardHarry branch January 15, 2021 14:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Startup Wizard - Addition of Startup Wizard #284

Startup Wizard - Addition of Startup Wizard #284

camc314 commented Nov 15, 2020 •

edited

Loading

heyhippari commented Nov 28, 2020

camc314 commented Nov 28, 2020

heyhippari commented Nov 28, 2020

heyhippari commented Jan 5, 2021

camc314 commented Jan 5, 2021

heyhippari commented Jan 5, 2021 •

edited

Loading

camc314 commented Jan 6, 2021

heyhippari commented Jan 6, 2021

heyhippari commented Jan 6, 2021 •

edited

Loading

camc314 commented Jan 6, 2021

heyhippari Jan 6, 2021

heyhippari Jan 6, 2021

codecov-io commented Jan 15, 2021

camc314 Jan 15, 2021

ferferga Jan 15, 2021

sonarqubecloud bot commented Jan 15, 2021

heyhippari left a comment

Startup Wizard - Addition of Startup Wizard #284

Startup Wizard - Addition of Startup Wizard #284

Conversation

camc314 commented Nov 15, 2020 • edited Loading

heyhippari commented Nov 28, 2020

camc314 commented Nov 28, 2020

heyhippari commented Nov 28, 2020

heyhippari commented Jan 5, 2021

camc314 commented Jan 5, 2021

heyhippari commented Jan 5, 2021 • edited Loading

camc314 commented Jan 6, 2021

heyhippari commented Jan 6, 2021

heyhippari commented Jan 6, 2021 • edited Loading

camc314 commented Jan 6, 2021

heyhippari Jan 6, 2021

Choose a reason for hiding this comment

heyhippari Jan 6, 2021

Choose a reason for hiding this comment

codecov-io commented Jan 15, 2021

Codecov Report

camc314 Jan 15, 2021

Choose a reason for hiding this comment

ferferga Jan 15, 2021

Choose a reason for hiding this comment

sonarqubecloud bot commented Jan 15, 2021

heyhippari left a comment

Choose a reason for hiding this comment

camc314 commented Nov 15, 2020 •

edited

Loading

heyhippari commented Jan 5, 2021 •

edited

Loading

heyhippari commented Jan 6, 2021 •

edited

Loading