ProfileService: read profiles from client data repo #661

seanlowjk · 2021-03-20T08:02:07Z

Summary

This PR fixes #641

Description

Every time we need to update the dropdown list of sessions for our CATcher application, we would need to edit the environment.*.ts files. As a result, it becomes pretty troublesome just to create a new commit just for it.

As a result, we can make use of simple HTTP requests to get the relevant profiles from an external repository, such as CATcher-org/client_data to help us retrieve the relevant profiles needed.

Thus, we will only have one single source of truth where updating of custom profiles can be done in CATcher-org/client_data.

To Dos:

Fix e2e tests with regards to session selection

Suggested Commit Message

ProfileService: read profiles from client data repo

The app's default profiles are stored within the
codebase (in the AppConfig). Hence, updating the profiles
requires us to release / re-deploy a new version of CATcher.

Let's fetch the profiles from a client data repository instead.
This makes it possible to update the profiles
without modifying our codebase.

We can define a new ProfileService that uses a HTTP request
to fetch these profiles from the repo.

codecov-io · 2021-03-20T08:37:22Z

Codecov Report

Merging #661 (ec7b2e3) into master (f005ee5) will increase coverage by 0.49%.
The diff coverage is 94.11%.

@@            Coverage Diff             @@
##           master     #661      +/-   ##
==========================================
+ Coverage   68.67%   69.17%   +0.49%     
==========================================
  Files          75       76       +1     
  Lines        2273     2290      +17     
  Branches      208      209       +1     
==========================================
+ Hits         1561     1584      +23     
+ Misses        667      660       -7     
- Partials       45       46       +1

Impacted Files	Coverage Δ
src/environments/environment.gen.ts	`83.33% <ø> (ø)`
src/app/core/services/profile.service.ts	`92.85% <92.85%> (ø)`
src/app/core/services/github.service.ts	`36.41% <100.00%> (+4.65%)`	⬆️
src/app/shared/issue-tables/issue-sorter.ts	`44.00% <0.00%> (-4.00%)`	⬇️
...rc/app/shared/issue/assignee/assignee.component.ts	`91.42% <0.00%> (ø)`
...github/cache-manager/issues-cache-manager.model.ts	`43.75% <0.00%> (+6.25%)`	⬆️
...cache-manager/issue-last-modified-manager.model.ts	`66.66% <0.00%> (+11.11%)`	⬆️

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 f005ee5...ec7b2e3. Read the comment docs.

dingyuchen

LGTM! just 1 small comment

dingyuchen · 2021-03-22T06:20:29Z

src/app/auth/profiles/profiles.component.ts

