Merge Request #29

← To merge requests From remove-article-confirmation into master
Merged
noosfero-themes/angular-theme!29
Created by Carlos Purificação

Remove article confirmation

Assignee: Ábner Oliveira
Milestone: 2016.05

Merged by Ábner Oliveira

Commits (2)
3 participants
participa serpro
Showing 5 changed files   Show diff stats
src/app/article/article-default-view-component.spec.ts
  Diff comments   Edit   View file @d6122de
... ... @@ -19,6 +19,7 @@ describe("Components", () => {
19 19 describe("Article Default View Component", () => {
20 20 let helper: ComponentTestHelper<ArticleDefaultViewComponent>;
21 21 const defaultViewTemplate: string = '<noosfero-default-article [article]="ctrl.article" [profile]="ctrl.profile"></noosfero-default-article>';
  22 + let notificationService = helpers.mocks.notificationService;
22 23 let articleService: any = helpers.mocks.articleService;
23 24 let article = <noosfero.Article>{
24 25 id: 1,
... ... @@ -29,7 +30,8 @@ describe(&quot;Components&quot;, () =&gt; {
29 30 let state = <ng.ui.IStateService>jasmine.createSpyObj("state", ["go", "transitionTo"]);
30 31 let providers = [
31 32 provide('$state', { useValue: state }),
32   - provide('ArticleService', { useValue: articleService })
  33 + provide('ArticleService', { useValue: articleService }),
  34 + helpers.createProviderToValue('NotificationService', notificationService),
33 35 ].concat(helpers.provideFilters("translateFilter"));
34 36  
35 37 /**
... ... @@ -67,13 +69,19 @@ describe(&quot;Components&quot;, () =&gt; {
67 69 * Execute the delete method on the target component
68 70 */
69 71 function doDeleteArticle() {
  72 + // Create a mock for the notification service confirmation
  73 + spyOn(helper.component.notificationService, 'confirmation').and.callFake(function (params: Function) {
2
  • Me
    Link here  
    Michel Felipe @mfdeveloper

    Não seria mais indicado utilizar jasmine.createSpy("confirmation") ou jasmine.createSpyObj("notificationService", ["confirmation"]) como já está amplamente utilizado em outros testes? As diversas formas para implementar coisas muito semelhantes pode causar confusão no futuro. Chegou a conversar rapidamente com @abner sobre isso?

    Choose File ...   File name...
    Cancel
  • 2c5c4299d62769e3da7d432cd2823dd6?s=40&d=identicon
    Link here  
    Carlos Purificação @carloseugenio

    Não sei direito qual a diferença. Como não estava conseguindo utilizar o jasmine.createSpy, vi isto na documentação do jasmine e utilizei.

    Choose File ...   File name...
    Cancel
  74 +
  75 + });
70 76 // Create a mock for the ArticleService removeArticle method
71   - spyOn(helper.component.articleService, 'removeArticle').and.callFake(function(param: noosfero.Article) {
  77 + spyOn(helper.component.articleService, 'removeArticle').and.callFake(function (param: noosfero.Article) {
72 78 return {
73   - catch: () => {}
  79 + catch: () => { }
74 80 };
75 81 });
76 82 helper.component.delete();
  83 + expect(notificationService.confirmation).toHaveBeenCalled();
  84 + helper.component.doDelete();
77 85 expect(articleService.removeArticle).toHaveBeenCalled();
78 86 // After the component delete method execution, fire the
79 87 // ArticleEvent.removed event
... ... @@ -81,9 +89,11 @@ describe(&quot;Components&quot;, () =&gt; {
81 89 }
82 90  
83 91 /**
84   - * Simulate the ArticleService ArticleEvent.removed event
  92 + * Simulate the Notification Service confirmation and ArticleService
  93 + * notifyArticleRemovedListeners event
85 94 */
86 95 function simulateRemovedEvent() {
  96 + helper.component.notificationService["confirmation"]({ title: "Title", message: "Message" }, () => { });
87 97 helper.component.articleService["notifyArticleRemovedListeners"](article);
88 98 }
89 99 });
... ...
src/app/article/article-default-view.component.ts
  Diff comments   Edit   View file @d6122de
... ... @@ -5,6 +5,7 @@ import {MacroDirective} from &quot;./macro/macro.directive&quot;;
5 5 import {ArticleToolbarHotspotComponent} from "../hotspot/article-toolbar-hotspot.component";
6 6 import {ArticleContentHotspotComponent} from "../hotspot/article-content-hotspot.component";
7 7 import {ArticleService} from "./../../lib/ng-noosfero-api/http/article.service";
  8 +import { NotificationService } from "./../shared/services/notification.service";
8 9  
9 10 /**
10 11 * @ngdoc controller
... ... @@ -17,13 +18,13 @@ import {ArticleService} from &quot;./../../lib/ng-noosfero-api/http/article.service&quot;;
17 18 selector: 'noosfero-default-article',
18 19 templateUrl: 'app/article/article.html'
19 20 })
20   -@Inject("$state", ArticleService)
  21 +@Inject("$state", ArticleService, NotificationService)
21 22 export class ArticleDefaultViewComponent {
22 23  
23 24 @Input() article: noosfero.Article;
24 25 @Input() profile: noosfero.Profile;
25 26  
26   - constructor(private $state: ng.ui.IStateService, public articleService: ArticleService) {
  27 + constructor(private $state: ng.ui.IStateService, public articleService: ArticleService, public notificationService: NotificationService) {
27 28 // Subscribe to the Article Removed Event
28 29 this.articleService.subscribeToArticleRemoved((article: noosfero.Article) => {
29 30 if (this.article.parent) {
... ... @@ -31,15 +32,21 @@ export class ArticleDefaultViewComponent {
31 32 } else {
32 33 this.$state.transitionTo('main.profile.info', { profile: this.article.profile.identifier });
33 34 }
  35 + this.notificationService.success({ title: "article.remove.success.title", message: "article.remove.success.message" });
34 36 });
35 37 }
36 38  
37 39 delete() {
38   - this.articleService.removeArticle(this.article).catch((cause: any) => {
39   - throw new Error(`Problem removing the article: ${cause}`);
  40 + this.notificationService.confirmation({ title: "article.remove.confirmation.title", message: "article.remove.confirmation.message" }, () => {
  41 + this.doDelete();
40 42 });
41 43 }
42 44  
  45 + doDelete() {
  46 + this.articleService.removeArticle(this.article).catch((cause: any) => {
  47 + throw new Error(`Problem removing the article: ${cause}`);
3
  • Me
    Link here  
    Michel Felipe @mfdeveloper

    Se ocorrer um problema, essa mensagem será exibida para o usuário final?

    Choose File ...   File name...
    Cancel
  • 2c5c4299d62769e3da7d432cd2823dd6?s=40&d=identicon
    Link here  
    Carlos Purificação @carloseugenio

    Está conforme foi feito nos outros callbacks de serviços. Ach que tem ser feita uma revisão geral de como será o padrão de funcionamento das mensagens de erro.

    Choose File ...   File name...
    Cancel
  • 0deafa1501ec8dd687ee70f90488d592?s=40&d=identicon
    Link here  
    Ábner Oliveira @abner

    Como esse catch aí é num componente de tela, então deveria estar utilizando o NotificationService para exibir a mensagem de erro para o usuário. Os serviços não devem realmente tratar a exibição de mensagens, mas os componentes de tela devem.

    Choose File ...   File name...
    Cancel
  48 + });
  49 + }
43 50 }
44 51  
45 52 /**
... ...
src/languages/en.json
  Diff comments   Edit   View file @d6122de
... ... @@ -56,6 +56,10 @@
56 56 "article.basic_editor.cancel": "Cancel",
57 57 "article.basic_editor.success.title": "Good job!",
58 58 "article.basic_editor.success.message": "Article saved!",
  59 + "article.remove.success.title": "Good job!",
  60 + "article.remove.success.message": "Article removed!",
  61 + "article.remove.confirmation.title": "Are you sure?",
  62 + "article.remove.confirmation.message": "You will not be able to recover this article!",
59 63 "article.basic_editor.visibility": "Visibility",
60 64 "article.basic_editor.visibility.public": "Public",
61 65 "article.basic_editor.visibility.private": "Private",
... ...
src/languages/pt.json
  Diff comments   Edit   View file @d6122de
... ... @@ -53,6 +53,10 @@
53 53 "article.basic_editor.cancel": "Cancelar",
54 54 "article.basic_editor.success.title": "Bom trabalho!",
55 55 "article.basic_editor.success.message": "Artigo salvo com sucesso!",
  56 + "article.remove.success.title": "Bom trabalho!",
1
  • Me
    Link here  
    Michel Felipe @mfdeveloper

    As chaves article.remove.success.title e article.remove.confirmation.title são específicas da tela de artigo? Outras telas não relacionadas com artigo poderão utilizar essas chaves?

    Choose File ...   File name...
    Cancel
  57 + "article.remove.success.message": "Artigo removido!",
  58 + "article.remove.confirmation.title": "Tem certeza?",
  59 + "article.remove.confirmation.message": "Não será possível recuperar este artigo!",
56 60 "article.basic_editor.visibility": "Visibilidade",
57 61 "article.basic_editor.visibility.public": "Público",
58 62 "article.basic_editor.visibility.private": "Privado",
... ...
src/spec/mocks.ts
  Diff comments   Edit   View file @d6122de
... ... @@ -192,6 +192,7 @@ export var mocks: any = {
192 192 translate: (text: string) => { return text; }
193 193 },
194 194 notificationService: {
195   - success: () => { }
  195 + success: () => { },
  196 + confirmation: () => { }
196 197 }
197 198 };
... ...
  • Me
    Link here  
    Michel Felipe @mfdeveloper

    Reassigned to @mfdeveloper

    Choose File ...   File name...
    Cancel
    • Me
    Show/hide discussion
    Michel Felipe started a discussion on the diff
    last updated by Carlos Purificação
    src/app/article/article-default-view-component.spec.ts
    67 69 * Execute the delete method on the target component
    68 70 */
    69 71 function doDeleteArticle() {
      72 + // Create a mock for the notification service confirmation
      73 + spyOn(helper.component.notificationService, 'confirmation').and.callFake(function (params: Function) {
    2
    • Me
      Link here  
      Michel Felipe @mfdeveloper

      Não seria mais indicado utilizar jasmine.createSpy("confirmation") ou jasmine.createSpyObj("notificationService", ["confirmation"]) como já está amplamente utilizado em outros testes? As diversas formas para implementar coisas muito semelhantes pode causar confusão no futuro. Chegou a conversar rapidamente com @abner sobre isso?

      Choose File ...   File name...
      Cancel
    • 2c5c4299d62769e3da7d432cd2823dd6?s=40&d=identicon
      Link here  
      Carlos Purificação @carloseugenio

      Não sei direito qual a diferença. Como não estava conseguindo utilizar o jasmine.createSpy, vi isto na documentação do jasmine e utilizei.

      Choose File ...   File name...
      Cancel
    Me
    Show/hide discussion
    Michel Felipe started a discussion on the diff
    last updated by Ábner Oliveira
    src/app/article/article-default-view.component.ts
    33 34 }
      35 + this.notificationService.success({ title: "article.remove.success.title", message: "article.remove.success.message" });
    34 36 });
    35 37 }
    36 38  
    37 39 delete() {
    38   - this.articleService.removeArticle(this.article).catch((cause: any) => {
    39   - throw new Error(`Problem removing the article: ${cause}`);
      40 + this.notificationService.confirmation({ title: "article.remove.confirmation.title", message: "article.remove.confirmation.message" }, () => {
      41 + this.doDelete();
    40 42 });
    41 43 }
    42 44  
      45 + doDelete() {
      46 + this.articleService.removeArticle(this.article).catch((cause: any) => {
      47 + throw new Error(`Problem removing the article: ${cause}`);
    3
    • Me
      Link here  
      Michel Felipe @mfdeveloper

      Se ocorrer um problema, essa mensagem será exibida para o usuário final?

      Choose File ...   File name...
      Cancel
    • 2c5c4299d62769e3da7d432cd2823dd6?s=40&d=identicon
      Link here  
      Carlos Purificação @carloseugenio

      Está conforme foi feito nos outros callbacks de serviços. Ach que tem ser feita uma revisão geral de como será o padrão de funcionamento das mensagens de erro.

      Choose File ...   File name...
      Cancel
    • 0deafa1501ec8dd687ee70f90488d592?s=40&d=identicon
      Link here  
      Ábner Oliveira @abner

      Como esse catch aí é num componente de tela, então deveria estar utilizando o NotificationService para exibir a mensagem de erro para o usuário. Os serviços não devem realmente tratar a exibição de mensagens, mas os componentes de tela devem.

      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
    53 53 "article.basic_editor.cancel": "Cancelar",
    54 54 "article.basic_editor.success.title": "Bom trabalho!",
    55 55 "article.basic_editor.success.message": "Artigo salvo com sucesso!",
      56 + "article.remove.success.title": "Bom trabalho!",
    1
    • Me
      Link here  
      Michel Felipe @mfdeveloper

      As chaves article.remove.success.title e article.remove.confirmation.title são específicas da tela de artigo? Outras telas não relacionadas com artigo poderão utilizar essas chaves?

      Choose File ...   File name...
      Cancel
  • 0deafa1501ec8dd687ee70f90488d592?s=40&d=identicon
    Link here  
    Ábner Oliveira @abner

    Added 1 new commit:

    Choose File ...   File name...
    Cancel
  • 0deafa1501ec8dd687ee70f90488d592?s=40&d=identicon
    Link here  
    Ábner Oliveira @abner

    Reassigned to @abner

    Choose File ...   File name...
    Cancel