Skip to content

Conversation

@PaulRC-ioet
Copy link
Contributor

  • change the design of "time in" field
  • add waitForAsync in the tests
  • change in the .json file the commit message

@codecov
Copy link

codecov bot commented Dec 10, 2020

Codecov Report

Merging #583 (e37c8dc) into master (13de42e) will increase coverage by 0.46%.
The diff coverage is 98.96%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #583      +/-   ##
==========================================
+ Coverage   93.09%   93.56%   +0.46%     
==========================================
  Files          85       92       +7     
  Lines        1564     1647      +83     
  Branches      107      107              
==========================================
+ Hits         1456     1541      +85     
- Misses         67       70       +3     
+ Partials       41       36       -5     
Impacted Files Coverage Δ
src/app/app-routing.module.ts 100.00% <ø> (ø)
...les/shared/components/sidebar/sidebar.component.ts 88.88% <ø> (ø)
...dules/time-entries/pages/time-entries.component.ts 82.81% <ø> (ø)
src/app/modules/users/store/user.reducers.ts 87.50% <87.50%> (ø)
src/app/guards/login-guard/login.guard.ts 100.00% <100.00%> (ø)
src/app/modules/login/login.component.ts 100.00% <100.00%> (+16.66%) ⬆️
...app/modules/login/services/azure.ad.b2c.service.ts 86.95% <100.00%> (+4.60%) ⬆️
...ponents/details-fields/details-fields.component.ts 89.61% <100.00%> (+2.43%) ⬆️
.../components/entry-fields/entry-fields.component.ts 98.48% <100.00%> (-1.52%) ⬇️
...project-list-hover/project-list-hover.component.ts 88.63% <100.00%> (+0.26%) ⬆️
... and 17 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 4844725...e37c8dc. Read the comment docs.

@@ -1,4 +1,4 @@
<form [formGroup]="entryForm" (ngSubmit)="onSubmit()">
<form [formGroup]="entryForm">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am concerned about this change. As I remember, the line you are removing, is in charge to update automatically the time entry, each time the user updates any other field, different from time in. If you remove this line, you remove that behavior, and user will have to explicitly clock out to save data. Make sure, that such behavior remains or is handled in another way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also thought the same when I removed that line, but then I realized that each field has (blur) that calls the onSubmit method. It is for this reason that in my previous ticket, it did not follow the correct order, because it ran onSubmit twice.

Captura de pantalla de 2020-12-10 15-58-02

I already tried it updating all the fields and these are perfectly updated because when clicking it runs the onSubmit method

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are sure that what @Angeluz-07 is not happening, go ahead. I insist that putting FF (Feature Toggle) would help to solve the problem.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PaulRC-ioet it's ok buddy if you checked it. Just wanted to highlight that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I tried it again and it didn't give me an error @josepato87 @Angeluz-07


<label class="col-12 col-sm-2">Time in:</label>
<div class="col-12 col-sm-4">
<input
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could put this under a Feature Toggle, so if something goes wrong we could change to the precious behavior without reverting changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, for the next tickets would be, now I have the idea to implement those feature toggles.


<label class="col-12 col-sm-2">Time out:</label>
<div class="col-12 col-sm-4">
<input
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the same as previous comment about FF (Feature Flags)

@@ -1,4 +1,4 @@
<form [formGroup]="entryForm" (ngSubmit)="onSubmit()">
<form [formGroup]="entryForm">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are sure that what @Angeluz-07 is not happening, go ahead. I insist that putting FF (Feature Toggle) would help to solve the problem.

Copy link
Contributor

@josepato87 josepato87 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGFM

@PaulRC-ioet PaulRC-ioet merged commit 0601f5b into master Dec 11, 2020
@PaulRC-ioet PaulRC-ioet deleted the TT-65_ChangeDesignTimeIn branch December 11, 2020 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants