Skip to content

TT-85 fix: fix overlapping seconds on time entries #591

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Dec 23, 2020
Merged

TT-85 fix: fix overlapping seconds on time entries #591

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8b50d4f
TT-85 fix: fix overlapping seconds on time entries
PaulRC-ioet Dec 17, 2020
3155406
TT-85 fix: correct word fit
PaulRC-ioet Dec 17, 2020
29da434
TT-85 fix: refactor start date and end date
PaulRC-ioet Dec 18, 2020
85a4c93
TT-85 fix: refactor code and improve tests
PaulRC-ioet Dec 22, 2020
1685b88
TT-85 fix: refactor code of time fields
PaulRC-ioet Dec 23, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
TT-85 fix: fix overlapping seconds on time entries
  • Loading branch information
@PaulRC-ioet
PaulRC-ioet committed Dec 22, 2020
commit 8b50d4fab39ab1e7b1abdc9a87a406c353408d71
22 changes: 20 additions & 2 deletions src/app/modules/shared/components/details-fields/details-fields.component.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,11 @@ describe('DetailsFieldsComponent', () => {
description: '',
technology: '',
};
const dateTest = moment().format('YYYY-MM-DD');
const endHourTest = moment().format('HH:mm:ss');
const endDateTest = new Date(`${dateTest}T${endHourTest.trim()}`);
const startHourTest = moment().subtract(3, 'hours').format('HH:mm:ss');
const startDateTest = new Date(`${dateTest}T${startHourTest.trim()}`);
Copy link
Contributor

@josepato87 josepato87 Dec 17, 2020

Choose a reason for hiding this comment

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

I would move these two lines before the end variables

Copy link
Contributor Author

@PaulRC-ioet PaulRC-ioet Dec 22, 2020

Choose a reason for hiding this comment

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

I have change all the tests, please check them.

Copy link
Contributor

@josepato87 josepato87 Dec 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Do you think we could have any issue about the timing when the tests run? For example, if we put this test running at 1:00 am, due you are subtracting 3 hours, it means that the current hour would be 23:00 of what day?

Copy link
Contributor Author

@PaulRC-ioet PaulRC-ioet Dec 22, 2020

Choose a reason for hiding this comment

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

I have deleted that block of lines, it's different now.


beforeEach(waitForAsync(() => {
TestBed.configureTestingModule({
Expand All @@ -101,8 +106,8 @@ describe('DetailsFieldsComponent', () => {
project_id: 'id',
activity_id: '',
uri: 'ticketUri',
start_date: null,
end_date: null,
start_date: startDateTest,
end_date: endDateTest,
description: '',
technologies: [],
id: 'xyz'
Expand Down Expand Up @@ -314,6 +319,19 @@ describe('DetailsFieldsComponent', () => {
expect(component.saveEntry.emit).toHaveBeenCalledWith(data);
});

fit('onSubmit an entry without change hours, should not modify the start_date and end_date ', () => {
component.entryToEdit = {...entryToEdit, description: 'test', };
fixture.componentInstance.ngOnChanges();

const startHourValue = moment().subtract(3, 'hours').format('HH:mm');
const endHourValue = moment().format('HH:mm');
Copy link
Contributor

@josepato87 josepato87 Dec 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

not sure if this should be the best approach for setting the start and end hours values, as I understand, you are setting those entry values in the before each section. Could you read the current entry "hours" and setting the test variables with those values instead?

Copy link
Contributor Author

@PaulRC-ioet PaulRC-ioet Dec 22, 2020

Choose a reason for hiding this comment

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

I have also deleted those hours, now is in the test those values.


component.onSubmit();

expect(component.starDateValue).toEqual(startHourValue);
expect(component.endDateValue).toEqual(endHourValue);
Copy link
Contributor

@josepato87 josepato87 Dec 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

How are you asserting that the other fields, like description are being updated?

Copy link
Contributor Author

@PaulRC-ioet PaulRC-ioet Dec 22, 2020

Choose a reason for hiding this comment

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

there's a new test to prove that.

});

it('displays error message when the date selected is in the future', () => {
spyOn(toastrServiceStub, 'error');

Expand Down
15 changes: 13 additions & 2 deletions src/app/modules/shared/components/details-fields/details-fields.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ export class DetailsFieldsComponent implements OnChanges, OnInit {
activities: Activity[] = [];
goingToWorkOnThis = false;
shouldRestartEntry = false;
starDateValue;
endDateValue;

constructor(private formBuilder: FormBuilder, private store: Store<Merged>,
private actionsSubject$: ActionsSubject, private toastrService: ToastrService) {
Expand Down Expand Up @@ -139,6 +141,8 @@ export class DetailsFieldsComponent implements OnChanges, OnInit {
uri: this.entryToEdit.uri,
technology: '',
});
this.starDateValue = formatDate(get(this.entryToEdit, 'start_date', '00:00'), 'HH:mm', 'en');
this.endDateValue = formatDate(get(this.entryToEdit, 'end_date', '00:00'), 'HH:mm', 'en');
} else {
this.cleanForm();
}
Expand Down Expand Up @@ -204,13 +208,20 @@ export class DetailsFieldsComponent implements OnChanges, OnInit {
}
const startDate = this.entryForm.value.start_date;
const endDate = this.entryForm.value.end_date;
this.starDateValue = this.entryForm.value.start_hour === this.starDateValue ?
this.entryToEdit.start_date :
new Date(`${startDate}T${this.entryForm.value.start_hour.trim()}`).toISOString();
this.endDateValue = this.entryForm.value.end_hour === this.endDateValue ?
this.entryToEdit.end_date :
new Date(`${endDate}T${this.entryForm.value.end_hour.trim()}`).toISOString();
Copy link
Contributor

@josepato87 josepato87 Dec 17, 2020

Choose a reason for hiding this comment

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

I see an opportunity to extract lines 213 and 216 into a method that receives the entryForm.value.X_VALUE and returns everything that is being done in that large sentence.

If you find that in other files, could be an utilitary. :)

Copy link
Contributor Author

@PaulRC-ioet PaulRC-ioet Dec 22, 2020

Choose a reason for hiding this comment

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

It's Done thanks.


const entry = {
project_id: this.entryForm.value.project_id,
activity_id: this.entryForm.value.activity_id,
technologies: get(this, 'selectedTechnologies', []),
description: this.entryForm.value.description,
start_date: new Date(`${startDate}T${this.entryForm.value.start_hour.trim()}`).toISOString(),
end_date: new Date(`${endDate}T${this.entryForm.value.end_hour.trim()}`).toISOString(),
start_date: this.starDateValue,
end_date: this.endDateValue,
uri: this.entryForm.value.uri,
timezone_offset: new Date().getTimezoneOffset(),
};
Expand Down