From 610fa8dab740411393c80571eecc90f57778d67c Mon Sep 17 00:00:00 2001 From: Kok Liang Date: Sat, 20 Jul 2024 22:08:53 +0800 Subject: [PATCH] Implement filter for Task Owners (#381) Having a function to filter issues/PRs by task owners can give users a more organised overview of the contributions made by each member in the project. Hence, let's add it to the list of existing filters we have currently for users. --- src/app/core/models/github-user.model.ts | 3 +- src/app/core/services/assignee.service.ts | 28 +++++++++++ src/app/core/services/filters.service.ts | 49 +++++++++++++++++-- .../assignee-grouping-strategy.service.ts | 3 +- .../card-view/card-view.component.ts | 8 ++- .../issues-viewer/issues-viewer.component.ts | 4 +- .../filter-bar/filter-bar.component.html | 18 +++++++ .../shared/filter-bar/filter-bar.component.ts | 12 +++++ .../shared/issue-tables/IssuesDataTable.ts | 4 +- src/app/shared/issue-tables/dropdownfilter.ts | 25 +++++++++- tests/constants/filter.constants.ts | 6 ++- tests/services/filters.service.spec.ts | 7 ++- 12 files changed, 152 insertions(+), 15 deletions(-) create mode 100644 src/app/core/services/assignee.service.ts diff --git a/src/app/core/models/github-user.model.ts b/src/app/core/models/github-user.model.ts index a68dcef1..f66e1079 100644 --- a/src/app/core/models/github-user.model.ts +++ b/src/app/core/models/github-user.model.ts @@ -18,7 +18,8 @@ export interface RawGithubUser { } /** - * Represents a GitHub user in WATcher + * Represents a GitHub user in WATcher, used for authentication + * of user as well as representing assignees for issues/PRs. */ export class GithubUser implements RawGithubUser, Group { static NO_ASSIGNEE: GithubUser = new GithubUser({ diff --git a/src/app/core/services/assignee.service.ts b/src/app/core/services/assignee.service.ts new file mode 100644 index 00000000..be9bd95b --- /dev/null +++ b/src/app/core/services/assignee.service.ts @@ -0,0 +1,28 @@ +import { Injectable } from '@angular/core'; +import { Observable } from 'rxjs'; +import { map } from 'rxjs/operators'; +import { GithubUser } from '../models/github-user.model'; +import { GithubService } from './github.service'; + +@Injectable({ + providedIn: 'root' +}) +export class AssigneeService { + assignees: GithubUser[] = []; + hasNoAssignees: boolean; + + constructor(private githubService: GithubService) {} + + /** + * Fetch all assignees from github. + */ + public fetchAssignees(): Observable { + return this.githubService.getUsersAssignable().pipe( + map((response) => { + this.assignees = response; + this.hasNoAssignees = response.length === 0; + return response; + }) + ); + } +} diff --git a/src/app/core/services/filters.service.ts b/src/app/core/services/filters.service.ts index 7d662a6d..4f9753c0 100644 --- a/src/app/core/services/filters.service.ts +++ b/src/app/core/services/filters.service.ts @@ -2,8 +2,10 @@ import { Injectable } from '@angular/core'; import { Sort } from '@angular/material/sort'; import { ActivatedRoute, Router } from '@angular/router'; import { BehaviorSubject, pipe } from 'rxjs'; +import { GithubUser } from '../models/github-user.model'; import { SimpleLabel } from '../models/label.model'; import { Milestone } from '../models/milestone.model'; +import { AssigneeService } from './assignee.service'; import { LoggingService } from './logging.service'; import { MilestoneService } from './milestone.service'; @@ -17,6 +19,7 @@ export type Filter = { hiddenLabels: Set; deselectedLabels: Set; itemsPerPage: number; + assignees: string[]; }; @Injectable({ @@ -39,7 +42,8 @@ export class FiltersService { milestones: [], hiddenLabels: new Set(), deselectedLabels: new Set(), - itemsPerPage: this.itemsPerPage + itemsPerPage: this.itemsPerPage, + assignees: [], }; readonly presetViews: { @@ -53,7 +57,8 @@ export class FiltersService { labels: [], milestones: this.getMilestonesForCurrentlyActive().map((milestone) => milestone.title), deselectedLabels: new Set(), - itemsPerPage: 20 + itemsPerPage: 20, + assignees: this.getAssigneesForCurrentlyActive().map((assignee) => assignee.login) }), contributions: () => ({ title: '', @@ -63,13 +68,14 @@ export class FiltersService { labels: [], milestones: this.milestoneService.milestones.map((milestone) => milestone.title), deselectedLabels: new Set(), - itemsPerPage: 20 + itemsPerPage: 20, + assignees: this.assigneeService.assignees.map((assignee) => assignee.login) }), custom: () => ({}) }; // List of keys in the new filter change that causes current filter to not qualify to be a preset view. - readonly presetChangingKeys = new Set(['status', 'type', 'sort', 'milestones', 'labels', 'deselectedLabels']); + readonly presetChangingKeys = new Set(['status', 'type', 'sort', 'milestones', 'labels', 'deselectedLabels', 'assignees']); public filter$ = new BehaviorSubject(this.defaultFilter); // Either 'currentlyActive', 'contributions', or 'custom'. @@ -77,12 +83,14 @@ export class FiltersService { // Helps in determining whether all milestones were selected from previous repo during sanitization of milestones private previousMilestonesLength = 0; + private previousAssigneesLength = 0; constructor( private logger: LoggingService, private router: Router, private route: ActivatedRoute, - private milestoneService: MilestoneService + private milestoneService: MilestoneService, + private assigneeService: AssigneeService ) { this.filter$.subscribe((filter: Filter) => { this.itemsPerPage = filter.itemsPerPage; @@ -111,6 +119,7 @@ export class FiltersService { case 'status': case 'labels': case 'milestones': + case 'assignees': if (filterValue.length === 0) { delete queryParams[filterName]; continue; @@ -148,6 +157,7 @@ export class FiltersService { this.updateFilters(this.defaultFilter); this.updatePresetView('currentlyActive'); this.previousMilestonesLength = 0; + this.previousAssigneesLength = 0; } initializeFromURLParams() { @@ -172,6 +182,7 @@ export class FiltersService { case 'status': case 'labels': case 'milestones': + case 'assignees': nextFilter[filterName] = filterData; break; // Sets @@ -280,6 +291,29 @@ export class FiltersService { }); } + sanitizeAssignees(allAssignees: GithubUser[]) { + const assignees = allAssignees.map((assignee) => assignee.login); + assignees.push(GithubUser.NO_ASSIGNEE.login); + const allAssigneesSet = new Set(assignees); + + // All previous assignees were selected, reset to all new assignees selected + if (this.filter$.value.assignees.length === this.previousAssigneesLength) { + this.updateFiltersWithoutUpdatingPresetView({ assignees: [...allAssigneesSet] }); + this.previousAssigneesLength = allAssigneesSet.size; + return; + } + + const newAssignees: string[] = []; + for (const assignee of this.filter$.value.assignees) { + if (allAssigneesSet.has(assignee)) { + newAssignees.push(assignee); + } + } + + this.updateFiltersWithoutUpdatingPresetView({ assignees: newAssignees }); + this.previousAssigneesLength = allAssigneesSet.size; + } + sanitizeMilestones(allMilestones: Milestone[]) { const milestones = allMilestones.map((milestone) => milestone.title); milestones.push(Milestone.IssueWithoutMilestone.title, Milestone.PRWithoutMilestone.title); @@ -320,4 +354,9 @@ export class FiltersService { } return [...this.milestoneService.milestones, Milestone.PRWithoutMilestone]; } + + getAssigneesForCurrentlyActive(): GithubUser[] { + // TODO Filter out assignees that have not contributed in currently active milestones + return [...this.assigneeService.assignees, GithubUser.NO_ASSIGNEE]; + } } diff --git a/src/app/core/services/grouping/assignee-grouping-strategy.service.ts b/src/app/core/services/grouping/assignee-grouping-strategy.service.ts index 41af8a8b..225f07e3 100644 --- a/src/app/core/services/grouping/assignee-grouping-strategy.service.ts +++ b/src/app/core/services/grouping/assignee-grouping-strategy.service.ts @@ -17,14 +17,13 @@ export class AssigneeGroupingStrategy implements GroupingStrategy { /** * Retrieves data for a specific assignee. - * If it is the"No Assignee" group, unassigned issues are returned. + * If it is the "No Assignee" group, unassigned issues are returned. * Otherwise, issues assigned to the specified user are returned. */ getDataForGroup(issues: Issue[], key: GithubUser): Issue[] { if (key === GithubUser.NO_ASSIGNEE) { return this.getUnassignedData(issues); } - return this.getDataAssignedToUser(issues, key); } diff --git a/src/app/issues-viewer/card-view/card-view.component.ts b/src/app/issues-viewer/card-view/card-view.component.ts index 1eaf2de8..13e1c42c 100644 --- a/src/app/issues-viewer/card-view/card-view.component.ts +++ b/src/app/issues-viewer/card-view/card-view.component.ts @@ -14,9 +14,11 @@ import { MatPaginator } from '@angular/material/paginator'; import { Observable, Subscription } from 'rxjs'; import { Group } from '../../core/models/github/group.interface'; import { Issue } from '../../core/models/issue.model'; +import { AssigneeService } from '../../core/services/assignee.service'; import { FiltersService } from '../../core/services/filters.service'; import { GroupBy, GroupingContextService } from '../../core/services/grouping/grouping-context.service'; import { IssueService } from '../../core/services/issue.service'; +import { LoggingService } from '../../core/services/logging.service'; import { MilestoneService } from '../../core/services/milestone.service'; import { FilterableComponent, FilterableSource } from '../../shared/issue-tables/filterableTypes'; import { IssuesDataTable } from '../../shared/issue-tables/IssuesDataTable'; @@ -60,7 +62,9 @@ export class CardViewComponent implements OnInit, AfterViewInit, OnDestroy, Filt public issueService: IssueService, public groupingContextService: GroupingContextService, private filtersService: FiltersService, - private milestoneService: MilestoneService + private milestoneService: MilestoneService, + private assigneeService: AssigneeService, + private logger: LoggingService, ) {} ngOnInit() { @@ -68,6 +72,7 @@ export class CardViewComponent implements OnInit, AfterViewInit, OnDestroy, Filt this.issueService, this.groupingContextService, this.filtersService, + this.assigneeService, this.milestoneService, this.paginator, this.headers, @@ -84,6 +89,7 @@ export class CardViewComponent implements OnInit, AfterViewInit, OnDestroy, Filt this.timeoutId = setTimeout(() => { this.issues.loadIssues(); this.issues$ = this.issues.connect(); + this.logger.debug('CardViewComponent: Issues loaded', this.issues$); // Emit event when issues change this.issuesLengthSubscription = this.issues$.subscribe(() => { diff --git a/src/app/issues-viewer/issues-viewer.component.ts b/src/app/issues-viewer/issues-viewer.component.ts index e1bb2118..cd598476 100644 --- a/src/app/issues-viewer/issues-viewer.component.ts +++ b/src/app/issues-viewer/issues-viewer.component.ts @@ -132,9 +132,11 @@ export class IssuesViewerComponent implements OnInit, AfterViewInit, OnDestroy { /** * Update the list of hidden group based on the new info. * @param issueLength The number of issues under this group. - * @param group The group. + * @param target The group. */ updateHiddenGroups(issueLength: number, target: Group) { + // If the group is in the hidden list, add it if it has no issues. + // Also add it if it is unchecked in the filter. if (issueLength === 0 && this.groupingContextService.isInHiddenList(target)) { this.addToHiddenGroups(target); } else { diff --git a/src/app/shared/filter-bar/filter-bar.component.html b/src/app/shared/filter-bar/filter-bar.component.html index 9c57f634..cc5b1c0b 100644 --- a/src/app/shared/filter-bar/filter-bar.component.html +++ b/src/app/shared/filter-bar/filter-bar.component.html @@ -91,6 +91,24 @@ 50 + + Assigned to + + + No Assignees + + + {{ assignee.login }} + + Unassigned + + diff --git a/src/app/shared/filter-bar/filter-bar.component.ts b/src/app/shared/filter-bar/filter-bar.component.ts index 1a06e698..9f0dba89 100644 --- a/src/app/shared/filter-bar/filter-bar.component.ts +++ b/src/app/shared/filter-bar/filter-bar.component.ts @@ -1,6 +1,7 @@ import { AfterViewInit, Component, Input, OnDestroy, OnInit, QueryList, ViewChild } from '@angular/core'; import { MatSelect } from '@angular/material/select'; import { BehaviorSubject, Subscription } from 'rxjs'; +import { AssigneeService } from '../../core/services/assignee.service'; import { Filter, FiltersService } from '../../core/services/filters.service'; import { GroupBy, GroupingContextService } from '../../core/services/grouping/grouping-context.service'; import { LoggingService } from '../../core/services/logging.service'; @@ -30,12 +31,14 @@ export class FilterBarComponent implements OnInit, OnDestroy { /** Milestone subscription */ milestoneSubscription: Subscription; + assigneeSubscription: Subscription; @ViewChild(LabelFilterBarComponent, { static: true }) labelFilterBar: LabelFilterBarComponent; @ViewChild('milestoneSelectorRef', { static: false }) milestoneSelectorRef: MatSelect; constructor( + public assigneeService: AssigneeService, public milestoneService: MilestoneService, public filtersService: FiltersService, private viewService: ViewService, @@ -59,6 +62,7 @@ export class FilterBarComponent implements OnInit, OnDestroy { ngOnDestroy(): void { this.milestoneSubscription.unsubscribe(); + this.assigneeSubscription.unsubscribe(); this.repoChangeSubscription.unsubscribe(); } @@ -94,5 +98,13 @@ export class FilterBarComponent implements OnInit, OnDestroy { (err) => {}, () => {} ); + this.assigneeSubscription = this.assigneeService.fetchAssignees().subscribe( + (response) => { + this.logger.debug('IssuesViewerComponent: Fetched assignees from Github'); + this.filtersService.sanitizeAssignees(this.assigneeService.assignees); + }, + (err) => {}, + () => {} + ); } } diff --git a/src/app/shared/issue-tables/IssuesDataTable.ts b/src/app/shared/issue-tables/IssuesDataTable.ts index 68eabb34..a820218d 100644 --- a/src/app/shared/issue-tables/IssuesDataTable.ts +++ b/src/app/shared/issue-tables/IssuesDataTable.ts @@ -4,6 +4,7 @@ import { BehaviorSubject, merge, Observable, Subscription } from 'rxjs'; import { map } from 'rxjs/operators'; import { Group } from '../../core/models/github/group.interface'; import { Issue } from '../../core/models/issue.model'; +import { AssigneeService } from '../../core/services/assignee.service'; import { Filter, FiltersService } from '../../core/services/filters.service'; import { GroupingContextService } from '../../core/services/grouping/grouping-context.service'; import { IssueService } from '../../core/services/issue.service'; @@ -26,6 +27,7 @@ export class IssuesDataTable extends DataSource implements FilterableSour private issueService: IssueService, private groupingContextService: GroupingContextService, private filtersService: FiltersService, + private assigneeService: AssigneeService, private milestoneService: MilestoneService, private paginator: MatPaginator, private displayedColumn: string[], @@ -69,7 +71,7 @@ export class IssuesDataTable extends DataSource implements FilterableSour data = this.groupingContextService.getDataForGroup(data, this.group); // Apply Filters - data = applyDropdownFilter(this.filter, data, !this.milestoneService.hasNoMilestones); + data = applyDropdownFilter(this.filter, data, !this.milestoneService.hasNoMilestones, !this.assigneeService.hasNoAssignees); data = applySearchFilter(this.filter.title, this.displayedColumn, this.issueService, data); this.count = data.length; diff --git a/src/app/shared/issue-tables/dropdownfilter.ts b/src/app/shared/issue-tables/dropdownfilter.ts index 13f08127..47cd5eae 100644 --- a/src/app/shared/issue-tables/dropdownfilter.ts +++ b/src/app/shared/issue-tables/dropdownfilter.ts @@ -20,7 +20,12 @@ const infoFromStatus = (statusString: string): StatusInfo => { * This module exports a single function applyDropDownFilter which is called by IssueList. * This functions returns the data passed in after all the filters of dropdownFilters are applied */ -export function applyDropdownFilter(filter: Filter, data: Issue[], isFilteringByMilestone: boolean): Issue[] { +export function applyDropdownFilter( + filter: Filter, + data: Issue[], + isFilteringByMilestone: boolean, + isFilteringByAssignee: boolean +): Issue[] { const filteredData: Issue[] = data.filter((issue) => { let ret = true; @@ -39,8 +44,26 @@ export function applyDropdownFilter(filter: Filter, data: Issue[], isFilteringBy } ret = ret && (!isFilteringByMilestone || filter.milestones.some((milestone) => issue.milestone.title === milestone)); + ret = ret && (!isFilteringByAssignee || isFilteredByAssignee(filter, issue)); ret = ret && issue.labels.every((label) => !filter.deselectedLabels.has(label)); return ret && filter.labels.every((label) => issue.labels.includes(label)); }); return filteredData; } + +function isFilteredByAssignee(filter: Filter, issue: Issue): boolean { + if (issue.issueOrPr === 'Issue') { + return ( + filter.assignees.some((assignee) => issue.assignees.includes(assignee)) || + (filter.assignees.includes('Unassigned') && issue.assignees.length === 0) + ); + } else if (issue.issueOrPr === 'PullRequest') { + return ( + filter.assignees.some((assignee) => issue.author === assignee) || (filter.assignees.includes('Unassigned') && issue.author === null) + ); + // note that issue.author is never == null for PRs, but is left for semantic reasons + } else { + // should never occur + throw new Error('Issue or PR is neither Issue nor PullRequest'); + } +} diff --git a/tests/constants/filter.constants.ts b/tests/constants/filter.constants.ts index d41eb21b..d9c3e522 100644 --- a/tests/constants/filter.constants.ts +++ b/tests/constants/filter.constants.ts @@ -9,7 +9,8 @@ export const DEFAULT_FILTER: Filter = { milestones: ['PR without a milestone'], hiddenLabels: new Set(), deselectedLabels: new Set(), - itemsPerPage: 20 + itemsPerPage: 20, + assignees: ['Unassigned'] }; export const CHANGED_FILTER: Filter = { @@ -21,5 +22,6 @@ export const CHANGED_FILTER: Filter = { milestones: ['V3.3.6'], hiddenLabels: new Set(['aspect-testing']), deselectedLabels: new Set(['aspect-documentation']), - itemsPerPage: 50 + itemsPerPage: 50, + assignees: ['test'] }; diff --git a/tests/services/filters.service.spec.ts b/tests/services/filters.service.spec.ts index e093ff1d..e8618408 100644 --- a/tests/services/filters.service.spec.ts +++ b/tests/services/filters.service.spec.ts @@ -1,4 +1,5 @@ import { ActivatedRoute, convertToParamMap, Router } from '@angular/router'; +import { AssigneeService } from '../../src/app/core/services/assignee.service'; import { FiltersService } from '../../src/app/core/services/filters.service'; import { LoggingService } from '../../src/app/core/services/logging.service'; import { MilestoneService } from '../../src/app/core/services/milestone.service'; @@ -9,6 +10,7 @@ let loggingServiceSpy: jasmine.SpyObj; let routerSpy: jasmine.SpyObj; let activatedRouteSpy: jasmine.SpyObj; let milestoneServiceSpy: jasmine.SpyObj; +let assigneeServiceSpy: jasmine.SpyObj; describe('FiltersService', () => { beforeEach(() => { @@ -22,7 +24,10 @@ describe('FiltersService', () => { milestoneServiceSpy = jasmine.createSpyObj('MilestoneService', ['milestones', 'getEarliestOpenMilestone', 'getLatestClosedMilestone'], { milestones: [] }); - filtersService = new FiltersService(loggingServiceSpy, routerSpy, activatedRouteSpy, milestoneServiceSpy); + assigneeServiceSpy = jasmine.createSpyObj('AssigneeService', ['assignees', 'hasNoAssignees'], { + assignees: [] + }); + filtersService = new FiltersService(loggingServiceSpy, routerSpy, activatedRouteSpy, milestoneServiceSpy, assigneeServiceSpy); filtersService.initializeFromURLParams(); });