Refactored signin controller to Octane patterns

refs https://github.com/TryGhost/Ghost/issues/14101

- migrated to native class syntax and glimmer component patterns
- removed use of jQuery, the workaround to trigger change events no longer appears necessary
This commit is contained in:
Kevin Ansfield 2022-11-01 14:11:41 +00:00
parent 498bec08cf
commit 2d2ac0102a
6 changed files with 92 additions and 89 deletions

View File

@ -1003,3 +1003,9 @@ remove|ember-template-lint|no-action|19|35|19|35|8b04fb9251c6a34b6bfc2995b527539
remove|ember-template-lint|no-action|31|35|31|35|ddfad6e48c0df2368eed5dd9faf83f2a96dc182e|1662681600000|1673053200000|1678237200000|app/templates/reset.hbs
remove|ember-template-lint|no-passed-in-event-handlers|19|28|19|28|ea0378c5df53f2be82f0fd59e0c538efd7176851|1662681600000|1673053200000|1678237200000|app/templates/reset.hbs
remove|ember-template-lint|no-passed-in-event-handlers|31|28|31|28|06311a82a9589d4c126863466d56c7c4a76409d0|1662681600000|1673053200000|1678237200000|app/templates/reset.hbs
remove|ember-template-lint|no-action|15|85|15|85|5f277699a01bbbcf2e74101096f9c9bff189af74|1662681600000|1673053200000|1678237200000|app/templates/signin.hbs
remove|ember-template-lint|no-action|33|35|33|35|85790bc3673715479aa2678f29d8a61e47281bc7|1662681600000|1673053200000|1678237200000|app/templates/signin.hbs
remove|ember-template-lint|no-action|34|39|34|39|e65f48edccba27e52c1f8358a9795dc8ff20d5ef|1662681600000|1673053200000|1678237200000|app/templates/signin.hbs
remove|ember-template-lint|no-action|50|35|50|35|7432725bd18c48f69bf22dc9487d14d25dc6c1b7|1662681600000|1673053200000|1678237200000|app/templates/signin.hbs
remove|ember-template-lint|no-passed-in-event-handlers|33|28|33|28|5b371baf419f247953b91b626611cb831c524af3|1662681600000|1673053200000|1678237200000|app/templates/signin.hbs
remove|ember-template-lint|no-passed-in-event-handlers|50|28|50|28|40caf07c7cebf6f4321c5b7e7f2f426b5c30217b|1662681600000|1673053200000|1678237200000|app/templates/signin.hbs

View File

