Skip to content

Conversation

wobravo
Copy link
Contributor

@wobravo wobravo commented Feb 26, 2021

disable future dates on date in and date out

@codecov
Copy link

codecov bot commented Feb 26, 2021

Codecov Report

Merging #642 (9cff77c) into master (13de42e) will increase coverage by 1.15%.
The diff coverage is 97.56%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #642      +/-   ##
==========================================
+ Coverage   93.09%   94.24%   +1.15%     
==========================================
  Files          85       95      +10     
  Lines        1564     1790     +226     
  Branches      107      119      +12     
==========================================
+ Hits         1456     1687     +231     
+ Misses         67       65       -2     
+ Partials       41       38       -3     
Impacted Files Coverage Δ
src/app/app-routing.module.ts 100.00% <ø> (ø)
...omponents/customer-list/customer-list.component.ts 92.10% <ø> (ø)
...time-entries-table/time-entries-table.component.ts 55.55% <ø> (ø)
...nents/time-range-form/time-range-form.component.ts 100.00% <ø> (ø)
...dules/time-entries/pages/time-entries.component.ts 85.13% <81.81%> (+2.32%) ⬆️
...ponents/details-fields/details-fields.component.ts 93.54% <94.44%> (+6.36%) ⬆️
src/app/modules/users/store/user.reducers.ts 94.73% <94.73%> (ø)
src/app/guards/login-guard/login.guard.ts 100.00% <100.00%> (ø)
...nologies-report-guard/technologies-report.guard.ts 100.00% <100.00%> (ø)
src/app/modules/login/login.component.ts 100.00% <100.00%> (+16.66%) ⬆️
... and 35 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 c138f40...4045b4c. Read the comment docs.

@@ -468,6 +468,12 @@ describe('DetailsFieldsComponent', () => {
expect(endDateInput.value).not.toEqual(startDateInput.value);
expect(startDateInput.value).toEqual(expectedStartDate);
});

it('on get actual date should return the actual date in isoString ', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we need proof: when we select dates in the future.

Copy link
Collaborator

Choose a reason for hiding this comment

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

update the name of the test: onGetActualDate should return the actual date in String

@wobravo wobravo requested a review from scastillo-jp March 1, 2021 21:18
@@ -171,6 +171,10 @@ export class DetailsFieldsComponent implements OnChanges, OnInit {
this.selectedTechnologies = $event;
}

getActualDate(): string {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you should change this to getCurrentDate. And update all references from actual date to current date

@wobravo wobravo requested a review from Angeluz-07 March 6, 2021 00:50
@@ -468,6 +468,31 @@ describe('DetailsFieldsComponent', () => {
expect(endDateInput.value).not.toEqual(startDateInput.value);
expect(startDateInput.value).toEqual(expectedStartDate);
});

it('on get actual date should return the current date', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

We should update the references and the name of the test. Something like:
On get current date should return expected date

Copy link
Contributor

Choose a reason for hiding this comment

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

actualDateExpected to expectedDate

const slectInput: HTMLInputElement = fixture.debugElement.
nativeElement.querySelector(`input[id="start_date"],input[max="${component.getCurrentDate()}"]`);

expect(slectInput.id).toEqual('start_date');
Copy link
Contributor

Choose a reason for hiding this comment

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

Why it is neccesary to assert the value of id?


it('on the input with id #start_date we could get the id and max value', () => {
fixture.detectChanges();
const expectDate = new Date().toISOString().split('T')[0];
Copy link
Contributor

Choose a reason for hiding this comment

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

expectDate to expectedDate

it('on the input with id #start_date we could get the id and max value', () => {
fixture.detectChanges();
const expectDate = new Date().toISOString().split('T')[0];
const slectInput: HTMLInputElement = fixture.debugElement.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think if you want to retrieve the selecInput. You only need to queryselector by the id.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, the name slectInput should be startDateInput or dateInput

@wobravo wobravo requested a review from Angeluz-07 March 8, 2021 20:41
@@ -468,6 +468,12 @@ describe('DetailsFieldsComponent', () => {
expect(endDateInput.value).not.toEqual(startDateInput.value);
expect(startDateInput.value).toEqual(expectedStartDate);
});

it('on get actual date should return the actual date in isoString ', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

update the name of the test: onGetActualDate should return the actual date in String

@scastillo-jp scastillo-jp merged commit f74adda into master Mar 8, 2021
@scastillo-jp scastillo-jp deleted the TT-167-Date-picker-disable-future-dates branch March 8, 2021 21:48
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.

4 participants