[AAE-9186] Filter tasks by assignee #7784
Conversation
tomgny
requested review from
eromano,
popovicsandras and
DenysVuika
as code owners
| case 'ASSIGNED': | ||
| this.assignmentType = AssignmentType.ASSIGNED_TO; | ||
| break; | ||
| case '': |
There was a problem hiding this comment.
is there such a case or is a typo? If there is do we need a break here?
There was a problem hiding this comment.
useless case removed
| } | ||
| }; | ||
|
|
||
| export const mockIdentityUsers: IdentityUserModel[] = [{ |
There was a problem hiding this comment.
- You have added mockIdentityUsers obj inside a file that's called edit-task-filter-cloud.mock.ts
- Have you checked if there are already mocks that you can use? I am pretty sure there are as these are quite common mock objects that we probably have tested elsewhere as well
There was a problem hiding this comment.
changed mock to one from people-cloud mocks
| expect(component.assignmentType).toEqual(AssignmentType.ASSIGNED_TO); | ||
| }); | ||
|
|
||
| it('should empty status set assignment type to NONE', () => { |
There was a problem hiding this comment.
what's empty status? Something like ALL?
There was a problem hiding this comment.
yes, it's equal ALL in this case. changed to ALL (+ used enum instead of string)
| type: 'assignment', | ||
| attributes: { assignedUsers: 'assignedUsers', candidateGroups: 'candidateGroups'} | ||
| }; | ||
| component.ngOnInit(); |
There was a problem hiding this comment.
is there a way to avoid force calling ngOnInit and do fixture.detectChanges() instead?
| expect(component.assignmentType).toEqual(AssignmentType.CANDIDATE_GROUPS); | ||
| }); | ||
|
|
||
| it('should set assignment type to CANDIDATE_GROUPS if initial candidateGroups and assignedUsers exists', () => { |
There was a problem hiding this comment.
is that possible from the UI to have both candidate groups and assigned users in this filter?
There was a problem hiding this comment.
no, unit test removed
| this.assignedUsersChange.emit(users); | ||
| } | ||
|
|
||
| private changeAssignmentTypeByStatus(status: string) { |
There was a problem hiding this comment.
is there any enum about the status that we could use instead of string?
There was a problem hiding this comment.
used enum instead of string
| if (assignmentChange.value === AssignmentType.CURRENT_USER) { | ||
| this.assignedUsersChange.emit([this.identityUserService.getCurrentUserInfo()]); | ||
| } else if (assignmentChange.value === AssignmentType.NONE) { | ||
| this.assignedUsersChange.emit(undefined); |
There was a problem hiding this comment.
could we emit an empty array instead of undefined? Would that be possible?
tomgny
force-pushed
the
dev-tomgny-AAE-9186
branch
from
0485984 to
e9be624
Compare
tomgny
force-pushed
the
dev-tomgny-AAE-9186
branch
from
7566dad to
499ed8f
Compare
Please check if the PR fulfills these requirements
What kind of change does this PR introduce? (check one with "x")
What is the current behaviour? (You can also link to an open issue here)
What is the new behaviour?
Does this PR introduce a breaking change? (check one with "x")
If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...
Other information: