TTA 115 time tracker doesnt notifies the user when they lost internet connection or have a slow connection #928
Conversation
|
Kudos, SonarCloud Quality Gate passed!
|
Codecov Report
@@ Coverage Diff @@
## master #928 +/- ##
==========================================
- Coverage 97.48% 96.41% -1.08%
==========================================
Files 107 117 +10
Lines 2430 2702 +272
Branches 203 241 +38
==========================================
+ Hits 2369 2605 +236
- Misses 23 44 +21
- Partials 38 53 +15
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
[infrastructure/][stage] Terraform Plan 📖
|
[infrastructure/][prod] Terraform Plan 📖
|
…ction directive and their component
[infrastructure/][stage] Terraform Plan 📖
|
[infrastructure/][prod] Terraform Plan 📖
|
[infrastructure/][stage] Terraform Plan 📖
|
[infrastructure/][prod] Terraform Plan 📖
|
[infrastructure/][stage] Terraform Plan 📖
|
[infrastructure/][prod] Terraform Plan 📖
|
[infrastructure/][stage] Terraform Plan 📖
|
[infrastructure/][prod] Terraform Plan 📖
|
…r-doesnt-notifies-the-user-when-they-lost-internet-connection-or-have-a-slow-connection
[infrastructure/][stage] Terraform Plan 📖
|
[infrastructure/][prod] Terraform Plan 📖
|
[infrastructure/][stage] Terraform Plan 📖
|
[infrastructure/][prod] Terraform Plan 📖
|
[infrastructure/][stage] Terraform Plan 📖
|
[infrastructure/][prod] Terraform Plan 📖
|
[infrastructure/][stage] Terraform Plan 📖
|
[infrastructure/][prod] Terraform Plan 📖
|
| @@ -1,7 +1,6 @@ | |||
| import { Injectable } from '@angular/core'; | |||
| import { HttpClient, HttpParams } from '@angular/common/http'; | |||
| import { Observable } from 'rxjs'; | |||
There was a problem hiding this comment.
| import { Observable } from 'rxjs'; | |
| import { Observable } from 'rxjs'; | |
creo q esa linea en blanco estaba bien porque separa las importaciones externas de los modulos internos
| if (!(/\fast-5g|3g|4g/.test(effectiveType)) || !(/\slow-2g|2g/.test(effectiveType))){ | ||
| networkSatus = this.offlineSrc; | ||
| } | ||
|
|
||
| if (/\fast-5g|3g|4g/.test(effectiveType)) { | ||
| networkSatus = this.fastSrc; | ||
| } | ||
|
|
||
| if (/\slow-2g|2g/.test(effectiveType)) { | ||
| networkSatus = this.slowSrc; | ||
| } |
There was a problem hiding this comment.
| if (!(/\fast-5g|3g|4g/.test(effectiveType)) || !(/\slow-2g|2g/.test(effectiveType))){ | |
| networkSatus = this.offlineSrc; | |
| } | |
| if (/\fast-5g|3g|4g/.test(effectiveType)) { | |
| networkSatus = this.fastSrc; | |
| } | |
| if (/\slow-2g|2g/.test(effectiveType)) { | |
| networkSatus = this.slowSrc; | |
| } | |
| if (/\fast-5g|3g|4g/.test(effectiveType)) { | |
| networkSatus = this.fastSrc; | |
| } else if (/\slow-2g|2g/.test(effectiveType)) { | |
| networkSatus = this.slowSrc; | |
| } else { | |
| networkSatus = this.offlineSrc; | |
| } |
There was a problem hiding this comment.
Me parece que así se ve mas limpio. Entiendo que quisiste poner la negación primero, pero lo más común es que la conección sea buena, y si programamos poniendo el caso mas común al principio mejora la legibilidad del código, se entiende mejor el comportamiento.
Además, de la otra manera evaluamos la condición dos veces (y repetimos código) mientras que con la propuesta que te hago solamente ejecutamos 2 veces el .test
| it('should create an instance', () => { | ||
| const directive = new FastDirective(undefined); | ||
| expect(directive).toBeTruthy(); | ||
| }); |
There was a problem hiding this comment.
| it('should create an instance', () => { | |
| const directive = new FastDirective(undefined); | |
| expect(directive).toBeTruthy(); | |
| }); | |
| it('should create an instance', () => { | |
| const directive = new FastDirective(undefined); | |
| expect(directive).toBeTruthy(); | |
| }); |
| it('should create an instance', () => { | ||
| const directive = new OfflineDirective(undefined); | ||
| expect(directive).toBeTruthy(); | ||
| }); |
There was a problem hiding this comment.
| it('should create an instance', () => { | |
| const directive = new OfflineDirective(undefined); | |
| expect(directive).toBeTruthy(); | |
| }); | |
| it('should create an instance', () => { | |
| const directive = new OfflineDirective(undefined); | |
| expect(directive).toBeTruthy(); | |
| }); |
...app/modules/internet-connection-status/internet-connection-directives/slow.directive.spec.ts
Outdated
Show resolved
Hide resolved
| if (!(/\fast-5g|3g|4g/.test(effectiveType)) || !(/\slow-2g|2g/.test(effectiveType))){ | ||
| this.toastrService.error('Your request was not completed, you are offline'); | ||
| this.isFast = false; | ||
| } | ||
|
|
||
| if (/\fast-5g|3g|4g/.test(effectiveType)){ | ||
| this.isFast = true; | ||
| } | ||
|
|
||
| if (/\slow-2g|2g/.test(effectiveType)) { | ||
| this.toastrService.warning('Caution your connection is slow'); | ||
| this.isFast = false; | ||
| } |
There was a problem hiding this comment.
diria lo mismo que en connection.directive.ts
pero viendo que una parte del código se repite: podría haber una funcion que haga eso?
| fixture.detectChanges(); | ||
| }); | ||
|
|
||
| it('should show a stable connection warning', () => { |
There was a problem hiding this comment.
| it('should show a stable connection warning', () => { | |
| it('isFast should be true when connection is 4g', () => { |
| expect(component.isFast).toBe(true); | ||
| }); | ||
|
|
||
| it('should show a slow connection warning', () => { |
There was a problem hiding this comment.
| it('should show a slow connection warning', () => { | |
| it('isFast should be false when connection is 2g', () => { |
| checkTypeError(dataError: ErrorType){ | ||
| const { isError = false, message = 'The server is disconnected', error} = dataError; | ||
| const effectiveTypenetwork = navigator.connection; | ||
| if ((effectiveTypenetwork.effectiveType !== '2g')){ | ||
| if (!isError){ | ||
| this.toastrService.warning(message); | ||
| } | ||
| if (isError){ | ||
| const errorMessa = error.error && error.error.message ? error.error.message : 'There is an error with the server, your request have not be completed'; | ||
| this.toastrService.error(errorMessa); | ||
| } | ||
| } | ||
| this.toastrService.warning('Your request was not completed, your connection is slow'); | ||
| } |
There was a problem hiding this comment.
creo q deberiamos refactorear esa parte
| private toastrService: ToastrService | ||
| ) { } | ||
|
|
||
| checkTypeError(dataError: ErrorType){ |
There was a problem hiding this comment.
le ponemos de nombre swhowMessage?
…-directives/slow.directive.spec.ts Co-authored-by: mmaquina <[email protected]>
[infrastructure/][stage] Terraform Plan 📖
|
[infrastructure/][prod] Terraform Plan 📖
|
[infrastructure/][stage] Terraform Plan 📖
|
[infrastructure/][prod] Terraform Plan 📖
|
|
Kudos, SonarCloud Quality Gate passed!
|
[infrastructure/][stage] Terraform Plan 📖
|
[infrastructure/][prod] Terraform Plan 📖
|
No description provided.