From 663dec3e90481c31c1411b2cd3f1829795c686f2 Mon Sep 17 00:00:00 2001 From: NereusWB922 <107099783+NereusWB922@users.noreply.github.com> Date: Wed, 20 Mar 2024 21:26:58 +0800 Subject: [PATCH 1/8] Refactor Phase Service and remove Phase (#291) The concept of "Phase" is inherited from CATcher. Instead of "Phase", WATcher provides different "Views" of contributions in a repository. Let's replace "Phase" with "View". --- src/app/app.module.ts | 6 +- src/app/auth/auth.component.ts | 8 +- src/app/core/models/issue.model.ts | 2 +- src/app/core/models/phase.model.ts | 8 -- src/app/core/models/session.model.ts | 26 +++--- src/app/core/models/view.model.ts | 4 + src/app/core/services/auth.service.ts | 29 +++---- .../factories/factory.auth.service.ts | 8 +- .../factories/factory.issue.service.ts | 8 +- src/app/core/services/github.service.ts | 6 +- src/app/core/services/issue.service.ts | 12 +-- .../{phase.service.ts => view.service.ts} | 76 ++++++++-------- .../issues-viewer.component.html | 2 +- .../issues-viewer/issues-viewer.component.ts | 10 +-- .../shared/filter-bar/filter-bar.component.ts | 6 +- src/app/shared/layout/header.component.html | 29 +++---- src/app/shared/layout/header.component.ts | 58 ++++++------- tests/app/core/models/session-model.spec.ts | 28 +++--- tests/constants/label.constants.ts | 1 - tests/constants/session.constants.ts | 6 +- ...e.service.spec.ts => view.service.spec.ts} | 86 +++++++++---------- 21 files changed, 203 insertions(+), 216 deletions(-) delete mode 100644 src/app/core/models/phase.model.ts create mode 100644 src/app/core/models/view.model.ts rename src/app/core/services/{phase.service.ts => view.service.ts} (72%) rename tests/services/{phase.service.spec.ts => view.service.spec.ts} (64%) diff --git a/src/app/app.module.ts b/src/app/app.module.ts index 98e6fd8cb..48a6671f6 100644 --- a/src/app/app.module.ts +++ b/src/app/app.module.ts @@ -27,9 +27,9 @@ import { GithubEventService } from './core/services/githubevent.service'; import { IssueService } from './core/services/issue.service'; import { LabelService } from './core/services/label.service'; import { LoggingService } from './core/services/logging.service'; -import { PhaseService } from './core/services/phase.service'; import { RepoSessionStorageService } from './core/services/repo-session-storage.service'; import { UserService } from './core/services/user.service'; +import { ViewService } from './core/services/view.service'; import { IssuesViewerModule } from './issues-viewer/issues-viewer.module'; import { LabelDefinitionPopupComponent } from './shared/label-definition-popup/label-definition-popup.component'; import { HeaderComponent } from './shared/layout'; @@ -64,7 +64,7 @@ import { SharedModule } from './shared/shared.module'; UserService, IssueService, LabelService, - PhaseService, + ViewService, GithubEventService, Title, ErrorHandlingService, @@ -74,7 +74,7 @@ import { SharedModule } from './shared/shared.module'; { provide: IssueService, useFactory: IssueServiceFactory, - deps: [GithubService, UserService, PhaseService] + deps: [GithubService, UserService, ViewService] }, { provide: ErrorHandler, diff --git a/src/app/auth/auth.component.ts b/src/app/auth/auth.component.ts index 08795668a..651f56143 100644 --- a/src/app/auth/auth.component.ts +++ b/src/app/auth/auth.component.ts @@ -10,8 +10,8 @@ import { AuthService, AuthState } from '../core/services/auth.service'; import { ErrorHandlingService } from '../core/services/error-handling.service'; import { ErrorMessageService } from '../core/services/error-message.service'; import { LoggingService } from '../core/services/logging.service'; -import { PhaseService } from '../core/services/phase.service'; import { UserService } from '../core/services/user.service'; +import { ViewService } from '../core/services/view.service'; @Component({ selector: 'app-auth', @@ -33,7 +33,7 @@ export class AuthComponent implements OnInit, OnDestroy { private userService: UserService, private errorHandlingService: ErrorHandlingService, private router: Router, - private phaseService: PhaseService, + private viewService: ViewService, private ngZone: NgZone, private activatedRoute: ActivatedRoute, private logger: LoggingService @@ -46,7 +46,7 @@ export class AuthComponent implements OnInit, OnDestroy { const state = this.activatedRoute.snapshot.queryParamMap.get('state'); if (this.authService.isAuthenticated()) { - this.router.navigate([this.phaseService.currentPhase]); + this.router.navigate([this.viewService.currentView]); return; } this.initAccessTokenSubscription(); @@ -141,7 +141,7 @@ export class AuthComponent implements OnInit, OnDestroy { } isRepoSet(): boolean { - return this.phaseService.isRepoSet(); + return this.viewService.isRepoSet(); } get currentSessionOrg(): string { diff --git a/src/app/core/models/issue.model.ts b/src/app/core/models/issue.model.ts index 19af1f018..7233a35b8 100644 --- a/src/app/core/models/issue.model.ts +++ b/src/app/core/models/issue.model.ts @@ -24,7 +24,7 @@ export class Issue { author: string; isDraft: boolean; - /** Depending on the phase, assignees attribute can be derived from Github's assignee feature OR from the Github's issue description */ + /** Depending on the view, assignees attribute can be derived from Github's assignee feature OR from the Github's issue description */ assignees?: string[]; labels?: string[]; githubLabels?: GithubLabel[]; diff --git a/src/app/core/models/phase.model.ts b/src/app/core/models/phase.model.ts deleted file mode 100644 index be6521060..000000000 --- a/src/app/core/models/phase.model.ts +++ /dev/null @@ -1,8 +0,0 @@ -/** - * A phase is terminology brought over from CATcher. - * It refers to a feature in WATcher. - */ -export enum Phase { - issuesViewer = 'issuesViewer', - activityDashboard = 'activityDashboard' -} diff --git a/src/app/core/models/session.model.ts b/src/app/core/models/session.model.ts index bdb88a8c5..fde7e0507 100644 --- a/src/app/core/models/session.model.ts +++ b/src/app/core/models/session.model.ts @@ -1,13 +1,13 @@ import { pipe } from 'rxjs'; import { throwIfFalse } from '../../shared/lib/custom-ops'; -import { Phase } from './phase.model'; import { Repo } from './repo.model'; +import { View } from './view.model'; /** - * Session repository comprises the phase and its corresponding repository array. + * Session repository comprises the view and its corresponding repository array. */ export interface SessionRepo { - phase: Phase; + view: View; repos: Repo[]; } @@ -20,8 +20,8 @@ export interface SessionData { export const SESSION_DATA_UNAVAILABLE = 'Session data does not exist.'; export const SESSION_DATA_MISSING_FIELDS = 'Session data does not contain any repositories.'; -export const NO_VALID_OPEN_PHASES = 'Invalid phases in Session data.'; -export const OPENED_PHASE_REPO_UNDEFINED = 'Phase has no repo defined.'; +export const NO_VALID_OPEN_VIEWS = 'Invalid views in Session data.'; +export const OPENED_VIEW_REPO_UNDEFINED = 'View has no repo defined.'; export function assertSessionDataIntegrity() { return pipe( @@ -30,8 +30,8 @@ export function assertSessionDataIntegrity() { () => new Error(SESSION_DATA_UNAVAILABLE) ), throwIfFalse(hasSessionRepo, () => new Error(SESSION_DATA_MISSING_FIELDS)), - throwIfFalse(arePhasesValid, () => new Error(NO_VALID_OPEN_PHASES)), - throwIfFalse(areReposDefined, () => new Error(OPENED_PHASE_REPO_UNDEFINED)) + throwIfFalse(areViewsValid, () => new Error(NO_VALID_OPEN_VIEWS)), + throwIfFalse(areReposDefined, () => new Error(OPENED_VIEW_REPO_UNDEFINED)) ); } @@ -44,24 +44,24 @@ function hasSessionRepo(sessionData: SessionData): boolean { } /** - * Checks if Phases belong to a pre-defined Phase. + * Checks if Views belong to a pre-defined View. * @param sessionData */ -function arePhasesValid(sessionData: SessionData): boolean { +function areViewsValid(sessionData: SessionData): boolean { return sessionData.sessionRepo.reduce( - (isPhaseValidSoFar: boolean, currentPhaseRepo: SessionRepo) => isPhaseValidSoFar && currentPhaseRepo.phase in Phase, + (isViewValidSoFar: boolean, currentViewRepo: SessionRepo) => isViewValidSoFar && currentViewRepo.view in View, true ); } /** - * Checks if each Phase has an associated repo defined as well. + * Checks if each View has an associated repo defined as well. * @param sessionData */ function areReposDefined(sessionData: SessionData): boolean { return sessionData.sessionRepo.reduce( - (isPhaseRepoDefinedSoFar: boolean, currentPhaseRepo: SessionRepo) => - isPhaseRepoDefinedSoFar && !!currentPhaseRepo.repos && Array.isArray(currentPhaseRepo.repos) && currentPhaseRepo.repos.length > 0, + (isViewRepoDefinedSoFar: boolean, currentViewRepo: SessionRepo) => + isViewRepoDefinedSoFar && !!currentViewRepo.repos && Array.isArray(currentViewRepo.repos) && currentViewRepo.repos.length > 0, true ); } diff --git a/src/app/core/models/view.model.ts b/src/app/core/models/view.model.ts new file mode 100644 index 000000000..5fdc2809e --- /dev/null +++ b/src/app/core/models/view.model.ts @@ -0,0 +1,4 @@ +export enum View { + issuesViewer = 'issuesViewer', + activityDashboard = 'activityDashboard' +} diff --git a/src/app/core/services/auth.service.ts b/src/app/core/services/auth.service.ts index 662a4776f..02bebbb24 100644 --- a/src/app/core/services/auth.service.ts +++ b/src/app/core/services/auth.service.ts @@ -1,5 +1,4 @@ -import { Injectable } from '@angular/core'; -import { NgZone } from '@angular/core'; +import { Injectable, NgZone } from '@angular/core'; import { Title } from '@angular/platform-browser'; import { Router, RouterStateSnapshot } from '@angular/router'; import { BehaviorSubject, from, Observable, of } from 'rxjs'; @@ -7,15 +6,15 @@ import { catchError, map, mergeMap } from 'rxjs/operators'; import { AppConfig } from '../../../environments/environment'; import { generateSessionId } from '../../shared/lib/session'; import { uuid } from '../../shared/lib/uuid'; -import { Phase } from '../models/phase.model'; +import { View } from '../models/view.model'; import { ErrorHandlingService } from './error-handling.service'; import { GithubService } from './github.service'; import { GithubEventService } from './githubevent.service'; import { IssueService } from './issue.service'; import { LabelService } from './label.service'; import { LoggingService } from './logging.service'; -import { PhaseService } from './phase.service'; import { UserService } from './user.service'; +import { ViewService } from './view.service'; export enum AuthState { 'NotAuthenticated', @@ -50,7 +49,7 @@ export class AuthService { private userService: UserService, private issueService: IssueService, private labelService: LabelService, - private phaseService: PhaseService, + private viewService: ViewService, private githubEventService: GithubEventService, private titleService: Title, private errorHandlingService: ErrorHandlingService, @@ -118,7 +117,7 @@ export class AuthService { */ redirectToNext() { const next = sessionStorage.getItem(AuthService.SESSION_NEXT_KEY); - this.phaseService + this.viewService .setupFromUrl(next) .pipe( mergeMap(() => this.setRepo()), @@ -156,7 +155,7 @@ export class AuthService { this.userService.reset(); this.issueService.reset(true); this.labelService.reset(); - this.phaseService.reset(); + this.viewService.reset(); this.githubEventService.reset(); this.logger.reset(); this.setLandingPageTitle(); @@ -164,9 +163,9 @@ export class AuthService { this.reset(); } - setTitleWithPhaseDetail(): void { + setTitleWithViewDetail(): void { const appSetting = require('../../../../package.json'); - const title = `${appSetting.name} ${appSetting.version} - ${this.phaseService.getCurrentRepositoryURL()}`; + const title = `${appSetting.name} ${appSetting.version} - ${this.viewService.getCurrentRepositoryURL()}`; this.logger.info(`AuthService: Setting Title as ${title}`); this.titleService.setTitle(title); } @@ -234,10 +233,10 @@ export class AuthService { * Handles the clean up required after authentication and setting up of repository is completed. */ handleSetRepoSuccess(repoName: string) { - this.setTitleWithPhaseDetail(); - this.router.navigate([Phase.issuesViewer], { + this.setTitleWithViewDetail(); + this.router.navigate([View.issuesViewer], { queryParams: { - [PhaseService.REPO_QUERY_PARAM_KEY]: repoName + [ViewService.REPO_QUERY_PARAM_KEY]: repoName } }); } @@ -246,13 +245,13 @@ export class AuthService { * Setup repository after authentication. */ setRepo(): Observable { - return from(this.phaseService.initializeCurrentRepository()).pipe( + return from(this.viewService.initializeCurrentRepository()).pipe( map(() => { - if (!this.phaseService.currentRepo) { + if (!this.viewService.currentRepo) { return false; } this.githubEventService.setLatestChangeEvent(); - this.handleSetRepoSuccess(this.phaseService.currentRepo.toString()); + this.handleSetRepoSuccess(this.viewService.currentRepo.toString()); return true; }), catchError((error) => { diff --git a/src/app/core/services/factories/factory.auth.service.ts b/src/app/core/services/factories/factory.auth.service.ts index aabdcadaa..2aa915b76 100644 --- a/src/app/core/services/factories/factory.auth.service.ts +++ b/src/app/core/services/factories/factory.auth.service.ts @@ -10,8 +10,8 @@ import { IssueService } from '../issue.service'; import { LabelService } from '../label.service'; import { LoggingService } from '../logging.service'; // import { MockAuthService } from '../mocks/mock.auth.service'; -import { PhaseService } from '../phase.service'; import { UserService } from '../user.service'; +import { ViewService } from '../view.service'; export function AuthServiceFactory( router: Router, @@ -20,7 +20,7 @@ export function AuthServiceFactory( userService: UserService, issueService: IssueService, labelService: LabelService, - phaseService: PhaseService, + viewService: ViewService, githubEventService: GithubEventService, titleService: Title, errorHandlingService: ErrorHandlingService, @@ -35,7 +35,7 @@ export function AuthServiceFactory( // userService, // issueService, // labelService, - // phaseService, + // viewService, // githubEventService, // titleService, // logger @@ -48,7 +48,7 @@ export function AuthServiceFactory( userService, issueService, labelService, - phaseService, + viewService, githubEventService, titleService, errorHandlingService, diff --git a/src/app/core/services/factories/factory.issue.service.ts b/src/app/core/services/factories/factory.issue.service.ts index da4a64399..719bc9038 100644 --- a/src/app/core/services/factories/factory.issue.service.ts +++ b/src/app/core/services/factories/factory.issue.service.ts @@ -2,13 +2,13 @@ import { GithubService } from '../github.service'; import { IssueService } from '../issue.service'; // import { MockIssueService } from '../mocks/mock.issue.service'; -import { PhaseService } from '../phase.service'; import { UserService } from '../user.service'; +import { ViewService } from '../view.service'; -export function IssueServiceFactory(githubService: GithubService, userService: UserService, phaseService: PhaseService) { +export function IssueServiceFactory(githubService: GithubService, userService: UserService, viewService: ViewService) { // TODO: Write Mocks // if (AppConfig.test) { - // return new MockIssueService(githubService, phaseService, dataService); + // return new MockIssueService(githubService, viewService, dataService); // } - return new IssueService(githubService, userService, phaseService); + return new IssueService(githubService, userService, viewService); } diff --git a/src/app/core/services/github.service.ts b/src/app/core/services/github.service.ts index 26dd9122f..6518ffec9 100644 --- a/src/app/core/services/github.service.ts +++ b/src/app/core/services/github.service.ts @@ -95,12 +95,12 @@ export class GithubService { /** * Sets repository to watch. This repository is used for fetching from Github. - * @param phaseRepoOwner Repository owner + * @param viewRepoOwner Repository owner * @param repoName Repository name */ - storePhaseDetails(phaseRepoOwner: string, repoName: string) { + storeViewDetails(viewRepoOwner: string, repoName: string) { REPO = repoName; - ORG_NAME = phaseRepoOwner; + ORG_NAME = viewRepoOwner; } /** diff --git a/src/app/core/services/issue.service.ts b/src/app/core/services/issue.service.ts index 4f001bef3..4bc7bb835 100644 --- a/src/app/core/services/issue.service.ts +++ b/src/app/core/services/issue.service.ts @@ -4,10 +4,10 @@ import { catchError, exhaustMap, finalize, map } from 'rxjs/operators'; import RestGithubIssueFilter from '../models/github/github-issue-filter.model'; import { GithubIssue } from '../models/github/github-issue.model'; import { Issue, Issues, IssuesFilter } from '../models/issue.model'; -import { Phase } from '../models/phase.model'; +import { View } from '../models/view.model'; import { GithubService } from './github.service'; -import { PhaseService } from './phase.service'; import { UserService } from './user.service'; +import { ViewService } from './view.service'; @Injectable({ providedIn: 'root' @@ -29,7 +29,7 @@ export class IssueService { /** Whether the IssueService is downloading the data from Github*/ public isLoading = new BehaviorSubject(false); - constructor(private githubService: GithubService, private userService: UserService, private phaseService: PhaseService) { + constructor(private githubService: GithubService, private userService: UserService, private viewService: ViewService) { this.issues$ = new BehaviorSubject(new Array()); } @@ -112,7 +112,7 @@ export class IssueService { private initializeData(): Observable { let issuesAPICallsByFilter: Observable>; - switch (IssuesFilter[this.phaseService.currentPhase][this.userService.currentUser.role]) { + switch (IssuesFilter[this.viewService.currentView][this.userService.currentUser.role]) { case 'FILTER_BY_CREATOR': issuesAPICallsByFilter = this.githubService.fetchIssuesGraphql( new RestGithubIssueFilter({ creator: this.userService.currentUser.loginId }) @@ -195,8 +195,8 @@ export class IssueService { } private createIssueModel(githubIssue: GithubIssue): Issue { - switch (this.phaseService.currentPhase) { - case Phase.issuesViewer: + switch (this.viewService.currentView) { + case View.issuesViewer: return Issue.createPhaseBugReportingIssue(githubIssue); default: return; diff --git a/src/app/core/services/phase.service.ts b/src/app/core/services/view.service.ts similarity index 72% rename from src/app/core/services/phase.service.ts rename to src/app/core/services/view.service.ts index f29a7043a..ac0dad288 100644 --- a/src/app/core/services/phase.service.ts +++ b/src/app/core/services/view.service.ts @@ -3,9 +3,9 @@ import { Router } from '@angular/router'; import { BehaviorSubject, Observable, of, Subject } from 'rxjs'; import { map } from 'rxjs/operators'; import { STORAGE_KEYS } from '../constants/storage-keys.constants'; -import { Phase } from '../models/phase.model'; import { Repo } from '../models/repo.model'; import { SessionData } from '../models/session.model'; +import { View } from '../models/view.model'; import { ErrorMessageService } from './error-message.service'; import { GithubService } from './github.service'; import { LoggingService } from './logging.service'; @@ -14,25 +14,25 @@ import { RepoUrlCacheService } from './repo-url-cache.service'; export const SESSION_AVALIABILITY_FIX_FAILED = 'Session Availability Fix failed.'; /** - * The title of each phase that appears in the header bar. + * The title of each view that appears in the header bar. */ -export const PhaseDescription = { - [Phase.issuesViewer]: 'Issues Dashboard', - [Phase.activityDashboard]: 'Activity Dashboard' +export const ViewDescription = { + [View.issuesViewer]: 'Issues Dashboard', + [View.activityDashboard]: 'Activity Dashboard' }; /** * All data of the session. - * Add accessible phases here. + * Add accessible views here. */ export const STARTING_SESSION_DATA: SessionData = { sessionRepo: [ - { phase: Phase.issuesViewer, repos: [] } - // { phase: Phase.activityDashboard, repos: [] } + { view: View.issuesViewer, repos: [] } + // { view: View.activityDashboard, repos: [] } ] }; -export const STARTING_PHASE = Phase.issuesViewer; +export const STARTING_VIEW = View.issuesViewer; @Injectable({ providedIn: 'root' @@ -41,15 +41,13 @@ export const STARTING_PHASE = Phase.issuesViewer; /** * Responsible for managing the current selected feature of WATcher as well as the * current session data and repository details related to the session. - * - * A phase is terminology from CATcher, in WATcher it refers to a feature of WATcher. */ -export class PhaseService { +export class ViewService { public static readonly REPO_QUERY_PARAM_KEY = 'repo'; - public currentPhase: Phase = STARTING_PHASE; - public currentRepo: Repo; // current or main repository of current phase - public otherRepos: Repo[]; // more repositories relevant to this phase + public currentView: View = STARTING_VIEW; + public currentRepo: Repo; // current or main repository of current view + public otherRepos: Repo[]; // more repositories relevant to this view repoSetSource = new BehaviorSubject(false); repoSetState = this.repoSetSource.asObservable(); @@ -62,7 +60,7 @@ export class PhaseService { */ public repoChanged$: Subject = new Subject(); - /** Whether the PhaseService is changing the repository */ + /** Whether the ViewService is changing the repository */ public isChangingRepo = new BehaviorSubject(false); public sessionData = STARTING_SESSION_DATA; // stores session data for the session @@ -76,7 +74,7 @@ export class PhaseService { /** * Sets the current main repository and additional repos if any. - * Updates session data in Phase Service and local storage. + * Updates session data in View Service and local storage. * Updates Github Service with current repository. * @param repo Main current repository * @param repos Additional repositories @@ -84,12 +82,12 @@ export class PhaseService { setRepository(repo: Repo, repos?: Repo[]): void { this.currentRepo = repo; this.otherRepos = repos ? repos : []; - this.sessionData.sessionRepo.find((x) => x.phase === this.currentPhase).repos = this.getRepository(); - this.githubService.storePhaseDetails(this.currentRepo.owner, this.currentRepo.name); + this.sessionData.sessionRepo.find((x) => x.view === this.currentView).repos = this.getRepository(); + this.githubService.storeViewDetails(this.currentRepo.owner, this.currentRepo.name); localStorage.setItem('sessionData', JSON.stringify(this.sessionData)); this.router.navigate(['issuesViewer'], { queryParams: { - [PhaseService.REPO_QUERY_PARAM_KEY]: repo.toString() + [ViewService.REPO_QUERY_PARAM_KEY]: repo.toString() } }); } @@ -99,10 +97,10 @@ export class PhaseService { * @param repo New current repository */ private changeCurrentRepository(repo: Repo): void { - this.logger.info(`PhaseService: Changing current repository to '${repo}'`); + this.logger.info(`ViewService: Changing current repository to '${repo}'`); - if (this.currentPhase === Phase.issuesViewer) { - /** Adds past repositories to phase */ + if (this.currentView === View.issuesViewer) { + /** Adds past repositories to view */ (this.otherRepos || []).push(this.currentRepo); } this.setRepository(repo, this.otherRepos); @@ -142,7 +140,7 @@ export class PhaseService { async initializeCurrentRepository() { const org = window.localStorage.getItem(STORAGE_KEYS.ORG); const repoName = window.localStorage.getItem(STORAGE_KEYS.DATA_REPO); - this.logger.info(`PhaseService: received initial org (${org}) and initial name (${repoName})`); + this.logger.info(`ViewService: received initial org (${org}) and initial name (${repoName})`); let repo: Repo; if (!org || !repoName) { repo = Repo.ofEmptyRepo(); @@ -153,20 +151,20 @@ export class PhaseService { if (!isValidRepository) { throw new Error(ErrorMessageService.repositoryNotPresentMessage()); } - this.logger.info(`PhaseService: Repo is ${repo}`); + this.logger.info(`ViewService: Repo is ${repo}`); this.setRepository(repo); this.repoSetSource.next(true); } /** * Set items in the local storage corresponding to the next URL. - * This includes checking if the phase is valid, and if the repo is of the correct format. + * This includes checking if the view is valid, and if the repo is of the correct format. * @param url The partial URL without the host, e.g. `/issuesViewer?repo=CATcher%2FWATcher. */ setupFromUrl(url: string): Observable { - return of(this.getPhaseAndRepoFromUrl(url)).pipe( - map(([phaseName, repoName]) => { - if (!this.isPhaseAllowed(phaseName)) { + return of(this.getViewAndRepoFromUrl(url)).pipe( + map(([viewName, repoName]) => { + if (!this.isViewAllowed(viewName)) { throw new Error(ErrorMessageService.invalidUrlMessage()); } @@ -184,15 +182,15 @@ export class PhaseService { ); } - getPhaseAndRepoFromUrl(url: string): [string, string] { + getViewAndRepoFromUrl(url: string): [string, string] { const urlObject = new URL(`${location.protocol}//${location.host}${url}`); const pathname = urlObject.pathname; - const reponame = urlObject.searchParams.get(PhaseService.REPO_QUERY_PARAM_KEY); + const reponame = urlObject.searchParams.get(ViewService.REPO_QUERY_PARAM_KEY); return [pathname, reponame]; } - isPhaseAllowed(phaseName: string) { - return phaseName === '/' + Phase.issuesViewer; + isViewAllowed(viewName: string) { + return viewName === '/' + View.issuesViewer; } isRepoSet(): boolean { @@ -200,14 +198,14 @@ export class PhaseService { } /** - * Changes phase and updates Phase Service's properties. - * @param phase New phase + * Changes view and updates View Service's properties. + * @param view New view */ - changePhase(phase: Phase) { - this.currentPhase = phase; + changeView(view: View) { + this.currentView = view; // For now, assumes repository stays the same - this.githubService.storePhaseDetails(this.currentRepo.owner, this.currentRepo.name); + this.githubService.storeViewDetails(this.currentRepo.owner, this.currentRepo.name); } public getCurrentRepositoryURL() { @@ -215,6 +213,6 @@ export class PhaseService { } reset() { - this.currentPhase = STARTING_PHASE; + this.currentView = STARTING_VIEW; } } diff --git a/src/app/issues-viewer/issues-viewer.component.html b/src/app/issues-viewer/issues-viewer.component.html index ad5d99cf0..6750b5e05 100644 --- a/src/app/issues-viewer/issues-viewer.component.html +++ b/src/app/issues-viewer/issues-viewer.component.html @@ -1,5 +1,5 @@
-
+
diff --git a/src/app/issues-viewer/issues-viewer.component.ts b/src/app/issues-viewer/issues-viewer.component.ts index 5a94a35e7..a701f0d80 100644 --- a/src/app/issues-viewer/issues-viewer.component.ts +++ b/src/app/issues-viewer/issues-viewer.component.ts @@ -7,7 +7,7 @@ import { GithubService } from '../core/services/github.service'; import { IssueService } from '../core/services/issue.service'; import { LabelService } from '../core/services/label.service'; import { MilestoneService } from '../core/services/milestone.service'; -import { PhaseService } from '../core/services/phase.service'; +import { ViewService } from '../core/services/view.service'; import { TABLE_COLUMNS } from '../shared/issue-tables/issue-tables-columns'; import { CardViewComponent } from './card-view/card-view.component'; @@ -36,13 +36,13 @@ export class IssuesViewerComponent implements OnInit, AfterViewInit, OnDestroy { views = new BehaviorSubject>(undefined); constructor( - public phaseService: PhaseService, + public viewService: ViewService, public githubService: GithubService, public issueService: IssueService, public labelService: LabelService, public milestoneService: MilestoneService ) { - this.repoChangeSubscription = this.phaseService.repoChanged$.subscribe((newRepo) => { + this.repoChangeSubscription = this.viewService.repoChanged$.subscribe((newRepo) => { this.issueService.reset(false); this.labelService.reset(); this.initialize(); @@ -83,10 +83,10 @@ export class IssuesViewerComponent implements OnInit, AfterViewInit, OnDestroy { } /** - * Checks if our current repository available on phase service is indeed a valid repository + * Checks if our current repository available on view service is indeed a valid repository */ private checkIfValidRepository() { - const currentRepo = this.phaseService.currentRepo; + const currentRepo = this.viewService.currentRepo; if (Repo.isInvalidRepoName(currentRepo)) { return of(false); diff --git a/src/app/shared/filter-bar/filter-bar.component.ts b/src/app/shared/filter-bar/filter-bar.component.ts index b7301da64..cb5320392 100644 --- a/src/app/shared/filter-bar/filter-bar.component.ts +++ b/src/app/shared/filter-bar/filter-bar.component.ts @@ -4,7 +4,7 @@ import { BehaviorSubject, Subscription } from 'rxjs'; import { DEFAULT_FILTER, Filter, FiltersService } from '../../core/services/filters.service'; import { LoggingService } from '../../core/services/logging.service'; import { MilestoneService } from '../../core/services/milestone.service'; -import { PhaseService } from '../../core/services/phase.service'; +import { ViewService } from '../../core/services/view.service'; import { FilterableComponent } from '../issue-tables/filterableTypes'; import { LabelFilterBarComponent } from './label-filter-bar/label-filter-bar.component'; @@ -35,10 +35,10 @@ export class FilterBarComponent implements OnInit, OnDestroy { constructor( public milestoneService: MilestoneService, public filtersService: FiltersService, - private phaseService: PhaseService, + private viewService: ViewService, private logger: LoggingService ) { - this.repoChangeSubscription = this.phaseService.repoChanged$.subscribe((newRepo) => this.newRepoInitialize()); + this.repoChangeSubscription = this.viewService.repoChanged$.subscribe((newRepo) => this.newRepoInitialize()); } ngOnInit() { diff --git a/src/app/shared/layout/header.component.html b/src/app/shared/layout/header.component.html index 781bc7840..419e8b807 100644 --- a/src/app/shared/layout/header.component.html +++ b/src/app/shared/layout/header.component.html @@ -8,29 +8,26 @@ > arrow_back_ios - WATcher v{{ this.getVersion() }} - - ({{ this.getPhaseDescription(phaseService.currentPhase) }}) + + ({{ this.getViewDescription(viewService.currentView) }}) -
+
@@ -40,13 +37,13 @@ -
- +
+ {{ this.currentRepo || 'No Repository Set' }}
- +
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 3b5a10765..51a075bd0 100644 --- a/src/app/issues-viewer/card-view/card-view.component.ts +++ b/src/app/issues-viewer/card-view/card-view.component.ts @@ -30,6 +30,8 @@ export class CardViewComponent implements OnInit, AfterViewInit, OnDestroy, Filt isLoading = true; issueLength = 0; + pageSize = 20; + @Output() issueLengthChange: EventEmitter = new EventEmitter(); constructor(public element: ElementRef, public issueService: IssueService) {} @@ -44,8 +46,8 @@ export class CardViewComponent implements OnInit, AfterViewInit, OnDestroy, Filt this.issues$ = this.issues.connect(); // Emit event when issues change - this.issues$.subscribe((issues) => { - this.issueLength = issues.length; + this.issues$.subscribe(() => { + this.issueLength = this.issues.count; this.issueLengthChange.emit(this.issueLength); }); @@ -65,4 +67,8 @@ export class CardViewComponent implements OnInit, AfterViewInit, OnDestroy, Filt retrieveFilterable(): FilterableSource { return this.issues; } + + updatePageSize(newPageSize: number) { + this.pageSize = newPageSize; + } } From 63ed6a2329d9343bdf18afc39b44653809e3370a Mon Sep 17 00:00:00 2001 From: AdityaMisra <114080910+MadLamprey@users.noreply.github.com> Date: Thu, 21 Mar 2024 09:01:00 +0800 Subject: [PATCH 3/8] Add tool tip for hidden users (#307) Currently, there is no tool tip for the Hidden Users mat-card. It is not immediately obvious what the column is referring to. Let's add a tool tip to make it more explanatory. Co-authored-by: Misra Aditya --- src/app/issues-viewer/hidden-users/hidden-users.component.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/issues-viewer/hidden-users/hidden-users.component.html b/src/app/issues-viewer/hidden-users/hidden-users.component.html index 0bf651c83..67bcf9cac 100644 --- a/src/app/issues-viewer/hidden-users/hidden-users.component.html +++ b/src/app/issues-viewer/hidden-users/hidden-users.component.html @@ -1,5 +1,5 @@
- + Hidden users
{{ users.length }}
From caadd668b7142b6efcd99080c87ac7855fc76b25 Mon Sep 17 00:00:00 2001 From: NereusWB922 <107099783+NereusWB922@users.noreply.github.com> Date: Fri, 22 Mar 2024 23:09:27 +0800 Subject: [PATCH 4/8] Setup grouping strategy and service (#308) Implement GroupBy feature to allow users to group the issues/prs based on different criteria such as milestone, status and etc. Let's set up the Grouping Strategy and Service. --- src/app/core/models/github-user.model.ts | 49 ++++++++++- src/app/core/models/github/group.interface.ts | 8 ++ src/app/core/services/github.service.ts | 10 ++- .../assignee-grouping-strategy.service.ts | 81 ++++++++++++++++++ .../grouping/grouping-context.service.ts | 84 +++++++++++++++++++ .../grouping/grouping-strategy.interface.ts | 31 +++++++ 6 files changed, 259 insertions(+), 4 deletions(-) create mode 100644 src/app/core/models/github/group.interface.ts create mode 100644 src/app/core/services/grouping/assignee-grouping-strategy.service.ts create mode 100644 src/app/core/services/grouping/grouping-context.service.ts create mode 100644 src/app/core/services/grouping/grouping-strategy.interface.ts diff --git a/src/app/core/models/github-user.model.ts b/src/app/core/models/github-user.model.ts index cb98fd806..a68dcef15 100644 --- a/src/app/core/models/github-user.model.ts +++ b/src/app/core/models/github-user.model.ts @@ -1,4 +1,9 @@ -export interface GithubUser { +import { Group } from './github/group.interface'; + +/** + * Represents raw data returned from the GitHub API about a user. + */ +export interface RawGithubUser { avatar_url: string; created_at: string; html_url: string; @@ -11,3 +16,45 @@ export interface GithubUser { updated_at: string; url: string; } + +/** + * Represents a GitHub user in WATcher + */ +export class GithubUser implements RawGithubUser, Group { + static NO_ASSIGNEE: GithubUser = new GithubUser({ + avatar_url: '', + created_at: '', + html_url: '', + login: 'Unassigned', + name: '', + node_id: '', + two_factor_authentication: false, + site_admin: false, + type: '', + updated_at: '', + url: '' + }); + + avatar_url: string; + created_at: string; + html_url: string; + login: string; + name: string; + node_id: string; + two_factor_authentication: boolean; + site_admin: false; + type: string; + updated_at: string; + url: string; + + constructor(rawData: RawGithubUser) { + Object.assign(this, rawData); + } + + equals(other: any) { + if (!(other instanceof GithubUser)) { + return false; + } + return this.login === other.login; + } +} diff --git a/src/app/core/models/github/group.interface.ts b/src/app/core/models/github/group.interface.ts new file mode 100644 index 000000000..6e2b6f1b3 --- /dev/null +++ b/src/app/core/models/github/group.interface.ts @@ -0,0 +1,8 @@ +/** + * Represents a group used for grouping purposes. + * Groups can be used to organize issues/prs based on certain criteria, + * such as milestones, assignees, or other properties. + */ +export interface Group { + equals(other: any): boolean; +} diff --git a/src/app/core/services/github.service.ts b/src/app/core/services/github.service.ts index 6518ffec9..e306d00fb 100644 --- a/src/app/core/services/github.service.ts +++ b/src/app/core/services/github.service.ts @@ -16,7 +16,7 @@ import { } from '../../../../graphql/graphql-types'; import { AppConfig } from '../../../environments/environment'; import { getNumberOfPages } from '../../shared/lib/github-paginator-parser'; -import { GithubUser } from '../models/github-user.model'; +import { GithubUser, RawGithubUser } from '../models/github-user.model'; import { IssueLastModifiedManagerModel } from '../models/github/cache-manager/issue-last-modified-manager.model'; import { IssuesCacheManager } from '../models/github/cache-manager/issues-cache-manager.model'; import { GithubEvent } from '../models/github/github-event.model'; @@ -314,7 +314,10 @@ export class GithubService { }) ).pipe( map((response) => { - return response['data']; + const data: RawGithubUser[] = response['data']; + return data.map((rawGithubUser) => { + return new GithubUser(rawGithubUser); + }); }), catchError((err) => throwError(ErrorMessageService.unableToFetchUsersMessage())) ); @@ -406,7 +409,8 @@ export class GithubService { fetchAuthenticatedUser(): Observable { return from(octokit.users.getAuthenticated()).pipe( map((response) => { - return response['data']; + const data: RawGithubUser = response['data']; + return new GithubUser(data); }), catchError((err) => throwError(ErrorMessageService.unableToFetchAuthenticatedUsersMessage())) ); diff --git a/src/app/core/services/grouping/assignee-grouping-strategy.service.ts b/src/app/core/services/grouping/assignee-grouping-strategy.service.ts new file mode 100644 index 000000000..41af8a8be --- /dev/null +++ b/src/app/core/services/grouping/assignee-grouping-strategy.service.ts @@ -0,0 +1,81 @@ +import { Injectable } from '@angular/core'; +import { Observable } from 'rxjs'; +import { map } from 'rxjs/operators'; +import { GithubUser } from '../../models/github-user.model'; +import { Issue } from '../../models/issue.model'; +import { GithubService } from '../github.service'; +import { GroupingStrategy } from './grouping-strategy.interface'; + +/** + * A GroupingStrategy that groups issues/prs based on their assignees. + */ +@Injectable({ + providedIn: 'root' +}) +export class AssigneeGroupingStrategy implements GroupingStrategy { + constructor(private githubService: GithubService) {} + + /** + * Retrieves data for a specific assignee. + * 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); + } + + /** + * Retrieves an Observable emitting users who can be assigned to issues, + * including a special "No Assignee" option. + */ + getGroups(): Observable { + return this.githubService.getUsersAssignable().pipe( + map((users) => { + users.push(GithubUser.NO_ASSIGNEE); + return users; + }) + ); + } + + /** + * Groups other than "No Assignee" need to be shown on the + * hidden group list if empty. + */ + isInHiddenList(group: GithubUser): boolean { + return group !== GithubUser.NO_ASSIGNEE; + } + + private getDataAssignedToUser(issues: Issue[], user: GithubUser): Issue[] { + const filteredIssues = issues.filter((issue) => { + if (this.isPullRequest(issue)) { + return this.isPullRequestCreatedByTarget(issue, user); + } + + return this.isIssueAssignedToTarget(issue, user); + }); + + return filteredIssues; + } + + private getUnassignedData(issues: Issue[]): Issue[] { + return issues.filter((issue) => !this.isPullRequest(issue) && issue.assignees.length === 0); + } + + private isPullRequest(issue: Issue): boolean { + return issue.issueOrPr === 'PullRequest'; + } + + private isPullRequestCreatedByTarget(issue: Issue, target: GithubUser): boolean { + return issue.author === target.login; + } + + private isIssueAssignedToTarget(issue: Issue, target: GithubUser): boolean { + const isAssigneesFieldDefined = !!issue.assignees; + + return isAssigneesFieldDefined && issue.assignees.includes(target.login); + } +} diff --git a/src/app/core/services/grouping/grouping-context.service.ts b/src/app/core/services/grouping/grouping-context.service.ts new file mode 100644 index 000000000..9bfb7503e --- /dev/null +++ b/src/app/core/services/grouping/grouping-context.service.ts @@ -0,0 +1,84 @@ +import { Injectable, Injector } from '@angular/core'; +import { BehaviorSubject, Observable } from 'rxjs'; +import { Group } from '../../models/github/group.interface'; +import { Issue } from '../../models/issue.model'; +import { AssigneeGroupingStrategy } from './assignee-grouping-strategy.service'; +import { GroupingStrategy } from './grouping-strategy.interface'; + +export enum GroupBy { + Assignee = 'assignee', + Milestone = 'milestone' +} + +export const DEFAULT_GROUPBY = GroupBy.Assignee; + +/** + * A service responsible for managing the current grouping strategy and providing grouped data. + */ +@Injectable({ + providedIn: 'root' +}) +export class GroupingContextService { + private currGroupBySubject: BehaviorSubject; + currGroupBy: GroupBy; + currGroupBy$: Observable; + + private groupingStrategyMap: Map; + + constructor(private injector: Injector) { + this.currGroupBy = DEFAULT_GROUPBY; + this.currGroupBySubject = new BehaviorSubject(this.currGroupBy); + this.currGroupBy$ = this.currGroupBySubject.asObservable(); + + this.groupingStrategyMap = new Map(); + + // Initialize the grouping strategy map with available strategies + this.groupingStrategyMap.set(GroupBy.Assignee, this.injector.get(AssigneeGroupingStrategy)); + } + + /** + * Sets the current grouping type. + * @param groupBy - The grouping type to set. + */ + setCurrentGroupingType(groupBy: GroupBy): void { + this.currGroupBy = groupBy; + this.currGroupBySubject.next(this.currGroupBy); + } + + /** + * Retrieves data for a specific group. + * @param issues - An array of issues to be grouped. + * @param group - The group by which issues are to be grouped. + * @returns An array of issues belonging to the specified group. + */ + getDataForGroup(issues: Issue[], group: Group): Issue[] { + const strategy = this.groupingStrategyMap.get(this.currGroupBy); + return strategy.getDataForGroup(issues, group); + } + + /** + * Retrieves all groups available for current grouping strategy. + * @returns An Observable emitting an array of groups. + */ + getGroups(): Observable { + const strategy = this.groupingStrategyMap.get(this.currGroupBy); + return strategy.getGroups(); + } + + /** + * Determines whether a group should be shown on hidden list if it contains no issues. + * @param group - The group to check. + * @returns A boolean indicating whether the group should be shown on hidden list if empty. + */ + isInHiddenList(group: Group): boolean { + const strategy = this.groupingStrategyMap.get(this.currGroupBy); + return strategy.isInHiddenList(group); + } + + /** + * Resets the current grouping type to the default. + */ + reset(): void { + this.setCurrentGroupingType(DEFAULT_GROUPBY); + } +} diff --git a/src/app/core/services/grouping/grouping-strategy.interface.ts b/src/app/core/services/grouping/grouping-strategy.interface.ts new file mode 100644 index 000000000..6c2f407ee --- /dev/null +++ b/src/app/core/services/grouping/grouping-strategy.interface.ts @@ -0,0 +1,31 @@ +import { Observable } from 'rxjs'; +import { Group } from '../../models/github/group.interface'; +import { Issue } from '../../models/issue.model'; + +/** + * Represent a strategy for grouping issues/prs. + * This interface follows the Strategy Pattern, allowing for different + * strategies to be implemented for grouping issues/prs based on different criteria. + */ +export interface GroupingStrategy { + /** + * Retrieves data for a specific group. + * @param issues - An array of issues to be grouped. + * @param key - The group by which issues are to be grouped. + * @returns An array of issues belonging to the specified group. + */ + getDataForGroup(issues: Issue[], key: Group): Issue[]; + + /** + * Retrieves observable emitting groups available for the grouping strategy. + * @returns An Observable emitting an array of groups. + */ + getGroups(): Observable; + + /** + * Determines whether a group should be shown on hidden list if it contains no issues. + * @param group - The group to check. + * @returns A boolean indicating whether the group should be shown on hidden list if empty. + */ + isInHiddenList(group: Group): boolean; +} From e3d4a3464879414ab4724dde0f9b4cc24aebf7df Mon Sep 17 00:00:00 2001 From: Nguyen <87511888+nknguyenhc@users.noreply.github.com> Date: Fri, 22 Mar 2024 23:29:59 +0800 Subject: [PATCH 5/8] Three-state labels (#282) Previously, each label only has 2 states, either selected or not selected. However, with such design, the feature of hiding labels can be confused with hiding issues/PRs with the label. We implement the three-state label filters, so that each label can also be used to hide issues/PRs with the label. --- src/app/core/services/filters.service.ts | 4 +- .../label-filter-bar.component.css | 25 ++++++-- .../label-filter-bar.component.html | 48 +++++++------- .../label-filter-bar.component.ts | 64 +++++++++++-------- src/app/shared/issue-tables/dropdownfilter.ts | 2 +- .../label-filter-bar.component.spec.ts | 14 ++-- 6 files changed, 90 insertions(+), 67 deletions(-) diff --git a/src/app/core/services/filters.service.ts b/src/app/core/services/filters.service.ts index de2e2d538..583d0ea31 100644 --- a/src/app/core/services/filters.service.ts +++ b/src/app/core/services/filters.service.ts @@ -11,6 +11,7 @@ export type Filter = { labels: string[]; milestones: string[]; hiddenLabels: Set; + deselectedLabels: Set; }; export const DEFAULT_FILTER: Filter = { @@ -20,7 +21,8 @@ export const DEFAULT_FILTER: Filter = { sort: { active: 'id', direction: 'asc' }, labels: [], milestones: [], - hiddenLabels: new Set() + hiddenLabels: new Set(), + deselectedLabels: new Set() }; @Injectable({ diff --git a/src/app/shared/filter-bar/label-filter-bar/label-filter-bar.component.css b/src/app/shared/filter-bar/label-filter-bar/label-filter-bar.component.css index 03e92e4be..69d9e392f 100644 --- a/src/app/shared/filter-bar/label-filter-bar/label-filter-bar.component.css +++ b/src/app/shared/filter-bar/label-filter-bar/label-filter-bar.component.css @@ -88,6 +88,26 @@ flex-direction: row; justify-content: flex-start; align-items: center; + border-radius: 10px; + height: 40px; + padding: 0px 12px; + margin: 8px 4px; + box-sizing: border-box; + position: relative; +} + +.flexbox-container:hover { + background-color: rgba(0, 0, 0, 0.04); +} + +.flexbox-container-strikethrough { + position: absolute; + top: 50%; + width: 90%; + left: 50%; + transform: translate(-50%, -50%); + height: 2px; + background-color: black; } .input-field { @@ -95,10 +115,6 @@ padding: 0 15px; } -.list-option { - width: 100%; -} - .mat-chip { height: auto; padding: 5.5px 7px; @@ -109,7 +125,6 @@ min-height: 16px; max-height: 42px; margin: 0px; - top: 50%; } .mat-stroked-button { diff --git a/src/app/shared/filter-bar/label-filter-bar/label-filter-bar.component.html b/src/app/shared/filter-bar/label-filter-bar/label-filter-bar.component.html index ae5b5ef32..ad12b066b 100644 --- a/src/app/shared/filter-bar/label-filter-bar/label-filter-bar.component.html +++ b/src/app/shared/filter-bar/label-filter-bar/label-filter-bar.component.html @@ -1,5 +1,5 @@ + + -
- - - - {{ label.name }} - -
- - + {{ label.name }} +
+
+
diff --git a/src/app/shared/filter-bar/label-filter-bar/label-filter-bar.component.ts b/src/app/shared/filter-bar/label-filter-bar/label-filter-bar.component.ts index 6d4326277..38adeec95 100644 --- a/src/app/shared/filter-bar/label-filter-bar/label-filter-bar.component.ts +++ b/src/app/shared/filter-bar/label-filter-bar/label-filter-bar.component.ts @@ -1,5 +1,4 @@ import { AfterViewInit, Component, OnDestroy, OnInit, ViewChild } from '@angular/core'; -import { MatListOption, MatSelectionList } from '@angular/material/list'; import { Observable, Subscription } from 'rxjs'; import { SimpleLabel } from '../../../core/models/label.model'; import { FiltersService } from '../../../core/services/filters.service'; @@ -12,11 +11,14 @@ import { LoggingService } from '../../../core/services/logging.service'; styleUrls: ['./label-filter-bar.component.css'] }) export class LabelFilterBarComponent implements OnInit, AfterViewInit, OnDestroy { - @ViewChild(MatSelectionList) matSelectionList; + private static readonly DEFAULT_LABEL_COLOR: string = 'transparent'; + private static readonly DESELECTED_LABEL_COLOR: string = '#b00020'; + private static readonly SELECTED_LABEL_COLOR: string = '#41c300'; labels$: Observable; allLabels: SimpleLabel[]; - selectedLabelNames: string[] = []; + selectedLabelNames: Set = new Set(); + deselectedLabelNames: Set = new Set(); hiddenLabelNames: Set = new Set(); loaded = false; @@ -35,7 +37,7 @@ export class LabelFilterBarComponent implements OnInit, AfterViewInit, OnDestroy this.labels$.subscribe((labels) => { this.allLabels = labels; this.filtersService.sanitizeLabels(this.allLabels); - this.selectedLabelNames = this.filtersService.filter$.value.labels; + this.selectedLabelNames = new Set(this.filtersService.filter$.value.labels); this.hiddenLabelNames = this.filtersService.filter$.value.hiddenLabels; }); }); @@ -64,16 +66,34 @@ export class LabelFilterBarComponent implements OnInit, AfterViewInit, OnDestroy } /** - * chip as of the current project version consumes click events - * this method is used as an workaround the issue. - * https://github.com/angular/components/issues/19759 + * Change label to the next state. + * Label has the following state rotation: default -> selected -> deselected. + * @param label The label to change state */ - simulateClick(el: MatListOption): void { - if (el.disabled) { - return; + changeLabelState(label: SimpleLabel) { + if (this.selectedLabelNames.has(label.name)) { + this.selectedLabelNames.delete(label.name); + this.deselectedLabelNames.add(label.name); + } else if (this.deselectedLabelNames.has(label.name)) { + this.deselectedLabelNames.delete(label.name); + } else { + this.selectedLabelNames.add(label.name); + } + this.updateSelection(); + } + + /** + * Returns the border color of the label. + * The border color represents the state of the label. + */ + getColor(label: SimpleLabel): string { + if (this.selectedLabelNames.has(label.name)) { + return LabelFilterBarComponent.SELECTED_LABEL_COLOR; + } else if (this.deselectedLabelNames.has(label.name)) { + return LabelFilterBarComponent.DESELECTED_LABEL_COLOR; + } else { + return LabelFilterBarComponent.DEFAULT_LABEL_COLOR; } - el.toggle(); - this.updateSelection([el]); } /** loads in the labels in the repository */ @@ -101,22 +121,16 @@ export class LabelFilterBarComponent implements OnInit, AfterViewInit, OnDestroy return this.allLabels.some((label) => !this.filter(filter, label.name)); } - updateSelection(options: MatListOption[]): void { - options.forEach((option) => { - if (option.selected && !this.selectedLabelNames.includes(option.value)) { - this.selectedLabelNames.push(option.value); - } - if (!option.selected && this.selectedLabelNames.includes(option.value)) { - const index = this.selectedLabelNames.indexOf(option.value); - this.selectedLabelNames.splice(index, 1); - } + updateSelection(): void { + this.filtersService.updateFilters({ + labels: Array.from(this.selectedLabelNames), + deselectedLabels: this.deselectedLabelNames }); - this.filtersService.updateFilters({ labels: this.selectedLabelNames }); } removeAllSelection(): void { - this.matSelectionList.deselectAll(); - this.selectedLabelNames = []; - this.filtersService.updateFilters({ labels: this.selectedLabelNames }); + this.selectedLabelNames = new Set(); + this.deselectedLabelNames = new Set(); + this.updateSelection(); } } diff --git a/src/app/shared/issue-tables/dropdownfilter.ts b/src/app/shared/issue-tables/dropdownfilter.ts index 0b9e8ebef..e1f122f77 100644 --- a/src/app/shared/issue-tables/dropdownfilter.ts +++ b/src/app/shared/issue-tables/dropdownfilter.ts @@ -27,7 +27,7 @@ export function applyDropdownFilter(filter: Filter, data: Issue[]): Issue[] { } ret = ret && filter.milestones.some((milestone) => issue.milestone.title === milestone); - + ret = ret && issue.labels.every((label) => !filter.deselectedLabels.has(label)); return ret && filter.labels.every((label) => issue.labels.includes(label)); }); return filteredData; diff --git a/tests/app/shared/filter-bar/label-filter-bar/label-filter-bar.component.spec.ts b/tests/app/shared/filter-bar/label-filter-bar/label-filter-bar.component.spec.ts index 9ee9bfd31..d0d85a591 100644 --- a/tests/app/shared/filter-bar/label-filter-bar/label-filter-bar.component.spec.ts +++ b/tests/app/shared/filter-bar/label-filter-bar/label-filter-bar.component.spec.ts @@ -119,23 +119,19 @@ describe('LabelFilterBarComponent', () => { describe('updateSelection', () => { it('should update filters service with selected labels', () => { const selectedLabels = [LABEL_NAME_SEVERITY_HIGH, LABEL_NAME_SEVERITY_LOW]; - component.selectedLabelNames = selectedLabels; + component.selectedLabelNames = new Set(selectedLabels); - component.updateSelection([]); + component.updateSelection(); - expect(filtersServiceSpy.updateFilters).toHaveBeenCalledWith({ labels: selectedLabels }); + expect(filtersServiceSpy.updateFilters).toHaveBeenCalledWith({ labels: selectedLabels, deselectedLabels: new Set() }); }); }); describe('removeAllSelection', () => { it('should deselect all labels and update the filter', () => { - const matSelectionListSpy = jasmine.createSpyObj('MatSelectionList', ['deselectAll']); - component.matSelectionList = matSelectionListSpy; - component.removeAllSelection(); - - expect(matSelectionListSpy.deselectAll).toHaveBeenCalled(); - expect(filtersServiceSpy.updateFilters).toHaveBeenCalledWith({ labels: [] }); + expect(component.selectedLabelNames).toEqual(new Set()); + expect(component.deselectedLabelNames).toEqual(new Set()); }); }); }); From a65bb59578c4157be05f3c38fd712e411e43a155 Mon Sep 17 00:00:00 2001 From: Nguyen <87511888+nknguyenhc@users.noreply.github.com> Date: Mon, 25 Mar 2024 12:04:08 +0800 Subject: [PATCH 6/8] Status filter checkboxes (#310) We implement checkboxes for status, so that multiple types of PRs/issues can be viewed concurrently. --- src/app/core/services/filters.service.ts | 41 ++++++------------- .../filter-bar/filter-bar.component.html | 11 ++--- .../shared/filter-bar/filter-bar.component.ts | 8 +++- .../label-filter-bar.component.ts | 1 + src/app/shared/issue-tables/dropdownfilter.ts | 28 +++++++++---- 5 files changed, 45 insertions(+), 44 deletions(-) diff --git a/src/app/core/services/filters.service.ts b/src/app/core/services/filters.service.ts index 583d0ea31..2f3c81bf0 100644 --- a/src/app/core/services/filters.service.ts +++ b/src/app/core/services/filters.service.ts @@ -5,7 +5,7 @@ import { SimpleLabel } from '../models/label.model'; export type Filter = { title: string; - status: string; + status: string[]; type: string; sort: Sort; labels: string[]; @@ -16,7 +16,7 @@ export type Filter = { export const DEFAULT_FILTER: Filter = { title: '', - status: 'all', + status: ['open pullrequest', 'merged pullrequest', 'open issue', 'closed issue'], type: 'all', sort: { active: 'id', direction: 'asc' }, labels: [], @@ -35,21 +35,16 @@ export const DEFAULT_FILTER: Filter = { export class FiltersService { public filter$ = new BehaviorSubject(DEFAULT_FILTER); - private _validateFilter = pipe(this.updateStatusPairing, this.updateTypePairing); - clearFilters(): void { this.filter$.next(DEFAULT_FILTER); } updateFilters(newFilters: Partial): void { - let nextFilter: Filter = { + const nextDropdownFilter: Filter = { ...this.filter$.value, ...newFilters }; - - nextFilter = this._validateFilter(nextFilter); - - this.filter$.next(nextFilter); + this.filter$.next(nextDropdownFilter); } sanitizeLabels(allLabels: SimpleLabel[]) { @@ -62,27 +57,15 @@ export class FiltersService { } } - const newLabels = this.filter$.value.labels.filter((label) => allLabelsSet.has(label)); - - this.updateFilters({ labels: newLabels, hiddenLabels: newHiddenLabels }); - } - /** - * Changes type to a valid, default value when an incompatible combination of type and status is encountered. - */ - updateTypePairing(filter: Filter): Filter { - if (filter.status === 'merged') { - filter.type = 'pullrequest'; + const newDeselectedLabels: Set = new Set(); + for (const deselectedLabel of this.filter$.value.deselectedLabels) { + if (allLabelsSet.has(deselectedLabel)) { + newDeselectedLabels.add(deselectedLabel); + } } - return filter; - } - /** - * Changes status to a valid, default value when an incompatible combination of type and status is encountered. - */ - updateStatusPairing(filter: Filter): Filter { - if (filter.status === 'merged' && filter.type === 'issue') { - filter.status = 'all'; - } - return filter; + const newLabels = this.filter$.value.labels.filter((label) => allLabelsSet.has(label)); + + this.updateFilters({ labels: newLabels, hiddenLabels: newHiddenLabels, deselectedLabels: newDeselectedLabels }); } } diff --git a/src/app/shared/filter-bar/filter-bar.component.html b/src/app/shared/filter-bar/filter-bar.component.html index c968fd97d..8ffe621f9 100644 --- a/src/app/shared/filter-bar/filter-bar.component.html +++ b/src/app/shared/filter-bar/filter-bar.component.html @@ -14,11 +14,12 @@