+   private fetchProfilesJson(): Promise<any> {
+    return fetch(AppConfig.clientDataUrl).then(res => res.json());


Suggested change

private fetchProfilesJson(): Promise<any> {

return fetch(AppConfig.clientDataUrl).then(res => res.json());

private fetchProfilesJson(): Promise<Profile[]> {

return fetch(AppConfig.clientDataUrl).then(res => res.json().profiles || []);

I think can include some error handling here and specify the promise type

Sounds good to me! 👍

…eanlowjk/CATcher into feature/external-repo-profiles

ptvrajsk

~~LGTM! 👍 Nice work with this.~~

Woops sorry, spotted a small bug

ptvrajsk

Just one minor bug but otherwise LGTM! 👍

src/app/auth/profiles/profiles.component.ts

ptvrajsk

Great! LGTM 👍

src/app/auth/profiles/profiles.component.ts

seanlowjk · 2021-04-10T14:21:28Z

Add GitHubService as a dependency to ProfileService. Within fetchExternalProfiles, invoke githubService.getProfilesData(). In the production GitHubService's getProfilesData method, fetch the profiles' data from the external repo. In the MockGitHubService's getProfilesData method, return a hardcoded profile.

I honestly feel that this is the best way to go about with respect to making sure our e2e tests passing offline.

is a bit too hardcoded and if we need to add more other methods for ProfileService to fetch data related to profiles, we need to add more hardcoded logic which shouldn't be advisable.

anubh-v · 2021-04-10T14:48:47Z

Add GitHubService as a dependency to ProfileService. Within fetchExternalProfiles, invoke githubService.getProfilesData(). In the production GitHubService's getProfilesData method, fetch the profiles' data from the external repo. In the MockGitHubService's getProfilesData method, return a hardcoded profile.

I honestly feel that this is the best way to go about with respect to making sure our e2e tests passing offline.

is a bit too hardcoded and if we need to add more other methods for ProfileService to fetch data related to profiles, we need to add more hardcoded logic which shouldn't be advisable.

Ok, let's go with the second approach. We can move fetch(AppConfig.clientDataUrl) into a method in GithubService.

tests/services/profile.service.spec.ts

src/app/auth/profiles/profiles.component.ts

anubh-v

A few final comments

src/app/app.module.ts

tests/services/profile.service.spec.ts

src/app/auth/profiles/profiles.component.ts

tests/services/profile.service.spec.ts

Co-authored-by: Anubhav <35621759+anubh-v@users.noreply.github.com>

…eanlowjk/CATcher into feature/external-repo-profiles

codecov-commenter · 2021-04-18T00:54:56Z

Codecov Report

Merging #661 (f05e769) into master (6a3c5e8) will increase coverage by 0.51%.
The diff coverage is 94.73%.

@@            Coverage Diff             @@
##           master     #661      +/-   ##
==========================================
+ Coverage   69.08%   69.60%   +0.51%     
==========================================
  Files          76       77       +1     
  Lines        2274     2293      +19     
  Branches      208      209       +1     
==========================================
+ Hits         1571     1596      +25     
+ Misses        660      652       -8     
- Partials       43       45       +2

Impacted Files	Coverage Δ
src/environments/environment.gen.ts	`83.33% <ø> (ø)`
src/app/core/services/profile.service.ts	`93.75% <93.75%> (ø)`
src/app/core/services/github.service.ts	`36.41% <100.00%> (+4.65%)`	⬆️
src/app/shared/issue-tables/issue-sorter.ts	`48.00% <0.00%> (-4.00%)`	⬇️
...github/cache-manager/issues-cache-manager.model.ts	`43.75% <0.00%> (+6.25%)`	⬆️
...cache-manager/issue-last-modified-manager.model.ts	`66.66% <0.00%> (+11.11%)`	⬆️

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 6a3c5e8...f05e769. Read the comment docs.

src/app/app.module.ts

seanlowjk added 5 commits March 20, 2021 15:39

Add Boiletplate API for Fetching External Profiles

4455c07

Test Concatenation of Profiles

50a137f

Add Constant Values for Repo

252aea0

Add fetchProfilesJson for Mock GithubService

d48f636

Fix Linting

61c37e4

seanlowjk added 5 commits March 21, 2021 11:12

Revert Changes to GithubService

7a8eb4e

Change GithubService Request to HTTP Request

f93feaf

Remove unused API

6ca59b4

Update URLs and Profiles for Environments

83ef23d

Remove Unused Imports

eb23634

seanlowjk requested a review from a team March 21, 2021 04:00

seanlowjk added the s.ToReview label Mar 21, 2021

seanlowjk marked this pull request as ready for review March 21, 2021 04:01

dingyuchen reviewed Mar 22, 2021

View reviewed changes

seanlowjk and others added 5 commits March 22, 2021 14:58

Change Return Type and Add Error Handling

497c4e1

Merge branch 'master' into feature/external-repo-profiles

6658d4a

Merge branch 'feature/external-repo-profiles' of https://github.com/s…

88b517e

…eanlowjk/CATcher into feature/external-repo-profiles

Merge branch 'master' into feature/external-repo-profiles

7315c8e

Merge branch 'master' into feature/external-repo-profiles

f631e37

seanlowjk requested a review from dingyuchen March 27, 2021 09:02

Tidy up Documentation

4ead260

anubh-v added this to the V3.3.8 milestone Mar 28, 2021

Merge branch 'master' into feature/external-repo-profiles

b8f420a

ptvrajsk approved these changes Mar 28, 2021

View reviewed changes

ptvrajsk reviewed Mar 28, 2021

View reviewed changes

src/app/auth/profiles/profiles.component.ts Outdated Show resolved Hide resolved

Fix Custom Profiles Behavior

176b29e

ptvrajsk approved these changes Mar 29, 2021

View reviewed changes

src/app/auth/profiles/profiles.component.ts Show resolved Hide resolved

ptvrajsk requested a review from anubh-v March 29, 2021 15:46

Merge branch 'master' into feature/external-repo-profiles

5e23e12

Move fetch API call to GithubService

ee5d750

anubh-v reviewed Apr 10, 2021

View reviewed changes

tests/services/profile.service.spec.ts Outdated Show resolved Hide resolved

src/app/auth/profiles/profiles.component.ts Outdated Show resolved Hide resolved

seanlowjk added 6 commits April 11, 2021 00:16

Update Dependencies in AppModule

48f773b

Update MockGithubService method

d4da1b3

Fix Linting

484d863

Add return statements for tests

04aaebb

Change Logic Flow for fetchExternalProfiles

25e2795

Fix Linting

ec7b2e3

seanlowjk requested a review from anubh-v April 10, 2021 16:47

Merge branch 'master' into feature/external-repo-profiles

6f8d512

anubh-v approved these changes Apr 17, 2021

View reviewed changes

seanlowjk and others added 5 commits April 18, 2021 08:42

Add Fail Statement for Invalid Profiles Test

f1a52b6

Fix Test Suite Names for ProfileService Tests

9a54fad

Fix Slight Typo in Docs

f6d6046

Co-authored-by: Anubhav <35621759+anubh-v@users.noreply.github.com>

Merge branch 'feature/external-repo-profiles' of https://github.com/s…

e92bbe7

…eanlowjk/CATcher into feature/external-repo-profiles

Replace Dependency with Injectable

b966081

anubh-v reviewed Apr 19, 2021

View reviewed changes

src/app/app.module.ts Outdated Show resolved Hide resolved

anubh-v changed the title ~~ProfileService: Read Profiles From External Repository, Add Tests~~ ProfileService: read profiles from external repository Apr 19, 2021

Remove Unused Import

f05e769

anubh-v added s.ToMerge and removed s.ToReview labels Apr 19, 2021

anubh-v changed the title ~~ProfileService: read profiles from external repository~~ ProfileService: read profiles from client data repo Apr 19, 2021

Merge branch 'master' into feature/external-repo-profiles

ebd6422

anubh-v merged commit e253dfb into CATcher-org:master Apr 19, 2021

This was referenced Jun 21, 2021

Read default profiles from a config file #364

Closed

Bug report / Team response: Replace placeholder text with a hint #716

Merged

seanlowjk mentioned this pull request Jun 25, 2021

PhaseService#storeSessionData: Remove current phase verification #721

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ProfileService: read profiles from client data repo #661

ProfileService: read profiles from client data repo #661

seanlowjk commented Mar 20, 2021 •

edited by anubh-v

codecov-io commented Mar 20, 2021 •

edited

dingyuchen left a comment

dingyuchen Mar 22, 2021

seanlowjk Mar 22, 2021

ptvrajsk left a comment •

edited

ptvrajsk left a comment •

edited

ptvrajsk left a comment

seanlowjk commented Apr 10, 2021

anubh-v commented Apr 10, 2021

anubh-v left a comment

codecov-commenter commented Apr 18, 2021 •

edited

		private fetchProfilesJson(): Promise<any> {
		return fetch(AppConfig.clientDataUrl).then(res => res.json());

ProfileService: read profiles from client data repo #661

ProfileService: read profiles from client data repo #661

Conversation

seanlowjk commented Mar 20, 2021 • edited by anubh-v

Summary

Description

To Dos:

Suggested Commit Message

codecov-io commented Mar 20, 2021 • edited

Codecov Report

dingyuchen left a comment

Choose a reason for hiding this comment

dingyuchen Mar 22, 2021

Choose a reason for hiding this comment

seanlowjk Mar 22, 2021

Choose a reason for hiding this comment

ptvrajsk left a comment • edited

Choose a reason for hiding this comment

ptvrajsk left a comment • edited

Choose a reason for hiding this comment

ptvrajsk left a comment

Choose a reason for hiding this comment

seanlowjk commented Apr 10, 2021

anubh-v commented Apr 10, 2021

anubh-v left a comment

Choose a reason for hiding this comment

codecov-commenter commented Apr 18, 2021 • edited

Codecov Report

seanlowjk commented Mar 20, 2021 •

edited by anubh-v

codecov-io commented Mar 20, 2021 •

edited

ptvrajsk left a comment •

edited

ptvrajsk left a comment •

edited

codecov-commenter commented Apr 18, 2021 •

edited