@ -1,22 +1,18 @@
// TODO: bump lint rules to be able to take advantage of https://github.com/ember-cli/eslint-plugin-ember/issues/560
/* eslint-disable ghost/ember/alias-model-in-controller */
import $ from 'jquery';
import Controller, {inject as controller} from '@ember/controller';
import ValidationEngine from 'ghost-admin/mixins/validation-engine';
import classic from 'ember-classic-decorator';
import {action} from '@ember/object';
import {alias} from '@ember/object/computed';
import {htmlSafe} from '@ember/template';
import {isArray as isEmberArray} from '@ember/array';
import {isVersionMismatchError} from 'ghost-admin/services/ajax';
import {inject as service} from '@ember/service';
import {task} from 'ember-concurrency';
import {tracked} from '@glimmer/tracking';
@classic
export default class SigninController extends Controller.extend(ValidationEngine) {
@controller
application;
@controller application;
@service ajax;
@service config;
@ -25,29 +21,32 @@ export default class SigninController extends Controller.extend(ValidationEngine
@service session;
@service settings;
submitting = false;
loggingIn = false;
authProperties = null;
flowErrors = '';
passwordResetEmailSent = false;
@tracked submitting = false;
@tracked loggingIn = false;
@tracked flowErrors = '';
@tracked passwordResetEmailSent = false;
// ValidationEngine settings
validationType = 'signin';
init() {
super.init(...arguments);
this.authProperties = ['identification', 'password'];
}
authProperties = ['identification', 'password'];
@alias('model')
signin;
get signin() {
return this.model;
}
@action
authenticate() {
return this.validateAndAuthenticate.perform();
handleInput(event) {
this.signin[event.target.name] = event.target.value;
}
@(task(function* (authStrategy, authentication) {
@action
validateProperty(property) {
return this.validate({property});
}
@task({drop: true})
*authenticateTask(authStrategy, authentication) {
try {
return yield this.session
.authenticate(authStrategy, ...authentication)
@ -63,18 +62,18 @@ export default class SigninController extends Controller.extend(ValidationEngine
mainError.message = htmlSafe(mainError.message || '');
mainError.context = htmlSafe(mainError.context || '');
this.set('flowErrors', (mainError.context.string || mainError.message.string));
this.flowErrors = (mainError.context.string || mainError.message.string);
if (mainError.type === 'PasswordResetRequiredError') {
this.set('passwordResetEmailSent', true);
this.passwordResetEmailSent = true;
}
if (mainError.context.string.match(/user with that email/i)) {
this.get('signin.errors').add('identification', '');
this.signin.errors.add('identification', '');
}
if (mainError.context.string.match(/password is incorrect/i)) {
this.get('signin.errors').add('password', '');
this.signin.errors.add('password', '');
}
} else {
console.error(error); // eslint-disable-line no-console
@ -87,17 +86,14 @@ export default class SigninController extends Controller.extend(ValidationEngine
return false;
}
}).drop())
authenticateTask;
}
@(task(function* () {
@task({drop: true})
*validateAndAuthenticateTask() {
let signin = this.signin;
let authStrategy = 'authenticator:cookie';
this.set('flowErrors', '');
// Manually trigger events for input fields, ensuring legacy compatibility with
// browsers and password managers that don't send proper events on autofill
$('#login').find('input').trigger('change');
this.flowErrors = '';
// This is a bit dirty, but there's no other way to ensure the properties are set as well as 'signin'
this.hasValidated.addObjects(this.authProperties);
@ -105,19 +101,19 @@ export default class SigninController extends Controller.extend(ValidationEngine
try {
yield this.validate({property: 'signin'});
return yield this.authenticateTask
.perform(authStrategy, [signin.get('identification'), signin.get('password')]);
.perform(authStrategy, [signin.identification, signin.password]);
} catch (error) {
this.set('flowErrors', 'Please fill out the form to sign in.');
this.flowErrors = 'Please fill out the form to sign in.';
}
}).drop())
validateAndAuthenticate;
}
@task(function* () {
let email = this.get('signin.identification');
let forgottenUrl = this.get('ghostPaths.url').api('authentication', 'password_reset');
@task
*forgotPasswordTask() {
let email = this.signin.identification;
let forgottenUrl = this.ghostPaths.url.api('authentication', 'password_reset');
let notifications = this.notifications;
this.set('flowErrors', '');
this.flowErrors = '';
// This is a bit dirty, but there's no other way to ensure the properties are set as well as 'forgotPassword'
this.hasValidated.addObject('identification');
@ -132,7 +128,7 @@ export default class SigninController extends Controller.extend(ValidationEngine
} catch (error) {
// ValidationEngine throws "undefined" for failed validation
if (!error) {
return this.set('flowErrors', 'We need your email address to reset your password!');
return this.flowErrors = 'We need your email address to reset your password!';
}
if (isVersionMismatchError(error)) {
@ -142,15 +138,14 @@ export default class SigninController extends Controller.extend(ValidationEngine
if (error && error.payload && error.payload.errors && isEmberArray(error.payload.errors)) {
let [{message}] = error.payload.errors;
this.set('flowErrors', message);
this.flowErrors = message;
if (message.match(/no user|not found/)) {
this.get('signin.errors').add('identification', '');
this.signin.errors.add('identification', '');
}
} else {
notifications.showAPIError(error, {defaultErrorText: 'There was a problem with the reset, please try again.', key: 'forgot-password.send'});
}
}
})
forgotten;
}
}

View File

@ -1,17 +1,20 @@
// TODO: remove usage of Ember Data's private `Errors` class when refactoring validations
// eslint-disable-next-line
import DS from 'ember-data';
import EmberObject from '@ember/object';
import UnauthenticatedRoute from 'ghost-admin/routes/unauthenticated';
import {tracked} from '@glimmer/tracking';
const {Errors} = DS;
class Signin {
@tracked identification = '';
@tracked password = '';
errors = Errors.create();
}
const defaultModel = function defaultModel() {
return EmberObject.create({
identification: '',
password: '',
errors: Errors.create()
});
return new Signin();
};
export default class SigninRoute extends UnauthenticatedRoute {
@ -21,12 +24,10 @@ export default class SigninRoute extends UnauthenticatedRoute {
// the deactivate hook is called after a route has been exited.
deactivate() {
let controller = this.controllerFor('signin');
super.deactivate(...arguments);
// clear the properties that hold the credentials when we're no longer on the signin screen
controller.set('signin', defaultModel());
this.controllerFor('signin').model = defaultModel();
}
buildRouteInfoMetadata() {

View File

@ -12,47 +12,47 @@
</header>
</div>
{{else}}
<form id="login" method="post" class="gh-signin" novalidate="novalidate" {{action "authenticate" on="submit"}}>
<form id="login" method="post" class="gh-signin" novalidate="novalidate" {{on "submit" (perform this.validateAndAuthenticateTask)}}>
<header>
<div class="gh-site-icon" style={{site-icon-style}}></div>
<h1>Sign in to {{this.config.blogTitle}}.</h1>
</header>
<GhFormGroup @errors={{this.signin.errors}} @hasValidated={{this.hasValidated}} @property="identification">
<label>Email address</label>
<label for="identification">Email address</label>
<span class="gh-input-icon gh-icon-mail">
<GhTrimFocusInput
@class="email"
@type="email"
@placeholder="jamie@example.com"
@name="identification"
@autocapitalize="off"
@autocorrect="off"
@autocomplete="username"
@tabindex="1"
@value={{readonly this.signin.identification}}
@input={{action (mut this.signin.identification) value="target.value"}}
@focus-out={{action "validate" "identification"}}
<input
id="identification"
type="email"
class="gh-input email"
placeholder="jamie@example.com"
name="identification"
autocapitalize="off"
autocorrect="off"
autocomplete="username"
value={{this.signin.identification}}
{{on "input" this.handleInput}}
{{on "blur" (fn this.validateProperty "identification")}}
/>
</span>
</GhFormGroup>
<GhFormGroup @errors={{this.signin.errors}} @hasValidated={{this.hasValidated}} @property="password">
<label>Password</label>
<label for="password">Password</label>
<span class="gh-input-icon gh-icon-lock forgotten-wrap">
<GhTextInput
@class="password"
@type="password"
@placeholder="•••••••••••••••"
@name="password"
@tabindex="2"
@autocomplete="current-password"
@autocorrect="off"
@value={{readonly this.signin.password}}
@input={{action (mut this.signin.password) value="target.value"}} />
<input
id="password"
type="password"
class="gh-input password"
placeholder="•••••••••••••••"
name="password"
autocomplete="current-password"
autocorrect="off"
value={{this.signin.password}}
{{on "input" this.handleInput}}
/>
<GhTaskButton
@task={{this.forgotten}}
@task={{this.forgotPasswordTask}}
@class="forgotten-link gh-btn gh-btn-link gh-btn-icon"
@tabindex="4"
@type="button"
@successClass=""
@failureClass=""
@ -63,13 +63,14 @@
</span>
</GhFormGroup>
<GhTaskButton @buttonText="Sign in &rarr;"
@task={{this.validateAndAuthenticate}}
<GhTaskButton
@buttonText="Sign in &rarr;"
@task={{this.validateAndAuthenticateTask}}
@showSuccess={{false}}
@class="login gh-btn gh-btn-login gh-btn-block gh-btn-icon js-login-button"
@class="login gh-btn gh-btn-login gh-btn-block gh-btn-icon"
@type="submit"
@tabindex="3"
style={{accent-color-background}} />
style={{accent-color-background}}
data-test-button="sign-in" />
</form>
<p class="main-error">{{if this.flowErrors this.flowErrors}}&nbsp;</p>

View File

@ -73,7 +73,7 @@ describe('Acceptance: Error Handling', function () {
await visit('/signin');
await fillIn('[name="identification"]', 'test@example.com');
await fillIn('[name="password"]', 'password');
await click('.js-login-button');
await click('[data-test-button="sign-in"]');
// has the refresh to update alert
expect(findAll('.gh-alert').length).to.equal(1);

View File

@ -62,7 +62,7 @@ describe('Acceptance: Signin', function () {
expect(findAll('input[name="password"]').length, 'password input field')
.to.equal(1);
await click('.js-login-button');
await click('[data-test-button="sign-in"]');
expect(findAll('.form-group.error').length, 'number of invalid fields')
.to.equal(2);
@ -72,7 +72,7 @@ describe('Acceptance: Signin', function () {
await fillIn('[name="identification"]', 'test@example.com');
await fillIn('[name="password"]', 'invalid');
await click('.js-login-button');
await click('[data-test-button="sign-in"]');
expect(currentURL(), 'current url').to.equal('/signin');
@ -91,7 +91,7 @@ describe('Acceptance: Signin', function () {
await fillIn('[name="identification"]', 'test@example.com');
await fillIn('[name="password"]', 'thisissupersafe');
await click('.js-login-button');
await click('[data-test-button="sign-in"]');
expect(currentURL(), 'currentURL').to.equal('/dashboard');
});
});