Skip to content

Conversation

@PaulRC-ioet
Copy link
Contributor

function to change the start and end time of previous time entry when switch project

@codecov
Copy link

codecov bot commented Nov 26, 2020

Codecov Report

Merging #581 (a5c28f8) into master (13de42e) will increase coverage by 0.23%.
The diff coverage is 98.57%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #581      +/-   ##
==========================================
+ Coverage   93.09%   93.32%   +0.23%     
==========================================
  Files          85       92       +7     
  Lines        1564     1633      +69     
  Branches      107      111       +4     
==========================================
+ Hits         1456     1524      +68     
- Misses         67       68       +1     
  Partials       41       41              
Impacted Files Coverage Δ
src/app/app-routing.module.ts 100.00% <ø> (ø)
...les/shared/components/sidebar/sidebar.component.ts 88.88% <ø> (ø)
src/app/modules/users/store/user.reducers.ts 87.50% <87.50%> (ø)
.../components/entry-fields/entry-fields.component.ts 100.00% <100.00%> (ø)
...project-list-hover/project-list-hover.component.ts 88.63% <100.00%> (+0.26%) ⬆️
...sers/components/users-list/users-list.component.ts 100.00% <100.00%> (ø)
src/app/modules/users/pages/users.component.ts 100.00% <100.00%> (ø)
src/app/modules/users/services/users.service.ts 100.00% <100.00%> (ø)
src/app/modules/users/store/user.actions.ts 100.00% <100.00%> (ø)
src/app/modules/users/store/user.effects.ts 100.00% <100.00%> (ø)
... and 8 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 c1aa529...a5c28f8. Read the comment docs.

expect(toastrServiceStub.error).toHaveBeenCalled();
});

it('displays error message when new hour entered is in the past of other entry', () => {
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 not quite sure about what are you testing here. Correct me if I am wrong please:

Displays an error message when new entry has start_time before (start time/end time???) of active entry

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, it's the same, the name could be:
Displays an error message when new entry has start_time before end time of last entry

component.setDataToUpdate(entry);
spyOn(toastrServiceStub, 'error');

const hourInTheFuture = moment().add(-6, 'hour').format('HH:mm:ss');
Copy link
Contributor

Choose a reason for hiding this comment

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

how the time can be in the future if you're actually subtracting hours. BTW, you can use the function subtract instead of add in this case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, I already change it

expect(toastrServiceStub.error).toHaveBeenCalled();
});

it('If start hour is in the past of other entry, reset to initial start_date in form', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't get what are you testing here. Feel free to send me a DM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test is to restart to the previous time when you enter a wrong time.
I corrected the name to:
should reset to current start_date of form when start_date is before the current entrie’s start_date

it('when a start hour is updated, then dispatch UpdateActiveEntry', () => {
component.activeEntry = entry ;
component.setDataToUpdate(entry);
const newHour = moment().format('HH:mm:ss');
Copy link
Contributor

Choose a reason for hiding this comment

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

let's use newTime or updatedTime instead of newHour

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I change to updatedTime

expect(store.dispatch).toHaveBeenCalled();
});

it('when a start hour is update, then select the last time entry', async(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Correct me if I am wrong, I think this is what you are testing here.

When start_time is updated, component.last_entry is equal to...

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, another way would be:
When start_time is updated, component.last_entry is equal to last time entry in the state

expect(component.lastEntry).toBe(state.entries.timeEntriesDataSource.data[1]);
}));

it('when a start hour is updated in other time entry, then dispatch UpdateEntry and UpdateEntryRunning', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

When start_time is updated for a time entry. UpdateEntry and UpdateEntryRuning actions are dispatched.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Of course, that phrase is better understood.

spyOn(store, 'dispatch');

component.onUpdateStartHour();
expect(store.dispatch).toHaveBeenCalledTimes(2);
Copy link
Contributor

Choose a reason for hiding this comment

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

You might want to verify here the above-mentioned actions have been dispatched.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, it's tested

const isFirstEntry = this.lastEntry !== undefined ? this.lastEntry.start_date : moment().add(-1, 'hours');
const isEntryDateInLastStartDate = moment(newHourEntered).isBefore(isFirstEntry);
if (isEntryDateInLastStartDate) {
this.toastrService.error('You cannot start a time-entry before another time-entry');
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is to control that the user does not enter a time equal to or prior to the last time entry.
I have changed the message to:
There is already a time-entry with the time entered. The entry time cannot be the same as another time-entry

@PaulRC-ioet PaulRC-ioet merged commit ae55794 into master Dec 1, 2020
@PaulRC-ioet PaulRC-ioet deleted the 572_switchEditingTime branch December 1, 2020 15:27
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.

3 participants