Merge Request #57

← To merge requests From registration_errors into master
Merged
noosfero-themes/angular-theme!57
Created by Daniela Feitosa

Enhancements on RegisterComponent 

* Move error messages to shared folder
* Refactor and addition of tests for signup on RegisterComponent
* Display error messages after sumbit registration
* Add new strings on translations files
* Add login link on registration page
Assignee: Caio Almeida
Milestone: None

Merged by Victor Costa

Source branch has been removed
Commits (1)
  • ecca2061   Enhancements on RegisterComponent ... Browse Code »
    1 
    Also:
* Move error messages to shared folder
* Refactor and addition of tests for signup on RegisterComponent
* Display error messages after sumbit registration
* Add new strings on translations files
* Add login link on registration page
    Daniela Feitosa
     
5 participants
Showing 10 changed files   Show diff stats
src/app/account/register-component.html
  Diff comments   Edit   View file @ecca206
... ... @@ -3,6 +3,14 @@
3 3 <h1>{{"account.register.welcomeMessageTitle" | translate }}</h1>
4 4 <div class="environment-signup-intro" ng-bind-html="ctrl.environment.signup_intro"></div>
5 5 </div>
  6 +
  7 + <div class="error-messages" ng-if="errorMessages">
  8 + <p>{{ "account.register.errorMessagesTitle" | translate }}</p>
  9 + <ul>
  10 + <li ng-repeat="error in errorMessages">{{ error.fieldName | translate }} {{ error.message | translate }}</li>
  11 + </ul>
  12 + </div>
  13 +
6 14 <form name="signupForm">
7 15 <div class="row">
8 16 <div class="col-md-12 register-field">
... ... @@ -17,10 +25,12 @@
17 25 <span class="input-group-addon" style="font-weight: bold;"><i class="fa fa-globe"></i>&nbsp;{{ ctrl.environment.host }}</span>
18 26 <input type="text" id="username" name="username" class="form-control" placeholder="{{'account.register.usernameLabel' | translate }}" ng-model="ctrl.account.login" required>
19 27 </div>
20   - <div class="help-block" ng-show="signupForm.username.$touched" ng-messages="signupForm.username.$error">
21   - <ul class="list-unstyled">
22   - <li ng-messages-include="languages/messages.html"></li>
23   - </ul>
  28 + <div class="help-block" ng-messages="signupForm.username.$error">
  29 + <div ng-if="signupForm.username.$touched && !signupForm.username.$empty">
  30 + <ul class="list-unstyled">
  31 + <li ng-messages-include="app/shared/messages/form-errors.html"></li>
1
  • Me
    Link here  
    Michel Felipe @mfdeveloper (Edited )

    @daniela Por que o arquivo .html contendo as mensagens de erro foram movidas? Essa pasta shared é um novo local para arquivos desse gênero? Vocês discutiram isso internamente? Esse tipo de abordagem é interessante, mas sugiro a atualização do README.md informando quais as arquivos devem estar nessa pasta, para que fique claro para todos os envolvidos :)

    Choose File ...   File name...
    Cancel
  32 + </ul>
  33 + </div>
24 34 </div>
25 35 </div>
26 36  
... ... @@ -29,22 +39,26 @@
29 39 <span class="input-group-addon"><i class="fa fa-envelope"></i></span>
30 40 <input type="email" class="form-control" id="email" name="email" placeholder="{{'account.register.emailLabel' | translate }}" ng-model="ctrl.account.email" required>
31 41 </div>
32   - <div class="help-block" ng-show="signupForm.email.$touched" ng-messages="signupForm.email.$error">
33   - <ul class="list-unstyled">
34   - <li ng-messages-include="languages/messages.html"></li>
35   - </ul>
  42 + <div class="help-block" ng-messages="signupForm.email.$error">
  43 + <div ng-if="signupForm.email.$touched && !signupForm.email.$empty">
  44 + <ul class="list-unstyled">
  45 + <li ng-messages-include="app/shared/messages/form-errors.html"></li>
  46 + </ul>
  47 + </div>
36 48 </div>
37 49 </div>
38 50  
39 51 <div class="form-group col-md-6 register-field" ng-class="ctrl.isInvalid(signupForm.password)">
40 52 <div class="input-group">
41 53 <span class="input-group-addon"><i class="fa fa-lock"></i></span>
42   - <input type="password" class="form-control" id="password" name="password" placeholder="{{'account.register.passwordLabel' | translate }}" ng-model="ctrl.account.password" required>
  54 + <input type="password" class="form-control" id="password" name="password" placeholder="{{'account.register.passwordLabel' | translate }}" ng-model="ctrl.account.password" required minlength="4">
43 55 </div>
44   - <div class="help-block" ng-show="signupForm.password.$touched" ng-messages="signupForm.password.$error">
45   - <ul class="list-unstyled">
46   - <li ng-messages-include="languages/messages.html"></li>
47   - </ul>
  56 + <div class="help-block" ng-messages="signupForm.password.$error">
  57 + <div ng-if="signupForm.password.$touched && !signupForm.password.$empty">
  58 + <ul class="list-unstyled">
  59 + <li ng-messages-include="app/shared/messages/form-errors.html"></li>
  60 + </ul>
  61 + </div>
48 62 </div>
49 63 </div>
50 64  
... ... @@ -55,7 +69,7 @@
55 69 </div>
56 70 <div class="help-block" ng-show="signupForm.passwordConfirm.$touched" ng-messages="signupForm.passwordConfirm.$error">
57 71 <ul class="list-unstyled">
58   - <li ng-messages-include="languages/messages.html"></li>
  72 + <li ng-messages-include="app/shared/messages/form-errors.html"></li>
59 73 <li ng-message="passwordMatch" translate="messages.invalid.passwordMatch"></li>
60 74 </ul>
61 75 </div>
... ... @@ -72,6 +86,5 @@
72 86 </div>
73 87 </form>
74 88  
75   - <p class="already-registered-message">{{"account.register.haveAccountMessage" | translate}}</p>
76   -
  89 + <a class="already-registered-message" ng-href="#" ng-click="ctrl.openLogin()">{{"account.register.haveAccountMessage" | translate}}</a>
77 90 </div>
... ...
src/app/account/register-terms.html
  Diff comments   Edit   View file @ecca206
1 1 <div class="modal-header">
2   - <h3 class="modal-title">Register terms</h3>
  2 + <h3 class="modal-title">{{ "account.register.termsOfUseTitle" | translate }} - {{ ctrl.environment.name }}</h3>
3 3 </div>
4 4 <div class="modal-body modal-body-overflow" ng-bind-html="ctrl.environment.terms_of_use"></div>
5 5 <div class="modal-footer">
... ...
src/app/account/register.component.spec.ts
  Diff comments   Edit   View file @ecca206
1 1 import { ComponentTestHelper, createClass } from "../../spec/component-test-helper";
2 2 import * as helpers from "../../spec/helpers";
3 3 import { RegisterComponent } from "./register.component";
4   -// import {RegisterService} from "../../lib/ng-noosfero-api/http/register.service"
5   -
6 4  
7 5 describe("Register Component", () => {
8 6 const htmlTemplate: string = '<noosfero-register></noosfero-register>';
9 7  
10 8 let helper: ComponentTestHelper<RegisterComponent>;
11 9 let registerService = helpers.mocks.registerService;
12   - let stateService = jasmine.createSpyObj("$state", ["transitionTo"]);
  10 + let stateService: angular.ui.IStateService;
13 11 let notificationService = helpers.mocks.notificationService;
14 12 notificationService.success = jasmine.createSpy('success');
15 13 notificationService.error = jasmine.createSpy('error');
16   -
17   -
18   - let account: any = {
19   - id: 1,
20   - login: 'test',
21   - email: 'test@email.com',
22   - password: 'xxx',
23   - passwordConfirmation: 'xxx'
24   - };
  14 + let user_data: any;
  15 + let response: any;
  16 + let deferred: any;
  17 + let $rootScope: ng.IRootScopeService;
  18 + let $q: ng.IQService;
25 19  
26 20 beforeEach(() => {
  21 + stateService = jasmine.createSpyObj("$state", ["transitionTo"]);
  22 +
27 23 angular.mock.module('templates');
28 24 angular.mock.module('ngSanitize');
29 25 angular.mock.module('ngMessages');
... ... @@ -45,8 +41,53 @@ describe(&quot;Register Component&quot;, () =&gt; {
45 41 helper = new ComponentTestHelper<RegisterComponent>(cls, done);
46 42 });
47 43  
  44 + beforeEach(inject((_$rootScope_: ng.IRootScopeService, _$q_: ng.IQService) => {
1
  • Me
    Link here  
    Michel Felipe @mfdeveloper

    Esse inject() é somente para injetar os serviços do angular? Se sim, existe uma classe AngularServiceFactory que contém o método getAngularService() voltado para esse tipo de "injeção". Salvo em algumas exceções, a idéia é tentar mitigar o máximo possível chamadas explicitas a elementos específicos do Angular 1.x, uma vez que estamos utilizando ngForward

    Choose File ...   File name...
    Cancel
  45 + $rootScope = _$rootScope_;
  46 + $q = _$q_;
  47 + }));
  48 +
48 49 it('register page was rendered', () => {
49 50 expect(helper.debugElement.query('div.register-page').length).toEqual(1);
50 51 });
51 52  
  53 + it("registers a new user", done => {
  54 + user_data = { username: "username", password: "password", password_confirmation: "password", email: "user@company.com" };
  55 + response = { };
  56 +
  57 + helper.component.account = user_data;
  58 +
  59 + deferred = $q.defer();
  60 + deferred.resolve({ data: response });
  61 + registerService.createAccount = jasmine.createSpy("createAccount").and.returnValue(deferred.promise);
  62 +
  63 + helper.component.signup();
  64 + $rootScope.$apply();
1
  • Me
    Link here  
    Michel Felipe @mfdeveloper

    È realmente necessário uma chamada explicita a $apply()? A classe do ngForward chamada ComponentFixture possui um método detectChanges(), que no final das contas faz uma chamada a $apply() por debaixo dos panos. Como vc está utilizando a classe ComponentTestHelper nesses testes, nela existe o método detectChanges() que é um "atalho". Verifique se isso atende ao que você precisa :)

    Choose File ...   File name...
    Cancel
  65 +
  66 + expect(registerService.createAccount).toHaveBeenCalledWith(user_data);
  67 + expect(stateService.transitionTo).toHaveBeenCalledWith("main.environment");
  68 + expect(notificationService.success).toHaveBeenCalled();
  69 +
  70 + done();
  71 + });
  72 +
  73 + it("gives error when registration fails", done => {
  74 + user_data = { password: "pas" };
  75 + response = { data: { message: '{ "password": ["is too short"] }'}};
  76 +
  77 + helper.component.account = user_data;
  78 +
  79 + deferred = $q.defer();
  80 + deferred.reject(response);
1
  • Me
    Link here  
    Michel Felipe @mfdeveloper

    Uma chamada direta a Promise.reject() não funcionaria ao invés de injetar o serviço $q?

    Choose File ...   File name...
    Cancel
  81 + registerService.createAccount = jasmine.createSpy("createAccount").and.returnValue(deferred.promise);
  82 +
  83 + helper.component.signup();
  84 + $rootScope.$apply();
  85 +
  86 + expect(registerService.createAccount).toHaveBeenCalledWith(user_data);
  87 +
  88 + expect(stateService.transitionTo).not.toHaveBeenCalledWith("main.environment");
  89 + expect(notificationService.error).toHaveBeenCalled();
  90 +
  91 + done();
  92 + });
52 93 });
... ...
src/app/account/register.component.ts
  Diff comments   Edit   View file @ecca206
... ... @@ -3,7 +3,8 @@ import { RegisterService } from &quot;./../../lib/ng-noosfero-api/http/register.servi
3 3 import { NotificationService } from "./../shared/services/notification.service";
4 4 import { EnvironmentService } from "../../lib/ng-noosfero-api/http/environment.service";
5 5 import { RegisterController } from "./register.controller";
6   -import { IModalComponent } from "../shared/components/interfaces";
  6 +import { AuthController } from "./../login";
  7 +import { IModalComponent, IErrorMessages } from "../shared/components/interfaces";
7 8  
8 9 @Component({
9 10 selector: 'noosfero-register',
... ... @@ -23,7 +24,7 @@ export class RegisterComponent {
23 24 constructor(
24 25 private $state: ng.ui.IStateService,
25 26 private $uibModal: ng.ui.bootstrap.IModalService,
26   - private $scope: ng.IScope,
  27 + private $scope: IErrorMessages,
27 28 public registerService: RegisterService,
28 29 private notificationService: NotificationService,
29 30 private environmentService: EnvironmentService
... ... @@ -33,19 +34,27 @@ export class RegisterComponent {
33 34 }
34 35  
35 36 signup() {
36   - if (this.account.password === this.account.passwordConfirmation) {
37   - this.registerService.createAccount(this.account).then((response) => {
38   -
39   - if (response.status === 201) {
40   - this.$state.transitionTo('main.environment');
41   - this.notificationService.success({ title: "account.register.success.title", message: "account.register.success.message" });
42   - } else {
43   - throw new Error('Invalid attributes');
44   - }
45   - });
46   - } else {
47   - this.notificationService.error({ message: "account.register.passwordConfirmation.failed" });
48   - }
  37 + let error = '';
  38 + let errors: any;
  39 + let field = '';
  40 + this.$scope.errorMessages = [];
  41 + this.registerService.createAccount(this.account).then((response) => {
  42 + this.$state.transitionTo('main.environment');
  43 + this.notificationService.success({ title: "account.register.success.title", message: "account.register.success.message" });
  44 + }).catch((response) => {
  45 + if ( response.data.error ) {
1
  • Me
    Link here  
    Michel Felipe @mfdeveloper (Edited )

    A utilização deste atributo errorMessages contendo os erros gerados pelo backend é realmente a melhor solução? Vocês discutiram internamente em equipe acerca disso @daniela ? Observe que, uma vez que é chamado simplesmente um catch() da Promise resultante da requisição REST, mensagens de erros relacionadas a status como 500, 503 entre outros serão exibidas para o usuário correto?

    Além disso, uma vez que a validação já é controlada pelo ngMessages, faz sentido requisitar o servidor para obter "campos requeridos" por exemplo? È bem verdade que é configurável no Noosfero quais campos são ou não obrigatórios no registro de usuário, e que "replicar" essas verificações no front-end parece uma duplicação da regra de negócio, mas de qualquer forma, o usuário teria as duas mensagens exibidas, tanto nos campos quanto acima do form, com o novo HTML adicionado por esse MR.

    Em suma, qual a avaliação de vocês sobre essas questões? Questiono isso para pensarmos juntos realmente xD

    Choose File ...   File name...
    Cancel
  46 + errors = response.data['error'].split(', ');
  47 + for (error in errors) {
  48 + this.$scope.errorMessages.push({ message: errors[error] });
  49 + }
  50 + } else if ( response.data.message ) {
  51 + errors = JSON.parse(response.data.message);
  52 + for (field in errors) {
  53 + this.$scope.errorMessages.push({ fieldName: field, message: errors[field][0] });
  54 + }
  55 + }
  56 + this.notificationService.error({ title: "account.register.save.failed" });
  57 + });
49 58 }
50 59  
51 60 isInvalid(field: any): any {
... ... @@ -63,4 +72,14 @@ export class RegisterComponent {
63 72 scope: this.$scope
64 73 });
65 74 }
  75 +
  76 + openLogin() {
1
  • Me
    Link here  
    Michel Felipe @mfdeveloper (Edited )

    O código deste método parece estar duplicado com a definição em Navbar#openLogin() nao? Acredito que seria melhor acessar o componente Navbar, ou até mesmo mover esse método para a classe AuthController, para que seja acessível e reutilizável entre os dois componentes. O que você acha @daniela ?

    Choose File ...   File name...
    Cancel
  77 + this.modalInstance = this.$uibModal.open({
  78 + templateUrl: 'app/login/login.html',
  79 + controller: AuthController,
  80 + controllerAs: 'vm',
  81 + bindToController: true
  82 + });
  83 + }
  84 +
66 85 }
... ...
src/app/account/scss/register.scss
  Diff comments   Edit   View file @ecca206
... ... @@ -29,6 +29,7 @@
29 29 .register-page .already-registered-message {
30 30 margin-top: 60px;
31 31 text-align: center;
  32 + display: block;
32 33 }
33 34  
34 35 .register-page input {
... ...
src/app/shared/components/interfaces.ts
  Diff comments   Edit   View file @ecca206
... ... @@ -6,3 +6,7 @@ export interface IModalComponent {
6 6 $uibModal: ng.ui.bootstrap.IModalService;
7 7 modalInstance: ng.ui.bootstrap.IModalServiceInstance;
8 8 }
  9 +
  10 +export interface IErrorMessages extends ng.IScope {
1
  • Me
    Link here  
    Michel Felipe @mfdeveloper

    Pq criar uma interface só para isso? errorMessages não poderia simplesmente ser um atributo do componente RegisterComponent? Observe que os atributos de um componente são inseridos automaticamente no $scope, uma vez q o ngForward faz esse de => para

    Choose File ...   File name...
    Cancel
  11 + errorMessages: any;
  12 +}
... ...
src/app/shared/messages/form-errors.html 0 → 100644
  Diff comments   Edit   View file @ecca206
... ... @@ -0,0 +1,4 @@
  1 +<li ng-message="required" translate="messages.invalid.required"></li>
  2 +<li ng-message="minlength" translate="messages.invalid.minlength"></li>
  3 +<li ng-message="maxlength" translate="messages.invalid.maxlength"></li>
  4 +<li ng-message="email" translate="messages.invalid.email"></li>
... ...
src/languages/en.json
  Diff comments   Edit   View file @ecca206
... ... @@ -112,6 +112,7 @@
112 112 "account.register.signupMessage": "Register",
113 113 "account.register.haveAccountMessage": "Already have an account?",
114 114 "account.register.termsOfUseMessage": "terms of use",
  115 + "account.register.termsOfUseTitle": "Terms of use",
1
  • Me
    Link here  
    Michel Felipe @mfdeveloper (Edited )

    Parece que há uma duplicidade de valores entre as chaves account.register.termsOfUseMessage e account.register.termsOfUseTitlenao? A única coisa diferente entre os seus valores é o "T" em caixa alta. Essas duas chaves são realmente necessárias?

    Choose File ...   File name...
    Cancel
115 116 "account.register.success.title": "Good job!",
116 117 "account.register.success.message": "Account created!",
117 118 "account.register.passwordConfirmation.failed": "Wrong password confirmation",
... ... @@ -119,5 +120,20 @@
119 120 "messages.invalid.maxlength": "This field is too long",
120 121 "messages.invalid.minlength": "This field is too short",
121 122 "messages.invalid.email": "This needs to be a valid email",
122   - "messages.invalid.passwordMatch": "Your passwords did not match"
  123 + "messages.invalid.passwordMatch": "Your passwords did not match",
  124 + "account.register.save.failed": "This account could not be created",
  125 + "account.register.mandatoryField": "This field is mandatory",
  126 + "account.register.invalidEmail": "This field must be a valid email address.",
  127 + "account.register.shortPassword": "The password must have at least 4 characters)",
  128 + "account.register.errorMessagesTitle": "There were problems with the following fields:",
  129 + "email is missing": "email can't be blank",
  130 + "login is missing": "username can't be blank",
  131 + "password is missing": "password can't be blank",
  132 + "login": "username",
  133 + "has already been taken": "has already been taken",
  134 + "email": "email",
  135 + "identifier": "identifier",
  136 + "is not available.": "is not available",
  137 + "user": "user",
  138 + "is invalid": "is invalid"
123 139 }
... ...
src/languages/messages.html
View file @26ff533
... ... @@ -1,4 +0,0 @@
1   -<li ng-message="required" translate="messages.invalid.required"></li>
2   -<li ng-message="minlength" translate="messages.invalid.minlength"></li>
3   -<li ng-message="maxlength" translate="messages.invalid.maxlength"></li>
4   -<li ng-message="email" translate="messages.invalid.email"></li>
src/languages/pt.json
  Diff comments   Edit   View file @ecca206
... ... @@ -113,6 +113,7 @@
113 113 "account.register.passwordConfirmationLabel": "Confirmar senha",
114 114 "account.register.accountCreatingMessage": "Ao criar uma conta, você está concordando com os ",
115 115 "account.register.termsOfUseMessage": "termos de uso",
  116 + "account.register.termsOfUseTitle": "Termos de uso",
116 117 "account.register.signupMessage": "Criar Conta",
117 118 "account.register.haveAccountMessage": "Já possui uma conta?",
118 119 "account.register.success.title": "Bom trabalho!",
... ... @@ -122,5 +123,20 @@
122 123 "messages.invalid.maxlength": "O valor é muito longo",
123 124 "messages.invalid.minlength": "O valor é muito curto",
124 125 "messages.invalid.email": "Informe um email válido",
125   - "messages.invalid.passwordMatch": "As senhas não coincidem"
  126 + "messages.invalid.passwordMatch": "As senhas não coincidem",
  127 + "account.register.save.failed": "A conta não pode ser criada",
  128 + "account.register.mandatoryField": "Este campo é obrigatório",
  129 + "account.register.invalidEmail": "Este campo deve ser um endereço de e-mail válido.",
  130 + "account.register.shortPassword": "A senha deve ter no mínimo 4 caracteres",
  131 + "account.register.errorMessagesTitle": "Houve erros com os seguintes campos:",
  132 + "email is missing": "email não pode ficar em branco",
  133 + "login is missing": "nome de usuário não pode ficar em branco",
  134 + "password is missing": "senha não pode ficar em branco",
  135 + "login": "nome de usuário",
  136 + "has already been taken": "já está sendo usado",
  137 + "email": "email",
1
  138 + "identifier": "identificador",
  139 + "is not available.": "não está disponível",
  140 + "user": "usuário",
  141 + "is invalid": "é inválido"
126 142 }
... ...
  • 0743c2eb05e68c92baa5f8c498995a20?s=40&d=identicon
    Link here  
    Daniela Feitosa @daniela

    @caiosba @mfdeveloper: esse MR é uma continuação do !47

    Choose File ...   File name...
    Cancel
  • Me
    Link here  
    Michel Felipe @mfdeveloper (Edited )

    Perfeito @daniela Irei revisá-lo :)

    Choose File ...   File name...
    Cancel
    • Me
    Show/hide discussion
    Michel Felipe started a discussion on the diff
    last updated by Michel Felipe
    src/app/account/register-component.html
    17 25 <span class="input-group-addon" style="font-weight: bold;"><i class="fa fa-globe"></i>&nbsp;{{ ctrl.environment.host }}</span>
    18 26 <input type="text" id="username" name="username" class="form-control" placeholder="{{'account.register.usernameLabel' | translate }}" ng-model="ctrl.account.login" required>
    19 27 </div>
    20   - <div class="help-block" ng-show="signupForm.username.$touched" ng-messages="signupForm.username.$error">
    21   - <ul class="list-unstyled">
    22   - <li ng-messages-include="languages/messages.html"></li>
    23   - </ul>
      28 + <div class="help-block" ng-messages="signupForm.username.$error">
      29 + <div ng-if="signupForm.username.$touched && !signupForm.username.$empty">
      30 + <ul class="list-unstyled">
      31 + <li ng-messages-include="app/shared/messages/form-errors.html"></li>
    1
    • Me
      Link here  
      Michel Felipe @mfdeveloper (Edited )

      @daniela Por que o arquivo .html contendo as mensagens de erro foram movidas? Essa pasta shared é um novo local para arquivos desse gênero? Vocês discutiram isso internamente? Esse tipo de abordagem é interessante, mas sugiro a atualização do README.md informando quais as arquivos devem estar nessa pasta, para que fique claro para todos os envolvidos :)

      Choose File ...   File name...
      Cancel
    Me
    Show/hide discussion
    Michel Felipe started a discussion on the diff
    last updated by Michel Felipe
    src/app/account/register.component.ts
    63 72 scope: this.$scope
    64 73 });
    65 74 }
      75 +
      76 + openLogin() {
    1
    • Me
      Link here  
      Michel Felipe @mfdeveloper (Edited )

      O código deste método parece estar duplicado com a definição em Navbar#openLogin() nao? Acredito que seria melhor acessar o componente Navbar, ou até mesmo mover esse método para a classe AuthController, para que seja acessível e reutilizável entre os dois componentes. O que você acha @daniela ?

      Choose File ...   File name...
      Cancel
    Me
    Show/hide discussion
    Michel Felipe started a discussion on the diff
    last updated by Michel Felipe
    src/languages/en.json
    112 112 "account.register.signupMessage": "Register",
    113 113 "account.register.haveAccountMessage": "Already have an account?",
    114 114 "account.register.termsOfUseMessage": "terms of use",
      115 + "account.register.termsOfUseTitle": "Terms of use",
    1
    • Me
      Link here  
      Michel Felipe @mfdeveloper (Edited )

      Parece que há uma duplicidade de valores entre as chaves account.register.termsOfUseMessage e account.register.termsOfUseTitlenao? A única coisa diferente entre os seus valores é o "T" em caixa alta. Essas duas chaves são realmente necessárias?

      Choose File ...   File name...
      Cancel
    Me
    Show/hide discussion
    Michel Felipe started a discussion on the diff
    last updated by Michel Felipe
    src/languages/pt.json
    122 123 "messages.invalid.maxlength": "O valor é muito longo",
    123 124 "messages.invalid.minlength": "O valor é muito curto",
    124 125 "messages.invalid.email": "Informe um email válido",
    125   - "messages.invalid.passwordMatch": "As senhas não coincidem"
      126 + "messages.invalid.passwordMatch": "As senhas não coincidem",
      127 + "account.register.save.failed": "A conta não pode ser criada",
      128 + "account.register.mandatoryField": "Este campo é obrigatório",
      129 + "account.register.invalidEmail": "Este campo deve ser um endereço de e-mail válido.",
      130 + "account.register.shortPassword": "A senha deve ter no mínimo 4 caracteres",
      131 + "account.register.errorMessagesTitle": "Houve erros com os seguintes campos:",
      132 + "email is missing": "email não pode ficar em branco",
      133 + "login is missing": "nome de usuário não pode ficar em branco",
      134 + "password is missing": "senha não pode ficar em branco",
      135 + "login": "nome de usuário",
      136 + "has already been taken": "já está sendo usado",
      137 + "email": "email",
    1
    Me
    Show/hide discussion
    Michel Felipe started a discussion on the diff
    last updated by Michel Felipe
    src/app/account/register.component.ts
    42   - } else {
    43   - throw new Error('Invalid attributes');
    44   - }
    45   - });
    46   - } else {
    47   - this.notificationService.error({ message: "account.register.passwordConfirmation.failed" });
    48   - }
      37 + let error = '';
      38 + let errors: any;
      39 + let field = '';
      40 + this.$scope.errorMessages = [];
      41 + this.registerService.createAccount(this.account).then((response) => {
      42 + this.$state.transitionTo('main.environment');
      43 + this.notificationService.success({ title: "account.register.success.title", message: "account.register.success.message" });
      44 + }).catch((response) => {
      45 + if ( response.data.error ) {
    1
    • Me
      Link here  
      Michel Felipe @mfdeveloper (Edited )

      A utilização deste atributo errorMessages contendo os erros gerados pelo backend é realmente a melhor solução? Vocês discutiram internamente em equipe acerca disso @daniela ? Observe que, uma vez que é chamado simplesmente um catch() da Promise resultante da requisição REST, mensagens de erros relacionadas a status como 500, 503 entre outros serão exibidas para o usuário correto?

      Além disso, uma vez que a validação já é controlada pelo ngMessages, faz sentido requisitar o servidor para obter "campos requeridos" por exemplo? È bem verdade que é configurável no Noosfero quais campos são ou não obrigatórios no registro de usuário, e que "replicar" essas verificações no front-end parece uma duplicação da regra de negócio, mas de qualquer forma, o usuário teria as duas mensagens exibidas, tanto nos campos quanto acima do form, com o novo HTML adicionado por esse MR.

      Em suma, qual a avaliação de vocês sobre essas questões? Questiono isso para pensarmos juntos realmente xD

      Choose File ...   File name...
      Cancel
    Me
    Show/hide discussion
    Michel Felipe started a discussion on the diff
    last updated by Michel Felipe
    src/app/shared/components/interfaces.ts
    6 6 $uibModal: ng.ui.bootstrap.IModalService;
    7 7 modalInstance: ng.ui.bootstrap.IModalServiceInstance;
    8 8 }
      9 +
      10 +export interface IErrorMessages extends ng.IScope {
    1
    • Me
      Link here  
      Michel Felipe @mfdeveloper

      Pq criar uma interface só para isso? errorMessages não poderia simplesmente ser um atributo do componente RegisterComponent? Observe que os atributos de um componente são inseridos automaticamente no $scope, uma vez q o ngForward faz esse de => para

      Choose File ...   File name...
      Cancel
    Me
    Show/hide discussion
    Michel Felipe started a discussion on the diff
    last updated by Michel Felipe
    src/app/account/register.component.spec.ts
    45 41 helper = new ComponentTestHelper<RegisterComponent>(cls, done);
    46 42 });
    47 43  
      44 + beforeEach(inject((_$rootScope_: ng.IRootScopeService, _$q_: ng.IQService) => {
    1
    • Me
      Link here  
      Michel Felipe @mfdeveloper

      Esse inject() é somente para injetar os serviços do angular? Se sim, existe uma classe AngularServiceFactory que contém o método getAngularService() voltado para esse tipo de "injeção". Salvo em algumas exceções, a idéia é tentar mitigar o máximo possível chamadas explicitas a elementos específicos do Angular 1.x, uma vez que estamos utilizando ngForward

      Choose File ...   File name...
      Cancel
    Me
    Show/hide discussion
    Michel Felipe started a discussion on the diff
    last updated by Michel Felipe
    src/app/account/register.component.spec.ts
    48 49 it('register page was rendered', () => {
    49 50 expect(helper.debugElement.query('div.register-page').length).toEqual(1);
    50 51 });
    51 52  
      53 + it("registers a new user", done => {
      54 + user_data = { username: "username", password: "password", password_confirmation: "password", email: "user@company.com" };
      55 + response = { };
      56 +
      57 + helper.component.account = user_data;
      58 +
      59 + deferred = $q.defer();
      60 + deferred.resolve({ data: response });
      61 + registerService.createAccount = jasmine.createSpy("createAccount").and.returnValue(deferred.promise);
      62 +
      63 + helper.component.signup();
      64 + $rootScope.$apply();
    1
    • Me
      Link here  
      Michel Felipe @mfdeveloper

      È realmente necessário uma chamada explicita a $apply()? A classe do ngForward chamada ComponentFixture possui um método detectChanges(), que no final das contas faz uma chamada a $apply() por debaixo dos panos. Como vc está utilizando a classe ComponentTestHelper nesses testes, nela existe o método detectChanges() que é um "atalho". Verifique se isso atende ao que você precisa :)

      Choose File ...   File name...
      Cancel
    Me
    Show/hide discussion
    Michel Felipe started a discussion on the diff
    last updated by Michel Felipe
    src/app/account/register.component.spec.ts
      65 +
      66 + expect(registerService.createAccount).toHaveBeenCalledWith(user_data);
      67 + expect(stateService.transitionTo).toHaveBeenCalledWith("main.environment");
      68 + expect(notificationService.success).toHaveBeenCalled();
      69 +
      70 + done();
      71 + });
      72 +
      73 + it("gives error when registration fails", done => {
      74 + user_data = { password: "pas" };
      75 + response = { data: { message: '{ "password": ["is too short"] }'}};
      76 +
      77 + helper.component.account = user_data;
      78 +
      79 + deferred = $q.defer();
      80 + deferred.reject(response);
    1
    • Me
      Link here  
      Michel Felipe @mfdeveloper

      Uma chamada direta a Promise.reject() não funcionaria ao invés de injetar o serviço $q?

      Choose File ...   File name...
      Cancel
  • C8b72d0556872a2aea21e8fed0a72001?s=40&d=identicon
    Link here  
    Melissa Wen @melissawen

    mentioned in issue #138

    Choose File ...   File name...
    Cancel
  • 4a20548511a65cfccc863520b70c3ee9?s=40&d=identicon
    Link here  
    Victor Costa @vfcosta

    Merged manually, thanks!

    Choose File ...   File name...
    Cancel
    • 0743c2eb05e68c92baa5f8c498995a20?s=40&d=identicon
    Show/hide discussion
    Daniela Feitosa started a discussion on commit ecca2061
    last updated by Daniela Feitosa
  • 0743c2eb05e68c92baa5f8c498995a20?s=40&d=identicon
    Link here  
    Daniela Feitosa @daniela

    mentioned in issue #138

    Choose File ...   File name...
    Cancel